[PATCH 5/6] net: add MV88E61xx DSA driver

[Tim Harvey]

On Thu, Mar 31, 2022 at 3:30 AM Marek Behún <marek.behun at nic.cz> wrote:
>
> On Wed, 30 Mar 2022 08:46:06 -0700
> Tim Harvey <tharvey at gateworks.com> wrote:
>
> > On Tue, Mar 29, 2022 at 4:22 PM Marek Behún <marek.behun at nic.cz> wrote:
> > >
> > > On Tue, 29 Mar 2022 15:52:39 -0700
> > > Tim Harvey <tharvey at gateworks.com> wrote:
> > >
> > > > Add a DSA driver for the MV88E61xx compatible GbE Ethernet switches.
> > > >
> > > > Signed-off-by: Tim Harvey <tharvey at gateworks.com>
> > >
> >
> > Marek,
> >
> > Thanks for looking at this.
> >
> > > Is this final version that should be accepted?
> >
> > This is the first submission of the new driver.
> >
> > >
> > > The drivers seems to support not only 61xx, but 6xxx (6096, 6250, 6352,
> > > ...).
> >
> > It should also support all the same devices that the non dsa driver it
> > was derived from (drivers/net/phy/mv88e61xx.c) but I only have a board
> > with 88E6176 to test with.
> >
> > It is not completely clear to me what devices are supported in the
> > original driver per Kconfig but from looking over the code I would say
> > the following appear to be supported:
> > mv88e6096
> > mv88e6097
> > mv88e6020
> > mv88e6070
> > mv88e6071
> > mv88e6172
> > mv88e6176
> > mv88e6220
> > mv88e6240
> > mv88e6250
> > mv88e6352
> >
> > >
> > > Also there are some commented lines, for example
> > >
> > > > +//   u16 port_stat_link_mask;/* Bitmask for port link status bits */
> > > > +//   u16 port_stat_dup_mask; /* Bitmask for port duplex status bits */
> > > > +//   u8 port_stat_speed_width;/* Width of speed status bitfield */
> > >
> > > What is their purpose?
> > >
> >
> > oops... that was a mistake. I'm surprised checkpatch didn't catch
> > those. Those were in the non-dsa driver and not used here so will be
> > removed.
> >
> > > Why is mv88e61xx_dsa_xmit() no-op?
> >
> > For DSA dsa-uclass calls the switch master eth device send function
> > after calling the dsa_ops->xmit function so that a dsa driver can add
> > any header/footer if needed. The function is required but in my case I
> > don't care about header/footer tagging or vlan as only 1 port is
> > active at a time in U-Boot so I just return success.
>
> So if I make one port active, the other are completely disabled? They
> won't even switch? Is that how DSA uclass is supposed to work in U-Boot?
>
> I would think that it should be somehow configurable instead.
>

Marek,

I'll let Vladimir correct me if I'm wrong but my understanding is DSA
in U-Boot is not intended to allow switches to forward packets on
their own from port to port but instead just for forwarding packets
between the active port and the MAC connected to the CPU (at least
that's what I intended when I wrote the ksz9477 dsa driver
previously).

In my opinion what a DSA driver provides is avoidance of putting
switches in forwarding mode having the potential of easily creating
bridge loops. With the existing mv88e61xx driver I've had users create
bridge loops often.

Best regards,

Tim