2023-04-27 20:06:27

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH] imx8mn-var-som: dts: fix PHY detection bug by adding deassert delay

Hi Hugo,

On Thu, Apr 27, 2023 at 4:56 PM Hugo Villeneuve <[email protected]> wrote:
>
> From: Hugo Villeneuve <[email protected]>
>
> While testing the ethernet interface on a symphony carrier
> board using an imx8mn SOM with an onboard PHY (EC hardware
> configuration), the ethernet PHY is not detected.
>
> The device tree in Variscite custom linux git repository uses the
> following property:
>
> phy-reset-post-delay = <20>;
>
> Add a new property 'reset-deassert-us' of 20ms to have the same delay
> inside the ethphy handle.

Which Ethernet PHY does this board use?

What does its datasheet recommend?


2023-04-27 20:13:29

by Hugo Villeneuve

[permalink] [raw]
Subject: Re: [PATCH] imx8mn-var-som: dts: fix PHY detection bug by adding deassert delay

On Thu, 27 Apr 2023 17:00:41 -0300
Fabio Estevam <[email protected]> wrote:

> Hi Hugo,
>
> On Thu, Apr 27, 2023 at 4:56 PM Hugo Villeneuve <[email protected]> wrote:
> >
> > From: Hugo Villeneuve <[email protected]>
> >
> > While testing the ethernet interface on a symphony carrier
> > board using an imx8mn SOM with an onboard PHY (EC hardware
> > configuration), the ethernet PHY is not detected.
> >
> > The device tree in Variscite custom linux git repository uses the
> > following property:
> >
> > phy-reset-post-delay = <20>;
> >
> > Add a new property 'reset-deassert-us' of 20ms to have the same delay
> > inside the ethphy handle.
>
> Which Ethernet PHY does this board use?
>
> What does its datasheet recommend?

Hi Fabio,
it uses a ADIN1300 PHY.

The datasheet indicate that the "Management interface active (t4)" state is reached at most 5ms after the reset signal is deasserted.

Hugo.

--
Hugo Villeneuve <[email protected]>

2023-04-27 20:15:12

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH] imx8mn-var-som: dts: fix PHY detection bug by adding deassert delay

On Thu, Apr 27, 2023 at 5:06 PM Hugo Villeneuve <[email protected]> wrote:

> Hi Fabio,
> it uses a ADIN1300 PHY.
>
> The datasheet indicate that the "Management interface active (t4)" state is reached at most 5ms after the reset signal is deasserted.

Please add this information to the commit log and please add a Fixes: tag.

Thanks

2023-04-27 20:21:21

by Hugo Villeneuve

[permalink] [raw]
Subject: Re: [PATCH] imx8mn-var-som: dts: fix PHY detection bug by adding deassert delay

On Thu, 27 Apr 2023 17:07:59 -0300
Fabio Estevam <[email protected]> wrote:

> On Thu, Apr 27, 2023 at 5:06 PM Hugo Villeneuve <[email protected]> wrote:
>
> > Hi Fabio,
> > it uses a ADIN1300 PHY.
> >
> > The datasheet indicate that the "Management interface active (t4)" state is reached at most 5ms after the reset signal is deasserted.
>
> Please add this information to the commit log and please add a Fixes: tag.

Good idea, will do.

Hugo.


--
Hugo Villeneuve <[email protected]>

2023-04-27 20:29:51

by Hugo Villeneuve

[permalink] [raw]
Subject: Re: [PATCH] imx8mn-var-som: dts: fix PHY detection bug by adding deassert delay

On Thu, 27 Apr 2023 17:07:59 -0300
Fabio Estevam <[email protected]> wrote:

> On Thu, Apr 27, 2023 at 5:06 PM Hugo Villeneuve <[email protected]> wrote:
>
> > Hi Fabio,
> > it uses a ADIN1300 PHY.
> >
> > The datasheet indicate that the "Management interface active (t4)" state is reached at most 5ms after the reset signal is deasserted.
>
> Please add this information to the commit log and please add a Fixes: tag.
>
> Thanks

Hi,
I am trying to properly add a "Fixes: " tag, but the description for this tag indicates that it is to report that "the patch fixes an issue in a previous commit".

In this case, I cannot identify a commit that introduced that bug, apart from the initial commit of the DTS file which didn't have the reset property present?

Thank you,
Hugo.

--
Hugo Villeneuve <[email protected]>

2023-04-27 20:53:03

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH] imx8mn-var-som: dts: fix PHY detection bug by adding deassert delay

On Thu, Apr 27, 2023 at 5:22 PM Hugo Villeneuve <[email protected]> wrote:

> Hi,
> I am trying to properly add a "Fixes: " tag, but the description for this tag indicates that it is to report that "the patch fixes an issue in a previous commit".
>
> In this case, I cannot identify a commit that introduced that bug, apart from the initial commit of the DTS file which didn't have the reset property present?

You can add:

Fixes: ade0176dd8a0 ("arm64: dts: imx8mn-var-som: Add Variscite
VAR-SOM-MX8MN System on Module")

2023-04-27 21:04:43

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH] imx8mn-var-som: dts: fix PHY detection bug by adding deassert delay

On Thu, Apr 27, 2023 at 04:22:51PM -0400, Hugo Villeneuve wrote:
> On Thu, 27 Apr 2023 17:07:59 -0300
> Fabio Estevam <[email protected]> wrote:
>
> > On Thu, Apr 27, 2023 at 5:06 PM Hugo Villeneuve <[email protected]> wrote:
> >
> > > Hi Fabio,
> > > it uses a ADIN1300 PHY.
> > >
> > > The datasheet indicate that the "Management interface active (t4)" state is reached at most 5ms after the reset signal is deasserted.
> >
> > Please add this information to the commit log and please add a Fixes: tag.
> >
> > Thanks
>
> Hi,
> I am trying to properly add a "Fixes: " tag, but the description for this tag indicates that it is to report that "the patch fixes an issue in a previous commit".
>
> In this case, I cannot identify a commit that introduced that bug, apart from the initial commit of the DTS file which didn't have the reset property present?

Is the PHY on the SOM or the carrier?

If the PHY is on the carrier, then the delay is a carrier property,
and should be in the carrier .dts file. So use the commit for when the
carrier DTS file was added.

If the PHY is on the SOM, then use the commit for when the SOM DTSI
was added.

Andrew

2023-04-27 21:31:31

by Hugo Villeneuve

[permalink] [raw]
Subject: Re: [PATCH] imx8mn-var-som: dts: fix PHY detection bug by adding deassert delay

On Thu, 27 Apr 2023 22:56:40 +0200
Andrew Lunn <[email protected]> wrote:

> On Thu, Apr 27, 2023 at 04:22:51PM -0400, Hugo Villeneuve wrote:
> > On Thu, 27 Apr 2023 17:07:59 -0300
> > Fabio Estevam <[email protected]> wrote:
> >
> > > On Thu, Apr 27, 2023 at 5:06 PM Hugo Villeneuve <[email protected]> wrote:
> > >
> > > > Hi Fabio,
> > > > it uses a ADIN1300 PHY.
> > > >
> > > > The datasheet indicate that the "Management interface active (t4)" state is reached at most 5ms after the reset signal is deasserted.
> > >
> > > Please add this information to the commit log and please add a Fixes: tag.
> > >
> > > Thanks
> >
> > Hi,
> > I am trying to properly add a "Fixes: " tag, but the description for this tag indicates that it is to report that "the patch fixes an issue in a previous commit".
> >
> > In this case, I cannot identify a commit that introduced that bug, apart from the initial commit of the DTS file which didn't have the reset property present?
>
> Is the PHY on the SOM or the carrier?

It is on the SOM.

>
> If the PHY is on the carrier, then the delay is a carrier property,
> and should be in the carrier .dts file. So use the commit for when the
> carrier DTS file was added.
>
> If the PHY is on the SOM, then use the commit for when the SOM DTSI
> was added.

Ok, will use that.

Hugo.

--
Hugo Villeneuve <[email protected]>