Re: [PATCHv2 1/5] net: dsa: Support internal phy on 'cpu' port

[Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>]

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-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>

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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html