PHY reset question

PHY reset question

[Oleksij Rempel]

Hello PHY experts,

Short version:
what is the proper way to handle the PHY reset before identifying PHY?

Long version:
I stumbled over following issue:
If PHY reset is registered within PHY node. Then, sometimes,  we will not be
able to identify it (read PHY ID), because PHY is under reset.

mdio {
	compatible = "virtual,mdio-gpio";

	[...]

	/* Microchip KSZ8081 */
	usbeth_phy: ethernet-phy@3 {
		reg = <0x3>;

		interrupts-extended = <&gpio5 12 IRQ_TYPE_LEVEL_LOW>;
		reset-gpios = <&gpio5 11 GPIO_ACTIVE_LOW>;
		reset-assert-us = <500>;
		reset-deassert-us = <1000>;
	};

	[...]
};

On simple boards with one PHY per MDIO bus, it is easy to workaround by using
phy-reset-gpios withing MAC node (illustrated in below DT example), instead of
using reset-gpios within PHY node (see above DT example).

&fec {
	[...]
	phy-mode = "rmii";
	phy-reset-gpios = <&gpio4 12 GPIO_ACTIVE_LOW>;
	[...]
};

On boards with multiple PHYs (for example attached to a switch) and separate
reset lines to each PHY, it becomes more challenging. In my case, after power
cycle the system is working as expected:
- pinmux is configured to GPIO mode with internal pull-up
- GPIO is by default in input state. So the internal pull-up will automatically
  dessert the PHY reset.

On reboot, the system will assert the reset. GPIO configuration will survive the
reboot and PHYs will stay in the reset state, and not detected by the system.

So far I have following options/workarounds:
- do all needed configurations in the bootloader.
  Disadvantage:
  - not clear at which init level it should be done?
    1. Boot ROM script (in case of iMX). One fix per each board. Ease to forget.
    2. Pre bootloader. Same as 1.
    3. GPIO driver in the bootloader. What if some configuration was done in
       1. or 2.?
  - we will go back to the same problem if we jumped to Kexec

- Use compatible ("compatible = "ethernet-phy-id0022.1560") in the devicetree,
  so that reading the PHYID is not needed
  - easy to solve.
  Disadvantage:
  - losing PHY auto-detection capability
  - need a new devicetree if different PHY is used (for example in different
    board revision)

- modify PHY framework to deassert reset before identifying the PHY. 
  Disadvantages?

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: PHY reset question

[Florian Fainelli]

On 10/6/2020 1:04 AM, Oleksij Rempel wrote:
> Hello PHY experts,
> 
> Short version:
> what is the proper way to handle the PHY reset before identifying PHY?
> 
> Long version:
> I stumbled over following issue:
> If PHY reset is registered within PHY node. Then, sometimes,  we will not be
> able to identify it (read PHY ID), because PHY is under reset.
> 
> mdio {
> 	compatible = "virtual,mdio-gpio";
> 
> 	[...]
> 
> 	/* Microchip KSZ8081 */
> 	usbeth_phy: ethernet-phy@3 {
> 		reg = <0x3>;
> 
> 		interrupts-extended = <&gpio5 12 IRQ_TYPE_LEVEL_LOW>;
> 		reset-gpios = <&gpio5 11 GPIO_ACTIVE_LOW>;
> 		reset-assert-us = <500>;
> 		reset-deassert-us = <1000>;
> 	};
> 
> 	[...]
> };
> 
> On simple boards with one PHY per MDIO bus, it is easy to workaround by using
> phy-reset-gpios withing MAC node (illustrated in below DT example), instead of
> using reset-gpios within PHY node (see above DT example).
> 
> &fec {
> 	[...]
> 	phy-mode = "rmii";
> 	phy-reset-gpios = <&gpio4 12 GPIO_ACTIVE_LOW>;
> 	[...]
> };
> 
> On boards with multiple PHYs (for example attached to a switch) and separate
> reset lines to each PHY, it becomes more challenging. In my case, after power
> cycle the system is working as expected:
> - pinmux is configured to GPIO mode with internal pull-up
> - GPIO is by default in input state. So the internal pull-up will automatically
>    dessert the PHY reset.
> 
> On reboot, the system will assert the reset. GPIO configuration will survive the
> reboot and PHYs will stay in the reset state, and not detected by the system.
> 
> So far I have following options/workarounds:
> - do all needed configurations in the bootloader.
>    Disadvantage:
>    - not clear at which init level it should be done?
>      1. Boot ROM script (in case of iMX). One fix per each board. Ease to forget.
>      2. Pre bootloader. Same as 1.
>      3. GPIO driver in the bootloader. What if some configuration was done in
>         1. or 2.?
>    - we will go back to the same problem if we jumped to Kexec
> 
> - Use compatible ("compatible = "ethernet-phy-id0022.1560") in the devicetree,
>    so that reading the PHYID is not needed
>    - easy to solve.
>    Disadvantage:
>    - losing PHY auto-detection capability
>    - need a new devicetree if different PHY is used (for example in different
>      board revision)

Or you can punt that to the boot loader to be able to tell the 
difference and populate different compatible, or even manage the PHY 
reset to be able to read the actual PHY OUI. To me that is still the 
best solution around.

> 
> - modify PHY framework to deassert reset before identifying the PHY.
>    Disadvantages?

The disadvantages would be that you would have to use 
__of_reset_control_get() against the PHY device tree node because there 
are no phy_device being created yet because you have not been able to 
identify it. Given that there are people working on the ACPIzation of 
the PHY framework, that would be a set back.
-- 
Florian

Re: PHY reset question

[Marek Vasut]

On 10/6/20 9:36 PM, Florian Fainelli wrote:
[...]
>> - Use compatible ("compatible = "ethernet-phy-id0022.1560") in the
>> devicetree,
>>    so that reading the PHYID is not needed
>>    - easy to solve.
>>    Disadvantage:
>>    - losing PHY auto-detection capability
>>    - need a new devicetree if different PHY is used (for example in
>> different
>>      board revision)
> 
> Or you can punt that to the boot loader to be able to tell the
> difference and populate different compatible, or even manage the PHY
> reset to be able to read the actual PHY OUI. To me that is still the
> best solution around.

Wasn't there some requirement for Linux to be bootloader-independent ?
Some systems cannot replace their bootloaders, e.g. if the bootloader is
in ROM, so this might not be a solution.

>> - modify PHY framework to deassert reset before identifying the PHY.
>>    Disadvantages?

If this happens on MX6 with FEC, can you please try these two patches?

https://patchwork.ozlabs.org/project/netdev/patch/20201006135253.97395-1-marex@denx.de/

https://patchwork.ozlabs.org/project/netdev/patch/20201006202029.254212-1-marex@denx.de/

Thanks!

Re: PHY reset question

[Florian Fainelli]

On 10/6/2020 1:24 PM, Marek Vasut wrote:
> On 10/6/20 9:36 PM, Florian Fainelli wrote:
> [...]
>>> - Use compatible ("compatible = "ethernet-phy-id0022.1560") in the
>>> devicetree,
>>>     so that reading the PHYID is not needed
>>>     - easy to solve.
>>>     Disadvantage:
>>>     - losing PHY auto-detection capability
>>>     - need a new devicetree if different PHY is used (for example in
>>> different
>>>       board revision)
>>
>> Or you can punt that to the boot loader to be able to tell the
>> difference and populate different compatible, or even manage the PHY
>> reset to be able to read the actual PHY OUI. To me that is still the
>> best solution around.
> 
> Wasn't there some requirement for Linux to be bootloader-independent ?

What kind of dependency does this create here? The fact that Linux is 
capable of parsing a compatible string of the form 
"ethernet-phyAAAA.BBBB" is not something that is exclusively applicable 
to Linux. Linux just so happens to support that, but so could FreeBSD or 
any OS for that matter.

This is exactly the way firmware should be going, that is to describe 
accurately the hardware, while making the life of the OS much easier 
when it can. If we supported ACPI that is exactly what would have to 
happen IMHO.

> Some systems cannot replace their bootloaders, e.g. if the bootloader is
> in ROM, so this might not be a solution.

It is always possible to chain load a field updateable boot loader, and 
even when that is not desirable you could devise a solution that allows 
to utilize say a slightly different DTB that you could append to the 
kernel. Again, if you want to use strictly the same DTB, then you have 
to do what I just suggested and have the boot loader absorb some of this 
complexit

> 
>>> - modify PHY framework to deassert reset before identifying the PHY.
>>>     Disadvantages?
> 
> If this happens on MX6 with FEC, can you please try these two patches?
> 
> https://patchwork.ozlabs.org/project/netdev/patch/20201006135253.97395-1-marex@denx.de/
> 
> https://patchwork.ozlabs.org/project/netdev/patch/20201006202029.254212-1-marex@denx.de/

Your patches are not scaling across multiple Ethernet MAC drivers 
unfortunately, so I am not sure this should be even remotely considered 
a viable solution.
-- 
Florian

Re: PHY reset question

[Marek Vasut]

On 10/6/20 11:11 PM, Florian Fainelli wrote:
> 
> 
> On 10/6/2020 1:24 PM, Marek Vasut wrote:
>> On 10/6/20 9:36 PM, Florian Fainelli wrote:
>> [...]
>>>> - Use compatible ("compatible = "ethernet-phy-id0022.1560") in the
>>>> devicetree,
>>>>     so that reading the PHYID is not needed
>>>>     - easy to solve.
>>>>     Disadvantage:
>>>>     - losing PHY auto-detection capability
>>>>     - need a new devicetree if different PHY is used (for example in
>>>> different
>>>>       board revision)
>>>
>>> Or you can punt that to the boot loader to be able to tell the
>>> difference and populate different compatible, or even manage the PHY
>>> reset to be able to read the actual PHY OUI. To me that is still the
>>> best solution around.
>>
>> Wasn't there some requirement for Linux to be bootloader-independent ?
> 
> What kind of dependency does this create here? The fact that Linux is
> capable of parsing a compatible string of the form
> "ethernet-phyAAAA.BBBB" is not something that is exclusively applicable
> to Linux. Linux just so happens to support that, but so could FreeBSD or
> any OS for that matter.
> 
> This is exactly the way firmware should be going, that is to describe
> accurately the hardware, while making the life of the OS much easier
> when it can. If we supported ACPI that is exactly what would have to
> happen IMHO.

I should have been more specific, I meant the part where bootloader
should handle the PHY reset. If the kernel code depends on the fact that
the bootloader did PHY reset, then it depends on the bootloader
behavior, and I think that used to be frowned upon.

>> Some systems cannot replace their bootloaders, e.g. if the bootloader is
>> in ROM, so this might not be a solution.
> 
> It is always possible to chain load a field updateable boot loader

Not always, but that's another discussion.

>, and
> even when that is not desirable you could devise a solution that allows
> to utilize say a slightly different DTB that you could append to the
> kernel. Again, if you want to use strictly the same DTB, then you have
> to do what I just suggested and have the boot loader absorb some of this
> complexit

That sounds like moving the problem one level down without really
solving it, the bootloader will have this exact same problem -- how does
it determine that the PHY needs reset if it cannot reads its ID ?

>>>> - modify PHY framework to deassert reset before identifying the PHY.
>>>>     Disadvantages?
>>
>> If this happens on MX6 with FEC, can you please try these two patches?
>>
>> https://patchwork.ozlabs.org/project/netdev/patch/20201006135253.97395-1-marex@denx.de/
>>
>>
>> https://patchwork.ozlabs.org/project/netdev/patch/20201006202029.254212-1-marex@denx.de/
>>
> 
> Your patches are not scaling across multiple Ethernet MAC drivers
> unfortunately, so I am not sure this should be even remotely considered
> a viable solution.

Sorry for that . Since Oleksij was running into this problem on MX6 and
I had similar issue on MX6 with LAN8710 PHY, I thought this might be
helpful.

Re: PHY reset question

[Marco Felsch]

Hi Marek,

On 20-10-06 14:11, Florian Fainelli wrote:
> On 10/6/2020 1:24 PM, Marek Vasut wrote:

...

> > If this happens on MX6 with FEC, can you please try these two patches?
> > 
> > https://patchwork.ozlabs.org/project/netdev/patch/20201006135253.97395-1-marex@denx.de/
> > 
> > https://patchwork.ozlabs.org/project/netdev/patch/20201006202029.254212-1-marex@denx.de/
> 
> Your patches are not scaling across multiple Ethernet MAC drivers
> unfortunately, so I am not sure this should be even remotely considered a
> viable solution.

Recently I added clk support for the smcs driver [1] and dropped the
PHY_RST_AFTER_CLK_EN flag for LAN8710/20 devices because I had the same
issues. Hope this will help you too.

[1] https://www.spinics.net/lists/netdev/msg682080.html

Regards,
  Marco

Re: PHY reset question

[Marek Vasut]

On 10/7/20 10:14 AM, Marco Felsch wrote:
> Hi Marek,

Hi,

[...]

> On 20-10-06 14:11, Florian Fainelli wrote:
>> On 10/6/2020 1:24 PM, Marek Vasut wrote:
> 
> ...
> 
>>> If this happens on MX6 with FEC, can you please try these two patches?
>>>
>>> https://patchwork.ozlabs.org/project/netdev/patch/20201006135253.97395-1-marex@denx.de/
>>>
>>> https://patchwork.ozlabs.org/project/netdev/patch/20201006202029.254212-1-marex@denx.de/
>>
>> Your patches are not scaling across multiple Ethernet MAC drivers
>> unfortunately, so I am not sure this should be even remotely considered a
>> viable solution.
> 
> Recently I added clk support for the smcs driver [1] and dropped the
> PHY_RST_AFTER_CLK_EN flag for LAN8710/20 devices because I had the same
> issues. Hope this will help you too.
> 
> [1] https://www.spinics.net/lists/netdev/msg682080.html

I feel this might be starting to go a bit off-topic here, but isn't the
last patch 5/5 breaking existing setups ? The LAN8710 surely does need
clock enabled before the reset line is toggled.

Re: PHY reset question

[Marco Felsch]

On 20-10-07 10:23, Marek Vasut wrote:
> On 10/7/20 10:14 AM, Marco Felsch wrote:
> > Hi Marek,
> 
> Hi,
> 
> [...]
> 
> > On 20-10-06 14:11, Florian Fainelli wrote:
> >> On 10/6/2020 1:24 PM, Marek Vasut wrote:
> > 
> > ...
> > 
> >>> If this happens on MX6 with FEC, can you please try these two patches?
> >>>
> >>> https://patchwork.ozlabs.org/project/netdev/patch/20201006135253.97395-1-marex@denx.de/
> >>>
> >>> https://patchwork.ozlabs.org/project/netdev/patch/20201006202029.254212-1-marex@denx.de/
> >>
> >> Your patches are not scaling across multiple Ethernet MAC drivers
> >> unfortunately, so I am not sure this should be even remotely considered a
> >> viable solution.
> > 
> > Recently I added clk support for the smcs driver [1] and dropped the
> > PHY_RST_AFTER_CLK_EN flag for LAN8710/20 devices because I had the same
> > issues. Hope this will help you too.
> > 
> > [1] https://www.spinics.net/lists/netdev/msg682080.html
> 
> I feel this might be starting to go a bit off-topic here,

You're right, just wanted to provide you a link :)

> but isn't the
> last patch 5/5 breaking existing setups ?

IMHO the solution proposed using the PHY_RST_AFTER_CLK_EN was wrong so
we needed to fix that. Yes we need to take care of DT backward
compatibility but we still must be able to fix wrong behaviours within
the driver. I could also argue that PHY_RST_AFTER_CLK_EN solution was
breaking exisitng setups too.

> The LAN8710 surely does need
> clock enabled before the reset line is toggled.

Yep and therefore you can specify it yet within the DT.

Regards,
  Marco

Re: PHY reset question

[Marek Vasut]

On 10/7/20 11:06 AM, Marco Felsch wrote:
> On 20-10-07 10:23, Marek Vasut wrote:
>> On 10/7/20 10:14 AM, Marco Felsch wrote:
>>> Hi Marek,
>>
>> Hi,
>>
>> [...]
>>
>>> On 20-10-06 14:11, Florian Fainelli wrote:
>>>> On 10/6/2020 1:24 PM, Marek Vasut wrote:
>>>
>>> ...
>>>
>>>>> If this happens on MX6 with FEC, can you please try these two patches?
>>>>>
>>>>> https://patchwork.ozlabs.org/project/netdev/patch/20201006135253.97395-1-marex@denx.de/
>>>>>
>>>>> https://patchwork.ozlabs.org/project/netdev/patch/20201006202029.254212-1-marex@denx.de/
>>>>
>>>> Your patches are not scaling across multiple Ethernet MAC drivers
>>>> unfortunately, so I am not sure this should be even remotely considered a
>>>> viable solution.
>>>
>>> Recently I added clk support for the smcs driver [1] and dropped the
>>> PHY_RST_AFTER_CLK_EN flag for LAN8710/20 devices because I had the same
>>> issues. Hope this will help you too.
>>>
>>> [1] https://www.spinics.net/lists/netdev/msg682080.html
>>
>> I feel this might be starting to go a bit off-topic here,
> 
> You're right, just wanted to provide you a link :)

Can you CC me on the next version of those patches ? I seems the LAN8710
is causing grief to many.

>> but isn't the
>> last patch 5/5 breaking existing setups ?
> 
> IMHO the solution proposed using the PHY_RST_AFTER_CLK_EN was wrong so
> we needed to fix that. Yes we need to take care of DT backward
> compatibility but we still must be able to fix wrong behaviours within
> the driver. I could also argue that PHY_RST_AFTER_CLK_EN solution was
> breaking exisitng setups too.
> 
>> The LAN8710 surely does need
>> clock enabled before the reset line is toggled.
> 
> Yep and therefore you can specify it yet within the DT.

So the idea is that the PHY enables the clock for itself . And if the
MAC doesn't export these clock as clk to which you can refer to in DT,
then you still need the PHY_RST_AFTER_CLK_EN flag, so the MAC can deal
with enabling the clock ? Or is the idea to fix the MAC drivers too ?

Re: PHY reset question

[Marco Felsch]

On 20-10-07 11:20, Marek Vasut wrote:
> On 10/7/20 11:06 AM, Marco Felsch wrote:

...

> > You're right, just wanted to provide you a link :)
> 
> Can you CC me on the next version of those patches ? I seems the LAN8710
> is causing grief to many.

No need to since this serie was already applied.

> >> but isn't the
> >> last patch 5/5 breaking existing setups ?
> > 
> > IMHO the solution proposed using the PHY_RST_AFTER_CLK_EN was wrong so
> > we needed to fix that. Yes we need to take care of DT backward
> > compatibility but we still must be able to fix wrong behaviours within
> > the driver. I could also argue that PHY_RST_AFTER_CLK_EN solution was
> > breaking exisitng setups too.
> > 
> >> The LAN8710 surely does need
> >> clock enabled before the reset line is toggled.
> > 
> > Yep and therefore you can specify it yet within the DT.
> 
> So the idea is that the PHY enables the clock for itself . And if the
> MAC doesn't export these clock as clk to which you can refer to in DT,
> then you still need the PHY_RST_AFTER_CLK_EN flag, so the MAC can deal
> with enabling the clock ? Or is the idea to fix the MAC drivers too ?

First we need to consider that the PHY_RST_AFTER_CLK_EN flag gets only
handled by the imx-fec driver currently. This particular MAC driver
don't provide clks instead it is just a clock consumer. The clock is
coming from the clock driver and the clk-id is IMX6QDL_CLK_ENET_REF. If
the imx host is the clock provider for smsc-phy you need to specify that
clock-id. There are other phy-drivers using the same mechanism currently.
But yes, you need at least one clock provider. My solution is not
complete as Florian proposal [1] since it rely on the fact that the MAC
enables all clocks before probing the mdio bus. Luckily the imx-fec
driver has this behaviour and my patches are valid till Florian's
patches are merged. Resetting the phy should only be done within the phy
state machine and not from outside without respecting the phy state
since this can cause undefined behaviours.

Florian did you send a new version of those patches?

[1] https://www.spinics.net/lists/netdev/msg680412.html

Re: PHY reset question

[Florian Fainelli]

On 10/7/2020 3:47 AM, Marco Felsch wrote:
> Florian did you send a new version of those patches?

I did not because we had a good conversation with Rob over IRC and the 
conclusion was that the only solution that scaled across drivers, 
subsystems and type of resources (regulators, clocks, resets, etc.) was 
to have a compatible string for the given device that contains the ID. 
For Ethernet PHY or MDIO device nodes that is "ethernet-phyAAAA.BBBB".

When the bus determines the presence of such a compatible string it 
needs to bypass the dynamic identification of the device and needs to 
bind the PHY driver and the device instance directly. MDIO does that, 
and so does I2C and SPI AFAICT with the modalias/compatible (there is 
not a standardized way to runtime detect an I2C or SPI client anyway), 
while PCI and USB do not, but arguably could in the future.

For the specific use case that I had which required turning on a clock 
to the Ethernet PHY, I ended up modifying the firmware to provide that 
compatible string "ethernetAAAA.BBBB" and have the driver request the 
clock from its probe function:

https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/drivers/net/phy/bcm7xxx.c?id=ba4ee3c053659119472135231dbef8f6880ce1fb
-- 
Florian

Re: PHY reset question

[Fabio Estevam]

Hi Oleksij,

On Tue, Oct 6, 2020 at 5:05 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
>
> Hello PHY experts,
>
> Short version:
> what is the proper way to handle the PHY reset before identifying PHY?
>
> Long version:
> I stumbled over following issue:
> If PHY reset is registered within PHY node. Then, sometimes,  we will not be
> able to identify it (read PHY ID), because PHY is under reset.
>
> mdio {
>         compatible = "virtual,mdio-gpio";
>
>         [...]
>
>         /* Microchip KSZ8081 */
>         usbeth_phy: ethernet-phy@3 {
>                 reg = <0x3>;
>
>                 interrupts-extended = <&gpio5 12 IRQ_TYPE_LEVEL_LOW>;
>                 reset-gpios = <&gpio5 11 GPIO_ACTIVE_LOW>;
>                 reset-assert-us = <500>;
>                 reset-deassert-us = <1000>;
>         };
>
>         [...]
> };
>
> On simple boards with one PHY per MDIO bus, it is easy to workaround by using
> phy-reset-gpios withing MAC node (illustrated in below DT example), instead of
> using reset-gpios within PHY node (see above DT example).
>
> &fec {
>         [...]
>         phy-mode = "rmii";
>         phy-reset-gpios = <&gpio4 12 GPIO_ACTIVE_LOW>;
>         [...]

I thought this has been fixed by Bruno's series:
https://www.spinics.net/lists/netdev/msg673611.html

Re: PHY reset question

[Bruno Thomsen]

Hi Fabio and Oleksij

Den ons. 7. okt. 2020 kl. 11.50 skrev Fabio Estevam <festevam@gmail.com>:
>
> Hi Oleksij,
>
> On Tue, Oct 6, 2020 at 5:05 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> >
> > Hello PHY experts,
> >
> > Short version:
> > what is the proper way to handle the PHY reset before identifying PHY?
> >
> > Long version:
> > I stumbled over following issue:
> > If PHY reset is registered within PHY node. Then, sometimes,  we will not be
> > able to identify it (read PHY ID), because PHY is under reset.
> >
> > mdio {
> >         compatible = "virtual,mdio-gpio";
> >
> >         [...]
> >
> >         /* Microchip KSZ8081 */
> >         usbeth_phy: ethernet-phy@3 {
> >                 reg = <0x3>;
> >
> >                 interrupts-extended = <&gpio5 12 IRQ_TYPE_LEVEL_LOW>;
> >                 reset-gpios = <&gpio5 11 GPIO_ACTIVE_LOW>;
> >                 reset-assert-us = <500>;
> >                 reset-deassert-us = <1000>;
> >         };
> >
> >         [...]
> > };
> >
> > On simple boards with one PHY per MDIO bus, it is easy to workaround by using
> > phy-reset-gpios withing MAC node (illustrated in below DT example), instead of
> > using reset-gpios within PHY node (see above DT example).
> >
> > &fec {
> >         [...]
> >         phy-mode = "rmii";
> >         phy-reset-gpios = <&gpio4 12 GPIO_ACTIVE_LOW>;
> >         [...]
>
> I thought this has been fixed by Bruno's series:
> https://www.spinics.net/lists/netdev/msg673611.html

Yes, that has fixed the Microchip/Micrel PHY ID auto detection
issue. I have send a DTS patch v3 that makes use of the newly
added device tree parameter:
https://lkml.org/lkml/2020/9/23/595

/Bruno

Re: PHY reset question

[Oleksij Rempel]

Hi all,

thank you for the feedback!

On Fri, Oct 09, 2020 at 04:25:49PM +0200, Bruno Thomsen wrote:
> Hi Fabio and Oleksij
> 
> Den ons. 7. okt. 2020 kl. 11.50 skrev Fabio Estevam <festevam@gmail.com>:
> >
> > Hi Oleksij,
> >
> > On Tue, Oct 6, 2020 at 5:05 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> > >
> > > Hello PHY experts,
> > >
> > > Short version:
> > > what is the proper way to handle the PHY reset before identifying PHY?
> > >
> > > Long version:
> > > I stumbled over following issue:
> > > If PHY reset is registered within PHY node. Then, sometimes,  we will not be
> > > able to identify it (read PHY ID), because PHY is under reset.
> > >
> > > mdio {
> > >         compatible = "virtual,mdio-gpio";
> > >
> > >         [...]
> > >
> > >         /* Microchip KSZ8081 */
> > >         usbeth_phy: ethernet-phy@3 {
> > >                 reg = <0x3>;
> > >
> > >                 interrupts-extended = <&gpio5 12 IRQ_TYPE_LEVEL_LOW>;
> > >                 reset-gpios = <&gpio5 11 GPIO_ACTIVE_LOW>;
> > >                 reset-assert-us = <500>;
> > >                 reset-deassert-us = <1000>;
> > >         };
> > >
> > >         [...]
> > > };
> > >
> > > On simple boards with one PHY per MDIO bus, it is easy to workaround by using
> > > phy-reset-gpios withing MAC node (illustrated in below DT example), instead of
> > > using reset-gpios within PHY node (see above DT example).
> > >
> > > &fec {
> > >         [...]
> > >         phy-mode = "rmii";
> > >         phy-reset-gpios = <&gpio4 12 GPIO_ACTIVE_LOW>;
> > >         [...]
> >
> > I thought this has been fixed by Bruno's series:
> > https://www.spinics.net/lists/netdev/msg673611.html
> 
> Yes, that has fixed the Microchip/Micrel PHY ID auto detection
> issue. I have send a DTS patch v3 that makes use of the newly
> added device tree parameter:
> https://lkml.org/lkml/2020/9/23/595

This way is suitable only for boards with single PHY and single reset
line. But it is not scale on boards with multiple PHY and multiple reset
lines.

So far, it looks like using compatible like "ethernet-phy-idXXXX.XXXX"
is the only way to go.

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: PHY reset question

[Marek Vasut]

On 10/12/20 7:48 AM, Oleksij Rempel wrote:
> Hi all,
> 
> thank you for the feedback!
> 
> On Fri, Oct 09, 2020 at 04:25:49PM +0200, Bruno Thomsen wrote:
>> Hi Fabio and Oleksij
>>
>> Den ons. 7. okt. 2020 kl. 11.50 skrev Fabio Estevam <festevam@gmail.com>:
>>>
>>> Hi Oleksij,
>>>
>>> On Tue, Oct 6, 2020 at 5:05 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
>>>>
>>>> Hello PHY experts,
>>>>
>>>> Short version:
>>>> what is the proper way to handle the PHY reset before identifying PHY?
>>>>
>>>> Long version:
>>>> I stumbled over following issue:
>>>> If PHY reset is registered within PHY node. Then, sometimes,  we will not be
>>>> able to identify it (read PHY ID), because PHY is under reset.
>>>>
>>>> mdio {
>>>>         compatible = "virtual,mdio-gpio";
>>>>
>>>>         [...]
>>>>
>>>>         /* Microchip KSZ8081 */
>>>>         usbeth_phy: ethernet-phy@3 {
>>>>                 reg = <0x3>;
>>>>
>>>>                 interrupts-extended = <&gpio5 12 IRQ_TYPE_LEVEL_LOW>;
>>>>                 reset-gpios = <&gpio5 11 GPIO_ACTIVE_LOW>;
>>>>                 reset-assert-us = <500>;
>>>>                 reset-deassert-us = <1000>;
>>>>         };
>>>>
>>>>         [...]
>>>> };
>>>>
>>>> On simple boards with one PHY per MDIO bus, it is easy to workaround by using
>>>> phy-reset-gpios withing MAC node (illustrated in below DT example), instead of
>>>> using reset-gpios within PHY node (see above DT example).
>>>>
>>>> &fec {
>>>>         [...]
>>>>         phy-mode = "rmii";
>>>>         phy-reset-gpios = <&gpio4 12 GPIO_ACTIVE_LOW>;
>>>>         [...]
>>>
>>> I thought this has been fixed by Bruno's series:
>>> https://www.spinics.net/lists/netdev/msg673611.html
>>
>> Yes, that has fixed the Microchip/Micrel PHY ID auto detection
>> issue. I have send a DTS patch v3 that makes use of the newly
>> added device tree parameter:
>> https://lkml.org/lkml/2020/9/23/595
> 
> This way is suitable only for boards with single PHY and single reset
> line. But it is not scale on boards with multiple PHY and multiple reset
> lines.
> 
> So far, it looks like using compatible like "ethernet-phy-idXXXX.XXXX"
> is the only way to go.

I did further digging in this, and I agree it is either this or reset in
boot loader, sigh.