[RFC PATCH 1/2] net: stmmac: allow drivers to explicitly select PHY device

[RFC PATCH 1/2] net: stmmac: allow drivers to explicitly select PHY device

[James Hilliard]

Some devices like the Allwinner H616 need the ability to select a phy
in cases where multiple PHY's may be present in a device tree due to
needing the ability to support multiple SoC variants with runtime
PHY selection.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 22 +++++++++++++------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 59d07d0d3369..949c4a8a1456 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1210,17 +1210,25 @@ static int stmmac_init_phy(struct net_device *dev)
 	 */
 	if (!phy_fwnode || IS_ERR(phy_fwnode)) {
 		int addr = priv->plat->phy_addr;
-		struct phy_device *phydev;
+		struct phy_device *phydev = NULL;
 
-		if (addr < 0) {
-			netdev_err(priv->dev, "no phy found\n");
-			return -ENODEV;
+		if (priv->plat->phy_node) {
+			phy_fwnode = of_fwnode_handle(priv->plat->phy_node);
+			phydev = fwnode_phy_find_device(phy_fwnode);
+			fwnode_handle_put(phy_fwnode);
 		}
 
-		phydev = mdiobus_get_phy(priv->mii, addr);
 		if (!phydev) {
-			netdev_err(priv->dev, "no phy at addr %d\n", addr);
-			return -ENODEV;
+			if (addr < 0) {
+				netdev_err(priv->dev, "no phy found\n");
+				return -ENODEV;
+			}
+
+			phydev = mdiobus_get_phy(priv->mii, addr);
+			if (!phydev) {
+				netdev_err(priv->dev, "no phy at addr %d\n", addr);
+				return -ENODEV;
+			}
 		}
 
 		ret = phylink_connect_phy(priv->phylink, phydev);
-- 
2.34.1

[RFC PATCH 2/2] net: stmmac: dwmac-sun8i: Allow runtime AC200/AC300 phy selection

[James Hilliard]

The Allwinner H616 ships with two different on-die phy variants, in
order to determine the phy being used we need to read an efuse and
then select the appropriate PHY based on the AC300 bit.

By defining an emac node without a phy-handle we can override the
default PHY selection logic in stmmac by passing a specific phy_node
selected based on the ac200 and ac300 names in a phys list.

This allows us to have a device tree that defines both PHY variants
even though only one will actually end up being used at runtime
based on the ac300 nvmem efuse bit.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 6c7e8655a7eb..e275f4caa684 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -11,6 +11,7 @@
 #include <linux/mdio-mux.h>
 #include <linux/mfd/syscon.h>
 #include <linux/module.h>
+#include <linux/nvmem-consumer.h>
 #include <linux/of.h>
 #include <linux/of_mdio.h>
 #include <linux/of_net.h>
@@ -280,6 +281,8 @@ static const struct emac_variant emac_variant_h6 = {
 #define SYSCON_ETCS_EXT_GMII	0x1
 #define SYSCON_ETCS_INT_GMII	0x2
 
+#define AC300_KEY		BIT(8)
+
 /* sun8i_dwmac_dma_reset() - reset the EMAC
  * Called from stmmac via stmmac_dma_ops->reset
  */
@@ -1159,6 +1162,7 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
 	struct net_device *ndev;
 	struct regmap *regmap;
 	int ret;
+	u16 val;
 
 	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
 	if (ret)
@@ -1222,6 +1226,21 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
 	if (IS_ERR(plat_dat))
 		return PTR_ERR(plat_dat);
 
+	if (!nvmem_cell_read_u16(dev, "ac300", &val)) {
+		const char *phy_name = (val & AC300_KEY) ? "ac300" : "ac200";
+		int index = of_property_match_string(dev->of_node, "phy-names", phy_name);
+		if (index < 0) {
+			dev_err(dev, "PHY name not found in device tree\n");
+			return -EINVAL;
+		}
+
+		plat_dat->phy_node = of_parse_phandle(dev->of_node, "phys", index);
+		if (!plat_dat->phy_node) {
+			dev_err(dev, "Failed to get PHY node from phys property\n");
+			return -EINVAL;
+		}
+	}
+
 	/* platform data specifying hardware features and callbacks.
 	 * hardware features were copied from Allwinner drivers.
 	 */
-- 
2.34.1

Re: [RFC PATCH 2/2] net: stmmac: dwmac-sun8i: Allow runtime AC200/AC300 phy selection

[Yanteng Si]

在 5/26/25 8:29 AM, James Hilliard 写道:
> The Allwinner H616 ships with two different on-die phy variants, in
> order to determine the phy being used we need to read an efuse and
> then select the appropriate PHY based on the AC300 bit.
>
> By defining an emac node without a phy-handle we can override the
> default PHY selection logic in stmmac by passing a specific phy_node
> selected based on the ac200 and ac300 names in a phys list.
>
> This allows us to have a device tree that defines both PHY variants
> even though only one will actually end up being used at runtime
> based on the ac300 nvmem efuse bit.
>
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
>   .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> index 6c7e8655a7eb..e275f4caa684 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> @@ -11,6 +11,7 @@
>   #include <linux/mdio-mux.h>
>   #include <linux/mfd/syscon.h>
>   #include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
>   #include <linux/of.h>
>   #include <linux/of_mdio.h>
>   #include <linux/of_net.h>
> @@ -280,6 +281,8 @@ static const struct emac_variant emac_variant_h6 = {
>   #define SYSCON_ETCS_EXT_GMII	0x1
>   #define SYSCON_ETCS_INT_GMII	0x2
>   

> +#define AC300_KEY		BIT(8)
> +

I have observed all the BIT macros in this file, and they all

have a line of comments. I think a comment is also needed here.

>   /* sun8i_dwmac_dma_reset() - reset the EMAC
>    * Called from stmmac via stmmac_dma_ops->reset
>    */
> @@ -1159,6 +1162,7 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
>   	struct net_device *ndev;
>   	struct regmap *regmap;
>   	int ret;
> +	u16 val;
>   
>   	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
>   	if (ret)
> @@ -1222,6 +1226,21 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
>   	if (IS_ERR(plat_dat))
>   		return PTR_ERR(plat_dat);
>   
> +	if (!nvmem_cell_read_u16(dev, "ac300", &val)) {
> +		const char *phy_name = (val & AC300_KEY) ? "ac300" : "ac200";
> +		int index = of_property_match_string(dev->of_node, "phy-names", phy_name);
> +		if (index < 0) {
> +			dev_err(dev, "PHY name not found in device tree\n");
> +			return -EINVAL;
> +		}
> +
> +		plat_dat->phy_node = of_parse_phandle(dev->of_node, "phys", index);
> +		if (!plat_dat->phy_node) {
> +			dev_err(dev, "Failed to get PHY node from phys property\n");
> +			return -EINVAL;
> +		}
> +	}
> +
How about preparing a separate function for it?  Then call it in probe().

If you are willing to do so, remember to write a code comment.


Thanks,

Yanteng

Re: [RFC PATCH 2/2] net: stmmac: dwmac-sun8i: Allow runtime AC200/AC300 phy selection

[Russell King (Oracle)]

On Sun, May 25, 2025 at 06:29:22PM -0600, James Hilliard wrote:
> +	if (!nvmem_cell_read_u16(dev, "ac300", &val)) {
> +		const char *phy_name = (val & AC300_KEY) ? "ac300" : "ac200";
> +		int index = of_property_match_string(dev->of_node, "phy-names", phy_name);
> +		if (index < 0) {
> +			dev_err(dev, "PHY name not found in device tree\n");
> +			return -EINVAL;
> +		}
> +
> +		plat_dat->phy_node = of_parse_phandle(dev->of_node, "phys", index);
> +		if (!plat_dat->phy_node) {
> +			dev_err(dev, "Failed to get PHY node from phys property\n");
> +			return -EINVAL;
> +		}
> +	}

1. You are re-using the drivers/phy binding for ethernet PHYs driven by
   phylib here.
2. You need to update
   Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml
   in a separate patch.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [RFC PATCH 2/2] net: stmmac: dwmac-sun8i: Allow runtime AC200/AC300 phy selection

[Andrew Lunn]

On Mon, May 26, 2025 at 09:04:40AM +0100, Russell King (Oracle) wrote:
> On Sun, May 25, 2025 at 06:29:22PM -0600, James Hilliard wrote:
> > +	if (!nvmem_cell_read_u16(dev, "ac300", &val)) {
> > +		const char *phy_name = (val & AC300_KEY) ? "ac300" : "ac200";
> > +		int index = of_property_match_string(dev->of_node, "phy-names", phy_name);
> > +		if (index < 0) {
> > +			dev_err(dev, "PHY name not found in device tree\n");
> > +			return -EINVAL;
> > +		}
> > +
> > +		plat_dat->phy_node = of_parse_phandle(dev->of_node, "phys", index);
> > +		if (!plat_dat->phy_node) {
> > +			dev_err(dev, "Failed to get PHY node from phys property\n");
> > +			return -EINVAL;
> > +		}
> > +	}
> 
> 1. You are re-using the drivers/phy binding for ethernet PHYs driven by
>    phylib here.
> 2. You need to update
>    Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml
>    in a separate patch.

A real user, i.e. a patch to a .dts file, would also be good.

  Andrew

Re: [RFC PATCH 2/2] net: stmmac: dwmac-sun8i: Allow runtime AC200/AC300 phy selection

[James Hilliard]

On Mon, May 26, 2025 at 8:14 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Mon, May 26, 2025 at 09:04:40AM +0100, Russell King (Oracle) wrote:
> > On Sun, May 25, 2025 at 06:29:22PM -0600, James Hilliard wrote:
> > > +   if (!nvmem_cell_read_u16(dev, "ac300", &val)) {
> > > +           const char *phy_name = (val & AC300_KEY) ? "ac300" : "ac200";
> > > +           int index = of_property_match_string(dev->of_node, "phy-names", phy_name);
> > > +           if (index < 0) {
> > > +                   dev_err(dev, "PHY name not found in device tree\n");
> > > +                   return -EINVAL;
> > > +           }
> > > +
> > > +           plat_dat->phy_node = of_parse_phandle(dev->of_node, "phys", index);
> > > +           if (!plat_dat->phy_node) {
> > > +                   dev_err(dev, "Failed to get PHY node from phys property\n");
> > > +                   return -EINVAL;
> > > +           }
> > > +   }
> >
> > 1. You are re-using the drivers/phy binding for ethernet PHYs driven by
> >    phylib here.
> > 2. You need to update
> >    Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml
> >    in a separate patch.
>
> A real user, i.e. a patch to a .dts file, would also be good.

That will be added that down the line, for now I added an example in the docs:
https://lore.kernel.org/netdev/20250526182939.2593553-3-james.hilliard1@gmail.com/

Currently there's a few other drivers needed to fully bring up the h616 emac1
with AC200/AC300 PHY's such as PWM driver support.

i.e. this(which will also need a few additional patches for the H616
PWM variant):
https://lore.kernel.org/all/20250427142500.151925-3-privatesub2@gmail.com/

I'm currently doing most of the PHY initialization in u-boot to simplify testing
of the efuse based PHY selection logic in the kernel. I'm sending this
separately as a number of subsequent drivers for kernel side PHY
initialization will be dependent upon specific PHY's being discovered at
runtime via the ac300 efuse bit.

I've currently verified this works on AC200 and AC300 boards by checking
that the appropriate phy address is used(address 0 on AC300 and address 1
on AC200).

>
>   Andrew

Re: [RFC PATCH 2/2] net: stmmac: dwmac-sun8i: Allow runtime AC200/AC300 phy selection

[Andrew Lunn]

> I'm currently doing most of the PHY initialization in u-boot to simplify testing
> of the efuse based PHY selection logic in the kernel. I'm sending this
> separately as a number of subsequent drivers for kernel side PHY
> initialization will be dependent upon specific PHY's being discovered at
> runtime via the ac300 efuse bit.

Do the different PHYs have different ID values in register 2 and 3?

	Andrew

Re: [RFC PATCH 2/2] net: stmmac: dwmac-sun8i: Allow runtime AC200/AC300 phy selection

[James Hilliard]

On Mon, May 26, 2025 at 1:58 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > I'm currently doing most of the PHY initialization in u-boot to simplify testing
> > of the efuse based PHY selection logic in the kernel. I'm sending this
> > separately as a number of subsequent drivers for kernel side PHY
> > initialization will be dependent upon specific PHY's being discovered at
> > runtime via the ac300 efuse bit.
>
> Do the different PHYs have different ID values in register 2 and 3?

Well...for the primary phy address in the device tree, no:
AC300(does not appear to support address 1 unlike AC200):
PHY at address 0:
0 - 0x3100
1 - 0x79ed
2 - 0x44
3 - 0x1400

AC300 address used for PHY initialization sequence on address 16(0x10),
appears to be used as replacement for i2c init sequence on AC200:
PHY at address 10:
0 - 0x1f80
1 - 0x1084
2 - 0xc000
3 - 0x0

AC200:
PHY at address 0:
0 - 0x3000
1 - 0x79ed
2 - 0x44
3 - 0x1400

AC200:
PHY at address 1:
0 - 0x3000
1 - 0x79ed
2 - 0x44
3 - 0x1400

AC200 appears to support either address 0 or address 1.

>
>         Andrew
>