Re: [PATCH 6/6] arm64: dts: qcom: sm8250-hdk: Add the 2 LEDs of PM8150b

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] arm64: dts: qcom: sm8250-hdk: deprecated LED label overrides color in sysfs
--

commit 8610d8172ae62463a8325cbe16b011b5f8fa6fc1
Author: redstonee <1516235445@qq.com>

arm64: dts: qcom: sm8250-hdk: Add the 2 LEDs of PM8150b

This commit enables the pm8150b lpg controller on sm8250-hdk and adds two
child nodes for the green heartbeat and red wlan indicator leds.

> diff --git a/arch/arm64/boot/dts/qcom/sm8250-hdk.dts b/arch/arm64/boot/dts/qcom/sm8250-hdk.dts
> index eb899c92fcfb7..d15e75a4ace5c 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250-hdk.dts
> +++ b/arch/arm64/boot/dts/qcom/sm8250-hdk.dts
> @@ -7,6 +7,7 @@
>  
>  #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
>  #include <dt-bindings/usb/pd.h>
> +#include <dt-bindings/leds/common.h>
>  #include "sm8250.dtsi"
>  #include "pm8150.dtsi"
>  #include "pm8150b.dtsi"
> @@ -157,6 +158,25 @@ vreg_pmu_pcie_1p8: ldo9 {
>  	};
>  };
>  
> +&pm8150b_lpg {
> +	status = "okay";
> +
> +	led@1 {
> +		reg = <1>;
> +		label = "led-heartbeat";
> +		linux,default-trigger = "heartbeat";
> +		color = <LED_COLOR_ID_GREEN>;
> +	};
> +
> +	led@2 {
> +		reg = <2>;
> +		label = "led-wlan";
> +		linux,default-trigger = "none";
> +		default-state = "off";
> +		color = <LED_COLOR_ID_RED>;
> +	};
> +};

[Severity: Medium]
When pm8150b_lpg probes in drivers/leds/rgb/leds-qcom-lpg.c, registering
led@1 and led@2 calls:

  devm_led_classdev_register_ext() ->
    led_classdev_register_ext() ->
      led_compose_name() ->
        led_parse_fwnode_props()

In drivers/leds/led-core.c:led_parse_fwnode_props(), the presence of the
deprecated label property causes an early return before color or function
are parsed:

	if (fwnode_property_present(fwnode, "label")) {
		ret = fwnode_property_read_string(fwnode, "label", &props->label);
		if (ret)
			dev_err(dev, "Error parsing 'label' property (%d)\n", ret);
		return;
	}

Because label is set on both nodes in
arch/arm64/boot/dts/qcom/sm8250-hdk.dts, color = <LED_COLOR_ID_GREEN> and
color = <LED_COLOR_ID_RED> are ignored at runtime, and the devices are
registered in sysfs as /sys/class/leds/led-heartbeat and
/sys/class/leds/led-wlan instead of /sys/class/leds/green:heartbeat and
/sys/class/leds/red:wlan.

Since label is also deprecated in
Documentation/devicetree/bindings/leds/common.yaml and
dt-bindings/leds/common.h is already included, should led@1 and led@2 use
the function property instead of label:

	led@1 {
		reg = <1>;
		function = LED_FUNCTION_HEARTBEAT;
		linux,default-trigger = "heartbeat";
		color = <LED_COLOR_ID_GREEN>;
	};

	led@2 {
		reg = <2>;
		function = LED_FUNCTION_WLAN;
		linux,default-trigger = "none";
		default-state = "off";
		color = <LED_COLOR_ID_RED>;
	};

so that the color and function properties take effect?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022113601.2320940-1-dc_redstone@outlook.com?part=6