* [PATCH] net: dsa: mv88e6xxx: Skip cmode writable for mv88e6*41 if unchanged
@ 2022-04-23 13:20 Nathan Rossi
2022-04-23 13:25 ` Marek Behún
0 siblings, 1 reply; 5+ messages in thread
From: Nathan Rossi @ 2022-04-23 13:20 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: Nathan Rossi, Andrew Lunn, Vivien Didelot, Florian Fainelli,
Vladimir Oltean, David S. Miller, Jakub Kicinski, Paolo Abeni
The mv88e6341_port_set_cmode function always calls the set writable
regardless of whether the current cmode is different from the desired
cmode. This is problematic for specific configurations of the mv88e6341
and mv88e6141 (in single chip adddressing mode?) where the hidden
registers are not accessible. This causes the set_cmode_writable to
fail, and causes teardown of the switch despite the cmode already being
configured in the correct mode (via external configuration).
This change adds checking of the current cmode compared to the desired
mode and returns if already in the desired mode. This skips the
set_cmode_writable setup if the port is already configured in the
desired mode, avoiding any issues with access of hidden registers.
Signed-off-by: Nathan Rossi <nathan@nathanrossi•com>
---
drivers/net/dsa/mv88e6xxx/port.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
index 795b312876..f2e9c8cae3 100644
--- a/drivers/net/dsa/mv88e6xxx/port.c
+++ b/drivers/net/dsa/mv88e6xxx/port.c
@@ -713,6 +713,7 @@ int mv88e6341_port_set_cmode(struct mv88e6xxx_chip *chip, int port,
phy_interface_t mode)
{
int err;
+ u8 cmode = chip->ports[port].cmode;
if (port != 5)
return -EOPNOTSUPP;
@@ -724,6 +725,23 @@ int mv88e6341_port_set_cmode(struct mv88e6xxx_chip *chip, int port,
case PHY_INTERFACE_MODE_XAUI:
case PHY_INTERFACE_MODE_RXAUI:
return -EINVAL;
+
+ /* Check before setting writable. Such that on devices that are already
+ * correctly configured, no attempt is made to make the cmode writable
+ * as it may fail.
+ */
+ case PHY_INTERFACE_MODE_1000BASEX:
+ if (cmode == MV88E6XXX_PORT_STS_CMODE_1000BASEX)
+ return 0;
+ break;
+ case PHY_INTERFACE_MODE_SGMII:
+ if (cmode == MV88E6XXX_PORT_STS_CMODE_SGMII)
+ return 0;
+ break;
+ case PHY_INTERFACE_MODE_2500BASEX:
+ if (cmode == MV88E6XXX_PORT_STS_CMODE_2500BASEX)
+ return 0;
+ break;
default:
break;
}
---
2.35.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] net: dsa: mv88e6xxx: Skip cmode writable for mv88e6*41 if unchanged
2022-04-23 13:20 [PATCH] net: dsa: mv88e6xxx: Skip cmode writable for mv88e6*41 if unchanged Nathan Rossi
@ 2022-04-23 13:25 ` Marek Behún
2022-04-23 13:59 ` Nathan Rossi
2022-04-26 7:50 ` Paolo Abeni
0 siblings, 2 replies; 5+ messages in thread
From: Marek Behún @ 2022-04-23 13:25 UTC (permalink / raw)
To: Nathan Rossi
Cc: netdev, linux-kernel, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, David S. Miller,
Jakub Kicinski, Paolo Abeni
On Sat, 23 Apr 2022 13:20:35 +0000
Nathan Rossi <nathan@nathanrossi•com> wrote:
> The mv88e6341_port_set_cmode function always calls the set writable
> regardless of whether the current cmode is different from the desired
> cmode. This is problematic for specific configurations of the mv88e6341
> and mv88e6141 (in single chip adddressing mode?) where the hidden
> registers are not accessible.
I don't have a problem with skipping setting cmode writable if cmode is
not being changed. But hidden registers should be accessible regardless
of whether you are using single chip addressing mode or not. You need
to find why it isn't working for you, this is a bug.
Marek
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] net: dsa: mv88e6xxx: Skip cmode writable for mv88e6*41 if unchanged
2022-04-23 13:25 ` Marek Behún
@ 2022-04-23 13:59 ` Nathan Rossi
2022-04-26 7:50 ` Paolo Abeni
1 sibling, 0 replies; 5+ messages in thread
From: Nathan Rossi @ 2022-04-23 13:59 UTC (permalink / raw)
To: Marek Behún
Cc: netdev, linux-kernel, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, David S. Miller,
Jakub Kicinski, Paolo Abeni
On Sat, 23 Apr 2022 at 23:25, Marek Behún <kabel@kernel•org> wrote:
>
> On Sat, 23 Apr 2022 13:20:35 +0000
> Nathan Rossi <nathan@nathanrossi•com> wrote:
>
> > The mv88e6341_port_set_cmode function always calls the set writable
> > regardless of whether the current cmode is different from the desired
> > cmode. This is problematic for specific configurations of the mv88e6341
> > and mv88e6141 (in single chip adddressing mode?) where the hidden
> > registers are not accessible.
>
> I don't have a problem with skipping setting cmode writable if cmode is
> not being changed. But hidden registers should be accessible regardless
> of whether you are using single chip addressing mode or not. You need
> to find why it isn't working for you, this is a bug.
I did try to debug the hidden register access, unfortunately with the
device I have the hidden registers do not behave correctly. It simply
times out waiting for the busy bit to change. I was not sure the
reason why and suspected it was something specific to the single mode,
and unfortunately the only information I have regarding these
registers is the kernel code itself. Perhaps it is specific to some
other pin configuration or the specific chip revision? If you have any
additional information for these hidden registers it would be very
helpful in debugging. For reference the device is a MV88E6141,
manufactured in 2019 week 47 (in a Netgate SG-3100).
Thanks,
Nathan
>
> Marek
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] net: dsa: mv88e6xxx: Skip cmode writable for mv88e6*41 if unchanged
2022-04-23 13:25 ` Marek Behún
2022-04-23 13:59 ` Nathan Rossi
@ 2022-04-26 7:50 ` Paolo Abeni
2022-04-26 10:45 ` Nathan Rossi
1 sibling, 1 reply; 5+ messages in thread
From: Paolo Abeni @ 2022-04-26 7:50 UTC (permalink / raw)
To: Marek Behún, Nathan Rossi
Cc: netdev, linux-kernel, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, David S. Miller,
Jakub Kicinski
Hello,
On Sat, 2022-04-23 at 15:25 +0200, Marek Behún wrote:
> On Sat, 23 Apr 2022 13:20:35 +0000
> Nathan Rossi <nathan@nathanrossi•com> wrote:
>
> > The mv88e6341_port_set_cmode function always calls the set writable
> > regardless of whether the current cmode is different from the desired
> > cmode. This is problematic for specific configurations of the mv88e6341
> > and mv88e6141 (in single chip adddressing mode?) where the hidden
> > registers are not accessible.
>
> I don't have a problem with skipping setting cmode writable if cmode is
> not being changed. But hidden registers should be accessible regardless
> of whether you are using single chip addressing mode or not. You need
> to find why it isn't working for you, this is a bug.
For the records, I read the above as requiring a fix the root cause, so
I'm not applying this patch. Please correct me if I'm wrong.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] net: dsa: mv88e6xxx: Skip cmode writable for mv88e6*41 if unchanged
2022-04-26 7:50 ` Paolo Abeni
@ 2022-04-26 10:45 ` Nathan Rossi
0 siblings, 0 replies; 5+ messages in thread
From: Nathan Rossi @ 2022-04-26 10:45 UTC (permalink / raw)
To: Paolo Abeni
Cc: Marek Behún, netdev, linux-kernel, Andrew Lunn,
Vivien Didelot, Florian Fainelli, Vladimir Oltean,
David S. Miller, Jakub Kicinski
On Tue, 26 Apr 2022 at 17:50, Paolo Abeni <pabeni@redhat•com> wrote:
>
> Hello,
>
> On Sat, 2022-04-23 at 15:25 +0200, Marek Behún wrote:
> > On Sat, 23 Apr 2022 13:20:35 +0000
> > Nathan Rossi <nathan@nathanrossi•com> wrote:
> >
> > > The mv88e6341_port_set_cmode function always calls the set writable
> > > regardless of whether the current cmode is different from the desired
> > > cmode. This is problematic for specific configurations of the mv88e6341
> > > and mv88e6141 (in single chip adddressing mode?) where the hidden
> > > registers are not accessible.
> >
> > I don't have a problem with skipping setting cmode writable if cmode is
> > not being changed. But hidden registers should be accessible regardless
> > of whether you are using single chip addressing mode or not. You need
> > to find why it isn't working for you, this is a bug.
>
> For the records, I read the above as requiring a fix the root cause, so
> I'm not applying this patch. Please correct me if I'm wrong.
You are correct. Sorry I forgot to reply to this thread after posting
the root cause fix.
For reference the root cause fix to the issue mentioned by this patch
is https://lore.kernel.org/netdev/20220425070454.348584-1-nathan@nathanrossi.com/.
Thanks,
Nathan
>
> Thanks,
>
> Paolo
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-04-26 10:47 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-04-23 13:20 [PATCH] net: dsa: mv88e6xxx: Skip cmode writable for mv88e6*41 if unchanged Nathan Rossi
2022-04-23 13:25 ` Marek Behún
2022-04-23 13:59 ` Nathan Rossi
2022-04-26 7:50 ` Paolo Abeni
2022-04-26 10:45 ` Nathan Rossi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox