netdev - Re: [RFC PATCH] drivers: net: mdio_bus: try indirect clause 45 regs access

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: <87lflq3afx.fsf@tarshish>
Date:   Sun, 17 May 2020 17:25:38 +0300
From:   Baruch Siach <baruch@...s.co.il>
To:     Russell King - ARM Linux admin <linux@...linux.org.uk>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>, netdev@...r.kernel.org
Subject: Re: [RFC PATCH] drivers: net: mdio_bus: try indirect clause 45 regs access

Hi Russell,

On Sun, May 17 2020, Russell King - ARM Linux admin wrote:
> On Sun, May 17, 2020 at 01:20:56PM +0300, Baruch Siach wrote:
>> When the MDIO bus does not support directly clause 45 access, fallback
>> to indirect registers access method to read/write clause 45 registers
>> using clause 22 registers.
>> 
>> Signed-off-by: Baruch Siach <baruch@...s.co.il>
>> ---
>> 
>> Is that the right course?
>> 
>> Currently, this does not really work on the my target machine, which is
>> using the Armada 385 native MDIO bus (mvmdio) connected to clause 45
>> Marvell 88E2110 PHY. Registers MDIO_DEVS1 and MDIO_DEVS1 read bogus
>> values which breaks PHY identification. However, the phytool utility
>> reads the same registers correctly:
>> 
>> phytool eth1/2:1/5
>> ieee-phy: reg:0x05 val:0x008a
>> 
>> eth1 is connected to another PHY (clause 22) on the same MDIO bus.
>> 
>> The same hardware works nicely with the mdio-gpio bus implementation,
>> when mdio pins are muxed as GPIOs.
>
> Not all C45 PHYs are required to provide C22.  I'm pretty sure that
> accessing a C45 PHY through the indirect method is likely something
> that isn't well tested with PHYs, so getting wrong device IDs doesn't
> surprise me.

The 88E2110 PHY datasheets mentions support for indirect C45 access
(FWIW: Rev B, section 3.9.3).

> Is there a reason to try switching back to mvmdio on this device?

No technical reason. U-Boot does not currently provide bit-band MDIO,
and hardware manufacturing testers would like to do their thing in
U-Boot, for some reason.

I just thought it would be nice to support C45 over C22 mdio if the
hardware allows that.

> Some comments on the patch:
>
>> ---
>>  drivers/net/phy/mdio_bus.c | 12 ++++++++++++
>>  drivers/net/phy/phy-core.c |  2 +-
>>  include/linux/phy.h        |  2 ++
>>  3 files changed, 15 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
>> index 7a4eb3f2cb74..12e39f794b29 100644
>> --- a/drivers/net/phy/mdio_bus.c
>> +++ b/drivers/net/phy/mdio_bus.c
>> @@ -790,6 +790,12 @@ int __mdiobus_read(struct mii_bus *bus, int addr, u32 regnum)
>>  	WARN_ON_ONCE(!mutex_is_locked(&bus->mdio_lock));
>>  
>>  	retval = bus->read(bus, addr, regnum);
>> +	if (retval == -EOPNOTSUPP && regnum & MII_ADDR_C45) {
>> +		int c45_devad = (regnum >> 16) & 0x1f;
>> +
>> +		mmd_phy_indirect(bus, addr, c45_devad, regnum & 0xfff);
>> +		retval = bus->read(bus, addr, MII_MMD_DATA);
>> +	}
>
> I don't think this should be done at mdiobus level; I think this is a
> layering violation.  It needs to happen at the PHY level because the
> indirect C45 access via C22 registers is specific to PHYs.
>
> It also needs to check in the general case that the PHY does indeed
> support the C22 register set - not all C45 PHYs do.
>
> So, I think we want this fallback to be conditional on:
>
> - are we probing for the PHY, trying to read its IDs and
>   devices-in-package registers - if yes, allow fallback.
> - does the C45 PHY support the C22 register set - if yes, allow
>   fallback.

I'll take a look. Thanks.

baruch

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@...s.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -