Re: [PATCH net-next v1 06/14] net: phy: add 1000baseT1 to phy_basic_t1_features

["Radu Pirea (OSS)" <radu-nicolae.pirea@oss.nxp.com>]

On 16.06.2023 23:49, Andrew Lunn wrote:
> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button
> 
> 
> On Fri, Jun 16, 2023 at 04:53:15PM +0300, Radu Pirea (NXP OSS) wrote:
>> Add 1000baseT1 bit to phy_basic_t1_features.
> 
> Please add an explanation why this is safe. For example, why the
> RTL9000AA does not start saying it supports 1000BaseT1_Full.

I added 1000BaseT1_Full to that array because all the baseT1 features 
are in the same array. However this is not looking right to me. For 
example, the output of ethtool for TJA1120 looks like this:
[root@alarm ~]# ethtool end0
Settings for end0:
         Supported ports: [ TP ]
         Supported link modes:   100baseT1/Full
                                 1000baseT1/Full
                                 10baseT1L/Full
         Supported pause frame use: Symmetric
         Supports auto-negotiation: No
         Supported FEC modes: Not reported
         Advertised link modes:  100baseT1/Full
                                 1000baseT1/Full
                                 10baseT1L/Full
         Advertised pause frame use: Symmetric
         Advertised auto-negotiation: No
         Advertised FEC modes: Not reported
         Speed: 100Mb/s
         Duplex: Full
         Auto-negotiation: off
         master-slave cfg: forced master
         master-slave status: master
         Port: Twisted Pair
         PHYAD: 1
         Transceiver: external
         MDI-X: Unknown
         Supports Wake-on: g
         Wake-on: d

Which is wrong. TJA1120 does not support 10baseT1L nor 100baseT1.
We should have a phylib function that determines the features for T1 PHYs.

> 
> Has 1000BaseT1_Full been standardised? If there a feature bit in a
> register to indicate the hardware supports it? That would be the
> preferred method to determine what the hardware can do.

Yes, it was standardized in IEEE802.3bp and there are bits in the MMD 1 
that tells you if the PHY supports 1000BaseT1. And here I am talking
about BASE_T1_PMA_XTD_ABILITY(0x12).1.

> 
>          Andrew

-- 
Radu P.