netdev - Re: [PATCH] net: dsa: mv88e6xxx: don't force settings on CPU port

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] [day] [month] [year] [list]

Message-ID: <9f0bb7db-f80c-759a-ada8-952f4f05aeba@zonque.org>
Date:   Mon, 22 Jun 2020 20:43:15 +0200
From:   Daniel Mack <daniel@...que.org>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     vivien.didelot@...il.com, f.fainelli@...il.com,
        davem@...emloft.net, netdev@...r.kernel.org
Subject: Re: [PATCH] net: dsa: mv88e6xxx: don't force settings on CPU port

Hi Andrew,

Picking up this ancient thread, sorry for the delay.

On 3/30/20 3:40 PM, Andrew Lunn wrote:
> On Mon, Mar 30, 2020 at 11:29:27AM +0200, Daniel Mack wrote:
>> On 3/28/20 12:52 AM, Andrew Lunn wrote:
>>> Did you turn off auto-neg on the external PHY and use fixed 100Full?
>>> Ethtool on the SoC interface should show you if the switch PHY is
>>> advertising anything. I'm guessing it is not, and hence you need to
>>> turn off auto neg on the external PHY.
>>>
>>> Another option would be something like
>>>
>>>                                         port@6 {
>>>                                                 reg = <6>;
>>>                                                 label = "cpu";
>>>                                                 ethernet = <&fec1>;
>>>
>>>                                                 phy-handle = <phy6>;
>>>                                         };
>>>                                 };
>>>
>>>                                 mdio {
>>>                                         #address-cells = <1>;
>>>                                         #size-cells = <0>;
>>>                                         phy6: ethernet-phy@6 {
>>>                                                 reg = <6>;
>>>                                                 interrupt-parent = <&switch0>;
>>>                                                 interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
>>>                                         };
>>>                                 };
>>>
>>> By explicitly saying there is a PHY for the CPU node, phylink might
>>> drive it.
> 
> You want to debug this. Although what you have is unusual, yours is
> not the only board. It is something we want to work. And ideally,
> there should be something controlling the PHY.

I spent some more time on this today, and the reason for why this fails
is simple. The PHY on port 4 is internal, and mv88e6xxx_mac_config()
hence decides to not touch the config of this port, unless it's a
fixed-linked config. And the latter is not an option the port has a
phy-handle.

This means that non-fixed ports with internal PHYs are only programmed
once at probe time, and userspace can't modify the settings later on.

I've sent a patch to relax that check, but tbh I'm not sure whether I
miss a relevant piece of detail about the current code.


Thanks,
Daniel