r8169: Fix PHY lookup for mdiobus_get_phy() and improve error reporting during probe

[PATCH next v1] r8169: Fix PHY lookup for mdiobus_get_phy() and improve error reporting during probe

Posted by Anand Moon 2 weeks, 6 days ago

The driver currently assumes the PHY is always at address 0. On some
hardware or buggy BIOS implementations, the PHY may reside at a
different address on the MDIO bus. Update r8169_mdio_register()
to scan the bus for the first available PHY instead of hardcoding
address 0.

Additionally, switch to dev_err_probe() in the main probe path to
provide standardized error handling and cleaner log output for
registration failures.

Link: https://lore.kernel.org/all/87bc37ee-234c-4568-b72e-955c130a6838@arm.com/
Fixes: ec392abc9593 ("PCI: dw-rockchip: Enable async probe by default")
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Danilo Krummrich <dakr@kernel.org>
Cc: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 34 ++++++++++++++++-------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 791277e750ba..0fd735beff92 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -5427,7 +5427,8 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
 {
 	struct pci_dev *pdev = tp->pci_dev;
 	struct mii_bus *new_bus;
-	int ret;
+	struct phy_device *phydev;
+	int ret, addr;
 
 	/* On some boards with this chip version the BIOS is buggy and misses
 	 * to reset the PHY page selector. This results in the PHY ID read
@@ -5462,18 +5463,31 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
 	if (ret)
 		return ret;
 
-	tp->phydev = mdiobus_get_phy(new_bus, 0);
-	if (!tp->phydev) {
+	/* find the first (lowest address) PHY on the current MAC's MII bus */
+	for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
+		struct phy_device *tmp = mdiobus_get_phy(new_bus, addr);
+
+		if (tmp) {
+			phydev = tmp;
+			break;
+		}
+	}
+
+	if (!phydev) {
+		dev_err(&pdev->dev, "no PHY found on bus\n");
 		return -ENODEV;
-	} else if (!tp->phydev->drv) {
-		/* Most chip versions fail with the genphy driver.
-		 * Therefore ensure that the dedicated PHY driver is loaded.
-		 */
+	}
+
+	/* Most chip versions fail with the genphy driver.
+	 * Therefore ensure that the dedicated PHY driver is loaded.
+	 */
+	if (!phydev->drv) {
 		dev_err(&pdev->dev, "no dedicated PHY driver found for PHY ID 0x%08x, maybe realtek.ko needs to be added to initramfs?\n",
-			tp->phydev->phy_id);
+			phydev->phy_id);
 		return -EUNATCH;
 	}
 
+	tp->phydev = phydev;
 	tp->phydev->mac_managed_pm = true;
 	if (rtl_supports_eee(tp))
 		phy_support_eee(tp->phydev);
@@ -5790,11 +5804,11 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	rc = r8169_mdio_register(tp);
 	if (rc)
-		return rc;
+		return dev_err_probe(&pdev->dev, rc, "mdio register failure\n");
 
 	rc = register_netdev(dev);
 	if (rc)
-		return rc;
+		return dev_err_probe(&pdev->dev, rc, "register newdev failure\n");
 
 	if (IS_ENABLED(CONFIG_R8169_LEDS)) {
 		if (rtl_is_8125(tp))

base-commit: 95c541ddfb0815a0ea8477af778bb13bb075079a
-- 
2.50.1

Re: [PATCH next v1] r8169: Fix PHY lookup for mdiobus_get_phy() and improve error reporting during probe

Posted by kernel test robot 2 weeks, 3 days ago

Hi Anand,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 95c541ddfb0815a0ea8477af778bb13bb075079a]

url:    https://github.com/intel-lab-lkp/linux/commits/Anand-Moon/r8169-Fix-PHY-lookup-for-mdiobus_get_phy-and-improve-error-reporting-during-probe/20260317-183327
base:   95c541ddfb0815a0ea8477af778bb13bb075079a
patch link:    https://lore.kernel.org/r/20260317061700.7734-1-linux.amoon%40gmail.com
patch subject: [PATCH next v1] r8169: Fix PHY lookup for mdiobus_get_phy() and improve error reporting during probe
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20260320/202603200758.vueafhDW-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260320/202603200758.vueafhDW-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/202603200758.vueafhDW-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/net/ethernet/realtek/r8169_main.c:5467:17: warning: variable 'phydev' is used uninitialized whenever 'for' loop exits because its condition is false [-Wsometimes-uninitialized]
    5467 |         for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
         |                        ^~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/realtek/r8169_main.c:5476:7: note: uninitialized use occurs here
    5476 |         if (!phydev) {
         |              ^~~~~~
   drivers/net/ethernet/realtek/r8169_main.c:5467:17: note: remove the condition if it is always true
    5467 |         for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
         |                        ^~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/realtek/r8169_main.c:5430:27: note: initialize the variable 'phydev' to silence this warning
    5430 |         struct phy_device *phydev;
         |                                  ^
         |                                   = NULL
   1 warning generated.


vim +5467 drivers/net/ethernet/realtek/r8169_main.c

  5425	
  5426	static int r8169_mdio_register(struct rtl8169_private *tp)
  5427	{
  5428		struct pci_dev *pdev = tp->pci_dev;
  5429		struct mii_bus *new_bus;
  5430		struct phy_device *phydev;
  5431		int ret, addr;
  5432	
  5433		/* On some boards with this chip version the BIOS is buggy and misses
  5434		 * to reset the PHY page selector. This results in the PHY ID read
  5435		 * accessing registers on a different page, returning a more or
  5436		 * less random value. Fix this by resetting the page selector first.
  5437		 */
  5438		if (tp->mac_version == RTL_GIGA_MAC_VER_25 ||
  5439		    tp->mac_version == RTL_GIGA_MAC_VER_26)
  5440			r8169_mdio_write(tp, 0x1f, 0);
  5441	
  5442		new_bus = devm_mdiobus_alloc(&pdev->dev);
  5443		if (!new_bus)
  5444			return -ENOMEM;
  5445	
  5446		new_bus->name = "r8169";
  5447		new_bus->priv = tp;
  5448		new_bus->parent = &pdev->dev;
  5449		new_bus->irq[0] = PHY_MAC_INTERRUPT;
  5450		new_bus->phy_mask = GENMASK(31, 1);
  5451		snprintf(new_bus->id, MII_BUS_ID_SIZE, "r8169-%x-%x",
  5452			 pci_domain_nr(pdev->bus), pci_dev_id(pdev));
  5453	
  5454		new_bus->read = r8169_mdio_read_reg;
  5455		new_bus->write = r8169_mdio_write_reg;
  5456	
  5457		if (tp->mac_version >= RTL_GIGA_MAC_VER_40) {
  5458			new_bus->read_c45 = r8169_mdio_read_reg_c45;
  5459			new_bus->write_c45 = r8169_mdio_write_reg_c45;
  5460		}
  5461	
  5462		ret = devm_mdiobus_register(&pdev->dev, new_bus);
  5463		if (ret)
  5464			return ret;
  5465	
  5466		/* find the first (lowest address) PHY on the current MAC's MII bus */
> 5467		for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
  5468			struct phy_device *tmp = mdiobus_get_phy(new_bus, addr);
  5469	
  5470			if (tmp) {
  5471				phydev = tmp;
  5472				break;
  5473			}
  5474		}
  5475	
  5476		if (!phydev) {
  5477			dev_err(&pdev->dev, "no PHY found on bus\n");
  5478			return -ENODEV;
  5479		}
  5480	
  5481		/* Most chip versions fail with the genphy driver.
  5482		 * Therefore ensure that the dedicated PHY driver is loaded.
  5483		 */
  5484		if (!phydev->drv) {
  5485			dev_err(&pdev->dev, "no dedicated PHY driver found for PHY ID 0x%08x, maybe realtek.ko needs to be added to initramfs?\n",
  5486				phydev->phy_id);
  5487			return -EUNATCH;
  5488		}
  5489	
  5490		tp->phydev = phydev;
  5491		tp->phydev->mac_managed_pm = true;
  5492		if (rtl_supports_eee(tp))
  5493			phy_support_eee(tp->phydev);
  5494		phy_support_asym_pause(tp->phydev);
  5495	
  5496		/* mimic behavior of r8125/r8126 vendor drivers */
  5497		if (tp->mac_version == RTL_GIGA_MAC_VER_61)
  5498			phy_disable_eee_mode(tp->phydev,
  5499					     ETHTOOL_LINK_MODE_2500baseT_Full_BIT);
  5500	
  5501		/* PHY will be woken up in rtl_open() */
  5502		phy_suspend(tp->phydev);
  5503	
  5504		return 0;
  5505	}
  5506	

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

Re: [PATCH next v1] r8169: Fix PHY lookup for mdiobus_get_phy() and improve error reporting during probe

Posted by Anand Moon 2 weeks, 3 days ago

Hi Kernel test robot,

On Fri, 20 Mar 2026 at 05:25, kernel test robot <lkp@intel.com> wrote:
>
> Hi Anand,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on 95c541ddfb0815a0ea8477af778bb13bb075079a]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Anand-Moon/r8169-Fix-PHY-lookup-for-mdiobus_get_phy-and-improve-error-reporting-during-probe/20260317-183327
> base:   95c541ddfb0815a0ea8477af778bb13bb075079a
> patch link:    https://lore.kernel.org/r/20260317061700.7734-1-linux.amoon%40gmail.com
> patch subject: [PATCH next v1] r8169: Fix PHY lookup for mdiobus_get_phy() and improve error reporting during probe
> config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20260320/202603200758.vueafhDW-lkp@intel.com/config)
> compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260320/202603200758.vueafhDW-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/202603200758.vueafhDW-lkp@intel.com/
>
> All warnings (new ones prefixed by >>):

Please discard this patch.

Thanks
-Anand

Re: [PATCH next v1] r8169: Fix PHY lookup for mdiobus_get_phy() and improve error reporting during probe

Posted by kernel test robot 2 weeks, 6 days ago

Hi Anand,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 95c541ddfb0815a0ea8477af778bb13bb075079a]

url:    https://github.com/intel-lab-lkp/linux/commits/Anand-Moon/r8169-Fix-PHY-lookup-for-mdiobus_get_phy-and-improve-error-reporting-during-probe/20260317-183327
base:   95c541ddfb0815a0ea8477af778bb13bb075079a
patch link:    https://lore.kernel.org/r/20260317061700.7734-1-linux.amoon%40gmail.com
patch subject: [PATCH next v1] r8169: Fix PHY lookup for mdiobus_get_phy() and improve error reporting during probe
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20260317/202603171855.OtM7sS7W-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260317/202603171855.OtM7sS7W-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/202603171855.OtM7sS7W-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/net/ethernet/realtek/r8169_main.c:5467:17: warning: variable 'phydev' is used uninitialized whenever 'for' loop exits because its condition is false [-Wsometimes-uninitialized]
    5467 |         for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
         |                        ^~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/realtek/r8169_main.c:5476:7: note: uninitialized use occurs here
    5476 |         if (!phydev) {
         |              ^~~~~~
   drivers/net/ethernet/realtek/r8169_main.c:5467:17: note: remove the condition if it is always true
    5467 |         for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
         |                        ^~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/realtek/r8169_main.c:5430:27: note: initialize the variable 'phydev' to silence this warning
    5430 |         struct phy_device *phydev;
         |                                  ^
         |                                   = NULL
   1 warning generated.


vim +5467 drivers/net/ethernet/realtek/r8169_main.c

  5425	
  5426	static int r8169_mdio_register(struct rtl8169_private *tp)
  5427	{
  5428		struct pci_dev *pdev = tp->pci_dev;
  5429		struct mii_bus *new_bus;
  5430		struct phy_device *phydev;
  5431		int ret, addr;
  5432	
  5433		/* On some boards with this chip version the BIOS is buggy and misses
  5434		 * to reset the PHY page selector. This results in the PHY ID read
  5435		 * accessing registers on a different page, returning a more or
  5436		 * less random value. Fix this by resetting the page selector first.
  5437		 */
  5438		if (tp->mac_version == RTL_GIGA_MAC_VER_25 ||
  5439		    tp->mac_version == RTL_GIGA_MAC_VER_26)
  5440			r8169_mdio_write(tp, 0x1f, 0);
  5441	
  5442		new_bus = devm_mdiobus_alloc(&pdev->dev);
  5443		if (!new_bus)
  5444			return -ENOMEM;
  5445	
  5446		new_bus->name = "r8169";
  5447		new_bus->priv = tp;
  5448		new_bus->parent = &pdev->dev;
  5449		new_bus->irq[0] = PHY_MAC_INTERRUPT;
  5450		new_bus->phy_mask = GENMASK(31, 1);
  5451		snprintf(new_bus->id, MII_BUS_ID_SIZE, "r8169-%x-%x",
  5452			 pci_domain_nr(pdev->bus), pci_dev_id(pdev));
  5453	
  5454		new_bus->read = r8169_mdio_read_reg;
  5455		new_bus->write = r8169_mdio_write_reg;
  5456	
  5457		if (tp->mac_version >= RTL_GIGA_MAC_VER_40) {
  5458			new_bus->read_c45 = r8169_mdio_read_reg_c45;
  5459			new_bus->write_c45 = r8169_mdio_write_reg_c45;
  5460		}
  5461	
  5462		ret = devm_mdiobus_register(&pdev->dev, new_bus);
  5463		if (ret)
  5464			return ret;
  5465	
  5466		/* find the first (lowest address) PHY on the current MAC's MII bus */
> 5467		for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
  5468			struct phy_device *tmp = mdiobus_get_phy(new_bus, addr);
  5469	
  5470			if (tmp) {
  5471				phydev = tmp;
  5472				break;
  5473			}
  5474		}
  5475	
  5476		if (!phydev) {
  5477			dev_err(&pdev->dev, "no PHY found on bus\n");
  5478			return -ENODEV;
  5479		}
  5480	
  5481		/* Most chip versions fail with the genphy driver.
  5482		 * Therefore ensure that the dedicated PHY driver is loaded.
  5483		 */
  5484		if (!phydev->drv) {
  5485			dev_err(&pdev->dev, "no dedicated PHY driver found for PHY ID 0x%08x, maybe realtek.ko needs to be added to initramfs?\n",
  5486				phydev->phy_id);
  5487			return -EUNATCH;
  5488		}
  5489	
  5490		tp->phydev = phydev;
  5491		tp->phydev->mac_managed_pm = true;
  5492		if (rtl_supports_eee(tp))
  5493			phy_support_eee(tp->phydev);
  5494		phy_support_asym_pause(tp->phydev);
  5495	
  5496		/* mimic behavior of r8125/r8126 vendor drivers */
  5497		if (tp->mac_version == RTL_GIGA_MAC_VER_61)
  5498			phy_disable_eee_mode(tp->phydev,
  5499					     ETHTOOL_LINK_MODE_2500baseT_Full_BIT);
  5500	
  5501		/* PHY will be woken up in rtl_open() */
  5502		phy_suspend(tp->phydev);
  5503	
  5504		return 0;
  5505	}
  5506	

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

Re: [PATCH next v1] r8169: Fix PHY lookup for mdiobus_get_phy() and improve error reporting during probe

Posted by Heiner Kallweit 2 weeks, 6 days ago

On 17.03.2026 07:16, Anand Moon wrote:
> The driver currently assumes the PHY is always at address 0. On some
> hardware or buggy BIOS implementations, the PHY may reside at a
> different address on the MDIO bus. Update r8169_mdio_register()
> to scan the bus for the first available PHY instead of hardcoding
> address 0.

No. The PHY always shows up at (virtual) address 0, due to the
MAC register based way of accessing the internal PHY.
Do you have any concrete example of such a "hardware or buggy bios"?

> 
> Additionally, switch to dev_err_probe() in the main probe path to
> provide standardized error handling and cleaner log output for
> registration failures.
> 
> Link: https://lore.kernel.org/all/87bc37ee-234c-4568-b72e-955c130a6838@arm.com/
> Fixes: ec392abc9593 ("PCI: dw-rockchip: Enable async probe by default")
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Danilo Krummrich <dakr@kernel.org>
> Cc: Niklas Cassel <cassel@kernel.org>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
>  drivers/net/ethernet/realtek/r8169_main.c | 34 ++++++++++++++++-------
>  1 file changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 791277e750ba..0fd735beff92 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -5427,7 +5427,8 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
>  {
>  	struct pci_dev *pdev = tp->pci_dev;
>  	struct mii_bus *new_bus;
> -	int ret;
> +	struct phy_device *phydev;
> +	int ret, addr;
>  
>  	/* On some boards with this chip version the BIOS is buggy and misses
>  	 * to reset the PHY page selector. This results in the PHY ID read
> @@ -5462,18 +5463,31 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
>  	if (ret)
>  		return ret;
>  
> -	tp->phydev = mdiobus_get_phy(new_bus, 0);
> -	if (!tp->phydev) {
> +	/* find the first (lowest address) PHY on the current MAC's MII bus */
> +	for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
> +		struct phy_device *tmp = mdiobus_get_phy(new_bus, addr);
> +
> +		if (tmp) {
> +			phydev = tmp;
> +			break;
> +		}
> +	}
> +
> +	if (!phydev) {
> +		dev_err(&pdev->dev, "no PHY found on bus\n");
>  		return -ENODEV;
> -	} else if (!tp->phydev->drv) {
> -		/* Most chip versions fail with the genphy driver.
> -		 * Therefore ensure that the dedicated PHY driver is loaded.
> -		 */

This check is needed. It's seems your actual intention is something completely
different from what you state in the commit message. You try to find a workaround
for the issues with request_module() from async contect, caused by the referenced
change to Rockchip PCI.

> +	}
> +
> +	/* Most chip versions fail with the genphy driver.
> +	 * Therefore ensure that the dedicated PHY driver is loaded.
> +	 */
> +	if (!phydev->drv) {
>  		dev_err(&pdev->dev, "no dedicated PHY driver found for PHY ID 0x%08x, maybe realtek.ko needs to be added to initramfs?\n",
> -			tp->phydev->phy_id);
> +			phydev->phy_id);
>  		return -EUNATCH;
>  	}
>  
> +	tp->phydev = phydev;
>  	tp->phydev->mac_managed_pm = true;
>  	if (rtl_supports_eee(tp))
>  		phy_support_eee(tp->phydev);
> @@ -5790,11 +5804,11 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  
>  	rc = r8169_mdio_register(tp);
>  	if (rc)
> -		return rc;
> +		return dev_err_probe(&pdev->dev, rc, "mdio register failure\n");
>  
>  	rc = register_netdev(dev);
>  	if (rc)
> -		return rc;
> +		return dev_err_probe(&pdev->dev, rc, "register newdev failure\n");
>  
>  	if (IS_ENABLED(CONFIG_R8169_LEDS)) {
>  		if (rtl_is_8125(tp))
> 
> base-commit: 95c541ddfb0815a0ea8477af778bb13bb075079a

Re: [PATCH next v1] r8169: Fix PHY lookup for mdiobus_get_phy() and improve error reporting during probe

Posted by Anand Moon 2 weeks, 6 days ago

Hi Heiner,

Thanks for your review comments.

On Tue, 17 Mar 2026 at 12:21, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> On 17.03.2026 07:16, Anand Moon wrote:
> > The driver currently assumes the PHY is always at address 0. On some
> > hardware or buggy BIOS implementations, the PHY may reside at a
> > different address on the MDIO bus. Update r8169_mdio_register()
> > to scan the bus for the first available PHY instead of hardcoding
> > address 0.
>
> No. The PHY always shows up at (virtual) address 0, due to the
> MAC register based way of accessing the internal PHY.
> Do you have any concrete example of such a "hardware or buggy bios"?
>
Sorry about that, I didn't realize. Unfortunately, my testing shows
this still doesn't
fix the problem, but I still perceive. I modeled the change after some
other drivers,
but clearly, that approach isn't working in this case.
> >
> > Additionally, switch to dev_err_probe() in the main probe path to
> > provide standardized error handling and cleaner log output for
> > registration failures.
> >
> > Link: https://lore.kernel.org/all/87bc37ee-234c-4568-b72e-955c130a6838@arm.com/
> > Fixes: ec392abc9593 ("PCI: dw-rockchip: Enable async probe by default")
> > Cc: Robin Murphy <robin.murphy@arm.com>
> > Cc: Danilo Krummrich <dakr@kernel.org>
> > Cc: Niklas Cassel <cassel@kernel.org>
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> >  drivers/net/ethernet/realtek/r8169_main.c | 34 ++++++++++++++++-------
> >  1 file changed, 24 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> > index 791277e750ba..0fd735beff92 100644
> > --- a/drivers/net/ethernet/realtek/r8169_main.c
> > +++ b/drivers/net/ethernet/realtek/r8169_main.c
> > @@ -5427,7 +5427,8 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
> >  {
> >       struct pci_dev *pdev = tp->pci_dev;
> >       struct mii_bus *new_bus;
> > -     int ret;
> > +     struct phy_device *phydev;
> > +     int ret, addr;
> >
> >       /* On some boards with this chip version the BIOS is buggy and misses
> >        * to reset the PHY page selector. This results in the PHY ID read
> > @@ -5462,18 +5463,31 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
> >       if (ret)
> >               return ret;
> >
> > -     tp->phydev = mdiobus_get_phy(new_bus, 0);
> > -     if (!tp->phydev) {
> > +     /* find the first (lowest address) PHY on the current MAC's MII bus */
> > +     for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
> > +             struct phy_device *tmp = mdiobus_get_phy(new_bus, addr);
> > +
> > +             if (tmp) {
> > +                     phydev = tmp;
> > +                     break;
> > +             }
> > +     }
> > +
> > +     if (!phydev) {
> > +             dev_err(&pdev->dev, "no PHY found on bus\n");
> >               return -ENODEV;
> > -     } else if (!tp->phydev->drv) {
> > -             /* Most chip versions fail with the genphy driver.
> > -              * Therefore ensure that the dedicated PHY driver is loaded.
> > -              */
>
> This check is needed. It's seems your actual intention is something completely
> different from what you state in the commit message. You try to find a workaround
> for the issues with request_module() from async contect, caused by the referenced
> change to Rockchip PCI.
That makes sense, sorry for the noise.

What are your thoughts on using dev_err_probe() for these registration failures?
I thought it might clean up the error path.

Thanks
-Anand
>
> > +     }
> > +
> > +     /* Most chip versions fail with the genphy driver.
> > +      * Therefore ensure that the dedicated PHY driver is loaded.
> > +      */
> > +     if (!phydev->drv) {
> >               dev_err(&pdev->dev, "no dedicated PHY driver found for PHY ID 0x%08x, maybe realtek.ko needs to be added to initramfs?\n",
> > -                     tp->phydev->phy_id);
> > +                     phydev->phy_id);
> >               return -EUNATCH;
> >       }
> >
> > +     tp->phydev = phydev;
> >       tp->phydev->mac_managed_pm = true;
> >       if (rtl_supports_eee(tp))
> >               phy_support_eee(tp->phydev);
> > @@ -5790,11 +5804,11 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> >
> >       rc = r8169_mdio_register(tp);
> >       if (rc)
> > -             return rc;
> > +             return dev_err_probe(&pdev->dev, rc, "mdio register failure\n");
> >
> >       rc = register_netdev(dev);
> >       if (rc)
> > -             return rc;
> > +             return dev_err_probe(&pdev->dev, rc, "register newdev failure\n");
> >
> >       if (IS_ENABLED(CONFIG_R8169_LEDS)) {
> >               if (rtl_is_8125(tp))
> >
> > base-commit: 95c541ddfb0815a0ea8477af778bb13bb075079a
>

Re: [PATCH next v1] r8169: Fix PHY lookup for mdiobus_get_phy() and improve error reporting during probe

Posted by Heiner Kallweit 2 weeks, 6 days ago

On 17.03.2026 08:59, Anand Moon wrote:
> Hi Heiner,
> 
> Thanks for your review comments.
> 
> On Tue, 17 Mar 2026 at 12:21, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>
>> On 17.03.2026 07:16, Anand Moon wrote:
>>> The driver currently assumes the PHY is always at address 0. On some
>>> hardware or buggy BIOS implementations, the PHY may reside at a
>>> different address on the MDIO bus. Update r8169_mdio_register()
>>> to scan the bus for the first available PHY instead of hardcoding
>>> address 0.
>>
>> No. The PHY always shows up at (virtual) address 0, due to the
>> MAC register based way of accessing the internal PHY.
>> Do you have any concrete example of such a "hardware or buggy bios"?
>>
> Sorry about that, I didn't realize. Unfortunately, my testing shows
> this still doesn't
> fix the problem, but I still perceive. I modeled the change after some
> other drivers,
> but clearly, that approach isn't working in this case.
>>>
>>> Additionally, switch to dev_err_probe() in the main probe path to
>>> provide standardized error handling and cleaner log output for
>>> registration failures.
>>>
>>> Link: https://lore.kernel.org/all/87bc37ee-234c-4568-b72e-955c130a6838@arm.com/
>>> Fixes: ec392abc9593 ("PCI: dw-rockchip: Enable async probe by default")
>>> Cc: Robin Murphy <robin.murphy@arm.com>
>>> Cc: Danilo Krummrich <dakr@kernel.org>
>>> Cc: Niklas Cassel <cassel@kernel.org>
>>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>>> ---
>>>  drivers/net/ethernet/realtek/r8169_main.c | 34 ++++++++++++++++-------
>>>  1 file changed, 24 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
>>> index 791277e750ba..0fd735beff92 100644
>>> --- a/drivers/net/ethernet/realtek/r8169_main.c
>>> +++ b/drivers/net/ethernet/realtek/r8169_main.c
>>> @@ -5427,7 +5427,8 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
>>>  {
>>>       struct pci_dev *pdev = tp->pci_dev;
>>>       struct mii_bus *new_bus;
>>> -     int ret;
>>> +     struct phy_device *phydev;
>>> +     int ret, addr;
>>>
>>>       /* On some boards with this chip version the BIOS is buggy and misses
>>>        * to reset the PHY page selector. This results in the PHY ID read
>>> @@ -5462,18 +5463,31 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
>>>       if (ret)
>>>               return ret;
>>>
>>> -     tp->phydev = mdiobus_get_phy(new_bus, 0);
>>> -     if (!tp->phydev) {
>>> +     /* find the first (lowest address) PHY on the current MAC's MII bus */
>>> +     for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
>>> +             struct phy_device *tmp = mdiobus_get_phy(new_bus, addr);
>>> +
>>> +             if (tmp) {
>>> +                     phydev = tmp;
>>> +                     break;
>>> +             }
>>> +     }
>>> +
>>> +     if (!phydev) {
>>> +             dev_err(&pdev->dev, "no PHY found on bus\n");
>>>               return -ENODEV;
>>> -     } else if (!tp->phydev->drv) {
>>> -             /* Most chip versions fail with the genphy driver.
>>> -              * Therefore ensure that the dedicated PHY driver is loaded.
>>> -              */
>>
>> This check is needed. It's seems your actual intention is something completely
>> different from what you state in the commit message. You try to find a workaround
>> for the issues with request_module() from async contect, caused by the referenced
>> change to Rockchip PCI.
> That makes sense, sorry for the noise.
> 
> What are your thoughts on using dev_err_probe() for these registration failures?
> I thought it might clean up the error path.
> 
I'm not aware of any realistic scenario where these calls would fail (w/o printing
an error message before). So adding an error message wouldn't hurt, but it also
wouldn't be beneficial enough to justify a patch.

> Thanks
> -Anand
>>
>>> +     }
>>> +
>>> +     /* Most chip versions fail with the genphy driver.
>>> +      * Therefore ensure that the dedicated PHY driver is loaded.
>>> +      */
>>> +     if (!phydev->drv) {
>>>               dev_err(&pdev->dev, "no dedicated PHY driver found for PHY ID 0x%08x, maybe realtek.ko needs to be added to initramfs?\n",
>>> -                     tp->phydev->phy_id);
>>> +                     phydev->phy_id);
>>>               return -EUNATCH;
>>>       }
>>>
>>> +     tp->phydev = phydev;
>>>       tp->phydev->mac_managed_pm = true;
>>>       if (rtl_supports_eee(tp))
>>>               phy_support_eee(tp->phydev);
>>> @@ -5790,11 +5804,11 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>>
>>>       rc = r8169_mdio_register(tp);
>>>       if (rc)
>>> -             return rc;
>>> +             return dev_err_probe(&pdev->dev, rc, "mdio register failure\n");
>>>
>>>       rc = register_netdev(dev);
>>>       if (rc)
>>> -             return rc;
>>> +             return dev_err_probe(&pdev->dev, rc, "register newdev failure\n");
>>>
>>>       if (IS_ENABLED(CONFIG_R8169_LEDS)) {
>>>               if (rtl_is_8125(tp))
>>>
>>> base-commit: 95c541ddfb0815a0ea8477af778bb13bb075079a
>>

Re: [PATCH next v1] r8169: Fix PHY lookup for mdiobus_get_phy() and improve error reporting during probe

Posted by Anand Moon 2 weeks, 6 days ago

Hi Heiner,

On Tue, 17 Mar 2026 at 15:14, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> On 17.03.2026 08:59, Anand Moon wrote:
> > Hi Heiner,
> >
> > Thanks for your review comments.
> >
> > On Tue, 17 Mar 2026 at 12:21, Heiner Kallweit <hkallweit1@gmail.com> wrote:
> >>
> >> On 17.03.2026 07:16, Anand Moon wrote:
> >>> The driver currently assumes the PHY is always at address 0. On some
> >>> hardware or buggy BIOS implementations, the PHY may reside at a
> >>> different address on the MDIO bus. Update r8169_mdio_register()
> >>> to scan the bus for the first available PHY instead of hardcoding
> >>> address 0.
> >>
> >> No. The PHY always shows up at (virtual) address 0, due to the
> >> MAC register based way of accessing the internal PHY.
> >> Do you have any concrete example of such a "hardware or buggy bios"?
> >>
> > Sorry about that, I didn't realize. Unfortunately, my testing shows
> > this still doesn't
> > fix the problem, but I still perceive. I modeled the change after some
> > other drivers,
> > but clearly, that approach isn't working in this case.
> >>>
> >>
> >> This check is needed. It's seems your actual intention is something completely
> >> different from what you state in the commit message. You try to find a workaround
> >> for the issues with request_module() from async contect, caused by the referenced
> >> change to Rockchip PCI.
> > That makes sense, sorry for the noise.
> >
> > What are your thoughts on using dev_err_probe() for these registration failures?
> > I thought it might clean up the error path.
> >
> I'm not aware of any realistic scenario where these calls would fail (w/o printing
> an error message before). So adding an error message wouldn't hurt, but it also
> wouldn't be beneficial enough to justify a patch.
>
If we enable .probe_type = PROBE_PREFER_ASYNCHRONOUS in pcie-dw-rockchip.c,
The r8169 module probe will fail due to the module’s request for
synchronous probing.

[1] https://lore.kernel.org/all/87bc37ee-234c-4568-b72e-955c130a6838@arm.com/

Some were in the module, but the probe failed to load the module or firmware.
I hope these changes will resolve the probe warning.

----->8-----
diff --git a/drivers/net/ethernet/realtek/r8169_main.c
b/drivers/net/ethernet/realtek/r8169_main.c
index 791277e750ba..8b7858f6dc6d 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -5790,11 +5790,11 @@ static int rtl_init_one(struct pci_dev *pdev,
const struct pci_device_id *ent)

        rc = r8169_mdio_register(tp);
        if (rc)
-               return rc;
+               return dev_err_probe(&pdev->dev, rc, "mdio register failure\n");

        rc = register_netdev(dev);
        if (rc)
-               return rc;
+               return dev_err_probe(&pdev->dev, rc, "register newdev
failure\n");

        if (IS_ENABLED(CONFIG_R8169_LEDS)) {
                if (rtl_is_8125(tp))

Thanks
-Anand