Re: [PATCH v4 7/7] riscv: dts: ultrarisc: Add Milk-V Titan board device tree

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] riscv: dts: ultrarisc: deprecated LED label overrides color and function
--

commit b602c792a6efba56c2935012c89e05f8d6efaf40
Author: Jia Wang <wangjia@ultrarisc.com>

riscv: dts: ultrarisc: Add Milk-V Titan board device tree

This commit adds the device tree and pinctrl definitions for the Milk-V
Titan ITX motherboard based on the UltraRISC DP1000 SoC.

Link: https://milkv.io/docs/titan/overview

> diff --git a/arch/riscv/boot/dts/ultrarisc/dp1000-milkv-titan.dts b/arch/riscv/boot/dts/ultrarisc/dp1000-milkv-titan.dts
> new file mode 100644
> index 0000000000000..59edd8157221a
> --- /dev/null
> +++ b/arch/riscv/boot/dts/ultrarisc/dp1000-milkv-titan.dts
> @@ -0,0 +1,203 @@

[ ... ]

> +&ethernet {
> +	phy-handle = <&phy0>;
> +	/*
> +	 * The PCB does not add RGMII TX or RX clock delays. The DP1000
> +	 * integration adds fixed delays in both directions.
> +	 */
> +	phy-mode = "rgmii-id";
> +
> +	status = "okay";
> +
> +	mdio {
> +		compatible = "snps,dwmac-mdio";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		phy0: phy@0 {
> +			reg = <0>;
> +
> +			leds {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				led@1 {
> +					label = "eth-link";
> +					reg = <1>;
> +					color = <LED_COLOR_ID_YELLOW>;
> +					function = LED_FUNCTION_INDICATOR;
> +					default-state = "keep";
> +					linux,default-trigger = "netdev";
> +				};
> +
> +				led@2 {
> +					label = "eth-activity";
> +					reg = <2>;
> +					color = <LED_COLOR_ID_GREEN>;
> +					function = LED_FUNCTION_ACTIVITY;
> +					default-state = "keep";
> +					linux,default-trigger = "netdev";
> +				};
> +			};
> +		};
> +	};
> +};

[Severity: Medium]
Should the label properties be dropped from led@1 and led@2 here?

When the PHY driver probes phy0, the call chain:

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

checks for the deprecated label property first in
drivers/leds/led-core.c:led_parse_fwnode_props():

	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 led_parse_fwnode_props() returns early when label is present, the
color and function properties on led@1 and led@2 are never parsed and are
completely ignored at runtime.

In addition, Documentation/devicetree/bindings/leds/common.yaml deprecates
label for new bindings in favor of color and function.

Would it make sense to remove label from both nodes so that the LED core
uses color and function to compose the standard LED names?

完美!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019-ultrarisc-dts-v4-0-3979747cda2e@ultrarisc.com?part=7