Re: [PATCH v5 3/3] arm64: dts: imx8qm-var-som: Add support for Variscite Symphony board

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Ethernet PHY ethphy1 is reset while its power supply is off, leaving it un-reset after power-up.
--

--- Patch [3]: [PATCH v5 3/3] arm64: dts: imx8qm-var-som: Add support for Variscite Symphony board ---
commit ed29f608ccf1d279621863c6ecef72b6e1b3be60
Author: Stefano Radaelli <stefano.r@variscite.com>

arm64: dts: imx8qm-var-som: Add support for Variscite Symphony board

This commit adds device tree support for the Variscite Symphony carrier board
equipped with the VAR-SOM-MX8 QuadMax system on module. It defines nodes and
pinmuxing for various peripherals including USB, Ethernet, SD card, I2C, and
PCIe.

Link: https://variscite.com/carrier-boards/symphony-board/

> diff --git a/arch/arm64/boot/dts/freescale/imx8qm-var-som-symphony.dts b/arch/arm64/boot/dts/freescale/imx8qm-var-som-symphony.dts
> new file mode 100644
> index 0000000000000..7c7fc8958efc0
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8qm-var-som-symphony.dts

[ ... ]

> +&fec1 {
> +	mdio {
> +		ethphy1: ethernet-phy@5 {
> +			compatible = "ethernet-phy-ieee802.3-c22";
> +			reg = <5>;
> +			vddio-supply = <&reg_phy_vddio>;
> +			reset-gpios = <&pca9534 5 GPIO_ACTIVE_LOW>;
> +			reset-assert-us = <10000>;
> +			reset-deassert-us = <100000>;

[Severity: High]
Does this sequence cause the PHY to be reset while its power is still off?

When fec1 probes and registers the MDIO bus, the MDIO core toggles the
reset-gpios for this PHY. However, the main power supply (reg_fec_phy) might
not be turned on yet because it's assigned to fec2 below.

[ ... ]

> +&fec2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_fec2>;
> +	/*
> +	 * The required RGMII TX and RX 2ns delays are implemented directly
> +	 * in hardware via passive delay elements on the SOM PCB.
> +	 * No delay configuration is needed in software via PHY driver.
> +	 */
> +	phy-mode = "rgmii";
> +	phy-handle = <&ethphy1>;
> +	phy-supply = <&reg_fec_phy>;

[Severity: High]
Should phy-supply be placed in the ethphy1 node rather than the fec2 node?

If phy-supply is here in the MAC node, power is only turned on when fec2
probes. This leaves the PHY un-reset after power-up, because the MDIO reset
already happened earlier during the fec1 probe.

This usually prevents the PHY from communicating over MDIO or establishing a
link.

> +	status = "okay";
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784142385.git.stefano.r@variscite.com?part=3