Messages in this thread |  | | From | Pierluigi Passaro <> | Subject | Re: [PATCH] net: mdio: force deassert MDIO reset signal | Date | Sun, 15 Jan 2023 23:16:57 +0000 |
| |
On Sun, Jan 15, 2023 at 11:56 PM Lars-Peter Clausen <lars@metafoo.de> wrote: > On 1/15/23 14:33, Pierluigi Passaro wrote: > > On Sun, Jan 15, 2023 at 10:59 PM Lars-Peter Clausen <lars@metafoo.de> wrote: > >> On 1/15/23 09:08, Andrew Lunn wrote: > >>> On Sun, Jan 15, 2023 at 05:10:06PM +0100, Pierluigi Passaro wrote: > >>>> When the reset gpio is defined within the node of the device tree > >>>> describing the PHY, the reset is initialized and managed only after > >>>> calling the fwnode_mdiobus_phy_device_register function. > >>>> However, before calling it, the MDIO communication is checked by the > >>>> get_phy_device function. > >>>> When this happen and the reset GPIO was somehow previously set down, > >>>> the get_phy_device function fails, preventing the PHY detection. > >>>> These changes force the deassert of the MDIO reset signal before > >>>> checking the MDIO channel. > >>>> The PHY may require a minimum deassert time before being responsive: > >>>> use a reasonable sleep time after forcing the deassert of the MDIO > >>>> reset signal. > >>>> Once done, free the gpio descriptor to allow managing it later. > >>> This has been discussed before. The problem is, it is not just a reset > >>> GPIO. There could also be a clock which needs turning on, a regulator, > >>> and/or a linux reset controller. And what order do you turn these on? > >>> > >>> The conclusions of the discussion is you assume the device cannot be > >>> found by enumeration, and you put the ID in the compatible. That is > >>> enough to get the driver to load, and the driver can then turn > >>> everything on in the correct order, with the correct delays, etc. > >> I've been running into this same problem again and again over the past > >> years. > >> > >> Specifying the ID as part of the compatible string works for clause 22 > >> PHYs, but for clause 45 PHYs it does not work. The code always wants to > >> read the ID from the PHY itself. But I do not understand things well > >> enough to tell whether that's a fundamental restriction of C45 or just > >> our implementation and the implementation can be changed to fix it. > >> > >> Do you have some thoughts on this? > >> > > IMHO, since the framework allows defining the reset GPIO, it does not sound > > reasonable to manage it only after checking if the PHY can communicate: > > if the reset is asserted, the PHY cannot communicate at all. > > This patch just ensures that, if the reset GPIO is defined, it's not asserted > > while checking the communication. > > I fully agree with you and I think this is the right approach, cause it > is required to make systems work. But I've seen two attempts in the past > that did the very same thing and they always got rejected. I can't find > the patches anymore, but I think one was maybe 2 years ago. > Rejection is always a chance ;) As long I can understand the reasons, I can at least try improving this patch.
|  |