Re: [PATCH V4 4/4] arm64: dts: freescale: Add chargebyte Charge Control Y

[Stefan Wahren <wahrenst@gmx.net>]

Hi,

Am 03.09.26 um 10:21 schrieb sashiko-bot@kernel.org:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Medium] Mismatched drive strengths on USDHC2 data lines within the same interface speed mode.
> - [Medium] The reset GPIO polarity for the Ethernet PHY (DP83825) is configured as active-high, but the PHY natively requires an active-low reset.
> - [Low] Incorrect node name used in `/delete-node/` for heartbeat LED pinctrl.
> --
>
> commit 228da9fe4e969182ceef8105169e14b5de5949a5
> Author: Stefan Wahren <wahrenst@gmx.net>
>
> arm64: dts: freescale: Add chargebyte Charge Control Y
>
> This adds device tree support for the chargebyte Charge Control Y, an
> Electrical Vehicle Supply Equipment (EVSE) based on the phyCORE-i.MX 93
> SOM.
>
>> diff --git a/arch/arm64/boot/dts/freescale/imx93-charge-control-y.dts b/arch/arm64/boot/dts/freescale/imx93-charge-control-y.dts
>> new file mode 100644
>> index 0000000000000..ba240f528f9b9
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/freescale/imx93-charge-control-y.dts
> [ ... ]
>> +/* Ethernet 2 PHY */
>> +&mdio {
>> +	ethphy2: ethernet-phy@2 {
>> +		reg = <2>;
>> +		compatible = "ethernet-phy-id2000.a140";
>> +		clocks = <&clk IMX93_CLK_ENET_REF_PHY>;
>> +		reset-gpios = <&gpio4 13 GPIO_ACTIVE_HIGH>;
> [Severity: Medium]
> Will this hold the Ethernet PHY in reset? The DP83825 PHY natively expects
> an active-low reset signal. By specifying GPIO_ACTIVE_HIGH here, the kernel
> will assert reset by driving the GPIO high, and normal operation will drive
> it low. This could potentially keep the PHY in reset permanently, preventing
> Ethernet link establishment, unless there is a physical hardware inverter
> between the GPIO and the PHY.
Ethernet-PHY and reset signal are not wired directly. There is a 
transistor, which negative the signal.
>
>> +		reset-assert-us = <30>;
>> +		reset-deassert-us = <50000>;
>> +	};
>> +};
> [ ... ]
>> +&iomuxc {
>> +
>> +	// Heartbeat LED of phyCore is not populated
>> +	/delete-node/ pinctrl_leds;
> [Severity: Low]
> Will the device tree compiler process this delete-node directive correctly?
> In imx93-phycore-som.dtsi, the node is defined with the name ledsgrp and
> the label pinctrl_leds.
>
> The delete-node syntax expects the exact node name, like:
> 	/delete-node/ ledsgrp;
>
> Or alternatively, using the phandle label at the root level:
> /delete-node/ &pinctrl_leds;
>
> Using just the label name inside &iomuxc might result in a "Node not found"
> compiler error or a silent failure that leaves unused pinmux data in the
> binary.
Nice catch. I'm surprised why make dtbs_check doesn't find it. I will 
fix it.
>
>> +
>> +	pinctrl_eqos: eqosgrp {
> [ ... ]
>> +	/* need to config the SION for data and cmd pad, refer to ERR052021 */
>> +	pinctrl_usdhc2_default: usdhc2grp {
>> +		fsl,pins = <
>> +			MX93_PAD_SD2_CLK__USDHC2_CLK		0x119e
>> +			MX93_PAD_SD2_CMD__USDHC2_CMD		0x4000139e
>> +			MX93_PAD_SD2_DATA0__USDHC2_DATA0	0x4000138e
>> +			MX93_PAD_SD2_DATA1__USDHC2_DATA1	0x4000138e
>> +			MX93_PAD_SD2_DATA2__USDHC2_DATA2	0x4000138e
>> +			MX93_PAD_SD2_DATA3__USDHC2_DATA3	0x4000139e
> [Severity: Medium]
> Are these drive strength settings intended to be different across the data
> lines?
Yes, DSE can be optimized for electromagnetic compatibility

Best regards
>
> In this default mode configuration, DATA0-2 use 0x4000138e (DSE bit 4 = 0),
> while DATA3 uses 0x4000139e (DSE bit 4 = 1). Mismatched drive strengths on
> the same SDIO bus can introduce signal skew and degrade signal integrity.
>
>> +			MX93_PAD_SD2_VSELECT__USDHC2_VSELECT	0x51e
>> +		>;
>> +	};
>> +
>> +	/* need to config the SION for data and cmd pad, refer to ERR052021 */
>> +	pinctrl_usdhc2_100mhz: usdhc2-100mhzgrp {
>> +		fsl,pins = <
>> +			MX93_PAD_SD2_CLK__USDHC2_CLK		0x119e
>> +			MX93_PAD_SD2_CMD__USDHC2_CMD		0x4000139e
>> +			MX93_PAD_SD2_DATA0__USDHC2_DATA0	0x4000138e
>> +			MX93_PAD_SD2_DATA1__USDHC2_DATA1	0x4000138e
>> +			MX93_PAD_SD2_DATA2__USDHC2_DATA2	0x4000139e
>> +			MX93_PAD_SD2_DATA3__USDHC2_DATA3	0x4000139e
> [Severity: Medium]
> Similarly, in the 100mhz configuration, DATA0-1 are set to 0x4000138e and
> DATA2-3 are set to 0x4000139e. Could this mismatch cause high-frequency
> enumeration or data corruption issues on the SD bus?
>
>> +			MX93_PAD_SD2_VSELECT__USDHC2_VSELECT	0x51e
>> +		>;
>> +	};


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy