diff mbox series

[2/4] of: phy: Warn about unexpected fixed-links in of_mdiobus_register

Message ID 20180820121238.7779-2-a.fatoum@pengutronix.de
State Changes Requested, archived
Delegated to: David Miller
Headers show
Series [1/4] net: macb: Fix regression breaking non-MDIO fixed-link PHYs | expand

Commit Message

Ahmad Fatoum Aug. 20, 2018, 12:12 p.m. UTC
fixed-links are currently not handled by of_mdiobus_register,
skip them with a warning instead of trying pointlessly to find their PHY
address:

	libphy: MACB_mii_bus: probed
	mdio_bus f0028000.ethernet-ffffffff: fixed-link has invalid PHY address
	mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 0
	[snip]
	mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 31
	macb f0028000.ethernet: broken fixed-link specification

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/of/of_mdio.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Ahmad Fatoum Aug. 20, 2018, 12:31 p.m. UTC | #1
On 08/20/2018 02:12 PM, Ahmad Fatoum wrote:
>  	/* Loop over the child nodes and register a phy_device for each phy */
>  	for_each_available_child_of_node(np, child) {
> +		if (of_phy_is_fixed_link(np)) {
> +			/* fixed-links are handled in the MAC drivers */
> +			dev_warn(&mdio->dev, FW_BUG
> +				"Skipping unexpected fixed-link in device tree");
> +			continue;
> +		}
> +

This would emit the warning even for normal PHYs,
because of_phy_is_fixed_link looks up a child...

I will correct this in v3 along with any other potential suggestions.
Andrew Lunn Aug. 20, 2018, 1:37 p.m. UTC | #2
On Mon, Aug 20, 2018 at 02:12:36PM +0200, Ahmad Fatoum wrote:
> fixed-links are currently not handled by of_mdiobus_register,
> skip them with a warning instead of trying pointlessly to find their PHY
> address:
> 
> 	libphy: MACB_mii_bus: probed
> 	mdio_bus f0028000.ethernet-ffffffff: fixed-link has invalid PHY address
> 	mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 0
> 	[snip]
> 	mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 31
> 	macb f0028000.ethernet: broken fixed-link specification
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/of/of_mdio.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index e92391d6d1bd..9a7ccd299daf 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -229,6 +229,13 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
>  
>  	/* Loop over the child nodes and register a phy_device for each phy */
>  	for_each_available_child_of_node(np, child) {
> +		if (of_phy_is_fixed_link(np)) {
> +			/* fixed-links are handled in the MAC drivers */
> +			dev_warn(&mdio->dev, FW_BUG
> +				"Skipping unexpected fixed-link in device tree");

Hi Ahmad

We should be more specific than "device tree". It is actually the "mdio bus subtree".

Is this patch on its own sufficient to fix your regression? Ideally we
want one small patch which we can add to stable to fix the regression,
and then all the new functionality goes into net-next.

    Andrew
Ahmad Fatoum Aug. 20, 2018, 1:51 p.m. UTC | #3
On 08/20/2018 03:37 PM, Andrew Lunn wrote:
> We should be more specific than "device tree". It is actually the "mdio bus subtree".

I wasn't sure if there are more drivers that call of_mdiobus_register for the MAC node.
I'll update the message.

> Is this patch on its own sufficient to fix your regression? Ideally we
> want one small patch which we can add to stable to fix the regression,
> and then all the new functionality goes into net-next.

This patch (or at least the fixed version in v3) only replaces the MDIO bus scan for
fixed-links with a warning. Patch 1/4 (the one Cc:ing stable) suffices to fix the regression.
diff mbox series

Patch

diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index e92391d6d1bd..9a7ccd299daf 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -229,6 +229,13 @@  int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
 
 	/* Loop over the child nodes and register a phy_device for each phy */
 	for_each_available_child_of_node(np, child) {
+		if (of_phy_is_fixed_link(np)) {
+			/* fixed-links are handled in the MAC drivers */
+			dev_warn(&mdio->dev, FW_BUG
+				"Skipping unexpected fixed-link in device tree");
+			continue;
+		}
+
 		addr = of_mdio_parse_addr(&mdio->dev, child);
 		if (addr < 0) {
 			scanphys = true;