All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Jiaxing Hu" <gahing@gahingwoo.com>
Cc: robh@kernel.org, conor+dt@kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v2 2/2] arm64: dts: rockchip: Add ArmSoM CM5 and CM5-IO
Date: Thu, 16 Jul 2026 07:12:15 +0000	[thread overview]
Message-ID: <20260716071216.3FE301F000E9@smtp.kernel.org> (raw)
In-Reply-To: <a86b8b22-6216-4926-a82f-9d4bd15cc397@smtp-relay.sendinblue.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The 25 MHz reference clock output (`clk_mac_refout`) for the crystal-less motorcomm YT8531 PHY will remain disabled because the `dwmac-rk` MAC driver ignores it for RGMII interfaces, leading to MDIO timeouts and a non-functional PHY.
- [Low] The ES8388 audio codec is claimed to be supported on I2C0 by the file header and block comments, but its node implementation is entirely missing.
- [Low] Use of deprecated 'gpio' property instead of 'gpios' in regulator-fixed node.
--

--- Patch [2]: [PATCH v2 2/2] arm64: dts: rockchip: Add ArmSoM CM5 and CM5-IO ---
commit e75134401985ec8dc82c1bb4f3f58610d1d03afd
Author: Jiaxing Hu <gahing@gahingwoo.com>

arm64: dts: rockchip: Add ArmSoM CM5 and CM5-IO

Add the ArmSoM CM5, an RK3576 compute module (rk3576-armsom-cm5.dtsi),
and its CM5-IO carrier (rk3576-armsom-cm5-io.dts).

> diff --git a/arch/arm64/boot/dts/rockchip/rk3576-armsom-cm5-io.dts b/arch/arm64/boot/dts/rockchip/rk3576-armsom-cm5-io.dts
> new file mode 100644
> index 0000000000000..79a903585197c
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3576-armsom-cm5-io.dts

[ ... ]

> +	/* PCIe 3.3 V for the M.2 slot, gated by GPIO0 PC3. */
> +	vcc_3v3_pcie: regulator-vcc-3v3-pcie {
> +		compatible = "regulator-fixed";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pcie_pwr_en>;
> +		regulator-name = "vcc_3v3_pcie";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		enable-active-high;
> +		gpio = <&gpio0 RK_PC3 GPIO_ACTIVE_HIGH>;

[Severity: Low]
Does this violate the regulator-fixed device tree binding schema? The schema
expects the gpios property instead of the deprecated gpio property, which may
trigger dtbs_check warnings.

> +		startup-delay-us = <5000>;
> +		vin-supply = <&vcc_5v0_sys>;
> +	};

[ ... ]

> +/*
> + * I2C0: ES8388 audio codec @ 0x10, FUSB302 USB-C PD controller @ 0x22.
> + * Pinctrl i2c0m1 (GPIO0 PC4 SCL / GPIO0 PC5 SDA).
> + */
> +&i2c0 {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c0m1_xfer>;
> +
> +	usbc0: usb-typec@22 {

[Severity: Low]
This isn't a bug, but the block comment mentions the ES8388 audio codec at
address 0x10, yet the node isn't implemented here. Could this cause confusion
for anyone attempting to use the audio functionality?

> +		compatible = "fcs,fusb302";
> +		reg = <0x22>;

[ ... ]

> diff --git a/arch/arm64/boot/dts/rockchip/rk3576-armsom-cm5.dtsi b/arch/arm64/boot/dts/rockchip/rk3576-armsom-cm5.dtsi
> new file mode 100644
> index 0000000000000..4612b7e1d6e8a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3576-armsom-cm5.dtsi

[ ... ]

> +/*
> + * GMAC0 drives an on-module MotorComm YT8531. The PHY has no local crystal, so
> + * it relies on the SoC 25 MHz reference: clk_mac_refout (REFCLKO25M_GMAC0_OUT)
> + * has to be named on the gmac node for dwmac-rk to enable it, and
> + * ethm0_clk0_25m_out routes it to the pad.
> + */
> +&gmac0 {
> +	/* The RX delay is added by the PHY, the TX delay by the MAC below. */
> +	phy-mode = "rgmii-rxid";
> +	clock_in_out = "output";
> +	clocks = <&cru CLK_GMAC0_125M_SRC>, <&cru CLK_GMAC0_RMII_CRU>,
> +		 <&cru PCLK_GMAC0>, <&cru ACLK_GMAC0>,
> +		 <&cru CLK_GMAC0_PTP_REF>, <&cru REFCLKO25M_GMAC0_OUT>;
> +	clock-names = "stmmaceth", "clk_mac_ref", "pclk_mac", "aclk_mac",
> +		      "ptp_ref", "clk_mac_refout";

[Severity: High]
Will the 25 MHz reference clock output remain disabled here? The dwmac-rk
driver (in drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c) only parses and
enables clk_mac_refout if the phy-mode is exactly RMII.

Since the mode is configured as rgmii-rxid, the MAC driver will ignore the
clock. Additionally, neither the generic PHY framework nor the motorcomm driver
automatically enable clocks specified in the PHY node before the MDIO bus scan,
which could lead to MDIO timeouts and a non-functional Ethernet interface.

> +	tx_delay = <0x21>;
> +	rx_delay = <0x00>;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716065548.2466198-1-gahing@gahingwoo.com?part=2

  reply	other threads:[~2026-07-16  7:12 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260716065548.2466198-1-gahing@gahingwoo.com>
2026-07-16  6:55 ` [PATCH v2 1/2] dt-bindings: arm: rockchip: Add ArmSoM CM5 and CM5-IO Jiaxing Hu
2026-07-16  6:55 ` Jiaxing Hu
2026-07-16  6:55 ` Jiaxing Hu
2026-07-16  6:55 ` Jiaxing Hu
2026-07-16  6:55 ` [PATCH v2 2/2] arm64: dts: " Jiaxing Hu
2026-07-16  6:55 ` Jiaxing Hu
2026-07-16  7:08   ` sashiko-bot
2026-07-16  6:55 ` Jiaxing Hu
2026-07-16  7:12   ` sashiko-bot [this message]
2026-07-16  6:55 ` Jiaxing Hu
2026-07-16  7:06   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260716071216.3FE301F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=gahing@gahingwoo.com \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.