netdev - Re: [PATCH 1/3] net: phy: add support to detect 100BASE-T1 capability

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 14 Dec 2017 10:21:56 +0100
From:   Lucas Stach <l.stach@...gutronix.de>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Florian Fainelli <f.fainelli@...il.com>, netdev@...r.kernel.org,
        kernel@...gutronix.de, patchwork-lst@...gutronix.de
Subject: Re: [PATCH 1/3] net: phy: add support to detect 100BASE-T1
 capability

Hi Andrew,

Am Mittwoch, den 13.12.2017, 21:11 +0100 schrieb Andrew Lunn:
> On Wed, Dec 13, 2017 at 06:37:49PM +0100, Lucas Stach wrote:
> > 100BASE-T1 is the automotive ethernet standard 802.3bw-2015.
> > Currently
> > we don't detect any valid modes for PHYs, which only support this
> > standard. Add support to detect the common 100Mbit full-duplex
> > mode.
> > 
> > Signed-off-by: Lucas Stach <l.stach@...gutronix.de>
> > ---
> >  drivers/net/phy/phy_device.c | 2 ++
> >  include/uapi/linux/mii.h     | 1 +
> >  2 files changed, 3 insertions(+)
> > 
> > diff --git a/drivers/net/phy/phy_device.c
> > b/drivers/net/phy/phy_device.c
> > index 67f25ac29025..8ef48b38d97b 100644
> > --- a/drivers/net/phy/phy_device.c
> > +++ b/drivers/net/phy/phy_device.c
> > @@ -1607,6 +1607,8 @@ int genphy_config_init(struct phy_device
> > *phydev)
> >  		if (val < 0)
> >  			return val;
> >  
> > +		if (val & ESTATUS_100T1_FULL)
> > +			features |= SUPPORTED_100baseT_Full;
> 
> Hi Lucas
> 
> Why did you decide to do this, and not add a SUPPORTED_100baseT1?
> 
> Could a device support both 100-BASE-T and 100-BASE-T1?  If at some
> point we need to differentiate between them, it is going to be
> hard. Especially since this is part of the kernel ABI.

Networking and especially PHY isn't really my primary area of
expertise, so excuse my ignorance. My reasoning was that we don't
differentiate between 100BASE-T2 and 100BASE-T4 in the kernel today, so
I thought it was fine to handle T1 the same way.

There are PHYs that can both do regular 100/1000 MBit Ethernet and
100BASE-T1, but definitely not at the same time or over the same
electrical wiring. 100BASE-T1 is really different in that it uses
capacitive coupling, instead of magnetic like on regular Ethernet. So
it is really a board level decision what gets used and is not something
I would expect to change at runtime.

I'll leave it to your judgment if this patch seems fine with the above
information in mind. Happy to rework if needed.

Regards,
Lucas