| Message ID | 20241203125430.2078090-1-kmlinuxm@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: phy: realtek: disable broadcast address feature of rtl8211f | expand |
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
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.
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
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
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
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;
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(+)