Re: [PATCH] net: dsa: mv88e6xxx: fix usable ports on 88e6020

[Matthias Schiffer]

On Tue, 2024-03-26 at 14:34 +0100, Lukasz Majewski wrote:
> Hi Matthias,
> 
> > From: Michael Krummsdorf <michael.krummsdorf@xxxxxxxxxxxx>
> > 
> > The switch has 4 ports with 2 internal PHYs, but ports are numbered up
> > to 6, with ports 0, 1, 5 and 6 being usable.
> > 
> > Fixes: 71d94a432a15 ("net: dsa: mv88e6xxx: add support for MV88E6020
> > switch") Signed-off-by: Michael Krummsdorf
> > <michael.krummsdorf@xxxxxxxxxxxx> Signed-off-by: Matthias Schiffer
> > <matthias.schiffer@xxxxxxxxxxxxxxx> ---
> > 
> > I was unfortunately too busy to notice the issue when the patch this
> > Fixes was resubmitted in my name. It would have been better to change
> > my From into a Based-on-patch-by or similar when modifying it
> 
> The "discussion" about this work was lasting at least a few months with
> several iterations and changing the design decisions ...
> 
> > - and
> > the final version obviously wasn't even tested on an 88E6020...
> 
> 
> Can you share on which kernel version have you tested the patch that
> you claim that testing was omitted?

Hi Lukasz,

we are currently testing with commit 71d94a432a15 backported onto a recent Linux 6.1.y. At least on
this kernel version, the driver will reject a Device Tree configuration that uses the ports 5 and 6
(rightfully so, as num_ports is set to 4), leaving only the internal-PHY ports 0 and 1, and none of
the *MII ports that are likely to be used as CPU ports.

So if the accepted version worked fine for you, your configuration possibly only used the first two
ports, or newer kernels somehow ignore num_ports when determining if a port number is valid.

We should be able to repeat our test on a newer kernel next week if needed.

Best regards,
Matthias


> 
> > 
> > 
> >  drivers/net/dsa/mv88e6xxx/chip.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c
> > b/drivers/net/dsa/mv88e6xxx/chip.c index 9ed1821184ece..c95787cb90867
> > 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c
> > +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> > @@ -5503,8 +5503,12 @@ static const struct mv88e6xxx_info
> > mv88e6xxx_table[] = { .family = MV88E6XXX_FAMILY_6250,
> >  		.name = "Marvell 88E6020",
> >  		.num_databases = 64,
> > -		.num_ports = 4,
> > +		/* Ports 2-4 are not routed to pins
> > +		 * => usable ports 0, 1, 5, 6
> > +		 */
> > +		.num_ports = 7,
> >  		.num_internal_phys = 2,
> > +		.invalid_port_mask = BIT(2) | BIT(3) | BIT(4),
> >  		.max_vid = 4095,
> >  		.port_base_addr = 0x8,
> >  		.phy_base_addr = 0x0,
> 
> 
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Erika Unter
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@xxxxxxx
> Achtung externe E-Mail: Öffnen Sie Anhänge und Links nur, wenn Sie wissen, dass diese aus einer sicheren Quelle stammen und sicher sind. Leiten Sie die E-Mail im Zweifelsfall zur Prüfung an den IT-Helpdesk weiter.
>  Attention external email: Open attachments and links only if you know that they are from a secure source and are safe. In doubt forward the email to the IT-Helpdesk to check it.
> 
>