| Message ID | 20200903043947.3272453-1-f.fainelli@gmail.com |
|---|---|
| Headers | show |
| Series | net: phy: Support enabling clocks prior to bus probe | expand |
On 9/2/2020 9:39 PM, Florian Fainelli wrote: > Hi all, > > This patch series takes care of enabling the Ethernet PHY clocks in > DT-based systems (we have no way to do it for ACPI, and ACPI would > likely keep all of this hardware enabled anyway). > > Please test on your respective platforms, mine still seems to have > a race condition that I am tracking down as it looks like we are not > waiting long enough post clock enable. > > The check on the clock reference count is necessary to avoid an > artificial bump of the clock reference count and to support the unbind > -> bind of the PHY driver. We could solve it in different ways. > > Comments and test results welcome! Andrew, while we figure out a proper way to support this with the Linux device driver model, would you be opposed in a single patch to drivers/net/mdio/mdio-bcm-unimac.c which takes care of enabling the PHY's clock during bus->reset just for the sake of getting those systems to work, and later on we move over to the pre-probe mechanism? That would allow me to continue working with upstream kernels on these systems without carrying a big pile of patches.
W dniu 2020-09-04 o 06:04, Florian Fainelli pisze: > > > On 9/2/2020 9:39 PM, Florian Fainelli wrote: >> Hi all, >> >> This patch series takes care of enabling the Ethernet PHY clocks in >> DT-based systems (we have no way to do it for ACPI, and ACPI would >> likely keep all of this hardware enabled anyway). >> >> Please test on your respective platforms, mine still seems to have >> a race condition that I am tracking down as it looks like we are not >> waiting long enough post clock enable. >> >> The check on the clock reference count is necessary to avoid an >> artificial bump of the clock reference count and to support the unbind >> -> bind of the PHY driver. We could solve it in different ways. >> >> Comments and test results welcome! > > Andrew, while we figure out a proper way to support this with the > Linux device driver model, would you be opposed in a single patch to > drivers/net/mdio/mdio-bcm-unimac.c which takes care of enabling the > PHY's clock during bus->reset just for the sake of getting those > systems to work, and later on we move over to the pre-probe mechanism? > > That would allow me to continue working with upstream kernels on these > systems without carrying a big pile of patches. Just a bunch of questions. Actually, why is it necessary to have a full MDIO bus scan already during probing peripherals? If during probing the peripherals enable their resources (like clocks), what's wrong in having the full MDIO bus scan after probing of all peripherals is complete (and all peripherals are up)? Also, what's wrong in letting the MDIO bus scan find only some PHYs in the first go, and then letting each driver instance (of particular peripheral) initiate scan only for its specific PHY, if it was not found yet? (Is it thatof_mdio.h provides public function of_mdiobus_register, but not something similar to add only specific devices/phys without destroying the existing state?) I'd say that it is not necessary to have a PHY getting found before it is needed to setup the complete interface. Best regards, Adam
> Just a bunch of questions. > > Actually, why is it necessary to have a full MDIO bus scan already during > probing peripherals? That is the Linux bus model. It does not matter what sort of bus it is, PCI, USB, MDIO, etc. When the bus driver is loaded, the bus is enumerated and drivers probe for each device found on the bus. > I'd say that it is not necessary to have a PHY getting found before it is > needed to setup the complete interface. It is like saying, we don't need to probe the keyboard until the first time the "Press Enter" prompt is given? Andrew
On Thu, Sep 03, 2020 at 09:04:11PM -0700, Florian Fainelli wrote: > > > On 9/2/2020 9:39 PM, Florian Fainelli wrote: > > Hi all, > > > > This patch series takes care of enabling the Ethernet PHY clocks in > > DT-based systems (we have no way to do it for ACPI, and ACPI would > > likely keep all of this hardware enabled anyway). > > > > Please test on your respective platforms, mine still seems to have > > a race condition that I am tracking down as it looks like we are not > > waiting long enough post clock enable. > > > > The check on the clock reference count is necessary to avoid an > > artificial bump of the clock reference count and to support the unbind > > -> bind of the PHY driver. We could solve it in different ways. > > > > Comments and test results welcome! > > Andrew, while we figure out a proper way to support this with the Linux > device driver model, would you be opposed in a single patch to > drivers/net/mdio/mdio-bcm-unimac.c which takes care of enabling the PHY's > clock during bus->reset just for the sake of getting those systems to work, > and later on we move over to the pre-probe mechanism? > > That would allow me to continue working with upstream kernels on these > systems without carrying a big pile of patches. We do have quite a need for the proper solution. I wouldn't want you dropping the proper solution because you have a hack in place. Please add a comment: "HORRIBLE TEMPORARY HACK", to give you motivation to remove it again :-) Andrew
W dniu 2020-09-04 o 15:45, Andrew Lunn pisze: >> Just a bunch of questions. >> >> Actually, why is it necessary to have a full MDIO bus scan already during >> probing peripherals? > That is the Linux bus model. It does not matter what sort of bus it > is, PCI, USB, MDIO, etc. When the bus driver is loaded, the bus is > enumerated and drivers probe for each device found on the bus. OK. But is it always expected to find all the devices on the bus in the first run? Does the bus model ever allow to just add any more devices? Kind of, "hotplug". :) >> I'd say that it is not necessary to have a PHY getting found before it is >> needed to setup the complete interface. > It is like saying, we don't need to probe the keyboard until the first > time the "Press Enter" prompt is given? I'm not sure what you mean. It's like saying that we don't need to care if we even have the keyboard until we are interested in any interaction with it. (This might be reading a key, an autotest, ..., or not using, but avoiding a conflict - depends on application.) Best regards, Adam
On Fri, Sep 04, 2020 at 04:00:55PM +0200, Adam Rudziński wrote: > W dniu 2020-09-04 o 15:45, Andrew Lunn pisze: > > > Just a bunch of questions. > > > > > > Actually, why is it necessary to have a full MDIO bus scan already during > > > probing peripherals? > > That is the Linux bus model. It does not matter what sort of bus it > > is, PCI, USB, MDIO, etc. When the bus driver is loaded, the bus is > > enumerated and drivers probe for each device found on the bus. > > OK. But is it always expected to find all the devices on the bus in the > first run? Yes. Cold plug expects to find all the device while scanning the bus. > Does the bus model ever allow to just add any more devices? Kind of, > "hotplug". :) Hotplug is triggered by hardware saying a new device has been added/removed after cold plug. This is not a hotplug case. The hardware has not suddenly appeared, it has always been there. Andrew
W dniu 2020-09-04 o 16:23, Andrew Lunn pisze: > On Fri, Sep 04, 2020 at 04:00:55PM +0200, Adam Rudziński wrote: >> W dniu 2020-09-04 o 15:45, Andrew Lunn pisze: >>>> Just a bunch of questions. >>>> >>>> Actually, why is it necessary to have a full MDIO bus scan already during >>>> probing peripherals? >>> That is the Linux bus model. It does not matter what sort of bus it >>> is, PCI, USB, MDIO, etc. When the bus driver is loaded, the bus is >>> enumerated and drivers probe for each device found on the bus. >> OK. But is it always expected to find all the devices on the bus in the >> first run? > Yes. Cold plug expects to find all the device while scanning the bus. > >> Does the bus model ever allow to just add any more devices? Kind of, >> "hotplug". :) > Hotplug is triggered by hardware saying a new device has been > added/removed after cold plug. > > This is not a hotplug case. The hardware has not suddenly appeared, it > has always been there. > > Andrew There still is something that I would like to have clarified. Let me ask a bit more. Even if the hardware is fixed, in general it doesn't mean that there is only one possible way of bringing it up in software (software in general, not necessarily Linux). Does the Linux driver/bus model define the allowed options more precisely? Or: There is "coldplug", there is "hotplug", and let's call the intermediate possibility "warmplug" - the hardware is fixed, but it is discovered not in the same scan. Some devices are found in scans triggered by something later. For example, by a driver which doesn't have its attached devices discovered yet. Let's assume for now that it makes sense, and this can result in a perfectly running system. Does the Linux driver/bus model explicitly and strictly require "coldplug" approach in the case of fixed hardware? Is "warmplug" explicitly and strictly not allowed, or it just "doesn't feel right"? Best regards, Adam