Re: [PATCH 2/2] riscv: dts: spacemit: k3: Initial support for CoM260-IFX board

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The PMIC node is incorrectly named `p1@41` instead of using a generic name.
- [Low] The pinctrl node for `gmac1` defines an unused label `gmac1_base_pins:` and uses an inconsistent subnode name compared to `gmac0`.
--

commit be83cb21ec8e4b52ccd315661d69460e85196cef
Author: Yixun Lan <dlan@kernel.org>

riscv: dts: spacemit: k3: Initial support for CoM260-IFX board

This commit introduces initial device tree support for the K3 CoM260-IFX
board, which combines a 260-pin compute module with a carrier board. It
enables basic features including Serial UART, ethernet, and the onboard PMIC.

> diff --git a/arch/riscv/boot/dts/spacemit/k3-com260.dtsi b/arch/riscv/boot/dts/spacemit/k3-com260.dtsi
> new file mode 100644
> index 0000000000000..1801a44d43559
> --- /dev/null
> +++ b/arch/riscv/boot/dts/spacemit/k3-com260.dtsi
[ ... ]
> +&i2c8 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c8_cfg>;
> +	status = "okay";
> +
> +	p1@41 {

[Severity: Low]
This isn't a bug, but according to the Devicetree specification, node names
should be generic and reflect the function of the device rather than the chip
name. Could this be named pmic@41 instead of p1@41?

> +		compatible = "spacemit,p1";
> +		reg = <0x41>;
> +		interrupts = <64 IRQ_TYPE_LEVEL_HIGH>;
[ ... ]
> diff --git a/arch/riscv/boot/dts/spacemit/k3-pinctrl.dtsi b/arch/riscv/boot/dts/spacemit/k3-pinctrl.dtsi
> index 23899d3f308a0..04efe64b8a3a2 100644
> --- a/arch/riscv/boot/dts/spacemit/k3-pinctrl.dtsi
> +++ b/arch/riscv/boot/dts/spacemit/k3-pinctrl.dtsi
[ ... ]
> @@ -45,6 +45,39 @@ gmac0-phy-0-pins {
>  		};
>  	};
>  
> +	gmac1_rgmii_0_cfg: gmac1-rgmii-0-cfg {
> +		gmac1_base_pins: gmac1-0-pins {

[Severity: Low]
This isn't a bug, but the gmac1_base_pins label appears to be unused.

Also, is there a reason this subnode is named gmac1-0-pins instead of
following the gmac0-rgmii-0-pins naming convention seen elsewhere?

> +			pinmux = <K3_PADCONF(21, 1)>,	/* gmac1_rxdv  */
> +				 <K3_PADCONF(22, 1)>,	/* gmac1 rx d0 */
> +				 <K3_PADCONF(23, 1)>,	/* gmac1 rx d1 */
> +				 <K3_PADCONF(24, 1)>,	/* gmac1 rx_clk */
> +				 <K3_PADCONF(25, 1)>,	/* gmac1 rx d2 */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520-02-k3-com260-ifx-v1-0-3e17055dd488@kernel.org?part=2