From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Maarten Zanders <maarten.zanders@mind.be>
Cc: Maarten Zanders <m.zanders@televic.com>,
Maxime Chevallier <maxime.chevallier@bootlin.com>,
Andrew Lunn <andrew@lunn.ch>,
Vivien Didelot <vivien.didelot@gmail.com>,
Florian Fainelli <f.fainelli@gmail.com>,
Vladimir Oltean <olteanv@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] net: dsa: mv88e6xxx: don't use PHY_DETECT on internal PHY's
Date: Sun, 17 Oct 2021 22:07:21 +0100 [thread overview]
Message-ID: <YWyQiSejqGNOG6ES@shell.armlinux.org.uk> (raw)
In-Reply-To: <20211011142720.42642-1-maarten.zanders@mind.be>
On Mon, Oct 11, 2021 at 04:27:20PM +0200, Maarten Zanders wrote:
> mv88e6xxx_port_ppu_updates() interpretes data in the PORT_STS
> register incorrectly for internal ports (ie no PPU). In these
> cases, the PHY_DETECT bit indicates link status. This results
> in forcing the MAC state whenever the PHY link goes down which
> is not intended. As a side effect, LED's configured to show
> link status stay lit even though the physical link is down.
I know this patch has been merged, but I'm going to say this anyway
for the record.
The description is not entirely correct. It is not true that internal
ports do not have the PHY_DETECT bit. 88E6176 and friends are documented
that bit 12 is always the PHY_DETECT bit even for internal ports. Bit 11
there is Link status.
Looking at the definitions in port.h, some switches are different
(88E6250 family) and do indeed use bit 12 as link status.
The point I'm making is that the commit description is not universally
true and only applies to a subset of mv88e6xxx supported switches. It is
a shame it wasn't better reviewed before merging.
At least the patch is harmless; we leave the PHY_DETECT bit set on the
internal ports, which means the PPU fetches the configuration and
configures the port appropriately. The change merely helps to ensure
that we don't force link status on the internal ports.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
prev parent reply other threads:[~2021-10-17 21:07 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-11 14:27 [PATCH] net: dsa: mv88e6xxx: don't use PHY_DETECT on internal PHY's Maarten Zanders
2021-10-17 21:07 ` Russell King (Oracle) [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YWyQiSejqGNOG6ES@shell.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=f.fainelli@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=m.zanders@televic.com \
--cc=maarten.zanders@mind.be \
--cc=maxime.chevallier@bootlin.com \
--cc=netdev@vger.kernel.org \
--cc=olteanv@gmail.com \
--cc=vivien.didelot@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).