[PATCH net-next] net: phy: realtek: disable broadcast address feature of rtl8211f

[PATCH net-next] net: phy: realtek: disable broadcast address feature of rtl8211f

[Zhiyuan Wan]

This feature is automatically enabled after a reset of this
transceiver. When this feature is enabled, the phy not only
responds to the configured PHY address by pin states on board,
but also responds to address 0, the optional broadcast address
of the MDIO bus.

But some MDIO device like mt7530 switch chip (integrated in mt7621
SoC), also use address 0 to configure a specific port, when use
mt7530 and rtl8211f together, it usually causes address conflict,
leads to the port of rtl8211f stops working.

This patch disables broadcast address feature of rtl8211f, and
returns -ENODEV if using broadcast address (0) as phy address.

Hardware design hint:
This PHY only support address 1-7, and DO NOT tie all PHYAD pins
ground when you connect more than one PHY on a MDIO bus.
If you do that, this PHY will automatically take the first address
appeared on the MDIO bus as it's address, causing address conflict.

Signed-off-by: Zhiyuan Wan <kmlinuxm@gmail.com>
---
 drivers/net/phy/realtek.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index f65d7f1f3..0ef636d7b 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -31,6 +31,7 @@
 #define RTL8211F_PHYCR1				0x18
 #define RTL8211F_PHYCR2				0x19
 #define RTL8211F_INSR				0x1d
+#define RTL8211F_PHYAD0_EN			BIT(13)
 
 #define RTL8211F_LEDCR				0x10
 #define RTL8211F_LEDCR_MODE			BIT(15)
@@ -139,6 +140,17 @@ static int rtl821x_probe(struct phy_device *phydev)
 		return dev_err_probe(dev, PTR_ERR(priv->clk),
 				     "failed to get phy clock\n");
 
+	dev_dbg(dev, "disabling MDIO address 0 for this phy");
+	ret = phy_modify_paged(phydev, 0xa43, RTL8211F_PHYCR1,
+				       RTL8211F_PHYAD0_EN, 0);
+	if (ret < 0) {
+		return dev_err_probe(dev, PTR_ERR(ret),
+				     "disabling MDIO address 0 failed\n");
+	}
+	/* Deny broadcast address as PHY address */
+	if (phydev->mdio.addr == 0)
+		return -ENODEV;
+
 	ret = phy_read_paged(phydev, 0xa43, RTL8211F_PHYCR1);
 	if (ret < 0)
 		return ret;
-- 
2.30.2

Re: [PATCH net-next] net: phy: realtek: disable broadcast address feature of rtl8211f

[Heiner Kallweit]

On 03.12.2024 13:54, Zhiyuan Wan wrote:
> This feature is automatically enabled after a reset of this
> transceiver. When this feature is enabled, the phy not only
> responds to the configured PHY address by pin states on board,
> but also responds to address 0, the optional broadcast address
> of the MDIO bus.
> 
> But some MDIO device like mt7530 switch chip (integrated in mt7621
> SoC), also use address 0 to configure a specific port, when use
> mt7530 and rtl8211f together, it usually causes address conflict,
> leads to the port of rtl8211f stops working.
> 
> This patch disables broadcast address feature of rtl8211f, and
> returns -ENODEV if using broadcast address (0) as phy address.
> 
> Hardware design hint:
> This PHY only support address 1-7, and DO NOT tie all PHYAD pins
> ground when you connect more than one PHY on a MDIO bus.
> If you do that, this PHY will automatically take the first address
> appeared on the MDIO bus as it's address, causing address conflict.
> 
> Signed-off-by: Zhiyuan Wan <kmlinuxm@gmail.com>
> ---
>  drivers/net/phy/realtek.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> index f65d7f1f3..0ef636d7b 100644
> --- a/drivers/net/phy/realtek.c
> +++ b/drivers/net/phy/realtek.c
> @@ -31,6 +31,7 @@
>  #define RTL8211F_PHYCR1				0x18
>  #define RTL8211F_PHYCR2				0x19
>  #define RTL8211F_INSR				0x1d
> +#define RTL8211F_PHYAD0_EN			BIT(13)
>  
>  #define RTL8211F_LEDCR				0x10
>  #define RTL8211F_LEDCR_MODE			BIT(15)
> @@ -139,6 +140,17 @@ static int rtl821x_probe(struct phy_device *phydev)
>  		return dev_err_probe(dev, PTR_ERR(priv->clk),
>  				     "failed to get phy clock\n");
>  
> +	dev_dbg(dev, "disabling MDIO address 0 for this phy");
> +	ret = phy_modify_paged(phydev, 0xa43, RTL8211F_PHYCR1,
> +				       RTL8211F_PHYAD0_EN, 0);
> +	if (ret < 0) {
> +		return dev_err_probe(dev, PTR_ERR(ret),

Why PTR_ERR()? Did you even compile-test?

> +				     "disabling MDIO address 0 failed\n");
> +	}
> +	/* Deny broadcast address as PHY address */
> +	if (phydev->mdio.addr == 0)
> +		return -ENODEV;
> +
>  	ret = phy_read_paged(phydev, 0xa43, RTL8211F_PHYCR1);
>  	if (ret < 0)
>  		return ret;

And more formal hints:
- If you send a new version of a patch, annotate it accordingly.
- Allow 24h before sending a new version
- Include a change log

https://docs.kernel.org/process/submitting-patches.html
https://www.kernel.org/doc/html/v6.1/process/maintainer-netdev.html

Re: [PATCH net-next] net: phy: realtek: disable broadcast address feature of rtl8211f

[Zhiyuan Wan]

On 2024/12/3 21:18, Heiner Kallweit wrote:
> On 03.12.2024 13:54, Zhiyuan Wan wrote:
> 
> Why PTR_ERR()? Did you even compile-test?
> 
I made a test on a real mt7621 based hardware,
I misunderstood what PTR_ERR() means.
> 
> And more formal hints:
> - If you send a new version of a patch, annotate it accordingly.
> - Allow 24h before sending a new version
> - Include a change log
> 
> https://docs.kernel.org/process/submitting-patches.html
> https://www.kernel.org/doc/html/v6.1/process/maintainer-netdev.html
> 
Ok, thank you.

Re: [PATCH net-next] net: phy: realtek: disable broadcast address feature of rtl8211f

[kernel test robot]

Hi Zhiyuan,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Zhiyuan-Wan/net-phy-realtek-disable-broadcast-address-feature-of-rtl8211f/20241203-205751
base:   net-next/main
patch link:    https://lore.kernel.org/r/20241203125430.2078090-1-kmlinuxm%40gmail.com
patch subject: [PATCH net-next] net: phy: realtek: disable broadcast address feature of rtl8211f
config: arm-randconfig-002-20241204 (https://download.01.org/0day-ci/archive/20241204/202412041255.6r9ogs5i-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 592c0fe55f6d9a811028b5f3507be91458ab2713)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241204/202412041255.6r9ogs5i-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412041255.6r9ogs5i-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/net/phy/realtek.c:12:
   In file included from include/linux/phy.h:16:
   In file included from include/linux/ethtool.h:18:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:8:
   In file included from include/linux/cacheflush.h:5:
   In file included from arch/arm/include/asm/cacheflush.h:10:
   In file included from include/linux/mm.h:2223:
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
>> drivers/net/phy/realtek.c:147:37: error: incompatible integer to pointer conversion passing 'int' to parameter of type 'const void *' [-Wint-conversion]
     147 |                 return dev_err_probe(dev, PTR_ERR(ret),
         |                                                   ^~~
   include/linux/err.h:52:61: note: passing argument to parameter 'ptr' here
      52 | static inline long __must_check PTR_ERR(__force const void *ptr)
         |                                                             ^
   1 warning and 1 error generated.


vim +147 drivers/net/phy/realtek.c

   126	
   127	static int rtl821x_probe(struct phy_device *phydev)
   128	{
   129		struct device *dev = &phydev->mdio.dev;
   130		struct rtl821x_priv *priv;
   131		u32 phy_id = phydev->drv->phy_id;
   132		int ret;
   133	
   134		priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
   135		if (!priv)
   136			return -ENOMEM;
   137	
   138		priv->clk = devm_clk_get_optional_enabled(dev, NULL);
   139		if (IS_ERR(priv->clk))
   140			return dev_err_probe(dev, PTR_ERR(priv->clk),
   141					     "failed to get phy clock\n");
   142	
   143		dev_dbg(dev, "disabling MDIO address 0 for this phy");
   144		ret = phy_modify_paged(phydev, 0xa43, RTL8211F_PHYCR1,
   145					       RTL8211F_PHYAD0_EN, 0);
   146		if (ret < 0) {
 > 147			return dev_err_probe(dev, PTR_ERR(ret),
   148					     "disabling MDIO address 0 failed\n");
   149		}
   150		/* Deny broadcast address as PHY address */
   151		if (phydev->mdio.addr == 0)
   152			return -ENODEV;
   153	
   154		ret = phy_read_paged(phydev, 0xa43, RTL8211F_PHYCR1);
   155		if (ret < 0)
   156			return ret;
   157	
   158		priv->phycr1 = ret & (RTL8211F_ALDPS_PLL_OFF | RTL8211F_ALDPS_ENABLE | RTL8211F_ALDPS_XTAL_OFF);
   159		if (of_property_read_bool(dev->of_node, "realtek,aldps-enable"))
   160			priv->phycr1 |= RTL8211F_ALDPS_PLL_OFF | RTL8211F_ALDPS_ENABLE | RTL8211F_ALDPS_XTAL_OFF;
   161	
   162		priv->has_phycr2 = !(phy_id == RTL_8211FVD_PHYID);
   163		if (priv->has_phycr2) {
   164			ret = phy_read_paged(phydev, 0xa43, RTL8211F_PHYCR2);
   165			if (ret < 0)
   166				return ret;
   167	
   168			priv->phycr2 = ret & RTL8211F_CLKOUT_EN;
   169			if (of_property_read_bool(dev->of_node, "realtek,clkout-disable"))
   170				priv->phycr2 &= ~RTL8211F_CLKOUT_EN;
   171		}
   172	
   173		phydev->priv = priv;
   174	
   175		return 0;
   176	}
   177	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next] net: phy: realtek: disable broadcast address feature of rtl8211f

[kernel test robot]

Hi Zhiyuan,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Zhiyuan-Wan/net-phy-realtek-disable-broadcast-address-feature-of-rtl8211f/20241203-205751
base:   net-next/main
patch link:    https://lore.kernel.org/r/20241203125430.2078090-1-kmlinuxm%40gmail.com
patch subject: [PATCH net-next] net: phy: realtek: disable broadcast address feature of rtl8211f
config: mips-ip22_defconfig (https://download.01.org/0day-ci/archive/20241204/202412041520.hGCSOuph-lkp@intel.com/config)
compiler: mips-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241204/202412041520.hGCSOuph-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412041520.hGCSOuph-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/net/phy/realtek.c: In function 'rtl821x_probe':
>> drivers/net/phy/realtek.c:147:51: error: passing argument 1 of 'PTR_ERR' makes pointer from integer without a cast [-Wint-conversion]
     147 |                 return dev_err_probe(dev, PTR_ERR(ret),
         |                                                   ^~~
         |                                                   |
         |                                                   int
   In file included from include/linux/kernfs.h:9,
                    from include/linux/sysfs.h:16,
                    from include/linux/kobject.h:20,
                    from include/linux/of.h:18,
                    from drivers/net/phy/realtek.c:11:
   include/linux/err.h:52:61: note: expected 'const void *' but argument is of type 'int'
      52 | static inline long __must_check PTR_ERR(__force const void *ptr)
         |                                                 ~~~~~~~~~~~~^~~


vim +/PTR_ERR +147 drivers/net/phy/realtek.c

   126	
   127	static int rtl821x_probe(struct phy_device *phydev)
   128	{
   129		struct device *dev = &phydev->mdio.dev;
   130		struct rtl821x_priv *priv;
   131		u32 phy_id = phydev->drv->phy_id;
   132		int ret;
   133	
   134		priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
   135		if (!priv)
   136			return -ENOMEM;
   137	
   138		priv->clk = devm_clk_get_optional_enabled(dev, NULL);
   139		if (IS_ERR(priv->clk))
   140			return dev_err_probe(dev, PTR_ERR(priv->clk),
   141					     "failed to get phy clock\n");
   142	
   143		dev_dbg(dev, "disabling MDIO address 0 for this phy");
   144		ret = phy_modify_paged(phydev, 0xa43, RTL8211F_PHYCR1,
   145					       RTL8211F_PHYAD0_EN, 0);
   146		if (ret < 0) {
 > 147			return dev_err_probe(dev, PTR_ERR(ret),
   148					     "disabling MDIO address 0 failed\n");
   149		}
   150		/* Deny broadcast address as PHY address */
   151		if (phydev->mdio.addr == 0)
   152			return -ENODEV;
   153	
   154		ret = phy_read_paged(phydev, 0xa43, RTL8211F_PHYCR1);
   155		if (ret < 0)
   156			return ret;
   157	
   158		priv->phycr1 = ret & (RTL8211F_ALDPS_PLL_OFF | RTL8211F_ALDPS_ENABLE | RTL8211F_ALDPS_XTAL_OFF);
   159		if (of_property_read_bool(dev->of_node, "realtek,aldps-enable"))
   160			priv->phycr1 |= RTL8211F_ALDPS_PLL_OFF | RTL8211F_ALDPS_ENABLE | RTL8211F_ALDPS_XTAL_OFF;
   161	
   162		priv->has_phycr2 = !(phy_id == RTL_8211FVD_PHYID);
   163		if (priv->has_phycr2) {
   164			ret = phy_read_paged(phydev, 0xa43, RTL8211F_PHYCR2);
   165			if (ret < 0)
   166				return ret;
   167	
   168			priv->phycr2 = ret & RTL8211F_CLKOUT_EN;
   169			if (of_property_read_bool(dev->of_node, "realtek,clkout-disable"))
   170				priv->phycr2 &= ~RTL8211F_CLKOUT_EN;
   171		}
   172	
   173		phydev->priv = priv;
   174	
   175		return 0;
   176	}
   177	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next] net: phy: realtek: disable broadcast address feature of rtl8211f

[kernel test robot]

Hi Zhiyuan,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Zhiyuan-Wan/net-phy-realtek-disable-broadcast-address-feature-of-rtl8211f/20241203-205751
base:   net-next/main
patch link:    https://lore.kernel.org/r/20241203125430.2078090-1-kmlinuxm%40gmail.com
patch subject: [PATCH net-next] net: phy: realtek: disable broadcast address feature of rtl8211f
config: i386-buildonly-randconfig-006 (https://download.01.org/0day-ci/archive/20241204/202412041557.EtqjkJE5-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241204/202412041557.EtqjkJE5-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412041557.EtqjkJE5-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/net/phy/realtek.c: In function 'rtl821x_probe':
>> drivers/net/phy/realtek.c:147:51: warning: passing argument 1 of 'PTR_ERR' makes pointer from integer without a cast [-Wint-conversion]
     147 |                 return dev_err_probe(dev, PTR_ERR(ret),
         |                                                   ^~~
         |                                                   |
         |                                                   int
   In file included from include/linux/kernfs.h:9,
                    from include/linux/sysfs.h:16,
                    from include/linux/kobject.h:20,
                    from include/linux/of.h:18,
                    from drivers/net/phy/realtek.c:11:
   include/linux/err.h:52:61: note: expected 'const void *' but argument is of type 'int'
      52 | static inline long __must_check PTR_ERR(__force const void *ptr)
         |                                                 ~~~~~~~~~~~~^~~


vim +/PTR_ERR +147 drivers/net/phy/realtek.c

   126	
   127	static int rtl821x_probe(struct phy_device *phydev)
   128	{
   129		struct device *dev = &phydev->mdio.dev;
   130		struct rtl821x_priv *priv;
   131		u32 phy_id = phydev->drv->phy_id;
   132		int ret;
   133	
   134		priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
   135		if (!priv)
   136			return -ENOMEM;
   137	
   138		priv->clk = devm_clk_get_optional_enabled(dev, NULL);
   139		if (IS_ERR(priv->clk))
   140			return dev_err_probe(dev, PTR_ERR(priv->clk),
   141					     "failed to get phy clock\n");
   142	
   143		dev_dbg(dev, "disabling MDIO address 0 for this phy");
   144		ret = phy_modify_paged(phydev, 0xa43, RTL8211F_PHYCR1,
   145					       RTL8211F_PHYAD0_EN, 0);
   146		if (ret < 0) {
 > 147			return dev_err_probe(dev, PTR_ERR(ret),
   148					     "disabling MDIO address 0 failed\n");
   149		}
   150		/* Deny broadcast address as PHY address */
   151		if (phydev->mdio.addr == 0)
   152			return -ENODEV;
   153	
   154		ret = phy_read_paged(phydev, 0xa43, RTL8211F_PHYCR1);
   155		if (ret < 0)
   156			return ret;
   157	
   158		priv->phycr1 = ret & (RTL8211F_ALDPS_PLL_OFF | RTL8211F_ALDPS_ENABLE | RTL8211F_ALDPS_XTAL_OFF);
   159		if (of_property_read_bool(dev->of_node, "realtek,aldps-enable"))
   160			priv->phycr1 |= RTL8211F_ALDPS_PLL_OFF | RTL8211F_ALDPS_ENABLE | RTL8211F_ALDPS_XTAL_OFF;
   161	
   162		priv->has_phycr2 = !(phy_id == RTL_8211FVD_PHYID);
   163		if (priv->has_phycr2) {
   164			ret = phy_read_paged(phydev, 0xa43, RTL8211F_PHYCR2);
   165			if (ret < 0)
   166				return ret;
   167	
   168			priv->phycr2 = ret & RTL8211F_CLKOUT_EN;
   169			if (of_property_read_bool(dev->of_node, "realtek,clkout-disable"))
   170				priv->phycr2 &= ~RTL8211F_CLKOUT_EN;
   171		}
   172	
   173		phydev->priv = priv;
   174	
   175		return 0;
   176	}
   177	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki