netdev - Re: [PATCHv2 1/5] net: dsa: Support internal phy 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: <20180115153842.GF21450@lunn.ch>
Date:   Mon, 15 Jan 2018 16:38:42 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Sebastian Reichel <sebastian.reichel@...labora.co.uk>
Cc:     Vivien Didelot <vivien.didelot@...oirfairelinux.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <kernel@...gutronix.de>,
        Fabio Estevam <fabio.estevam@....com>,
        Ian Ray <ian.ray@...com>, Nandor Han <nandor.han@...com>,
        Rob Herring <robh+dt@...nel.org>,
        "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCHv2 1/5] net: dsa: Support internal phy on 'cpu' port

On Mon, Jan 15, 2018 at 01:15:04PM +0100, Sebastian Reichel wrote:
> This adds support for enabling the internal PHY for a 'cpu' port.
> It has been tested on GE B850v3,  B650v3 and B450v3, which have a
> built-in MV88E6240 switch connected to a PCIe based network card.
> Without this patch the link does not come up and no traffic can be
> routed through the switch.
> 
> The PHY interface, that is being used on the above test systems is
> part of the MV88E6240 and since mv88e6xxx driver resets the chip
> during probe, it is definitely disabled without this patch.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@...labora.co.uk>

Hi Sebastian

I was not very happy about the original implementation. This is a bit
better, but i still think it needs improvement. The enum
PHY_INTERFACE_MODE_INTERNAL means the PHY is connected to the MAC
using something other than traditional MII. We don't use this for any
other of the PHYs inside the Marvell switch, or for any other vendors
switch which has internal PHYs, since they are connected by MII.

You also make the assumption that the PHY for the CPU port actually is
internal. It does not need to be. It could be an external PHY.

The key thing here is to know there is a PHY for the CPU port. The
standard way to represent that is to have a phy-handle. That
phy-handle also tells you which PHY it is, without making any
assumptions.

Please can you re-write this patch to look for a phy-handle in the cpu
node.

Thanks
	Andrew