From mboxrd@z Thu Jan  1 00:00:00 1970
From: Heiner Kallweit <hkallweit1@gmail.com>
Subject: Re: [PATCH net-next v2 01/10] r8169: add basic phylib support
Date: Tue, 10 Jul 2018 21:20:09 +0200
Message-ID: <ae6f72ec-a2af-3082-e481-58899d5ff5c9@gmail.com>
References: <84ac1cb7-bafd-5f15-8cda-e8dfd7d0deb3@gmail.com>
 <9816ca13-1347-5a1c-62d6-1d7c799bde60@gmail.com>
 <20180710191051.GB11754@lunn.ch>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Cc: David Miller <davem@davemloft.net>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Realtek linux nic maintainers <nic_swsd@realtek.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>
To: Andrew Lunn <andrew@lunn.ch>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-wr1-f67.google.com ([209.85.221.67]:45943 "EHLO
        mail-wr1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1732556AbeGJTUn (ORCPT
        <rfc822;netdev@vger.kernel.org>); Tue, 10 Jul 2018 15:20:43 -0400
Received: by mail-wr1-f67.google.com with SMTP id c4-v6so3070936wrs.12
        for <netdev@vger.kernel.org>; Tue, 10 Jul 2018 12:20:16 -0700 (PDT)
In-Reply-To: <20180710191051.GB11754@lunn.ch>
Content-Language: en-US
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

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
>