* [net-next PATCH v2 0/3] ACPI support for xgmac_mdio drivers.
@ 2020-06-22 8:19 Calvin Johnson
2020-06-22 8:19 ` [net-next PATCH v2 1/3] net: phy: Allow mdio buses to auto-probe c45 devices Calvin Johnson
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Calvin Johnson @ 2020-06-22 8:19 UTC (permalink / raw)
To: Jeremy Linton, Russell King - ARM Linux admin, Jon,
Cristi Sovaiala, Ioana Ciornei, Andrew Lunn, Andy Shevchenko,
Florian Fainelli, Madalin Bucur
Cc: linux.cj, netdev, Calvin Johnson
This patch series provides ACPI support for xgmac_mdio driver.
Changes in v2:
- Reserve "0" to mean that no mdiobus capabilities have been declared.
- bus->id: change to appropriate printk format specifier
- clean up xgmac_acpi_match
- clariy platform_get_resource() usage with comments
Calvin Johnson (1):
net/fsl: acpize xgmac_mdio
Jeremy Linton (2):
net: phy: Allow mdio buses to auto-probe c45 devices
net/fsl: enable extended scanning in xgmac_mdio
drivers/net/ethernet/freescale/xgmac_mdio.c | 33 ++++++++++++++-------
drivers/net/phy/mdio_bus.c | 17 +++++++++--
include/linux/phy.h | 8 +++++
3 files changed, 46 insertions(+), 12 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 9+ messages in thread* [net-next PATCH v2 1/3] net: phy: Allow mdio buses to auto-probe c45 devices 2020-06-22 8:19 [net-next PATCH v2 0/3] ACPI support for xgmac_mdio drivers Calvin Johnson @ 2020-06-22 8:19 ` Calvin Johnson 2020-06-22 9:29 ` Madalin Bucur (OSS) 2020-06-22 11:59 ` kernel test robot 2020-06-22 8:19 ` [net-next PATCH v2 2/3] net/fsl: acpize xgmac_mdio Calvin Johnson 2020-06-22 8:19 ` [net-next PATCH v2 3/3] net/fsl: enable extended scanning in xgmac_mdio Calvin Johnson 2 siblings, 2 replies; 9+ messages in thread From: Calvin Johnson @ 2020-06-22 8:19 UTC (permalink / raw) To: Jeremy Linton, Russell King - ARM Linux admin, Jon, Cristi Sovaiala, Ioana Ciornei, Andrew Lunn, Andy Shevchenko, Florian Fainelli, Madalin Bucur Cc: linux.cj, netdev, Calvin Johnson From: Jeremy Linton <jeremy.linton@arm.com> The mdiobus_scan logic is currently hardcoded to only work with c22 devices. This works fairly well in most cases, but its possible that a c45 device doesn't respond despite being a standard phy. If the parent hardware is capable, it makes sense to scan for c22 devices before falling back to c45. As we want this to reflect the capabilities of the STA, lets add a field to the mii_bus structure to represent the capability. That way devices can opt into the extended scanning. Existing users should continue to default to c22 only scanning as long as they are zero'ing the structure before use. Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com> --- Changes in v2: - Reserve "0" to mean that no mdiobus capabilities have been declared. drivers/net/phy/mdio_bus.c | 17 +++++++++++++++-- include/linux/phy.h | 8 ++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c index 6ceee82b2839..e6c179b89907 100644 --- a/drivers/net/phy/mdio_bus.c +++ b/drivers/net/phy/mdio_bus.c @@ -739,10 +739,23 @@ EXPORT_SYMBOL(mdiobus_free); */ struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr) { - struct phy_device *phydev; + struct phy_device *phydev = ERR_PTR(-ENODEV); int err; - phydev = get_phy_device(bus, addr, false); + switch (bus->probe_capabilities) { + case MDIOBUS_C22: + phydev = get_phy_device(bus, addr, false); + break; + case MDIOBUS_C45: + phydev = get_phy_device(bus, addr, true); + break; + case MDIOBUS_C22_C45: + phydev = get_phy_device(bus, addr, false); + if (IS_ERR(phydev)) + phydev = get_phy_device(bus, addr, true); + break; + } + if (IS_ERR(phydev)) return phydev; diff --git a/include/linux/phy.h b/include/linux/phy.h index 9248dd2ce4ca..7860d56c6bf5 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -298,6 +298,14 @@ struct mii_bus { /* RESET GPIO descriptor pointer */ struct gpio_desc *reset_gpiod; + /* bus capabilities, used for probing */ + enum { + MDIOBUS_NO_CAP = 0, + MDIOBUS_C22, + MDIOBUS_C45, + MDIOBUS_C22_C45, + } probe_capabilities; + /* protect access to the shared element */ struct mutex shared_lock; -- 2.17.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* RE: [net-next PATCH v2 1/3] net: phy: Allow mdio buses to auto-probe c45 devices 2020-06-22 8:19 ` [net-next PATCH v2 1/3] net: phy: Allow mdio buses to auto-probe c45 devices Calvin Johnson @ 2020-06-22 9:29 ` Madalin Bucur (OSS) 2020-06-22 13:16 ` Calvin Johnson 2020-06-22 11:59 ` kernel test robot 1 sibling, 1 reply; 9+ messages in thread From: Madalin Bucur (OSS) @ 2020-06-22 9:29 UTC (permalink / raw) To: Calvin Johnson (OSS), Jeremy Linton, Russell King - ARM Linux admin, Jon, Cristi Sovaiala, Ioana Ciornei, Andrew Lunn, Andy Shevchenko, Florian Fainelli, Madalin Bucur (OSS) Cc: linux.cj@gmail.com, netdev@vger.kernel.org > -----Original Message----- > From: Calvin Johnson (OSS) <calvin.johnson@oss.nxp.com> > Sent: Monday, June 22, 2020 11:19 AM > To: Jeremy Linton <jeremy.linton@arm.com>; Russell King - ARM Linux admin > <linux@armlinux.org.uk>; Jon <jon@solid-run.com>; Cristi Sovaiala > <cristian.sovaiala@nxp.com>; Ioana Ciornei <ioana.ciornei@nxp.com>; Andrew > Lunn <andrew@lunn.ch>; Andy Shevchenko <andy.shevchenko@gmail.com>; > Florian Fainelli <f.fainelli@gmail.com>; Madalin Bucur (OSS) > <madalin.bucur@oss.nxp.com> > Cc: linux.cj@gmail.com; netdev@vger.kernel.org; Calvin Johnson (OSS) > <calvin.johnson@oss.nxp.com> > Subject: [net-next PATCH v2 1/3] net: phy: Allow mdio buses to auto-probe > c45 devices > > From: Jeremy Linton <jeremy.linton@arm.com> > > The mdiobus_scan logic is currently hardcoded to only > work with c22 devices. This works fairly well in most > cases, but its possible that a c45 device doesn't respond > despite being a standard phy. If the parent hardware > is capable, it makes sense to scan for c22 devices before > falling back to c45. > > As we want this to reflect the capabilities of the STA, > lets add a field to the mii_bus structure to represent > the capability. That way devices can opt into the extended > scanning. Existing users should continue to default to c22 > only scanning as long as they are zero'ing the structure > before use. How is this default working for existing users, the code below does not seem to do anything for a zeroed struct, as there is no default in the switch? > > Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> > Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com> > > --- > > Changes in v2: > - Reserve "0" to mean that no mdiobus capabilities have been declared. > > drivers/net/phy/mdio_bus.c | 17 +++++++++++++++-- > include/linux/phy.h | 8 ++++++++ > 2 files changed, 23 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c > index 6ceee82b2839..e6c179b89907 100644 > --- a/drivers/net/phy/mdio_bus.c > +++ b/drivers/net/phy/mdio_bus.c > @@ -739,10 +739,23 @@ EXPORT_SYMBOL(mdiobus_free); > */ > struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr) > { > - struct phy_device *phydev; > + struct phy_device *phydev = ERR_PTR(-ENODEV); > int err; > > - phydev = get_phy_device(bus, addr, false); > + switch (bus->probe_capabilities) { > + case MDIOBUS_C22: > + phydev = get_phy_device(bus, addr, false); > + break; > + case MDIOBUS_C45: > + phydev = get_phy_device(bus, addr, true); > + break; > + case MDIOBUS_C22_C45: > + phydev = get_phy_device(bus, addr, false); > + if (IS_ERR(phydev)) > + phydev = get_phy_device(bus, addr, true); > + break; > + } > + > if (IS_ERR(phydev)) > return phydev; > > diff --git a/include/linux/phy.h b/include/linux/phy.h > index 9248dd2ce4ca..7860d56c6bf5 100644 > --- a/include/linux/phy.h > +++ b/include/linux/phy.h > @@ -298,6 +298,14 @@ struct mii_bus { > /* RESET GPIO descriptor pointer */ > struct gpio_desc *reset_gpiod; > > + /* bus capabilities, used for probing */ > + enum { > + MDIOBUS_NO_CAP = 0, > + MDIOBUS_C22, > + MDIOBUS_C45, > + MDIOBUS_C22_C45, > + } probe_capabilities; > + > /* protect access to the shared element */ > struct mutex shared_lock; > > -- > 2.17.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [net-next PATCH v2 1/3] net: phy: Allow mdio buses to auto-probe c45 devices 2020-06-22 9:29 ` Madalin Bucur (OSS) @ 2020-06-22 13:16 ` Calvin Johnson 2020-06-22 13:36 ` Russell King - ARM Linux admin 0 siblings, 1 reply; 9+ messages in thread From: Calvin Johnson @ 2020-06-22 13:16 UTC (permalink / raw) To: Madalin Bucur (OSS) Cc: Jeremy Linton, Russell King - ARM Linux admin, Jon, Cristi Sovaiala, Ioana Ciornei, Andrew Lunn, Andy Shevchenko, Florian Fainelli, linux.cj@gmail.com, netdev@vger.kernel.org On Mon, Jun 22, 2020 at 09:29:17AM +0000, Madalin Bucur (OSS) wrote: > > -----Original Message----- > > From: Calvin Johnson (OSS) <calvin.johnson@oss.nxp.com> > > Sent: Monday, June 22, 2020 11:19 AM > > To: Jeremy Linton <jeremy.linton@arm.com>; Russell King - ARM Linux admin > > <linux@armlinux.org.uk>; Jon <jon@solid-run.com>; Cristi Sovaiala > > <cristian.sovaiala@nxp.com>; Ioana Ciornei <ioana.ciornei@nxp.com>; Andrew > > Lunn <andrew@lunn.ch>; Andy Shevchenko <andy.shevchenko@gmail.com>; > > Florian Fainelli <f.fainelli@gmail.com>; Madalin Bucur (OSS) > > <madalin.bucur@oss.nxp.com> > > Cc: linux.cj@gmail.com; netdev@vger.kernel.org; Calvin Johnson (OSS) > > <calvin.johnson@oss.nxp.com> > > Subject: [net-next PATCH v2 1/3] net: phy: Allow mdio buses to auto-probe > > c45 devices > > > > From: Jeremy Linton <jeremy.linton@arm.com> > > > > The mdiobus_scan logic is currently hardcoded to only > > work with c22 devices. This works fairly well in most > > cases, but its possible that a c45 device doesn't respond > > despite being a standard phy. If the parent hardware > > is capable, it makes sense to scan for c22 devices before > > falling back to c45. > > > > As we want this to reflect the capabilities of the STA, > > lets add a field to the mii_bus structure to represent > > the capability. That way devices can opt into the extended > > scanning. Existing users should continue to default to c22 > > only scanning as long as they are zero'ing the structure > > before use. > > How is this default working for existing users, the code below does not seem > to do anything for a zeroed struct, as there is no default in the switch? Looking further into this, I think MDIOBUS_C22 = 0, was correct. Prior to this patch, get_phy_device() was executed for C22 in this path. I'll discuss with Russell and Andrew on this and get back. > > > > > Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> > > Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com> > > > > --- > > > > Changes in v2: > > - Reserve "0" to mean that no mdiobus capabilities have been declared. > > > > drivers/net/phy/mdio_bus.c | 17 +++++++++++++++-- > > include/linux/phy.h | 8 ++++++++ > > 2 files changed, 23 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c > > index 6ceee82b2839..e6c179b89907 100644 > > --- a/drivers/net/phy/mdio_bus.c > > +++ b/drivers/net/phy/mdio_bus.c > > @@ -739,10 +739,23 @@ EXPORT_SYMBOL(mdiobus_free); > > */ > > struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr) > > { > > - struct phy_device *phydev; > > + struct phy_device *phydev = ERR_PTR(-ENODEV); > > int err; > > > > - phydev = get_phy_device(bus, addr, false); > > + switch (bus->probe_capabilities) { > > + case MDIOBUS_C22: > > + phydev = get_phy_device(bus, addr, false); > > + break; > > + case MDIOBUS_C45: > > + phydev = get_phy_device(bus, addr, true); > > + break; > > + case MDIOBUS_C22_C45: > > + phydev = get_phy_device(bus, addr, false); > > + if (IS_ERR(phydev)) > > + phydev = get_phy_device(bus, addr, true); > > + break; > > + } > > + > > if (IS_ERR(phydev)) > > return phydev; > > > > diff --git a/include/linux/phy.h b/include/linux/phy.h > > index 9248dd2ce4ca..7860d56c6bf5 100644 > > --- a/include/linux/phy.h > > +++ b/include/linux/phy.h > > @@ -298,6 +298,14 @@ struct mii_bus { > > /* RESET GPIO descriptor pointer */ > > struct gpio_desc *reset_gpiod; > > > > + /* bus capabilities, used for probing */ > > + enum { > > + MDIOBUS_NO_CAP = 0, > > + MDIOBUS_C22, > > + MDIOBUS_C45, > > + MDIOBUS_C22_C45, > > + } probe_capabilities; > > + > > /* protect access to the shared element */ > > struct mutex shared_lock; > > > > -- > > 2.17.1 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [net-next PATCH v2 1/3] net: phy: Allow mdio buses to auto-probe c45 devices 2020-06-22 13:16 ` Calvin Johnson @ 2020-06-22 13:36 ` Russell King - ARM Linux admin 2020-06-22 14:15 ` Calvin Johnson 0 siblings, 1 reply; 9+ messages in thread From: Russell King - ARM Linux admin @ 2020-06-22 13:36 UTC (permalink / raw) To: Calvin Johnson Cc: Madalin Bucur (OSS), Jeremy Linton, Jon, Cristi Sovaiala, Ioana Ciornei, Andrew Lunn, Andy Shevchenko, Florian Fainelli, linux.cj@gmail.com, netdev@vger.kernel.org On Mon, Jun 22, 2020 at 06:46:52PM +0530, Calvin Johnson wrote: > On Mon, Jun 22, 2020 at 09:29:17AM +0000, Madalin Bucur (OSS) wrote: > > > -----Original Message----- > > > From: Calvin Johnson (OSS) <calvin.johnson@oss.nxp.com> > > > Sent: Monday, June 22, 2020 11:19 AM > > > To: Jeremy Linton <jeremy.linton@arm.com>; Russell King - ARM Linux admin > > > <linux@armlinux.org.uk>; Jon <jon@solid-run.com>; Cristi Sovaiala > > > <cristian.sovaiala@nxp.com>; Ioana Ciornei <ioana.ciornei@nxp.com>; Andrew > > > Lunn <andrew@lunn.ch>; Andy Shevchenko <andy.shevchenko@gmail.com>; > > > Florian Fainelli <f.fainelli@gmail.com>; Madalin Bucur (OSS) > > > <madalin.bucur@oss.nxp.com> > > > Cc: linux.cj@gmail.com; netdev@vger.kernel.org; Calvin Johnson (OSS) > > > <calvin.johnson@oss.nxp.com> > > > Subject: [net-next PATCH v2 1/3] net: phy: Allow mdio buses to auto-probe > > > c45 devices > > > > > > From: Jeremy Linton <jeremy.linton@arm.com> > > > > > > The mdiobus_scan logic is currently hardcoded to only > > > work with c22 devices. This works fairly well in most > > > cases, but its possible that a c45 device doesn't respond > > > despite being a standard phy. If the parent hardware > > > is capable, it makes sense to scan for c22 devices before > > > falling back to c45. > > > > > > As we want this to reflect the capabilities of the STA, > > > lets add a field to the mii_bus structure to represent > > > the capability. That way devices can opt into the extended > > > scanning. Existing users should continue to default to c22 > > > only scanning as long as they are zero'ing the structure > > > before use. > > > > How is this default working for existing users, the code below does not seem > > to do anything for a zeroed struct, as there is no default in the switch? > > Looking further into this, I think MDIOBUS_C22 = 0, was correct. Prior to > this patch, get_phy_device() was executed for C22 in this path. I'll discuss > with Russell and Andrew on this and get back. It is not correct for the reasons I stated when I made the comment. When you introduce "probe_capabilities", every MDIO bus will have that field as zero. In your original patch, that means the bus only supports clause 22. However, we have buses today that _that_ is factually incorrect. Therefore, introducing probe_capabilities with zero meaning MDIOBUS_C22 is wrong. It means we can _never_ assume that bus->probe_capabilities means the bus does not support Clause 45. Now, as per your patch below, that is better. It means we're able to identify those drivers that have not declared which bus access methods are supported, while we can positively identify those which have. All that's needed is for your switch() statement to maintain today's behaviour where no declared probe_capabilities means that the bus should be probed for clause 22 PHYs. This means we can later introduce the ability to prevent clause 45 probing for PHYs that declare themselves as explicitly only supporting clause 22 if we need to without having been backed into a corner, and left wondering whether the lack of probe_capabilities is because someone decided "it's zero, so doesn't need to be initialised" and didn't bother explicitly stating .probe_capabilities = MDIOBUS_C22. > > > Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> > > > Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com> > > > > > > --- > > > > > > Changes in v2: > > > - Reserve "0" to mean that no mdiobus capabilities have been declared. > > > > > > drivers/net/phy/mdio_bus.c | 17 +++++++++++++++-- > > > include/linux/phy.h | 8 ++++++++ > > > 2 files changed, 23 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c > > > index 6ceee82b2839..e6c179b89907 100644 > > > --- a/drivers/net/phy/mdio_bus.c > > > +++ b/drivers/net/phy/mdio_bus.c > > > @@ -739,10 +739,23 @@ EXPORT_SYMBOL(mdiobus_free); > > > */ > > > struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr) > > > { > > > - struct phy_device *phydev; > > > + struct phy_device *phydev = ERR_PTR(-ENODEV); > > > int err; > > > > > > - phydev = get_phy_device(bus, addr, false); > > > + switch (bus->probe_capabilities) { > > > + case MDIOBUS_C22: > > > + phydev = get_phy_device(bus, addr, false); > > > + break; > > > + case MDIOBUS_C45: > > > + phydev = get_phy_device(bus, addr, true); > > > + break; > > > + case MDIOBUS_C22_C45: > > > + phydev = get_phy_device(bus, addr, false); > > > + if (IS_ERR(phydev)) > > > + phydev = get_phy_device(bus, addr, true); > > > + break; > > > + } > > > + > > > if (IS_ERR(phydev)) > > > return phydev; > > > > > > diff --git a/include/linux/phy.h b/include/linux/phy.h > > > index 9248dd2ce4ca..7860d56c6bf5 100644 > > > --- a/include/linux/phy.h > > > +++ b/include/linux/phy.h > > > @@ -298,6 +298,14 @@ struct mii_bus { > > > /* RESET GPIO descriptor pointer */ > > > struct gpio_desc *reset_gpiod; > > > > > > + /* bus capabilities, used for probing */ > > > + enum { > > > + MDIOBUS_NO_CAP = 0, > > > + MDIOBUS_C22, > > > + MDIOBUS_C45, > > > + MDIOBUS_C22_C45, > > > + } probe_capabilities; > > > + > > > /* protect access to the shared element */ > > > struct mutex shared_lock; > > > > > > -- > > > 2.17.1 > > > -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [net-next PATCH v2 1/3] net: phy: Allow mdio buses to auto-probe c45 devices 2020-06-22 13:36 ` Russell King - ARM Linux admin @ 2020-06-22 14:15 ` Calvin Johnson 0 siblings, 0 replies; 9+ messages in thread From: Calvin Johnson @ 2020-06-22 14:15 UTC (permalink / raw) To: Russell King - ARM Linux admin Cc: Madalin Bucur (OSS), Jeremy Linton, Jon, Cristi Sovaiala, Ioana Ciornei, Andrew Lunn, Andy Shevchenko, Florian Fainelli, linux.cj@gmail.com, netdev@vger.kernel.org On Mon, Jun 22, 2020 at 02:36:19PM +0100, Russell King - ARM Linux admin wrote: > On Mon, Jun 22, 2020 at 06:46:52PM +0530, Calvin Johnson wrote: > > On Mon, Jun 22, 2020 at 09:29:17AM +0000, Madalin Bucur (OSS) wrote: > > > > -----Original Message----- > > > > From: Calvin Johnson (OSS) <calvin.johnson@oss.nxp.com> > > > > Sent: Monday, June 22, 2020 11:19 AM > > > > To: Jeremy Linton <jeremy.linton@arm.com>; Russell King - ARM Linux admin > > > > <linux@armlinux.org.uk>; Jon <jon@solid-run.com>; Cristi Sovaiala > > > > <cristian.sovaiala@nxp.com>; Ioana Ciornei <ioana.ciornei@nxp.com>; Andrew > > > > Lunn <andrew@lunn.ch>; Andy Shevchenko <andy.shevchenko@gmail.com>; > > > > Florian Fainelli <f.fainelli@gmail.com>; Madalin Bucur (OSS) > > > > <madalin.bucur@oss.nxp.com> > > > > Cc: linux.cj@gmail.com; netdev@vger.kernel.org; Calvin Johnson (OSS) > > > > <calvin.johnson@oss.nxp.com> > > > > Subject: [net-next PATCH v2 1/3] net: phy: Allow mdio buses to auto-probe > > > > c45 devices > > > > > > > > From: Jeremy Linton <jeremy.linton@arm.com> > > > > > > > > The mdiobus_scan logic is currently hardcoded to only > > > > work with c22 devices. This works fairly well in most > > > > cases, but its possible that a c45 device doesn't respond > > > > despite being a standard phy. If the parent hardware > > > > is capable, it makes sense to scan for c22 devices before > > > > falling back to c45. > > > > > > > > As we want this to reflect the capabilities of the STA, > > > > lets add a field to the mii_bus structure to represent > > > > the capability. That way devices can opt into the extended > > > > scanning. Existing users should continue to default to c22 > > > > only scanning as long as they are zero'ing the structure > > > > before use. > > > > > > How is this default working for existing users, the code below does not seem > > > to do anything for a zeroed struct, as there is no default in the switch? > > > > Looking further into this, I think MDIOBUS_C22 = 0, was correct. Prior to > > this patch, get_phy_device() was executed for C22 in this path. I'll discuss > > with Russell and Andrew on this and get back. > > It is not correct for the reasons I stated when I made the comment. > When you introduce "probe_capabilities", every MDIO bus will have > that field as zero. > > In your original patch, that means the bus only supports clause 22. > However, we have buses today that _that_ is factually incorrect. > Therefore, introducing probe_capabilities with zero meaning MDIOBUS_C22 > is wrong. It means we can _never_ assume that bus->probe_capabilities > means the bus does not support Clause 45. > > Now, as per your patch below, that is better. It means we're able to > identify those drivers that have not declared which bus access methods > are supported, while we can positively identify those which have. > > All that's needed is for your switch() statement to maintain today's > behaviour where no declared probe_capabilities means that the bus > should be probed for clause 22 PHYs. > > This means we can later introduce the ability to prevent clause 45 > probing for PHYs that declare themselves as explicitly only supporting > clause 22 if we need to without having been backed into a corner, and > left wondering whether the lack of probe_capabilities is because someone > decided "it's zero, so doesn't need to be initialised" and didn't bother > explicitly stating .probe_capabilities = MDIOBUS_C22. Got it. I'll add the MDIOBUS_NO_CAP case also. Thanks Calvin ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [net-next PATCH v2 1/3] net: phy: Allow mdio buses to auto-probe c45 devices 2020-06-22 8:19 ` [net-next PATCH v2 1/3] net: phy: Allow mdio buses to auto-probe c45 devices Calvin Johnson 2020-06-22 9:29 ` Madalin Bucur (OSS) @ 2020-06-22 11:59 ` kernel test robot 1 sibling, 0 replies; 9+ messages in thread From: kernel test robot @ 2020-06-22 11:59 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 3217 bytes --] Hi Calvin, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on net-next/master] url: https://github.com/0day-ci/linux/commits/Calvin-Johnson/ACPI-support-for-xgmac_mdio-drivers/20200622-162324 base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 29a720c1042f469c8fea317cb5e7f496b116e07d config: x86_64-allyesconfig (attached as .config) compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 1d4c87335d5236ea1f35937e1014980ba961ae34) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>, old ones prefixed by <<): >> drivers/net/phy/mdio_bus.c:745:10: warning: enumeration value 'MDIOBUS_NO_CAP' not handled in switch [-Wswitch] switch (bus->probe_capabilities) { ^ 1 warning generated. vim +/MDIOBUS_NO_CAP +745 drivers/net/phy/mdio_bus.c 727 728 /** 729 * mdiobus_scan - scan a bus for MDIO devices. 730 * @bus: mii_bus to scan 731 * @addr: address on bus to scan 732 * 733 * This function scans the MDIO bus, looking for devices which can be 734 * identified using a vendor/product ID in registers 2 and 3. Not all 735 * MDIO devices have such registers, but PHY devices typically 736 * do. Hence this function assumes anything found is a PHY, or can be 737 * treated as a PHY. Other MDIO devices, such as switches, will 738 * probably not be found during the scan. 739 */ 740 struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr) 741 { 742 struct phy_device *phydev = ERR_PTR(-ENODEV); 743 int err; 744 > 745 switch (bus->probe_capabilities) { 746 case MDIOBUS_C22: 747 phydev = get_phy_device(bus, addr, false); 748 break; 749 case MDIOBUS_C45: 750 phydev = get_phy_device(bus, addr, true); 751 break; 752 case MDIOBUS_C22_C45: 753 phydev = get_phy_device(bus, addr, false); 754 if (IS_ERR(phydev)) 755 phydev = get_phy_device(bus, addr, true); 756 break; 757 } 758 759 if (IS_ERR(phydev)) 760 return phydev; 761 762 /* 763 * For DT, see if the auto-probed phy has a correspoding child 764 * in the bus node, and set the of_node pointer in this case. 765 */ 766 of_mdiobus_link_mdiodev(bus, &phydev->mdio); 767 768 err = phy_device_register(phydev); 769 if (err) { 770 phy_device_free(phydev); 771 return ERR_PTR(-ENODEV); 772 } 773 774 return phydev; 775 } 776 EXPORT_SYMBOL(mdiobus_scan); 777 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org [-- Attachment #2: config.gz --] [-- Type: application/gzip, Size: 75302 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* [net-next PATCH v2 2/3] net/fsl: acpize xgmac_mdio 2020-06-22 8:19 [net-next PATCH v2 0/3] ACPI support for xgmac_mdio drivers Calvin Johnson 2020-06-22 8:19 ` [net-next PATCH v2 1/3] net: phy: Allow mdio buses to auto-probe c45 devices Calvin Johnson @ 2020-06-22 8:19 ` Calvin Johnson 2020-06-22 8:19 ` [net-next PATCH v2 3/3] net/fsl: enable extended scanning in xgmac_mdio Calvin Johnson 2 siblings, 0 replies; 9+ messages in thread From: Calvin Johnson @ 2020-06-22 8:19 UTC (permalink / raw) To: Jeremy Linton, Russell King - ARM Linux admin, Jon, Cristi Sovaiala, Ioana Ciornei, Andrew Lunn, Andy Shevchenko, Florian Fainelli, Madalin Bucur Cc: linux.cj, netdev, Calvin Johnson Add ACPI support for xgmac MDIO bus registration while maintaining the existing DT support. The function mdiobus_register() inside of_mdiobus_register(), brings up all the PHYs on the mdio bus and attach them to the bus. Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com> --- Changes in v2: - bus->id: change to appropriate printk format specifier - clean up xgmac_acpi_match - clariy platform_get_resource() usage with comments drivers/net/ethernet/freescale/xgmac_mdio.c | 32 ++++++++++++++------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c index c82c85ef5fb3..b4ed5f837975 100644 --- a/drivers/net/ethernet/freescale/xgmac_mdio.c +++ b/drivers/net/ethernet/freescale/xgmac_mdio.c @@ -245,14 +245,19 @@ static int xgmac_mdio_probe(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node; struct mii_bus *bus; - struct resource res; + struct resource *res; struct mdio_fsl_priv *priv; int ret; - ret = of_address_to_resource(np, 0, &res); - if (ret) { + /* In DPAA-1, MDIO is one of the many FMan sub-devices. The FMan + * defines a register space that spans a large area, covering all the + * subdevice areas. Therefore, MDIO cannot claim exclusive access to + * this register area. + */ + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) { dev_err(&pdev->dev, "could not obtain address\n"); - return ret; + return -EINVAL; } bus = mdiobus_alloc_size(sizeof(struct mdio_fsl_priv)); @@ -263,21 +268,21 @@ static int xgmac_mdio_probe(struct platform_device *pdev) bus->read = xgmac_mdio_read; bus->write = xgmac_mdio_write; bus->parent = &pdev->dev; - snprintf(bus->id, MII_BUS_ID_SIZE, "%llx", (unsigned long long)res.start); + snprintf(bus->id, MII_BUS_ID_SIZE, "%pa", &res->start); /* Set the PHY base address */ priv = bus->priv; - priv->mdio_base = of_iomap(np, 0); + priv->mdio_base = ioremap(res->start, resource_size(res)); if (!priv->mdio_base) { ret = -ENOMEM; goto err_ioremap; } - priv->is_little_endian = of_property_read_bool(pdev->dev.of_node, - "little-endian"); + priv->is_little_endian = device_property_read_bool(&pdev->dev, + "little-endian"); - priv->has_a011043 = of_property_read_bool(pdev->dev.of_node, - "fsl,erratum-a011043"); + priv->has_a011043 = device_property_read_bool(&pdev->dev, + "fsl,erratum-a011043"); ret = of_mdiobus_register(bus, np); if (ret) { @@ -320,10 +325,17 @@ static const struct of_device_id xgmac_mdio_match[] = { }; MODULE_DEVICE_TABLE(of, xgmac_mdio_match); +static const struct acpi_device_id xgmac_acpi_match[] = { + { "NXP0006" }, + { } +}; +MODULE_DEVICE_TABLE(acpi, xgmac_acpi_match); + static struct platform_driver xgmac_mdio_driver = { .driver = { .name = "fsl-fman_xmdio", .of_match_table = xgmac_mdio_match, + .acpi_match_table = xgmac_acpi_match, }, .probe = xgmac_mdio_probe, .remove = xgmac_mdio_remove, -- 2.17.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [net-next PATCH v2 3/3] net/fsl: enable extended scanning in xgmac_mdio 2020-06-22 8:19 [net-next PATCH v2 0/3] ACPI support for xgmac_mdio drivers Calvin Johnson 2020-06-22 8:19 ` [net-next PATCH v2 1/3] net: phy: Allow mdio buses to auto-probe c45 devices Calvin Johnson 2020-06-22 8:19 ` [net-next PATCH v2 2/3] net/fsl: acpize xgmac_mdio Calvin Johnson @ 2020-06-22 8:19 ` Calvin Johnson 2 siblings, 0 replies; 9+ messages in thread From: Calvin Johnson @ 2020-06-22 8:19 UTC (permalink / raw) To: Jeremy Linton, Russell King - ARM Linux admin, Jon, Cristi Sovaiala, Ioana Ciornei, Andrew Lunn, Andy Shevchenko, Florian Fainelli, Madalin Bucur Cc: linux.cj, netdev, Calvin Johnson From: Jeremy Linton <jeremy.linton@arm.com> Since we know the xgmac hardware always has a c45 compliant bus, let's try scanning for c22 capable PHYs first. If we fail to find any, then it will fall back to c45 automatically. Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com> --- Changes in v2: None drivers/net/ethernet/freescale/xgmac_mdio.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c index b4ed5f837975..98be51d8b08c 100644 --- a/drivers/net/ethernet/freescale/xgmac_mdio.c +++ b/drivers/net/ethernet/freescale/xgmac_mdio.c @@ -268,6 +268,7 @@ static int xgmac_mdio_probe(struct platform_device *pdev) bus->read = xgmac_mdio_read; bus->write = xgmac_mdio_write; bus->parent = &pdev->dev; + bus->probe_capabilities = MDIOBUS_C22_C45; snprintf(bus->id, MII_BUS_ID_SIZE, "%pa", &res->start); /* Set the PHY base address */ -- 2.17.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-06-22 14:15 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-06-22 8:19 [net-next PATCH v2 0/3] ACPI support for xgmac_mdio drivers Calvin Johnson 2020-06-22 8:19 ` [net-next PATCH v2 1/3] net: phy: Allow mdio buses to auto-probe c45 devices Calvin Johnson 2020-06-22 9:29 ` Madalin Bucur (OSS) 2020-06-22 13:16 ` Calvin Johnson 2020-06-22 13:36 ` Russell King - ARM Linux admin 2020-06-22 14:15 ` Calvin Johnson 2020-06-22 11:59 ` kernel test robot 2020-06-22 8:19 ` [net-next PATCH v2 2/3] net/fsl: acpize xgmac_mdio Calvin Johnson 2020-06-22 8:19 ` [net-next PATCH v2 3/3] net/fsl: enable extended scanning in xgmac_mdio Calvin Johnson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.