netdev - Re: [PATCH net-next v2] net: phy: broadcom: add 1000Base-X support for BCM54616S

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <7e397b2a-b2bc-19e3-5104-a596e88e2a9a@fb.com>
Date:   Fri, 2 Aug 2019 19:52:19 +0000
From:   Tao Ren <taoren@...com>
To:     Andrew Lunn <andrew@...n.ch>
CC:     Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        "Arun Parameswaran" <arun.parameswaran@...adcom.com>,
        Justin Chen <justinpopo6@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "openbmc@...ts.ozlabs.org" <openbmc@...ts.ozlabs.org>
Subject: Re: [PATCH net-next v2] net: phy: broadcom: add 1000Base-X support
 for BCM54616S

On 8/2/19 7:50 AM, Andrew Lunn wrote:
>> +static int bcm54616s_read_status(struct phy_device *phydev)
>> +{
>> +	int err;
>> +
>> +	err = genphy_read_status(phydev);
>> +
>> +	/* 1000Base-X register set doesn't provide speed fields: the
>> +	 * link speed is always 1000 Mb/s as long as link is up.
>> +	 */
>> +	if (phydev->dev_flags & PHY_BCM_FLAGS_MODE_1000BX &&
>> +	    phydev->link)
>> +		phydev->speed = SPEED_1000;
>> +
>> +	return err;
>> +}
> 
> This function is equivalent to bcm5482_read_status(). You should use
> it, rather than add a new function.

Thank you for pointing it out. Will fix the code.

BTW, should I update the patch subject to something more descriptive (such as "net: phy: broadcom: fix BCM54616S read_status in 1000X mode")? Or I should use the same title to avoid confusion?


Thanks,

Tao