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

["Marek Behún" <marek.behun@nic.cz>]

On Wed, 30 Mar 2022 08:46:06 -0700
Tim Harvey <tharvey@gateworks.com> wrote:

> On Tue, Mar 29, 2022 at 4:22 PM Marek Behún <marek.behun@nic.cz> wrote:
> >
> > On Tue, 29 Mar 2022 15:52:39 -0700
> > Tim Harvey <tharvey@gateworks.com> wrote:
> >  
> > > Add a DSA driver for the MV88E61xx compatible GbE Ethernet switches.
> > >
> > > Signed-off-by: Tim Harvey <tharvey@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