Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] Removal of the TPS61161 backlight controller PWM mode initialization sequence, potentially breaking backlight control. - [Low] The 'pandora-backlight' platform device is still registered in legacy pdata quirks despite the driver being removed, resulting in an unbound dead device. - [Low] Stale Kconfig reference to `CONFIG_BACKLIGHT_PANDORA` remains in `omap2plus_defconfig`. --
--- Patch [15]: [PATCH 15/16] backlight: remove pandora_bl --- commit 6eef80f63fc65c8cd1ed05d5c2e07cc3c726434e Author: H. Nikolaus Schaller <[email protected]> backlight: remove pandora_bl Retire the platform specific pandora backlight driver since it can now be replaced by twl_pm and device tree. [Severity: Low] Does this commit leave an unbound dead device registered in the system? The pandora-backlight platform driver is removed here, but it appears arch/arm/mach-omap2/pdata-quirks.c still statically defines and registers the device in omap3_pandora_legacy_init(): platform_device_register(&pandora_backlight); Should this platform device registration be removed as well? > diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig > index 7aa1c4b21111f..a250b3edc152c 100644 > --- a/drivers/video/backlight/Kconfig > +++ b/drivers/video/backlight/Kconfig > @@ -451,13 +451,6 @@ config BACKLIGHT_MP3309C [ ... ] > -config BACKLIGHT_PANDORA > - tristate "Backlight driver for Pandora console" > - depends on TWL4030_CORE > - help > - If you have a Pandora console, say Y to enable the > - backlight driver. > - [Severity: Low] Will this removal trigger a Kconfig warning during omap2plus_defconfig generation? The defconfig file still explicitly enables CONFIG_BACKLIGHT_PANDORA=m. Should arch/arm/configs/omap2plus_defconfig be updated to remove this stale entry? > diff --git a/drivers/video/backlight/pandora_bl.c > b/drivers/video/backlight/pandora_bl.c > deleted file mode 100644 > index 8a63ded0fa90f..0000000000000 > --- a/drivers/video/backlight/pandora_bl.c > +++ /dev/null > @@ -1,159 +0,0 @@ [ ... ] > - if (priv->old_state == PANDORABL_WAS_OFF) { > - /* > - * set PWM duty cycle to max. TPS61161 seems to use this > - * to calibrate it's PWM sensitivity when it starts. > - */ > - twl_i2c_write_u8(TWL_MODULE_PWM, MAX_VALUE, TWL_PWM0_OFF); > - > - /* first enable clock, then PWM0 out */ > - twl_i2c_read_u8(TWL4030_MODULE_INTBR, &r, TWL_INTBR_GPBR1); > - r &= ~PWM0_ENABLE; > - r |= PWM0_CLK_ENABLE; > - twl_i2c_write_u8(TWL4030_MODULE_INTBR, r, TWL_INTBR_GPBR1); > - r |= PWM0_ENABLE; > - twl_i2c_write_u8(TWL4030_MODULE_INTBR, r, TWL_INTBR_GPBR1); > - > - /* > - * TI made it very easy to enable digital control, so easy that > - * it often triggers unintentionally and disabes PWM control, > - * so wait until 1 wire mode detection window ends. > - */ > - usleep_range(2000, 10000); > - } [Severity: High] When switching to the generic pwm-backlight driver, how is the TPS61161 initialized into PWM mode? Because the TPS61161 shares its EN pin for both 1-wire EasyScale digital control and PWM control, the removed pandora_bl driver included this hardware workaround to force a 100% PWM duty cycle and sleep for 2-10ms at startup. Without this initialization pulse, could the TPS61161 misinterpret the initial PWM signal as a 1-wire EasyScale command and disable PWM control, preventing the backlight from turning on properly? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=15
