netdev - Re: net: phy: Dealing with 88e1543 dual-port mode

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: <20201120103601.313a166b@bootlin.com>
Date:   Fri, 20 Nov 2020 10:36:01 +0100
From:   Maxime Chevallier <maxime.chevallier@...tlin.com>
To:     Tobias Waldekranz <tobias@...dekranz.com>
Cc:     Russell King - ARM Linux admin <linux@...linux.org.uk>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Antoine Tenart <atenart@...nel.org>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        netdev@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: net: phy: Dealing with 88e1543 dual-port mode

Hi Tobias,

On Fri, 20 Nov 2020 01:11:12 +0100
Tobias Waldekranz <tobias@...dekranz.com> wrote:

>On Thu, Nov 19, 2020 at 23:16, Russell King - ARM Linux admin <linux@...linux.org.uk> wrote:
>> On Thu, Nov 19, 2020 at 11:43:39PM +0100, Tobias Waldekranz wrote:  
>>> On Thu, Nov 19, 2020 at 16:24, Maxime Chevallier <maxime.chevallier@...tlin.com> wrote:  
>>> > I don't think we have a way to distinguish from the DT if we are in
>>> > SGMII-to-Fibre or in SGMII-to-{Copper + Fibre}, since the description is
>>> > the same, we don't have any information in DT about wether or not the
>>> > PHY is wired to a Copper RJ45 port.
>>> >
>>> > Maybe we should have a way to indicate if a PHY is wired to a Copper
>>> > port in DT ?  
>>> 
>>> Do you mean something like:
>>> 
>>> SGMII->SGMII (Fibre):
>>> ethernet-phy@0 {
>>>    sfp = <&sfp0>;
>>> };
>>> 
>>> SGMII->MDI (Copper):
>>> ethernet-phy@0 {
>>>     mdi;
>>> };
>>> 
>>> SGMII->Auto Media Detect
>>> ethernet-phy@0 {
>>>     mdi;
>>>     sfp = <&sfp0>;
>>> };  
>>
>> This isn't something we could realistically do - think about how many
>> DT files are out there today which would not have this for an existing
>> PHY. The default has to be that today's DT descriptions continue to work
>> as-is, and that includes ones which already support copper and fibre
>> either with or without a sfp property.
>>
>> So, we can't draw any conclusion about whether the fiber interface is
>> wired from whether there is a sfp property or not.
>>
>> We also can't draw a conclusion about whether the copper side is wired
>> using a "mdi" property, or whether there is a "sfp" property or not.
>>
>> The only thing we could realistically do today is to introduce a
>> property like:
>>
>> 	mdi = "disabled" | "okay";
>>
>> to indicate whether the copper port can be used, and maybe something
>> similar for the fiber interface.  Maybe as you suggest, not "okay"
>> but specifying the number of connected pairs would be a good idea,
>> or maybe that should be a separate property?  
>
>Maybe you could have optional media nodes under the PHY instead, so that
>you don't involve the SFP property in the logic (SGMII can be connected
>to lots of things after all):
>
>    ethernet-phy@0 {
>        ...
>
>        sgmii {
>            status = "okay";
>            preferred;
>        };
>
>        mdi {
>           status = "okay";
>           pairs = <2>;
>        };
>    };

I like that approach too, and I agree that we do need to be very careful
with not breaking existing PHYs, where most of the time we assume that
a PHY simply has a 8P8C (RJ45) connector.

Maybe the term MDI is a bit misused here, my understanding was that MDI,
standing for "Media Dependent Interface" represents the media-side
interface in general, and not a particular technology such as
xxxBaseT/X/K or Copper of Fibre.

So maybe we could be a bit more generic, with something along these lines :

    ethernet-phy@0 {
        ...

        mdi {
            port@0 {
                media = "10baseT", "100baseT", "1000baseT";
                pairs = <1>;
	    };

            port@1 {
                media = "1000baseX", "10gbaseR"
            };
        };
    };

This would allow us to explicitely indicate which modes are supported
by each port.

And in absence of the mdi node, we indeed fallback to the usual behaviour.

>In the absence of any media declarations, you fall back to the driver's
>default behavior (keeping compatibility with older DTs). But you can
>still add support for more configurations if the information is
>available.

I also like the idea of having a way to express the "preferred" media,
although I wonder if that's something we want to include in DT or that
we would want to tweak at runtime, through ethtool for example.

What do you think ?

Thanks,

Maxime