Re: [PATCH v2 7/9] ARM: tegra: lg-x3: Add backlight LED controller node

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] driver core: ti,lm3533 unconditional probe failure without platform data
- [High] gpio: lm3533-core requests unnamed GPIO while DT provides enable-gpios
- [Medium] dts: tegra30-lg-x3: undocumented addition of unrelated hardware nodes
--

commit ff4e61027a1ca9aeb5769dcf34e03e97e0cb0683
Author: Svyatoslav Ryhel <clamor95@gmail.com>

    ARM: tegra: lg-x3: Add backlight LED controller node
    
    Add node for TI LM3533 LED controller used in this device for panel
    backlight.

>     ARM: tegra: lg-x3: Add backlight LED controller node
>    
>     Add node for TI LM3533 LED controller used in this device for panel
>     backlight.

[Severity: Medium]
Does this patch also silently introduce other hardware modifications, such as
a light sensor (avago,apds9900), haptic engine (imagis,isa1200), camera
flash LEDs (ti,lm3559), and critical power management parameters like
nvidia,suspend-mode = <1>?

The commit message explicitly claims the sole purpose of the patch is to add
the backlight LED controller node. Including unrelated changes hides potential
regressions.

> diff --git a/arch/arm/boot/dts/nvidia/tegra30-lg-p880.dts b/arch/arm/boot/dts/nvidia/tegra30-lg-p880.dts
> index f888987ce1340..3a5e66d869b1b 100644
> --- a/arch/arm/boot/dts/nvidia/tegra30-lg-p880.dts
> +++ b/arch/arm/boot/dts/nvidia/tegra30-lg-p880.dts
> @@ -155,6 +155,8 @@ panel@1 {
>  				vdd-supply = <&vcc_3v0_lcd>;
>  				vddio-supply = <&iovcc_1v8_lcd>;
>  
> +				backlight = <&backlight>;
> +

[Severity: High]
Since the lm3533-core.c driver lacks Device Tree parsing and explicitly
requires platform data, won't lm3533_device_init() immediately fail with
-EINVAL?

drivers/mfd/lm3533-core.c:lm3533_device_init() {
    ...
    if (!pdata) {
        dev_err(lm3533->dev, "no platform data\n");
        return -EINVAL;
    }
    ...
}

Because the backlight device never registers, will the panel driver
indefinitely wait for it via drm_panel_of_backlight(), returning
-EPROBE_DEFER and permanently breaking the primary display initialization?

[ ... ]

> diff --git a/arch/arm/boot/dts/nvidia/tegra30-lg-x3.dtsi b/arch/arm/boot/dts/nvidia/tegra30-lg-x3.dtsi
> index 1f10f4bfb5f24..2dbef0185a0b8 100644
> --- a/arch/arm/boot/dts/nvidia/tegra30-lg-x3.dtsi
> +++ b/arch/arm/boot/dts/nvidia/tegra30-lg-x3.dtsi
> @@ -1170,6 +1170,30 @@ rmi4-f11@11 {
>  			};
>  		};
>  
> +		led-controller@36 {
> +			compatible = "ti,lm3533";
> +			reg = <0x36>;
> +
> +			enable-gpios = <&gpio TEGRA_GPIO(N, 6) GPIO_ACTIVE_HIGH>;

[Severity: High]
The device tree node specifies enable-gpios, but the driver acquires the GPIO
using a NULL connection ID:

drivers/mfd/lm3533-core.c:lm3533_device_init() {
    ...
    lm3533->hwen = devm_gpiod_get(lm3533->dev, NULL, GPIOD_OUT_LOW);
    ...
}

With a NULL con_id, the GPIO subsystem only searches for properties named
'gpios' or 'gpio'. Will this fail to match 'enable-gpios' and cause the driver
to fail probe with -ENOENT?

> +			vin-supply = <&vdd_3v3_vbat>;
> +
> +			ti,boost-ovp-microvolt = <24000000>;
> +			ti,boost-freq-hz = <500000>;
> +
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			backlight: backlight@0 {
> +				compatible = "ti,lm3533-backlight";
> +				reg = <0>;
> +
> +				led-max-microamp = <23400>;
> +				led-sources = <0 1>;
> +
> +				ti,linear-mapping-mode;
> +			};
> +		};
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261004154456.145362-1-clamor95@gmail.com?part=7