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

[Heiner Kallweit]

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@xxxxxxx/
> Fixes: ec392abc9593 ("PCI: dw-rockchip: Enable async probe by default")
> Cc: Robin Murphy <robin.murphy@xxxxxxx>
> Cc: Danilo Krummrich <dakr@xxxxxxxxxx>
> Cc: Niklas Cassel <cassel@xxxxxxxxxx>
> Signed-off-by: Anand Moon <linux.amoon@xxxxxxxxx>
> ---
>  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