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

[Radu Pirea (OSS)]

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.