Messages in this thread | | | Subject | Re: [PATCH] net: dsa: mv88e6xxx: fix usable ports on 88e6020 | From | Matthias Schiffer <> | Date | Wed, 27 Mar 2024 10:42:41 +0100 |
| |
On Tue, 2024-03-26 at 16:52 +0100, Lukasz Majewski wrote: > Hi Matthias, > > > On Tue, 2024-03-26 at 14:34 +0100, Lukasz Majewski wrote: > > > Hi Matthias, > > > > > > > From: Michael Krummsdorf <michael.krummsdorf@tq-group.com> > > > > > > > > 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@tq-group.com> Signed-off-by: Matthias Schiffer > > > > <matthias.schiffer@ew.tq-group.com> --- > > > > > > > > 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, > > And then the penny has dropped.... > > > switch@10 { > compatible = "marvell,mv88e6250"; > reg = <0x10>; > > ports { > #address-cells = <1>; > #size-cells = <0>; > > port@0 { > reg = <0>; > label = "lan1"; > }; > > port@1 { > reg = <1>; > label = "lan2"; > }; > > port@6 { > reg = <6>; > label = "cpu"; > phy-mode = "rmii"; > ethernet = <&fec2>; > > fixed-link { > speed = <100>; > full-duplex; > }; > }; > }; > }; > > > > > 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. > > I think that considering the above snippet - customer was using only > port 0 and 1.
Hmm, I don't think that's right - the DTS has port 6 as CPU port, which should make the whole probe of the switch fail with num_ports == 4 (`reg >= ds->num_ports` check in dsa_switch_parse_ports_of()) - that's what we saw on kernel 6.1.y with our board for CPU on port 5.
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@denx.de 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. > > > > > > > > > > > > > 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@denx.de > 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. > >
| |