Received-SPF: pass (google.com: domain of linux-kernel+bounces-119105-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <35cc888230a0146a7687d8b859e5a6ceffec581a.camel@ew.tq-group.com>
Subject: Re: [PATCH] net: dsa: mv88e6xxx: fix usable ports on 88e6020
From: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
To: Lukasz Majewski <lukma@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>, Florian Fainelli <f.fainelli@gmail.com>, 
 Vladimir Oltean <olteanv@gmail.com>, "David S. Miller"
 <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>,  Jakub Kicinski
 <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, netdev@vger.kernel.org,
  linux-kernel@vger.kernel.org, linux@ew.tq-group.com, Michael Krummsdorf
 <michael.krummsdorf@tq-group.com>
Date: Tue, 26 Mar 2024 15:00:12 +0100
In-Reply-To: <20240326143424.3368d9b1@wsk>
References: <20240326123655.40666-1-matthias.schiffer@ew.tq-group.com>
	 <20240326143424.3368d9b1@wsk>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4-0ubuntu2 
Precedence: bulk
MIME-Version: 1.0

On Tue, 2024-03-26 at 14:34 +0100, Lukasz Majewski wrote:
> Hi Matthias,
>=20
> > From: Michael Krummsdorf <michael.krummsdorf@tq-group.com>
> >=20
> > 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.
> >=20
> > Fixes: 71d94a432a15 ("net: dsa: mv88e6xxx: add support for MV88E6020
> > switch") Signed-off-by: Michael Krummsdorf
> > <michael.krummsdorf@tq-group.com> Signed-off-by: Matthias Schiffer
> > <matthias.schiffer@ew.tq-group.com> ---
> >=20
> > 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
>=20
> The "discussion" about this work was lasting at least a few months with
> several iterations and changing the design decisions ...
>=20
> > - and
> > the final version obviously wasn't even tested on an 88E6020...
>=20
>=20
> 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 tha=
t uses the=C2=A0ports 5 and 6
(rightfully so, as num_ports is set to 4), leaving only the internal-PHY po=
rts 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


>=20
> >=20
> >=20
> >  drivers/net/dsa/mv88e6xxx/chip.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >=20
> > 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[] =3D { .family =3D MV88E6XXX_FAMILY_6250,
> >  		.name =3D "Marvell 88E6020",
> >  		.num_databases =3D 64,
> > -		.num_ports =3D 4,
> > +		/* Ports 2-4 are not routed to pins
> > +		 * =3D> usable ports 0, 1, 5, 6
> > +		 */
> > +		.num_ports =3D 7,
> >  		.num_internal_phys =3D 2,
> > +		.invalid_port_mask =3D BIT(2) | BIT(3) | BIT(4),
> >  		.max_vid =3D 4095,
> >  		.port_base_addr =3D 0x8,
> >  		.phy_base_addr =3D 0x0,
>=20
>=20
>=20
>=20
> Best regards,
>=20
> Lukasz Majewski
>=20
> --
>=20
> 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@denx.de
> Achtung externe E-Mail:=C2=A0=C3=96ffnen Sie Anh=C3=A4nge und Links nur, =
wenn Sie wissen, dass diese aus einer sicheren Quelle stammen und sicher si=
nd. Leiten Sie die E-Mail im Zweifelsfall zur Pr=C3=BCfung an den IT-Helpde=
sk weiter.
>  Attention external email:=C2=A0Open attachments and links only if you kn=
ow that they are from a secure source and are safe. In doubt forward the em=
ail to the IT-Helpdesk to check it.
>=20
> =C2=A0