On Mon, Jun 21, 2021 at 4:49 PM Vladimir Oltean olteanv@gmail.com wrote:
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?
Yes, each downward port's link status can be detected. Each port has standard GbE PHY registers which can be indirectly read.
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).
Correct, the FEC does not configure delays, only the PHY side.
In the case of DSA for dt it is correct to have phy-mode defined both in the MAC and the PHY (each cpu port) correct?
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@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)?
Right, this is wrong and just did not get caught. I will remove it.
ports { #address-cells = <1>; #size-cells = <0>; lan1: port@0 { reg = <0>; label = "lan1"; local-mac-address = [00 00 00 00 00 00]; }; lan2: port@1 { reg = <1>; label = "lan2"; local-mac-address = [00 00 00 00 00 00]; }; lan3: port@2 { reg = <2>; label = "lan3"; local-mac-address = [00 00 00 00 00 00]; }; lan4: port@3 { reg = <3>; label = "lan4"; local-mac-address = [00 00 00 00 00 00]; }; port@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?
Yes, I do have that patch and it seems I did not understand the issue properly.
The issue I'm seeing looks like it has to do with the fec driver. Whe fecmxc_probe is called it calls fec_get_miibus()->fec_mii_setspeed()->fec_get_ckl_rate() which fails because it uses 'uclass_get_device(UCLASS_ETH, 0, &dev) to find the first eth device assuming its the fec and instead its the lan1 port. It seems the fec_get_clk_rate() makes a poor assumption that the first UCLASS_ETH is the fec device and I'll have to find a way to fix that.
I've also found that if I do not have a 'fixed-link' node in each of my downstream rj45 ports (lan1/lan2/lan3/lan4 above) else dsa_port_probe() will return -ENODEV due to dm_eth_phy_connect()->dm_eth_connect_phy_handle() failing because it does not see the link as fixed. Should I really need a 'fixed-link' in my non-cpu port children above? Not only is it not needed for Linux dsa to work if I add it to the Linux dt, the Linux ksz9477 driver crashes (at least in 5.4). I feel like perhaps dm_eth_phy_connect() should default to thinking of the ports as fixed links if they have no phy-handle/phy/phy-device node.
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 understand that tagging is needed here in order to determine the port the packet was meant for. When I add tagging of any kind the fec driver appears to crash (memory alignment issue?) so I started off by not tagging (dsa_set_tagging(dev, 0, 0)) and only enabling lan1 but found a ping fails. I know that I've got my RGMII delay configured correctly for the switch's cpu port so I'm not sure where to look next.
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.
I can really use anything I want for tagging because the tag is only present to/from master to cpu port right? So I can use bytes on head, or bytes on tail, and any magic I want right?
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-olte...
Thanks - I'll look that over. Currently the only dsa driver examples in U-Boot are the felix_switch.c and dsa_sandbox.c
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.
I can have my ksz dsa driver register an mdio bus for this and this is probably the piece that I'm currently missing
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.
Thanks for the feedback!
Tim