netdev - Re: [PATCH RFC 4/7] net: phy: add support for probing MMDs >= 8 for devices-in-package

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <be1dad52-c199-f4d7-fa36-8198151fe2dd@arm.com>
Date:   Tue, 26 May 2020 12:20:10 -0500
From:   Jeremy Linton <jeremy.linton@....com>
To:     Russell King - ARM Linux admin <linux@...linux.org.uk>,
        Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>
Cc:     Florian Fainelli <f.fainelli@...il.com>, netdev@...r.kernel.org
Subject: Re: [PATCH RFC 4/7] net: phy: add support for probing MMDs >= 8 for
 devices-in-package

Hi,

On 5/26/20 12:14 PM, Russell King - ARM Linux admin wrote:
> On Tue, May 26, 2020 at 03:31:16PM +0100, Russell King wrote:
>> Add support for probing MMDs above 7 for a valid devices-in-package
>> specifier.
>>
>> Signed-off-by: Russell King <rmk+kernel@...linux.org.uk>
>> ---
>>   drivers/net/phy/phy_device.c | 39 ++++++++++++++++++++++++++++++++++--
>>   include/linux/phy.h          |  2 ++
>>   2 files changed, 39 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>> index 0d6b6ca66216..fa9164ac0f3d 100644
>> --- a/drivers/net/phy/phy_device.c
>> +++ b/drivers/net/phy/phy_device.c
>> @@ -659,6 +659,28 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, u32 phy_id,
>>   }
>>   EXPORT_SYMBOL(phy_device_create);
>>   
>> +/* phy_c45_probe_present - checks to see if a MMD is present in the package
>> + * @bus: the target MII bus
>> + * @prtad: PHY package address on the MII bus
>> + * @devad: PHY device (MMD) address
>> + *
>> + * Read the MDIO_STAT2 register, and check whether a device is responding
>> + * at this address.
>> + *
>> + * Returns: negative error number on bus access error, zero if no device
>> + * is responding, or positive if a device is present.
>> + */
>> +static int phy_c45_probe_present(struct mii_bus *bus, int prtad, int devad)
>> +{
>> +	int stat2;
>> +
>> +	stat2 = mdiobus_c45_read(bus, prtad, devad, MDIO_STAT2);
>> +	if (stat2 < 0)
>> +		return stat2;
>> +
>> +	return (stat2 & MDIO_STAT2_DEVPRST) == MDIO_STAT2_DEVPRST_VAL;
>> +}
>> +
>>   /* get_phy_c45_devs_in_pkg - reads a MMD's devices in package registers.
>>    * @bus: the target MII bus
>>    * @addr: PHY address on the MII bus
>> @@ -709,12 +731,25 @@ static int get_phy_c45_ids(struct mii_bus *bus, int addr, u32 *phy_id,
>>   {
>>   	const int num_ids = ARRAY_SIZE(c45_ids->device_ids);
>>   	u32 *devs = &c45_ids->devices_in_package;
>> -	int i, phy_reg;
>> +	int i, ret, phy_reg;
>>   
>>   	/* Find first non-zero Devices In package. Device zero is reserved
>>   	 * for 802.3 c45 complied PHYs, so don't probe it at first.
>>   	 */
>> -	for (i = 1; i < num_ids && *devs == 0; i++) {
>> +	for (i = 1; i < MDIO_MMD_NUM && *devs == 0; i++) {
>> +		if (i >= 8) {
>> +			/* Only probe for the devices-in-package if there
>> +			 * is a PHY reporting as present here; this avoids
>> +			 * picking up on PHYs that implement non-IEEE802.3
>> +			 * compliant register spaces.
>> +			 */
>> +			ret = phy_c45_probe_present(bus, addr, i);
>> +			if (ret < 0)
>> +				return -EIO;
>> +
>> +			if (!ret)
>> +				continue;
>> +		}
> 
> A second look at 802.3, this can't be done for all MMDs (which becomes
> visible when I look at the results from the 88x3310.)  Only MMDs 1, 2,
> 3, 4, 5, 30 and 31 are defined to have this register with the "Device
> Present" bit pair.
> 

I'm not sure it helps, but my thought process following some of the 
discussion last night was:

something to the effect:

	for (i = 1; i < MDIO_MMD_NUM && *devs == 0; i++) {
+		if (i & RESERVED_MMDS)
+			continue;

where RESERVED_MMDS was a hardcoded bitfield matching the IEEE reserved 
MMDs. or maybe a "IGNORE_MMDS" which also includes BIT0 and other MMDs 
the code doesn't understand.