Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230115161006.16431-1-pierluigi.p@variscite.com>
 <Y8QzI2VUY6//uBa/@lunn.ch> <f691f339-9e50-b968-01e1-1821a2f696e7@metafoo.de>
In-Reply-To: <f691f339-9e50-b968-01e1-1821a2f696e7@metafoo.de>
From:   Pierluigi Passaro <pierluigi.passaro@gmail.com>
Date:   Sun, 15 Jan 2023 23:33:31 +0100
Message-ID: <CAJ=UCjUo3t+D9S=J_yEhxCOo5OMj3d-UW6Z6HdwY+O+Q6JO0+A@mail.gmail.com>
Subject: Re: [PATCH] net: mdio: force deassert MDIO reset signal
To:     Lars-Peter Clausen <lars@metafoo.de>
Cc:     Andrew Lunn <andrew@lunn.ch>, hkallweit1@gmail.com,
        linux@armlinux.org.uk, davem@davemloft.net, edumazet@google.com,
        kuba@kernel.org, pabeni@redhat.com, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, eran.m@variscite.com,
        nate.d@variscite.com, francesco.f@variscite.com,
        pierluigi.p@variscite.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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.