On 10.07.2018 21:10, Andrew Lunn wrote: >> +static int r8169_mdio_read_reg(struct mii_bus *mii_bus, int phyaddr, int >> phyreg) >> +{ >> + struct rtl8169_private *tp = mii_bus->priv; >> + >> + if (phyaddr > 0) >> + return -EINVAL; > > Please use ENODEV. > > The mdio bus is scanned for devices in __mdiobus_register(). If > mdiobus_scan() returns -ENODEV, it is not considered an error, and it > will continue scanning other addresses on the bus. Any other error is > a real error, and will abort the scan. That will probably abort the > bus registration. > OK
>> +static int r8169_mdio_register(struct rtl8169_private *tp) >> +{ >> + struct pci_dev *pdev = tp->pci_dev; >> + struct mii_bus *new_bus; >> + int ret; >> + >> + new_bus = devm_mdiobus_alloc(&pdev->dev); >> + if (!new_bus) >> + return -ENOMEM; >> + >> + new_bus->name = "r8169"; >> + new_bus->phy_mask = ~1; > > Once your handling of addr > 0 is correct, you don't need this. Let > is scan all addresses, just like a normal MDIO bus. The more we can > keep it normal, the better. > Sounds good. I'll wait for feedback on other patches of the series and then prepare a v3. Heiner > Andrew >