U-Boot DSA driver for microchip KSZ9477 suggestions needed

[Vladimir Oltean]

On Mon, Jun 21, 2021 at 04:10:51PM -0700, Tim Harvey wrote:
> Greetings,
> 
> I've written a U-Boot phy driver for a Microchip KSZ9477 ethernet
> switch that I submitted some time ago as an RFC [1] which simply
> enables the ports in the dt and puts them in forwarding mode with link
> detect. However I think this would be much better suited as a DSA
> driver now that UCLASS_DSA exists.

Define 'link detect'. I don't know how the switch-as-PHY drivers work.
I suppose the DSA master is made to connect to a PHY device which is
implemented by the switch driver, but whose port's link status is being
reported?

> The KSZ9477 switch is used in the Gateworks GW7901 IMX8M Mini based
> board and is hooked up via I2C (instead of MDIO) so the relevant dt
> (already in Mainline Linux) looks like this:
> 
> / {
>         aliases {
>                 ethernet0 = &fec1;
>                 ethernet1 = &lan1;
>                 ethernet2 = &lan2;
>                 ethernet3 = &lan3;
>                 ethernet4 = &lan4;
>                 usb0 = &usbotg1;
>                 usb1 = &usbotg2;
>         };
> };
> 
> &fec1 {
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_fec1>;
>         phy-mode = "rgmii-id";

Can you confirm that the FEC will not attempt to add internal RGMII
delays in this case? Either the KSZ switch or the FEC should add delays,
but not both (the link will not work otherwise).

>         local-mac-address = [00 00 00 00 00 00];
>         status = "okay";
> 
>         fixed-link {
>                 speed = <1000>;
>                 full-duplex;
>         };
> };
> 
> &i2c3 {
>         clock-frequency = <400000>;
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_i2c3>;
>         status = "okay";
> 
>         switch: switch at 5f {
>                 compatible = "microchip,ksz9897";
>                 reg = <0x5f>;
>                 pinctrl-0 = <&pinctrl_ksz>;
>                 interrupt-parent = <&gpio4>;
>                 interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
>                 phy-mode = "rgmii-id";

This is strange. Why is phy-mode in the 'switch' node (it is a per-port
property)?

> 
>                 ports {
>                         #address-cells = <1>;
>                         #size-cells = <0>;
>                         lan1: port at 0 {
>                                 reg = <0>;
>                                 label = "lan1";
>                                 local-mac-address = [00 00 00 00 00 00];
>                         };
>                         lan2: port at 1 {
>                                 reg = <1>;
>                                 label = "lan2";
>                                 local-mac-address = [00 00 00 00 00 00];
>                         };
>                         lan3: port at 2 {
>                                 reg = <2>;
>                                 label = "lan3";
>                                 local-mac-address = [00 00 00 00 00 00];
>                         };
>                         lan4: port at 3 {
>                                 reg = <3>;
>                                 label = "lan4";
>                                 local-mac-address = [00 00 00 00 00 00];
>                         };
>                         port at 5 {
>                                 reg = <5>;
>                                 label = "cpu";
>                                 ethernet = <&fec1>;
>                                 phy-mode = "rgmii-id";
> 
>                                 fixed-link {
>                                         speed = <1000>;
>                                         full-duplex;
>                                 };
>                         };
>                 };
>         };
> };
> 
> I'm running into an issue trying to get a DSA driver working for this
> switch on this board. My UCLASS_DSA driver is getting registered and
> dsa_post_bind successfully sees the ports and the master_dev however
> when eth_initialize() is called my dsa driver probes before the fec1
> master because i2c comes before fec in the imx8mm.dtsi device-tree.
> Any suggestions on how and where to properly resolve this (Likely
> there is a way to force a probe)?
> 
> If I hack around that my moving i2c in imx8mm.dtsi after fec I see a
> U-Boot net device for all the ports including the cpu port.

Do you have patch e5d7d119287e ("net: dsa: probe master device")? Does
that not work?

> I'm not quite sure what the necessity of the cpu port is here because
> I assume the idea is to set 'ethact' to one of the physical ports
> before initiating a cmd that sends/receives traffic but I suppose the
> cpu interface is there because that's what Linux dsa does.

I am quite unsure what you mean. 'CPU port' == 'DSA master', i.e. the FEC?
You're asking why does the FEC driver register a UCLASS_ETH device,
considering that it should know it's a DSA master and you should only
use the DSA UCLASS_ETH devices for RX/TX?
I mean, if you can make that change and keep the drivers unaware of the
fact that they are DSA masters, I guess patches are welcome...

> Without adding any tagging and just doing a 'setenv ethact lan1; ping
> <serverip>' I'm not seeing any response.

And are you expecting too? Is the switch configured to forward packets
received from the host port which have no tail tag? Does it do that? If
not, is it an electrical problem (see the RGMII delay question)?

> I'm not exactly clear what to be using here for tagging. The Linux ksz
> driver [3] appears to add a 2 byte tag on ingress packets but a 1 byte
> on egress packets.

That's the expectation, to use the tail tagger to be able to steer
packets towards the desired port, correct.
If you have problems with that you can always use VLANs too, like
sja1105 does:
https://patchwork.ozlabs.org/project/uboot/patch/20191215205302.13325-4-olteanv@gmail.com/

> I'm also not quite clear if I should be implementing link detect on the ports.

DSA assumes that every port has a phy-handle to a PHY driver. In Linux,
the integrated PHYs from the KSZ switches are implemented using an MDIO
bus registered by the DSA switch driver, with custom logic to access
each PHY register from the port memory space, because these switches are
kinda quirky and don't really expose a clause 22 MDIO register map. I'm
not sure how that would work out in U-Boot, but it is the starting point
I would say.

> Any suggestions would be greatly appreciated. I would like to get this
> dsa driver working and submitted and then perhaps move on to added a
> DSA driver or DSA support to drivers/net/phy/mv88e61xx.c

That would be nice. I also need to find some time to resend the sja1105
driver.