Re: [RFC PATCH v3 net-next 10/10] docs: devicetree: add documentation for the VSC7512 SPI device

From: Colin Foster
Date: Sat Aug 14 2021 - 19:42:14 EST

On Sat, Aug 14, 2021 at 10:08:54PM +0300, Vladimir Oltean wrote:
> On Sat, Aug 14, 2021 at 11:40:40AM -0700, Colin Foster wrote:
> > On Sat, Aug 14, 2021 at 02:47:21PM +0300, Vladimir Oltean wrote:
> > > On Fri, Aug 13, 2021 at 07:50:03PM -0700, Colin Foster wrote:
> > > > +* phy_mode = "sgmii": on ports 0, 1, 2, 3
> > >
> > > > + port@0 {
> > > > + reg = <0>;
> > > > + ethernet = <&mac>;
> > > > + phy-mode = "sgmii";
> > > > +
> > > > + fixed-link {
> > > > + speed = <100>;
> > > > + full-duplex;
> > > > + };
> > > > + };
> > >
> > > Your driver is unconditionally setting up the NPI port at gigabit and
> > > you claim it works, yet the device tree sees a 100Mbps fixed-link? Which
> > > one is right, what speed does the port operate at?
> >
> > Good catch!
> >
> > I made the change to ocelot_spi_vsc7512 yesterday to set it up as
> > gigabit, tested it, and it still works. Previously for my testing I'd
> > had it hard-coded to 100, because the Beaglebone I'm using only supports
> > 100Mbps on eth0.
> >
> > # phytool print swp1/0
>
> Why are you showing the PHY registers of swp1? Why are these relevant at all?

Some sleight of hand. I'm not sure there's a different way to do it, but
running print on swp1/0 gives phy address 0 on the bus that contains
swp1. So in this setup, swp1/0 is the same as swp2/0, swp3/0 and swp4/0,
all of which point to the registers associated with "swp0" from what I
understand.

>
> >
> > ieee-phy: id:0x00070540
> >
> > ieee-phy: reg:BMCR(0x00) val:0x1040
> > flags: -reset -loopback +aneg-enable -power-down -isolate -aneg-restart -collision-test
> > speed: 1000-half
>
> Also, 1000/half sounds like an odd speed to end negotiation at.

Agreed. Possibly a misunderstanding by me during
vsc7512_enable_npi_port? I'll look into this.

>
> >
> > ieee-phy: reg:BMSR(0x01) val:0x796d
> > capabilities: -100-b4 +100-f +100-h +10-f +10-h -100-t2-f -100-t2-h
> > flags: +ext-status +aneg-complete -remote-fault +aneg-capable +link -jabber +ext-register
> >
> >
> > Of course I understand that "it works" is not the same as "it's correct"
> >
> > What I wanted to accomplish was to use the speed parameter and set up
> > the link based on that. I looked through all the DSA drivers and
> > couldn't find anything that seems to do that. The closest thing I saw
> > was in mt7531_cpu_port_config where they set the speed to either 2500 or
> > 1000 based on the interface. But nothing that I saw would explicitly set
> > the speed based on this parameter.
>
> As I mentioned in the other email, .phylink_mac_link_up is the function
> you are looking for. Phylink parses the fixed-link and calls that
> function for fixed-link ports with the speed and duplex specified. Check
> and see if felix_phylink_mac_link_up is not in fact called with
> link_an_mode == MLO_AN_FIXED, speed == SPEED_100 and duplex == DUPLEX_FULL,
> then what you are doing with that and if it makes sense for what you are
> trying to do.

I'll reply there once I've absorbed everything.

>
> >
> > So I think there's something I'm missing. The fixed-link speed should apply to
> > the CPU port on the switch (port@0)?
>
> Is this a question? It is under port@0, the port with the 'ethernet'
> property i.e. the CPU port, so why should it not?
>
> > Then eth0 can be manually set to a specific speed, but if it doesn't
> > match the fixed-link speed I'd be out of luck? Or should an ip link or
> > ethtool command to eth0 modify the speeds of both sides of the
> > connection? It feels like setting port@0 to the fastest speed and
> > letting it negotiate down to eth0 makes sense...
> >
> > To ask the same question a different way:
> >
> > I can currently run "ethtool -s eth0 speed 10 duplex full autoneg on"
> > and the link at eth0 drops to 10Mbps. Pinging my desktop jumps from
> > about 400us to about 600us when I do that.
>
> If eth0 is also a fixed-link, you should not be able to do that, no.
> But the fact that you are able to do that means it's not a fixed-link,
> you have a pair of PHYs that freely auto-negotiate the speed between the
> BeagleBone and the switch.

Yes, that is my setup. Addressed below.

>
> >
> > Should I not be able to do that? It should be fixed at 100Mbps without
> > autoneg, end of story? Because in the current configuration it feels
> > like the fixed-link settings are more a suggestion than a rule...
> >
>
> It should describe the hardware configuration, of course. It is
> incorrect to describe one side of a copper PHY connection as fixed-link
> and the other as having a phy-handle, and it sounds like this is what
> you're doing. We need to see the device tree binding for eth0, and
> maybe a picture of your setup if that is possible. How do you connect
> the switch board to the BeagleBone? Is it an RJ45 cable or some sort of
> PCIe-style connector with fingers for an SGMII SERDES lane, in which the
> board is plugged?
>
> The device tree says SGMII, the behavior says RJ45.

I'm using the standard BeagleBone devicetree, so &mac is defined in
arch/arm/boot/dts/am335x-bone-common.dtsi. For this stage of development
I'm using an ethernet cable plugged from the BeagleBone to port 0 of the
VSC7512 dev board. I haven't done anything more to the
am335x-boneblack.dts other than add the spi and switch configurations.
The connections between the beaglebone and dev board are limited to the
4 SPI lines, a ground, and an ethernet cable.

So DSA requires a fixed-link property. And that makes sense... who in
their right mind would connect switches on a board using an RJ45
connection :) Then the only reason any of this is working is because I
have eth0 set up as an RJ45 connection, and because of that I need the
hack to enable the phy on the switch port 0...

Maybe that's a question:
Is my devicetree incorrect for claiming the connection is SGMII when it
should be RJ45? Or is my setup incorrect for using RJ45 and there's no
way to configure it that way, so the fact that it functions is an
anomaly?