[PATCH] arm64: dts: renesas: white-hawk-cpu: Move avb0 reset gpio to mdio node

[PATCH] arm64: dts: renesas: white-hawk-cpu: Move avb0 reset gpio to mdio node

[Niklas Söderlund]

When creating a dedicated mdio node to describe the bus the gpio reset
property was erroneously left in the phy node. The reason for adding
mdio nodes on WhiteHawk was to ensure the PHYs where reset before they
were probed, keeping the property in the phy node prevented this.

Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Fixes: 54bf0c27380b ("arm64: dts: renesas: r8a779g0: Use MDIO node for all AVB devices")
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 arch/arm64/boot/dts/renesas/white-hawk-cpu-common.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/renesas/white-hawk-cpu-common.dtsi b/arch/arm64/boot/dts/renesas/white-hawk-cpu-common.dtsi
index 80496fb3d476..4f0230327868 100644
--- a/arch/arm64/boot/dts/renesas/white-hawk-cpu-common.dtsi
+++ b/arch/arm64/boot/dts/renesas/white-hawk-cpu-common.dtsi
@@ -156,6 +156,8 @@ mdio {
 		#address-cells = <1>;
 		#size-cells = <0>;
 
+		reset-gpios = <&gpio7 10 GPIO_ACTIVE_LOW>;
+
 		avb0_phy: ethernet-phy@0 {
 			compatible = "ethernet-phy-id0022.1622",
 				     "ethernet-phy-ieee802.3-c22";
@@ -163,7 +165,6 @@ avb0_phy: ethernet-phy@0 {
 			reg = <0>;
 			interrupt-parent = <&gpio7>;
 			interrupts = <5 IRQ_TYPE_LEVEL_LOW>;
-			reset-gpios = <&gpio7 10 GPIO_ACTIVE_LOW>;
 		};
 	};
 };
-- 
2.45.2

Re: [PATCH] arm64: dts: renesas: white-hawk-cpu: Move avb0 reset gpio to mdio node

[Geert Uytterhoeven]

Hi Marek,

What is your stance on this?
Thanks!

On Thu, Jul 4, 2024 at 5:26 PM Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> When creating a dedicated mdio node to describe the bus the gpio reset
> property was erroneously left in the phy node. The reason for adding
> mdio nodes on WhiteHawk was to ensure the PHYs where reset before they
> were probed, keeping the property in the phy node prevented this.
>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Fixes: 54bf0c27380b ("arm64: dts: renesas: r8a779g0: Use MDIO node for all AVB devices")
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  arch/arm64/boot/dts/renesas/white-hawk-cpu-common.dtsi | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/renesas/white-hawk-cpu-common.dtsi b/arch/arm64/boot/dts/renesas/white-hawk-cpu-common.dtsi
> index 80496fb3d476..4f0230327868 100644
> --- a/arch/arm64/boot/dts/renesas/white-hawk-cpu-common.dtsi
> +++ b/arch/arm64/boot/dts/renesas/white-hawk-cpu-common.dtsi
> @@ -156,6 +156,8 @@ mdio {
>                 #address-cells = <1>;
>                 #size-cells = <0>;
>
> +               reset-gpios = <&gpio7 10 GPIO_ACTIVE_LOW>;
> +
>                 avb0_phy: ethernet-phy@0 {
>                         compatible = "ethernet-phy-id0022.1622",
>                                      "ethernet-phy-ieee802.3-c22";
> @@ -163,7 +165,6 @@ avb0_phy: ethernet-phy@0 {
>                         reg = <0>;
>                         interrupt-parent = <&gpio7>;
>                         interrupts = <5 IRQ_TYPE_LEVEL_LOW>;
> -                       reset-gpios = <&gpio7 10 GPIO_ACTIVE_LOW>;
>                 };
>         };
>  };

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] arm64: dts: renesas: white-hawk-cpu: Move avb0 reset gpio to mdio node

[Marek Vasut]

On 8/2/24 10:33 AM, Geert Uytterhoeven wrote:
> Hi Marek,

Hi,

> What is your stance on this?
> Thanks!
> 
> On Thu, Jul 4, 2024 at 5:26 PM Niklas Söderlund
> <niklas.soderlund+renesas@ragnatech.se> wrote:
>> When creating a dedicated mdio node to describe the bus the gpio reset
>> property was erroneously left in the phy node. The reason for adding
>> mdio nodes on WhiteHawk was to ensure the PHYs where reset before they
>> were probed, keeping the property in the phy node prevented this.

If the PHYs should be reset before they are probed, that is something 
the PHY driver should take care of, right ? The PHY driver can bind to 
the PHY via compatible string. Does the PHY driver not reset the PHYs ?

Re: [PATCH] arm64: dts: renesas: white-hawk-cpu: Move avb0 reset gpio to mdio node

[Geert Uytterhoeven]

Hi Marek,

On Fri, Aug 2, 2024 at 7:16 PM Marek Vasut <marex@denx.de> wrote:
> On 8/2/24 10:33 AM, Geert Uytterhoeven wrote:
> > What is your stance on this?

> > On Thu, Jul 4, 2024 at 5:26 PM Niklas Söderlund
> > <niklas.soderlund+renesas@ragnatech.se> wrote:
> >> When creating a dedicated mdio node to describe the bus the gpio reset
> >> property was erroneously left in the phy node. The reason for adding
> >> mdio nodes on WhiteHawk was to ensure the PHYs where reset before they
> >> were probed, keeping the property in the phy node prevented this.
>
> If the PHYs should be reset before they are probed, that is something
> the PHY driver should take care of, right ? The PHY driver can bind to
> the PHY via compatible string. Does the PHY driver not reset the PHYs ?

AFAIK, there is no requirement to reset the PHY before it is probed.
However, the reset signal may be in asserted state when the PHY is
probed (e.g. after unbind from the Ethernet driver, or during kexec).
Identifying the PHY by reading the ID register requires deasserting
the reset first.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] arm64: dts: renesas: white-hawk-cpu: Move avb0 reset gpio to mdio node

[Niklas Söderlund]

Hello Geert and Marek,

On 2024-08-22 15:56:44 +0200, Geert Uytterhoeven wrote:
> Hi Marek,
> 
> On Fri, Aug 2, 2024 at 7:16 PM Marek Vasut <marex@denx.de> wrote:
> > On 8/2/24 10:33 AM, Geert Uytterhoeven wrote:
> > > What is your stance on this?
> 
> > > On Thu, Jul 4, 2024 at 5:26 PM Niklas Söderlund
> > > <niklas.soderlund+renesas@ragnatech.se> wrote:
> > >> When creating a dedicated mdio node to describe the bus the gpio reset
> > >> property was erroneously left in the phy node. The reason for adding
> > >> mdio nodes on WhiteHawk was to ensure the PHYs where reset before they
> > >> were probed, keeping the property in the phy node prevented this.
> >
> > If the PHYs should be reset before they are probed, that is something
> > the PHY driver should take care of, right ? The PHY driver can bind to
> > the PHY via compatible string. Does the PHY driver not reset the PHYs ?
> 
> AFAIK, there is no requirement to reset the PHY before it is probed.
> However, the reset signal may be in asserted state when the PHY is
> probed (e.g. after unbind from the Ethernet driver, or during kexec).
> Identifying the PHY by reading the ID register requires deasserting
> the reset first.

Did we reach consensus on this? My primary motivation for this was to 
align it what is done for the other AVB instances on WhiteHawk, which do 
have the reset-gpios property in the mdio node and not the phy node.

As having a mdio node at all is a new-ish thing for AVB as this was
needed or the mv88q2110 PHYs connected to the other AVBs to function. If 
we are happy to have this here for AVB0 I will drop this patch.

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH] arm64: dts: renesas: white-hawk-cpu: Move avb0 reset gpio to mdio node

[Marek Vasut]

On 8/22/24 3:56 PM, Geert Uytterhoeven wrote:
> Hi Marek,

Hi,

sorry for the delay.

> On Fri, Aug 2, 2024 at 7:16 PM Marek Vasut <marex@denx.de> wrote:
>> On 8/2/24 10:33 AM, Geert Uytterhoeven wrote:
>>> What is your stance on this?
> 
>>> On Thu, Jul 4, 2024 at 5:26 PM Niklas Söderlund
>>> <niklas.soderlund+renesas@ragnatech.se> wrote:
>>>> When creating a dedicated mdio node to describe the bus the gpio reset
>>>> property was erroneously left in the phy node. The reason for adding
>>>> mdio nodes on WhiteHawk was to ensure the PHYs where reset before they
>>>> were probed, keeping the property in the phy node prevented this.
>>
>> If the PHYs should be reset before they are probed, that is something
>> the PHY driver should take care of, right ? The PHY driver can bind to
>> the PHY via compatible string. Does the PHY driver not reset the PHYs ?
> 
> AFAIK, there is no requirement to reset the PHY before it is probed.

That would mean the PHY is in undefined state before it is probed, which 
is not good.

> However, the reset signal may be in asserted state when the PHY is
> probed (e.g. after unbind from the Ethernet driver, or during kexec).
> Identifying the PHY by reading the ID register requires deasserting
> the reset first.
That may not be the entire precondition. For example the SMSC LAN87xx 
PHYs also require PHY clock to be enabled before the reset is toggled, 
but such information is available only to the specific PHY driver.

The MDIO-level reset GPIO handling, as far as I understand it, applies 
in case there are more PHYs on the MDIO bus which share the same reset 
GPIO line.

In this case there is only one PHY on the MDIO bus, so the only bit 
which applies is the potential PHY-specific reset requirement handling. 
If the PHY driver ever gets extended with such a thing in the future, 
then having the reset-gpios in the PHY node is beneficial over having it 
in MDIO node.

It will always be a compromise between the above and best-effort PHY 
auto-detection though.

Re: [PATCH] arm64: dts: renesas: white-hawk-cpu: Move avb0 reset gpio to mdio node

[Niklas Söderlund]

Hello,

On 2024-09-06 20:09:19 +0200, Marek Vasut wrote:
> On 8/22/24 3:56 PM, Geert Uytterhoeven wrote:
> > Hi Marek,
> 
> Hi,
> 
> sorry for the delay.
> 
> > On Fri, Aug 2, 2024 at 7:16 PM Marek Vasut <marex@denx.de> wrote:
> > > On 8/2/24 10:33 AM, Geert Uytterhoeven wrote:
> > > > What is your stance on this?
> > 
> > > > On Thu, Jul 4, 2024 at 5:26 PM Niklas Söderlund
> > > > <niklas.soderlund+renesas@ragnatech.se> wrote:
> > > > > When creating a dedicated mdio node to describe the bus the gpio reset
> > > > > property was erroneously left in the phy node. The reason for adding
> > > > > mdio nodes on WhiteHawk was to ensure the PHYs where reset before they
> > > > > were probed, keeping the property in the phy node prevented this.
> > > 
> > > If the PHYs should be reset before they are probed, that is something
> > > the PHY driver should take care of, right ? The PHY driver can bind to
> > > the PHY via compatible string. Does the PHY driver not reset the PHYs ?
> > 
> > AFAIK, there is no requirement to reset the PHY before it is probed.
> 
> That would mean the PHY is in undefined state before it is probed, which is
> not good.
> 
> > However, the reset signal may be in asserted state when the PHY is
> > probed (e.g. after unbind from the Ethernet driver, or during kexec).
> > Identifying the PHY by reading the ID register requires deasserting
> > the reset first.
> That may not be the entire precondition. For example the SMSC LAN87xx PHYs
> also require PHY clock to be enabled before the reset is toggled, but such
> information is available only to the specific PHY driver.
> 
> The MDIO-level reset GPIO handling, as far as I understand it, applies in
> case there are more PHYs on the MDIO bus which share the same reset GPIO
> line.
> 
> In this case there is only one PHY on the MDIO bus, so the only bit which
> applies is the potential PHY-specific reset requirement handling. If the PHY
> driver ever gets extended with such a thing in the future, then having the
> reset-gpios in the PHY node is beneficial over having it in MDIO node.
> 
> It will always be a compromise between the above and best-effort PHY
> auto-detection though.

I agree this is not needed if the PHY is identified by the compatible 
string, but might be if it is not. In this case it works and the reason 
for this patch was just to align the style used here.

I'm happy to drop this patch, or send a rebased version that applies 
since the context changed ;-) Marek, Geert what is your view? I'm happy 
with either option.

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH] arm64: dts: renesas: white-hawk-cpu: Move avb0 reset gpio to mdio node

[Marek Vasut]

On 10/15/24 4:48 PM, Niklas Söderlund wrote:

Hi,

>>> However, the reset signal may be in asserted state when the PHY is
>>> probed (e.g. after unbind from the Ethernet driver, or during kexec).
>>> Identifying the PHY by reading the ID register requires deasserting
>>> the reset first.
>> That may not be the entire precondition. For example the SMSC LAN87xx PHYs
>> also require PHY clock to be enabled before the reset is toggled, but such
>> information is available only to the specific PHY driver.
>>
>> The MDIO-level reset GPIO handling, as far as I understand it, applies in
>> case there are more PHYs on the MDIO bus which share the same reset GPIO
>> line.
>>
>> In this case there is only one PHY on the MDIO bus, so the only bit which
>> applies is the potential PHY-specific reset requirement handling. If the PHY
>> driver ever gets extended with such a thing in the future, then having the
>> reset-gpios in the PHY node is beneficial over having it in MDIO node.
>>
>> It will always be a compromise between the above and best-effort PHY
>> auto-detection though.
> 
> I agree this is not needed if the PHY is identified by the compatible
> string, but might be if it is not. In this case it works and the reason
> for this patch was just to align the style used here.
> 
> I'm happy to drop this patch, or send a rebased version that applies
> since the context changed ;-) Marek, Geert what is your view? I'm happy
> with either option.
I was hoping Geert would comment on this first, but seems like maybe no. 
I think, since the PHY node does have a compatible string AND the reset 
is connected to the PHY, I would keep the reset property in the PHY 
node. Sorry.

Re: [PATCH] arm64: dts: renesas: white-hawk-cpu: Move avb0 reset gpio to mdio node

[Geert Uytterhoeven]

Hi Marek,

On Mon, Oct 21, 2024 at 2:13 AM Marek Vasut <marex@denx.de> wrote:
> On 10/15/24 4:48 PM, Niklas Söderlund wrote:
> >>> However, the reset signal may be in asserted state when the PHY is
> >>> probed (e.g. after unbind from the Ethernet driver, or during kexec).
> >>> Identifying the PHY by reading the ID register requires deasserting
> >>> the reset first.
> >> That may not be the entire precondition. For example the SMSC LAN87xx PHYs
> >> also require PHY clock to be enabled before the reset is toggled, but such
> >> information is available only to the specific PHY driver.
> >>
> >> The MDIO-level reset GPIO handling, as far as I understand it, applies in
> >> case there are more PHYs on the MDIO bus which share the same reset GPIO
> >> line.
> >>
> >> In this case there is only one PHY on the MDIO bus, so the only bit which
> >> applies is the potential PHY-specific reset requirement handling. If the PHY
> >> driver ever gets extended with such a thing in the future, then having the
> >> reset-gpios in the PHY node is beneficial over having it in MDIO node.
> >>
> >> It will always be a compromise between the above and best-effort PHY
> >> auto-detection though.
> >
> > I agree this is not needed if the PHY is identified by the compatible
> > string, but might be if it is not. In this case it works and the reason
> > for this patch was just to align the style used here.
> >
> > I'm happy to drop this patch, or send a rebased version that applies
> > since the context changed ;-) Marek, Geert what is your view? I'm happy
> > with either option.
>
> I was hoping Geert would comment on this first, but seems like maybe no.
> I think, since the PHY node does have a compatible string AND the reset
> is connected to the PHY, I would keep the reset property in the PHY
> node. Sorry.

You are inverting the reasoning ;-) The compatible strings were added
because otherwise the PHY core can not identify the PHY when the
reset is asserted (e.g. after kexec). If possible, I'd rather remove
the compatible strings again, as different PHYs may be mounted on
different PHY revisions, causing a headache for DTB management.

So, what would you suggest when the PHY nodes would not have compatible
strings?

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] arm64: dts: renesas: white-hawk-cpu: Move avb0 reset gpio to mdio node

[Marek Vasut]

On 10/21/24 9:13 AM, Geert Uytterhoeven wrote:
> Hi Marek,

Hi,

> On Mon, Oct 21, 2024 at 2:13 AM Marek Vasut <marex@denx.de> wrote:
>> On 10/15/24 4:48 PM, Niklas Söderlund wrote:
>>>>> However, the reset signal may be in asserted state when the PHY is
>>>>> probed (e.g. after unbind from the Ethernet driver, or during kexec).
>>>>> Identifying the PHY by reading the ID register requires deasserting
>>>>> the reset first.
>>>> That may not be the entire precondition. For example the SMSC LAN87xx PHYs
>>>> also require PHY clock to be enabled before the reset is toggled, but such
>>>> information is available only to the specific PHY driver.
>>>>
>>>> The MDIO-level reset GPIO handling, as far as I understand it, applies in
>>>> case there are more PHYs on the MDIO bus which share the same reset GPIO
>>>> line.
>>>>
>>>> In this case there is only one PHY on the MDIO bus, so the only bit which
>>>> applies is the potential PHY-specific reset requirement handling. If the PHY
>>>> driver ever gets extended with such a thing in the future, then having the
>>>> reset-gpios in the PHY node is beneficial over having it in MDIO node.
>>>>
>>>> It will always be a compromise between the above and best-effort PHY
>>>> auto-detection though.
>>>
>>> I agree this is not needed if the PHY is identified by the compatible
>>> string, but might be if it is not. In this case it works and the reason
>>> for this patch was just to align the style used here.
>>>
>>> I'm happy to drop this patch, or send a rebased version that applies
>>> since the context changed ;-) Marek, Geert what is your view? I'm happy
>>> with either option.
>>
>> I was hoping Geert would comment on this first, but seems like maybe no.
>> I think, since the PHY node does have a compatible string AND the reset
>> is connected to the PHY, I would keep the reset property in the PHY
>> node. Sorry.
> 
> You are inverting the reasoning ;-) The compatible strings were added
> because otherwise the PHY core can not identify the PHY when the
> reset is asserted (e.g. after kexec).

... or because the PHY requires some complex sequence to bring it up, it 
is not just reset.

> If possible, I'd rather remove
> the compatible strings again, as different PHYs may be mounted on
> different PHY revisions, causing a headache for DTB management.

Will that ever be the case with this hardware ?

> So, what would you suggest when the PHY nodes would not have compatible
> strings?
I would suggest keep the PHY compatible strings, because that is the 
most accurate method to describe the hardware and fulfill the PHY bring 
up requirements. If the PHY changes on this hardware in some future 
revision, we can revisit this discussion ? Maybe bootloader-applied DTOs 
could work then ?

Re: [PATCH] arm64: dts: renesas: white-hawk-cpu: Move avb0 reset gpio to mdio node

[Geert Uytterhoeven]

Hi Marek,

On Tue, Oct 22, 2024 at 4:07 AM Marek Vasut <marex@denx.de> wrote:
> On 10/21/24 9:13 AM, Geert Uytterhoeven wrote:
> > On Mon, Oct 21, 2024 at 2:13 AM Marek Vasut <marex@denx.de> wrote:
> >> On 10/15/24 4:48 PM, Niklas Söderlund wrote:
> >>>>> However, the reset signal may be in asserted state when the PHY is
> >>>>> probed (e.g. after unbind from the Ethernet driver, or during kexec).
> >>>>> Identifying the PHY by reading the ID register requires deasserting
> >>>>> the reset first.
> >>>> That may not be the entire precondition. For example the SMSC LAN87xx PHYs
> >>>> also require PHY clock to be enabled before the reset is toggled, but such
> >>>> information is available only to the specific PHY driver.
> >>>>
> >>>> The MDIO-level reset GPIO handling, as far as I understand it, applies in
> >>>> case there are more PHYs on the MDIO bus which share the same reset GPIO
> >>>> line.
> >>>>
> >>>> In this case there is only one PHY on the MDIO bus, so the only bit which
> >>>> applies is the potential PHY-specific reset requirement handling. If the PHY
> >>>> driver ever gets extended with such a thing in the future, then having the
> >>>> reset-gpios in the PHY node is beneficial over having it in MDIO node.
> >>>>
> >>>> It will always be a compromise between the above and best-effort PHY
> >>>> auto-detection though.
> >>>
> >>> I agree this is not needed if the PHY is identified by the compatible
> >>> string, but might be if it is not. In this case it works and the reason
> >>> for this patch was just to align the style used here.
> >>>
> >>> I'm happy to drop this patch, or send a rebased version that applies
> >>> since the context changed ;-) Marek, Geert what is your view? I'm happy
> >>> with either option.
> >>
> >> I was hoping Geert would comment on this first, but seems like maybe no.
> >> I think, since the PHY node does have a compatible string AND the reset
> >> is connected to the PHY, I would keep the reset property in the PHY
> >> node. Sorry.
> >
> > You are inverting the reasoning ;-) The compatible strings were added
> > because otherwise the PHY core can not identify the PHY when the
> > reset is asserted (e.g. after kexec).
>
> ... or because the PHY requires some complex sequence to bring it up, it
> is not just reset.

That is your hypothetical case, but not the reason behind commit
722d55f3a9bd810f ("arm64: dts: renesas: Add compatible properties to
KSZ9031 Ethernet PHYs").

> > If possible, I'd rather remove
> > the compatible strings again, as different PHYs may be mounted on
> > different PHY revisions, causing a headache for DTB management.
>
> Will that ever be the case with this hardware ?

Dunno. It did happen with the Beacon boards.

> > So, what would you suggest when the PHY nodes would not have compatible
> > strings?
> I would suggest keep the PHY compatible strings, because that is the
> most accurate method to describe the hardware and fulfill the PHY bring
> up requirements. If the PHY changes on this hardware in some future

That issue is moot for KSZ9031.

> revision, we can revisit this discussion ? Maybe bootloader-applied DTOs
> could work then ?

So, what would you suggest when the PHY nodes would not have compatible
strings?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] arm64: dts: renesas: white-hawk-cpu: Move avb0 reset gpio to mdio node

[Marek Vasut]

On 10/22/24 9:38 AM, Geert Uytterhoeven wrote:
> Hi Marek,

Hello Geert,

>>>> I was hoping Geert would comment on this first, but seems like maybe no.
>>>> I think, since the PHY node does have a compatible string AND the reset
>>>> is connected to the PHY, I would keep the reset property in the PHY
>>>> node. Sorry.
>>>
>>> You are inverting the reasoning ;-) The compatible strings were added
>>> because otherwise the PHY core can not identify the PHY when the
>>> reset is asserted (e.g. after kexec).
>>
>> ... or because the PHY requires some complex sequence to bring it up, it
>> is not just reset.
> 
> That is your hypothetical case, but not the reason behind commit
> 722d55f3a9bd810f ("arm64: dts: renesas: Add compatible properties to
> KSZ9031 Ethernet PHYs").

We can stick to the "reset line in unknown state" here for the sake of 
this argument, it makes no difference.

>>> If possible, I'd rather remove
>>> the compatible strings again, as different PHYs may be mounted on
>>> different PHY revisions, causing a headache for DTB management.
>>
>> Will that ever be the case with this hardware ?
> 
> Dunno. It did happen with the Beacon boards.

Let's cross that bridge when we come to it ?

>>> So, what would you suggest when the PHY nodes would not have compatible
>>> strings?
>> I would suggest keep the PHY compatible strings, because that is the
>> most accurate method to describe the hardware and fulfill the PHY bring
>> up requirements. If the PHY changes on this hardware in some future
> 
> That issue is moot for KSZ9031.

If the PHY won't change, then we can keep the compatible strings ?

>> revision, we can revisit this discussion ? Maybe bootloader-applied DTOs
>> could work then ?
> 
> So, what would you suggest when the PHY nodes would not have compatible
> strings?
I hope I already answered that question before.

[...]

Re: [PATCH] arm64: dts: renesas: white-hawk-cpu: Move avb0 reset gpio to mdio node

[Geert Uytterhoeven]

Hi Marek,

On Sun, Oct 27, 2024 at 5:09 PM Marek Vasut <marex@denx.de> wrote:
> On 10/22/24 9:38 AM, Geert Uytterhoeven wrote:
> >>>> I was hoping Geert would comment on this first, but seems like maybe no.
> >>>> I think, since the PHY node does have a compatible string AND the reset
> >>>> is connected to the PHY, I would keep the reset property in the PHY
> >>>> node. Sorry.
> >>>
> >>> You are inverting the reasoning ;-) The compatible strings were added
> >>> because otherwise the PHY core can not identify the PHY when the
> >>> reset is asserted (e.g. after kexec).
> >>
> >> ... or because the PHY requires some complex sequence to bring it up, it
> >> is not just reset.
> >
> > That is your hypothetical case, but not the reason behind commit
> > 722d55f3a9bd810f ("arm64: dts: renesas: Add compatible properties to
> > KSZ9031 Ethernet PHYs").
>
> We can stick to the "reset line in unknown state" here for the sake of
> this argument, it makes no difference.
>
> >>> If possible, I'd rather remove
> >>> the compatible strings again, as different PHYs may be mounted on
> >>> different PHY revisions, causing a headache for DTB management.
> >>
> >> Will that ever be the case with this hardware ?
> >
> > Dunno. It did happen with the Beacon boards.
>
> Let's cross that bridge when we come to it ?
>
> >>> So, what would you suggest when the PHY nodes would not have compatible
> >>> strings?
> >> I would suggest keep the PHY compatible strings, because that is the
> >> most accurate method to describe the hardware and fulfill the PHY bring
> >> up requirements. If the PHY changes on this hardware in some future
> >
> > That issue is moot for KSZ9031.
>
> If the PHY won't change, then we can keep the compatible strings ?

Sorry for being unclear. I should have written "the PHY bring-up
requirements are moot for KSZ9031".

> >> revision, we can revisit this discussion ? Maybe bootloader-applied DTOs
> >> could work then ?
> >
> > So, what would you suggest when the PHY nodes would not have compatible
> > strings?
> I hope I already answered that question before.

Sorry, I may have missed that?

I really prefer not having the PHY compatible strings, as DT should
describe only what cannot be auto-detected.

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] arm64: dts: renesas: white-hawk-cpu: Move avb0 reset gpio to mdio node

[Marek Vasut]

On 10/28/24 11:13 AM, Geert Uytterhoeven wrote:
> Hi Marek,

Hello Geert,

>>>>> So, what would you suggest when the PHY nodes would not have compatible
>>>>> strings?
>>>> I would suggest keep the PHY compatible strings, because that is the
>>>> most accurate method to describe the hardware and fulfill the PHY bring
>>>> up requirements. If the PHY changes on this hardware in some future
>>>
>>> That issue is moot for KSZ9031.
>>
>> If the PHY won't change, then we can keep the compatible strings ?
> 
> Sorry for being unclear. I should have written "the PHY bring-up
> requirements are moot for KSZ9031".

Perhaps, (*) but odd erratas do show up every once in a while, so unless 
you can surely say no such errata will show up for the KSZ9031, can you 
really dismiss the bring up requirements ?

>>>> revision, we can revisit this discussion ? Maybe bootloader-applied DTOs
>>>> could work then ?
>>>
>>> So, what would you suggest when the PHY nodes would not have compatible
>>> strings?
>> I hope I already answered that question before.
> 
> Sorry, I may have missed that?
> 
> I really prefer not having the PHY compatible strings, as DT should
> describe only what cannot be auto-detected.
See paragraph above (*). My take on this is the exact opposite, better 
describe the PHY in DT fully, including compatible strings, so that if 
the PHY driver needs to do some sort of bring up tweak/fix/errata 
workaround/... , it can do so by matching on the compatible string 
without trying to bring the PHY up in some generic and potentially 
problematic way.

The MDIO bus is not discoverable the same way as PCIe or USB is, so I 
don't think the "DT should describe only what cannot be detected" is 
really applicable to MDIO bus the same way it applies to PCIe or USB.

Re: [PATCH] arm64: dts: renesas: white-hawk-cpu: Move avb0 reset gpio to mdio node

[Geert Uytterhoeven]

On Mon, Oct 28, 2024 at 7:58 PM Marek Vasut <marex@denx.de> wrote:
> On 10/28/24 11:13 AM, Geert Uytterhoeven wrote:
> >>>>> So, what would you suggest when the PHY nodes would not have compatible
> >>>>> strings?
> >>>> I would suggest keep the PHY compatible strings, because that is the
> >>>> most accurate method to describe the hardware and fulfill the PHY bring
> >>>> up requirements. If the PHY changes on this hardware in some future
> >>>
> >>> That issue is moot for KSZ9031.
> >>
> >> If the PHY won't change, then we can keep the compatible strings ?
> >
> > Sorry for being unclear. I should have written "the PHY bring-up
> > requirements are moot for KSZ9031".
>
> Perhaps, (*) but odd erratas do show up every once in a while, so unless
> you can surely say no such errata will show up for the KSZ9031, can you
> really dismiss the bring up requirements ?
>
> >>>> revision, we can revisit this discussion ? Maybe bootloader-applied DTOs
> >>>> could work then ?
> >>>
> >>> So, what would you suggest when the PHY nodes would not have compatible
> >>> strings?
> >> I hope I already answered that question before.
> >
> > Sorry, I may have missed that?
> >
> > I really prefer not having the PHY compatible strings, as DT should
> > describe only what cannot be auto-detected.
> See paragraph above (*). My take on this is the exact opposite, better
> describe the PHY in DT fully, including compatible strings, so that if
> the PHY driver needs to do some sort of bring up tweak/fix/errata
> workaround/... , it can do so by matching on the compatible string
> without trying to bring the PHY up in some generic and potentially
> problematic way.
>
> The MDIO bus is not discoverable the same way as PCIe or USB is, so I
> don't think the "DT should describe only what cannot be detected" is
> really applicable to MDIO bus the same way it applies to PCIe or USB.

So you think this is similar to SPI NOR, where most FLASHes can be
discovered with the JEDEC READ ID opcode?  See commit 4b0cb4e7ab2f777c
("dt-bindings: mtd: spi-nor: clarify the need for spi-nor compatibles"),
which clarified why no new compatible values are accepted.

Any guidance/opinion from the DT people?
Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] arm64: dts: renesas: white-hawk-cpu: Move avb0 reset gpio to mdio node

[Marek Vasut]

On 10/29/24 9:26 AM, Geert Uytterhoeven wrote:

[...]

>>>>>> revision, we can revisit this discussion ? Maybe bootloader-applied DTOs
>>>>>> could work then ?
>>>>>
>>>>> So, what would you suggest when the PHY nodes would not have compatible
>>>>> strings?
>>>> I hope I already answered that question before.
>>>
>>> Sorry, I may have missed that?
>>>
>>> I really prefer not having the PHY compatible strings, as DT should
>>> describe only what cannot be auto-detected.
>> See paragraph above (*). My take on this is the exact opposite, better
>> describe the PHY in DT fully, including compatible strings, so that if
>> the PHY driver needs to do some sort of bring up tweak/fix/errata
>> workaround/... , it can do so by matching on the compatible string
>> without trying to bring the PHY up in some generic and potentially
>> problematic way.
>>
>> The MDIO bus is not discoverable the same way as PCIe or USB is, so I
>> don't think the "DT should describe only what cannot be detected" is
>> really applicable to MDIO bus the same way it applies to PCIe or USB.
> 
> So you think this is similar to SPI NOR, where most FLASHes can be
> discovered with the JEDEC READ ID opcode?

Possibly, if you take broken-flash-reset DT property into account 
somehow. Even SPI NOR does require a proper reset after all, else the 
READ ID opcode may not work.

> See commit 4b0cb4e7ab2f777c
> ("dt-bindings: mtd: spi-nor: clarify the need for spi-nor compatibles"),
> which clarified why no new compatible values are accepted.
This works as long as your SPI NOR reset works.

Re: [PATCH] arm64: dts: renesas: white-hawk-cpu: Move avb0 reset gpio to mdio node

[Niklas Söderlund]

Hello Geert, Marek,

On 2024-10-30 15:45:30 +0100, Marek Vasut wrote:
> On 10/29/24 9:26 AM, Geert Uytterhoeven wrote:
> 
> [...]
> 
> > > > > > > revision, we can revisit this discussion ? Maybe bootloader-applied DTOs
> > > > > > > could work then ?
> > > > > > 
> > > > > > So, what would you suggest when the PHY nodes would not have compatible
> > > > > > strings?
> > > > > I hope I already answered that question before.
> > > > 
> > > > Sorry, I may have missed that?
> > > > 
> > > > I really prefer not having the PHY compatible strings, as DT should
> > > > describe only what cannot be auto-detected.
> > > See paragraph above (*). My take on this is the exact opposite, better
> > > describe the PHY in DT fully, including compatible strings, so that if
> > > the PHY driver needs to do some sort of bring up tweak/fix/errata
> > > workaround/... , it can do so by matching on the compatible string
> > > without trying to bring the PHY up in some generic and potentially
> > > problematic way.
> > > 
> > > The MDIO bus is not discoverable the same way as PCIe or USB is, so I
> > > don't think the "DT should describe only what cannot be detected" is
> > > really applicable to MDIO bus the same way it applies to PCIe or USB.
> > 
> > So you think this is similar to SPI NOR, where most FLASHes can be
> > discovered with the JEDEC READ ID opcode?
> 
> Possibly, if you take broken-flash-reset DT property into account somehow.
> Even SPI NOR does require a proper reset after all, else the READ ID opcode
> may not work.
> 
> > See commit 4b0cb4e7ab2f777c
> > ("dt-bindings: mtd: spi-nor: clarify the need for spi-nor compatibles"),
> > which clarified why no new compatible values are accepted.
> This works as long as your SPI NOR reset works.

Seems we can't find a way forward with this. The core argument as I 
understand it more about if we shall use device specific compatibles, or 
probe the MDIO bus when possible.

As that issue would require more changes then the one done here. This 
patch was more to align all nodes to behave the same and not adding any 
new feature or fixing a bug. I will drop this patch from my backlog. I 
did find the discussion informative and I learnt a bit, thanks for 
taking time for that!

If the larger question ever is settled I'm happy to pick this up at that 
point.

-- 
Kind Regards,
Niklas Söderlund