[v1] arm64: dts: freescale: imx93-phycore-som: Delay the phy reset by a gpio

[PATCH] arm64: dts: freescale: imx93-phycore-som: Delay the phy reset by a gpio

Posted by Christoph Stoidner 7 months ago

According to the datasheet the phy needs to be held in reset until the
reference clock got stable. Even though no issue was observed, fix this
as the software should always comply with the specification.

Fix this in the bootloader, as this is the point where the reference
clock gets initialized and stable first.

Use gpio4 23, which is connected to the phy reset pin. On the same pin
RX_ER was used before, but this signal is optional and can be dropped.

Note: This comes into effect with the phyCOREs SOM hardware revision 4.
In revisions before, this gpio is not connected, and the phy reset is
managed with the global hardware reset circuit.

Signed-off-by: Christoph Stoidner <c.stoidner@phytec.de>
---
 arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi b/arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi
index 88c2657b50e6..f8e2f3f3baa8 100644
--- a/arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi
@@ -58,6 +58,9 @@ &fec {
 				 <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>,
 				 <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>;
 	assigned-clock-rates = <100000000>, <50000000>, <50000000>;
+	phy-reset-gpios = <&gpio4 23 GPIO_ACTIVE_HIGH>;
+	phy-reset-duration = <1>;
+	phy-reset-post-delay = <0>;
 	status = "okay";
 
 	mdio: mdio {
@@ -91,14 +94,16 @@ pinctrl_fec: fecgrp {
 		fsl,pins = <
 			MX93_PAD_ENET2_MDC__ENET1_MDC			0x50e
 			MX93_PAD_ENET2_MDIO__ENET1_MDIO			0x502
-			MX93_PAD_ENET2_RD0__ENET1_RGMII_RD0		0x57e
-			MX93_PAD_ENET2_RD1__ENET1_RGMII_RD1		0x57e
-			MX93_PAD_ENET2_RXC__ENET1_RX_ER			0x5fe
+			/* the three pins below are connected to PHYs straps,
+			 * that is what the pull-up/down setting is for. */
+			MX93_PAD_ENET2_RD0__ENET1_RGMII_RD0		0x37e
+			MX93_PAD_ENET2_RD1__ENET1_RGMII_RD1		0x37e
 			MX93_PAD_ENET2_RX_CTL__ENET1_RGMII_RX_CTL	0x57e
 			MX93_PAD_ENET2_TD0__ENET1_RGMII_TD0		0x50e
 			MX93_PAD_ENET2_TD1__ENET1_RGMII_TD1		0x50e
 			MX93_PAD_ENET2_TX_CTL__ENET1_RGMII_TX_CTL	0x50e
 			MX93_PAD_ENET2_TD2__ENET1_TX_CLK		0x4000050e
+			MX93_PAD_ENET2_RXC__GPIO4_IO23			0x51e
 		>;
 	};
 
-- 
2.43.0

Re: [Upstream] [PATCH] arm64: dts: freescale: imx93-phycore-som: Delay the phy reset by a gpio

Posted by Primoz Fiser 7 months ago

Hi Christoph,

On 20. 05. 25 09:34, Christoph Stoidner wrote:
> According to the datasheet the phy needs to be held in reset until the
> reference clock got stable. Even though no issue was observed, fix this
> as the software should always comply with the specification.
> 
> Fix this in the bootloader, as this is the point where the reference
> clock gets initialized and stable first.

I would remove this paragraph about the "Fix this in the bootloader..."

Doesn't patch apply to both the kernel and the bootloader FEC driver?

> 
> Use gpio4 23, which is connected to the phy reset pin. On the same pin
> RX_ER was used before, but this signal is optional and can be dropped.
> 
> Note: This comes into effect with the phyCOREs SOM hardware revision 4.
> In revisions before, this gpio is not connected, and the phy reset is
> managed with the global hardware reset circuit.
> 
> Signed-off-by: Christoph Stoidner <c.stoidner@phytec.de>
> ---
>  arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi b/arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi
> index 88c2657b50e6..f8e2f3f3baa8 100644
> --- a/arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi
> @@ -58,6 +58,9 @@ &fec {
>  				 <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>,
>  				 <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>;
>  	assigned-clock-rates = <100000000>, <50000000>, <50000000>;
> +	phy-reset-gpios = <&gpio4 23 GPIO_ACTIVE_HIGH>;
> +	phy-reset-duration = <1>;
> +	phy-reset-post-delay = <0>;
>  	status = "okay";
>  
>  	mdio: mdio {
> @@ -91,14 +94,16 @@ pinctrl_fec: fecgrp {
>  		fsl,pins = <
>  			MX93_PAD_ENET2_MDC__ENET1_MDC			0x50e
>  			MX93_PAD_ENET2_MDIO__ENET1_MDIO			0x502
> -			MX93_PAD_ENET2_RD0__ENET1_RGMII_RD0		0x57e
> -			MX93_PAD_ENET2_RD1__ENET1_RGMII_RD1		0x57e
> -			MX93_PAD_ENET2_RXC__ENET1_RX_ER			0x5fe
> +			/* the three pins below are connected to PHYs straps,
> +			 * that is what the pull-up/down setting is for. */

I would remove this comment and maybe move it to the commit msg why are
you changing the PD/PU configuration.

Anyway, if you decide to keep it, you need to fix the following warning:

WARNING: Block comments use a trailing */ on a separate line
#46: FILE: arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi:98:
+                        * that is what the pull-up/down setting is for. */

BR,
Primoz

> +			MX93_PAD_ENET2_RD0__ENET1_RGMII_RD0		0x37e
> +			MX93_PAD_ENET2_RD1__ENET1_RGMII_RD1		0x37e
>  			MX93_PAD_ENET2_RX_CTL__ENET1_RGMII_RX_CTL	0x57e
>  			MX93_PAD_ENET2_TD0__ENET1_RGMII_TD0		0x50e
>  			MX93_PAD_ENET2_TD1__ENET1_RGMII_TD1		0x50e
>  			MX93_PAD_ENET2_TX_CTL__ENET1_RGMII_TX_CTL	0x50e
>  			MX93_PAD_ENET2_TD2__ENET1_TX_CLK		0x4000050e
> +			MX93_PAD_ENET2_RXC__GPIO4_IO23			0x51e
>  		>;
>  	};
>  

-- 
Primoz Fiser
phone: +386-41-390-545
email: primoz.fiser@norik.com
--
Norik systems d.o.o.
Your embedded software partner
Slovenia, EU
phone: +386-41-540-545
email: info@norik.com

Re: [Upstream] [PATCH] arm64: dts: freescale: imx93-phycore-som: Delay the phy reset by a gpio

Posted by Christoph Stoidner 7 months ago

Hi Primoz,

On Di, 2025-05-20 at 09:56 +0200, Primoz Fiser wrote:
> Hi Christoph,
> 
> On 20. 05. 25 09:34, Christoph Stoidner wrote:
> > According to the datasheet the phy needs to be held in reset until
> > the
> > reference clock got stable. Even though no issue was observed, fix
> > this
> > as the software should always comply with the specification.
> > 
> > Fix this in the bootloader, as this is the point where the
> > reference
> > clock gets initialized and stable first.
> 
> I would remove this paragraph about the "Fix this in the
> bootloader..."
> 
> Doesn't patch apply to both the kernel and the bootloader FEC driver?

You are right. I missed that the kernel driver also has implemented
that phy-reset-gpios handling. I will remove that paragraph in a v2.

> 
> > 
> > Use gpio4 23, which is connected to the phy reset pin. On the same
> > pin
> > RX_ER was used before, but this signal is optional and can be
> > dropped.
> > 
> > Note: This comes into effect with the phyCOREs SOM hardware
> > revision 4.
> > In revisions before, this gpio is not connected, and the phy reset
> > is
> > managed with the global hardware reset circuit.
> > 
> > Signed-off-by: Christoph Stoidner <c.stoidner@phytec.de>
> > ---
> >  arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi | 11
> > ++++++++---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi
> > b/arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi
> > index 88c2657b50e6..f8e2f3f3baa8 100644
> > --- a/arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi
> > @@ -58,6 +58,9 @@ &fec {
> >   <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>,
> >   <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>;
> >   assigned-clock-rates = <100000000>, <50000000>, <50000000>;
> > + phy-reset-gpios = <&gpio4 23 GPIO_ACTIVE_HIGH>;
> > + phy-reset-duration = <1>;
> > + phy-reset-post-delay = <0>;
> >   status = "okay";
> >  
> >   mdio: mdio {
> > @@ -91,14 +94,16 @@ pinctrl_fec: fecgrp {
> >   fsl,pins = <
> >   MX93_PAD_ENET2_MDC__ENET1_MDC 0x50e
> >   MX93_PAD_ENET2_MDIO__ENET1_MDIO 0x502
> > - MX93_PAD_ENET2_RD0__ENET1_RGMII_RD0 0x57e
> > - MX93_PAD_ENET2_RD1__ENET1_RGMII_RD1 0x57e
> > - MX93_PAD_ENET2_RXC__ENET1_RX_ER 0x5fe
> > + /* the three pins below are connected to PHYs straps,
> > + * that is what the pull-up/down setting is for. */
> 
> I would remove this comment and maybe move it to the commit msg why
> are
> you changing the PD/PU configuration.

I think it is important to know why these PD/PU are configured, because
for the normal MDIO functionality PU/PD is not needed. And it is not
self explaining here, that changing those PD/PU might break phy config.

However I will of course fix the warning in a v2.

Thanks,
Christoph

> 
> Anyway, if you decide to keep it, you need to fix the following
> warning:
> 
> WARNING: Block comments use a trailing */ on a separate line
> #46: FILE: arch/arm64/boot/dts/freescale/imx93-phycore-som.dtsi:98:
> +                        * that is what the pull-up/down setting is
> for. */
> 
> BR,
> Primoz
> 
> > + MX93_PAD_ENET2_RD0__ENET1_RGMII_RD0 0x37e
> > + MX93_PAD_ENET2_RD1__ENET1_RGMII_RD1 0x37e
> >   MX93_PAD_ENET2_RX_CTL__ENET1_RGMII_RX_CTL 0x57e
> >   MX93_PAD_ENET2_TD0__ENET1_RGMII_TD0 0x50e
> >   MX93_PAD_ENET2_TD1__ENET1_RGMII_TD1 0x50e
> >   MX93_PAD_ENET2_TX_CTL__ENET1_RGMII_TX_CTL 0x50e
> >   MX93_PAD_ENET2_TD2__ENET1_TX_CLK 0x4000050e
> > + MX93_PAD_ENET2_RXC__GPIO4_IO23 0x51e
> >   >;
> >   };
> >  
>