Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] Missing TPS61161 hardware initialization quirk causes backlight to fail. - [High] Concurrent probing of legacy and generic backlight drivers causes I2C register corruption. - [Medium] Hardware misconfiguration due to unsupported PWM period, leading to artificially restricted brightness range. --
--- Patch [14]: [PATCH 14/16] arm: dts: omap3-pandora-common: backlight: switch to twl4030 pwm and pwm_bl --- commit 062b6b894a7997d211a245d15a6bd9071869adad Author: H. Nikolaus Schaller <[email protected]> arm: dts: omap3-pandora-common: backlight: switch to twl4030 pwm and pwm_bl This commit adds a generic pwm-backlight device tree node for the Pandora backlight. The commit message states this allows removing the pandora_bl driver and pdata-quirks. > diff --git a/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi > b/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi > index 13821d926afb0..fea7132ce7c5e 100644 > --- a/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi > +++ b/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi > @@ -31,6 +31,14 @@ hfclk_26m: oscillator { > clock-frequency = <26000000>; > }; > > + backlight { > + compatible = "pwm-backlight"; [Severity: High] Will this result in a hardware conflict if the legacy driver isn't actually removed in this patch? The commit message mentions this allows removing the legacy pandora_bl driver and pdata-quirks.c registration, but this commit doesn't remove or conditionally disable the legacy platform device registration. If CONFIG_BACKLIGHT_PANDORA is enabled, both the legacy pandora_bl driver and the new pwm-backlight driver will probe concurrently. Since pandora_bl writes directly to I2C registers and bypasses the PWM framework, this dual driver setup can cause race conditions, register corruption, and duplicate sysfs entries for the backlight. [Severity: High] Does switching to the generic driver cause the backlight to fail on startup? The legacy pandora_bl driver included a TPS61161 hardware quirk that forced a 100% duty cycle for 2-10ms at startup. Without this continuous >2.5ms high pulse, the TPS61161 enters 1-wire detection mode and disables PWM control. The generic pwm-backlight driver immediately applies the computed duty cycle on power-up, which lacks this required hardware initialization sequence. > + pwms = <&twl_pwm 0 1922710>; > + > + brightness-levels = <0 150 158 166 174 185 205 230 255>; [Severity: Medium] Is this artificially restricted minimum brightness of 150/255 (59%) a workaround for an unsupported PWM period? The device tree requests a period of 1922710ns (64-tick mode), but the pwm-twl driver unconditionally sets TWL_PWM0_ON to 1, forcing 128-tick mode (~3.9ms period). To prevent the longer off-times of this slower frequency from exceeding the 2.5ms threshold that triggers the TPS61161 1-wire mode, it appears the minimum brightness was raised to 59%. This seems to be a significant functional regression from the original 14% minimum brightness. > + default-brightness-level = <6>; > + }; > + > tv: connector { > compatible = "connector-analog-tv"; > label = "tv"; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=14
