[PATCH V2 net-next 1/2] net: bgmac: improve handling PHY

[PATCH V2 net-next 1/2] net: bgmac: improve handling PHY

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

1. Use info from DT if available

It allows describing for example a fixed link. It's more accurate than
just guessing there may be one (depending on a chipset).

2. Verify PHY ID before trying to connect PHY

PHY addr 0x1e (30) is special in Broadcom routers and means a switch
connected as MDIO devices instead of a real PHY. Don't try connecting to
it.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Promote it out of RFC and send together with MDIO patch per
    Florian's request.
---
 drivers/net/ethernet/broadcom/bgmac-bcma.c | 33 ++++++++++++++--------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma.c b/drivers/net/ethernet/broadcom/bgmac-bcma.c
index 28759062d68d..7190e3f0da91 100644
--- a/drivers/net/ethernet/broadcom/bgmac-bcma.c
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma.c
@@ -11,6 +11,7 @@
 #include <linux/bcma/bcma.h>
 #include <linux/brcmphy.h>
 #include <linux/etherdevice.h>
+#include <linux/of_mdio.h>
 #include <linux/of_net.h>
 #include "bgmac.h"
 
@@ -86,17 +87,28 @@ static int bcma_phy_connect(struct bgmac *bgmac)
 	struct phy_device *phy_dev;
 	char bus_id[MII_BUS_ID_SIZE + 3];
 
+	/* DT info should be the most accurate */
+	phy_dev = of_phy_get_and_connect(bgmac->net_dev, bgmac->dev->of_node,
+					 bgmac_adjust_link);
+	if (phy_dev)
+		return 0;
+
 	/* Connect to the PHY */
-	snprintf(bus_id, sizeof(bus_id), PHY_ID_FMT, bgmac->mii_bus->id,
-		 bgmac->phyaddr);
-	phy_dev = phy_connect(bgmac->net_dev, bus_id, bgmac_adjust_link,
-			      PHY_INTERFACE_MODE_MII);
-	if (IS_ERR(phy_dev)) {
-		dev_err(bgmac->dev, "PHY connection failed\n");
-		return PTR_ERR(phy_dev);
+	if (bgmac->mii_bus && bgmac->phyaddr != BGMAC_PHY_NOREGS) {
+		snprintf(bus_id, sizeof(bus_id), PHY_ID_FMT, bgmac->mii_bus->id,
+			 bgmac->phyaddr);
+		phy_dev = phy_connect(bgmac->net_dev, bus_id, bgmac_adjust_link,
+				      PHY_INTERFACE_MODE_MII);
+		if (IS_ERR(phy_dev)) {
+			dev_err(bgmac->dev, "PHY connection failed\n");
+			return PTR_ERR(phy_dev);
+		}
+
+		return 0;
 	}
 
-	return 0;
+	/* Assume a fixed link to the switch port */
+	return bgmac_phy_connect_direct(bgmac);
 }
 
 static const struct bcma_device_id bgmac_bcma_tbl[] = {
@@ -297,10 +309,7 @@ static int bgmac_probe(struct bcma_device *core)
 	bgmac->cco_ctl_maskset = bcma_bgmac_cco_ctl_maskset;
 	bgmac->get_bus_clock = bcma_bgmac_get_bus_clock;
 	bgmac->cmn_maskset32 = bcma_bgmac_cmn_maskset32;
-	if (bgmac->mii_bus)
-		bgmac->phy_connect = bcma_phy_connect;
-	else
-		bgmac->phy_connect = bgmac_phy_connect_direct;
+	bgmac->phy_connect = bcma_phy_connect;
 
 	err = bgmac_enet_probe(bgmac);
 	if (err)
-- 
2.26.2

[PATCH V2 net-next 2/2] net: bgmac: support MDIO described in DT

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

Check ethernet controller DT node for "mdio" subnode and use it with
of_mdiobus_register() when present. That allows specifying MDIO and its
PHY devices in a standard DT based way.

This is required for BCM53573 SoC support. That family is sometimes
called Northstar (by marketing?) but is quite different from it. It uses
different CPU(s) and many different hw blocks.

One of shared blocks in BCM53573 is Ethernet controller. Switch however
is not SRAB accessible (as it Northstar) but is MDIO attached.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Update commit message to reference BCM53573. I thought it's worth it
    after Florian's question.
    Florian suggested I may need of_node_put() but it doesn't seem that
    of_get_child_by_name() or of_mdiobus_register() inc. any refcount.
---
 drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c b/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
index 6ce80cbcb48e..086739e4f40a 100644
--- a/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma-mdio.c
@@ -10,6 +10,7 @@
 
 #include <linux/bcma/bcma.h>
 #include <linux/brcmphy.h>
+#include <linux/of_mdio.h>
 #include "bgmac.h"
 
 static bool bcma_mdio_wait_value(struct bcma_device *core, u16 reg, u32 mask,
@@ -211,6 +212,7 @@ struct mii_bus *bcma_mdio_mii_register(struct bgmac *bgmac)
 {
 	struct bcma_device *core = bgmac->bcma.core;
 	struct mii_bus *mii_bus;
+	struct device_node *np;
 	int err;
 
 	mii_bus = mdiobus_alloc();
@@ -229,7 +231,9 @@ struct mii_bus *bcma_mdio_mii_register(struct bgmac *bgmac)
 	mii_bus->parent = &core->dev;
 	mii_bus->phy_mask = ~(1 << bgmac->phyaddr);
 
-	err = mdiobus_register(mii_bus);
+	np = of_get_child_by_name(core->dev.of_node, "mdio");
+
+	err = of_mdiobus_register(mii_bus, np);
 	if (err) {
 		dev_err(&core->dev, "Registration of mii bus failed\n");
 		goto err_free_bus;
-- 
2.26.2

Re: [PATCH V2 net-next 1/2] net: bgmac: improve handling PHY

[Jakub Kicinski]

On Sat,  2 Oct 2021 19:58:11 +0200 Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> 1. Use info from DT if available
> 
> It allows describing for example a fixed link. It's more accurate than
> just guessing there may be one (depending on a chipset).
> 
> 2. Verify PHY ID before trying to connect PHY
> 
> PHY addr 0x1e (30) is special in Broadcom routers and means a switch
> connected as MDIO devices instead of a real PHY. Don't try connecting to
> it.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> V2: Promote it out of RFC and send together with MDIO patch per
>     Florian's request.

Florian, ack?

Re: [PATCH V2 net-next 2/2] net: bgmac: support MDIO described in DT

[Florian Fainelli]

On 10/2/2021 10:58 AM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Check ethernet controller DT node for "mdio" subnode and use it with
> of_mdiobus_register() when present. That allows specifying MDIO and its
> PHY devices in a standard DT based way.
> 
> This is required for BCM53573 SoC support. That family is sometimes
> called Northstar (by marketing?) but is quite different from it. It uses
> different CPU(s) and many different hw blocks.
> 
> One of shared blocks in BCM53573 is Ethernet controller. Switch however
> is not SRAB accessible (as it Northstar) but is MDIO attached.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>

Reviewed-y: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH V2 net-next 1/2] net: bgmac: improve handling PHY

[Florian Fainelli]

On 10/2/2021 10:58 AM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> 1. Use info from DT if available
> 
> It allows describing for example a fixed link. It's more accurate than
> just guessing there may be one (depending on a chipset).
> 
> 2. Verify PHY ID before trying to connect PHY
> 
> PHY addr 0x1e (30) is special in Broadcom routers and means a switch
> connected as MDIO devices instead of a real PHY. Don't try connecting to
> it.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH V2 net-next 1/2] net: bgmac: improve handling PHY

[Florian Fainelli]

On 10/5/2021 6:21 PM, Jakub Kicinski wrote:
> On Sat,  2 Oct 2021 19:58:11 +0200 Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> 1. Use info from DT if available
>>
>> It allows describing for example a fixed link. It's more accurate than
>> just guessing there may be one (depending on a chipset).
>>
>> 2. Verify PHY ID before trying to connect PHY
>>
>> PHY addr 0x1e (30) is special in Broadcom routers and means a switch
>> connected as MDIO devices instead of a real PHY. Don't try connecting to
>> it.
>>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>> V2: Promote it out of RFC and send together with MDIO patch per
>>      Florian's request.
> 
> Florian, ack?
> 

Roger roger, thanks for the ping.
-- 
Florian