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:

quoted

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@pengutronix.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