From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Robert Marko <robimarko@gmail.com>
Cc: andrew@lunn.ch, hkallweit1@gmail.com, davem@davemloft.net,
edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
ansuelsmth@gmail.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next 1/2] net: phy: aquantia: validate PHY mode on AQR107
Date: Fri, 17 Nov 2023 12:46:18 +0000 [thread overview]
Message-ID: <ZVdgmpvI+F4ClNmG@shell.armlinux.org.uk> (raw)
In-Reply-To: <20231117100958.425354-1-robimarko@gmail.com>
On Fri, Nov 17, 2023 at 11:09:48AM +0100, Robert Marko wrote:
> The Aquantia driver is not setting the PHY mode itself, but it does however
> still check if the PHY mode set in DTS is one of the supported modes.
>
> However, the set PHY mode does not have to match the actual one, so lets
> add update the PHY mode during .config_init and warn if they differ.
This looks completely wrong to me. These PHYs can be configured to
change their MAC-facing interface mode according to the media negotiated
speed, but you are only checking that _if_ the media is up, then the
interface that has resulted from that negotiation matches what is in
DTS. That could be dependent on the link partner, so what works for a
platform when connected to one link partner may issue your "info"-level
warning when connected to a different link partner.
So no, this to me looks completely wrong.
You need to check the VEND1_GLOBAL_CFG_* registers, and determine from
those what interface mode(s) will be used, and then use that to validate
the mode.
It just so happens that...
http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=net-queue&id=f7b531ee8855f81d267a8a42c44da51576f48daf
http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=net-queue&id=f55389aa5d11da8a32dfd65a1b98049878ce09f0
builds a bitmap that can then be tested to check this. Whether the
above is entirely correct or not, I can't really say, I don't have
enough information on this PHY.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
next prev parent reply other threads:[~2023-11-17 12:46 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-17 10:09 [PATCH net-next 1/2] net: phy: aquantia: validate PHY mode on AQR107 Robert Marko
2023-11-17 10:09 ` [PATCH net-next 2/2] net: phy: aquantia: enable USXGMII autoneg " Robert Marko
2023-11-17 12:47 ` Russell King (Oracle)
2023-11-28 11:57 ` Robert Marko
2023-11-17 12:46 ` Russell King (Oracle) [this message]
2023-11-28 11:47 ` [PATCH net-next 1/2] net: phy: aquantia: validate PHY mode " Robert Marko
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=ZVdgmpvI+F4ClNmG@shell.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=andrew@lunn.ch \
--cc=ansuelsmth@gmail.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=hkallweit1@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=robimarko@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).