From: Alvin Šipraga <[email protected]>
Since commit a18e6521a7d9 ("net: phylink: handle NA interface mode in
phylink_fwnode_phy_connect()"), phylib defaults to GMII when no phy-mode
or phy-connection-type property is specified in a DSA port node of the
device tree. The same commit caused a regression in rtl8365mb whereby
phylink would fail to connect, because the driver did not advertise
support for GMII for ports with internal PHY.
It should be noted that the aforementioned regression is not because the
blamed commit was incorrect: on the contrary, the blamed commit is
correcting the previous behaviour whereby unspecified phy-mode would
cause the internal interface mode to be PHY_INTERFACE_MODE_NA. The
rtl8365mb driver only worked by accident before because it _did_
advertise support for PHY_INTERFACE_MODE_NA, despite NA being reserved
for internal use by phylink. With one mistake fixed, the other was
exposed.
Commit a5dba0f207e5 ("net: dsa: rtl8365mb: add GMII as user port mode")
then introduced implicit support for GMII mode on ports with internal
PHY to allow a PHY connection for device trees where the phy-mode is not
explicitly set to "internal". At this point everything was working OK
again.
Subsequently, commit 6ff6064605e9 ("net: dsa: realtek: convert to
phylink_generic_validate()") broke this behaviour again by discarding
the usage of rtl8365mb_phy_mode_supported() - where this GMII support
was indicated - while switching to the new .phylink_get_caps API.
With the new API, rtl8365mb_phy_mode_supported() is no longer needed.
Remove it altogether and add back the GMII capability - this time to
rtl8365mb_phylink_get_caps() - so that the above default behaviour works
for ports with internal PHY again.
Fixes: 6ff6064605e9 ("net: dsa: realtek: convert to phylink_generic_validate()")
Signed-off-by: Alvin Šipraga <[email protected]>
---
v1 -> v2:
- no code changes
- added more detail in the commit description after Luiz and Russel's
help finding commit a18e6521a7d9
---
drivers/net/dsa/realtek/rtl8365mb.c | 38 +++++++----------------------
1 file changed, 9 insertions(+), 29 deletions(-)
diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c
index 3bb42a9f236d..769f672e9128 100644
--- a/drivers/net/dsa/realtek/rtl8365mb.c
+++ b/drivers/net/dsa/realtek/rtl8365mb.c
@@ -955,35 +955,21 @@ static int rtl8365mb_ext_config_forcemode(struct realtek_priv *priv, int port,
return 0;
}
-static bool rtl8365mb_phy_mode_supported(struct dsa_switch *ds, int port,
- phy_interface_t interface)
-{
- int ext_int;
-
- ext_int = rtl8365mb_extint_port_map[port];
-
- if (ext_int < 0 &&
- (interface == PHY_INTERFACE_MODE_NA ||
- interface == PHY_INTERFACE_MODE_INTERNAL ||
- interface == PHY_INTERFACE_MODE_GMII))
- /* Internal PHY */
- return true;
- else if ((ext_int >= 1) &&
- phy_interface_mode_is_rgmii(interface))
- /* Extension MAC */
- return true;
-
- return false;
-}
-
static void rtl8365mb_phylink_get_caps(struct dsa_switch *ds, int port,
struct phylink_config *config)
{
- if (dsa_is_user_port(ds, port))
+ if (dsa_is_user_port(ds, port)) {
__set_bit(PHY_INTERFACE_MODE_INTERNAL,
config->supported_interfaces);
- else if (dsa_is_cpu_port(ds, port))
+
+ /* GMII is the default interface mode for phylib, so
+ * we have to support it for ports with integrated PHY.
+ */
+ __set_bit(PHY_INTERFACE_MODE_GMII,
+ config->supported_interfaces);
+ } else if (dsa_is_cpu_port(ds, port)) {
phy_interface_set_rgmii(config->supported_interfaces);
+ }
config->mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE |
MAC_10 | MAC_100 | MAC_1000FD;
@@ -996,12 +982,6 @@ static void rtl8365mb_phylink_mac_config(struct dsa_switch *ds, int port,
struct realtek_priv *priv = ds->priv;
int ret;
- if (!rtl8365mb_phy_mode_supported(ds, port, state->interface)) {
- dev_err(priv->dev, "phy mode %s is unsupported on port %d\n",
- phy_modes(state->interface), port);
- return;
- }
-
if (mode != MLO_AN_PHY && mode != MLO_AN_FIXED) {
dev_err(priv->dev,
"port %d supports only conventional PHY or fixed-link\n",
--
2.36.0
On Tue, Jun 07, 2022 at 08:46:24PM +0200, Alvin Šipraga wrote:
> From: Alvin Šipraga <[email protected]>
>
> Since commit a18e6521a7d9 ("net: phylink: handle NA interface mode in
> phylink_fwnode_phy_connect()"), phylib defaults to GMII when no phy-mode
> or phy-connection-type property is specified in a DSA port node of the
> device tree. The same commit caused a regression in rtl8365mb whereby
> phylink would fail to connect, because the driver did not advertise
> support for GMII for ports with internal PHY.
>
> It should be noted that the aforementioned regression is not because the
> blamed commit was incorrect: on the contrary, the blamed commit is
> correcting the previous behaviour whereby unspecified phy-mode would
> cause the internal interface mode to be PHY_INTERFACE_MODE_NA. The
> rtl8365mb driver only worked by accident before because it _did_
> advertise support for PHY_INTERFACE_MODE_NA, despite NA being reserved
> for internal use by phylink. With one mistake fixed, the other was
> exposed.
>
> Commit a5dba0f207e5 ("net: dsa: rtl8365mb: add GMII as user port mode")
> then introduced implicit support for GMII mode on ports with internal
> PHY to allow a PHY connection for device trees where the phy-mode is not
> explicitly set to "internal". At this point everything was working OK
> again.
>
> Subsequently, commit 6ff6064605e9 ("net: dsa: realtek: convert to
> phylink_generic_validate()") broke this behaviour again by discarding
> the usage of rtl8365mb_phy_mode_supported() - where this GMII support
> was indicated - while switching to the new .phylink_get_caps API.
>
> With the new API, rtl8365mb_phy_mode_supported() is no longer needed.
> Remove it altogether and add back the GMII capability - this time to
> rtl8365mb_phylink_get_caps() - so that the above default behaviour works
> for ports with internal PHY again.
>
> Fixes: 6ff6064605e9 ("net: dsa: realtek: convert to phylink_generic_validate()")
> Signed-off-by: Alvin Šipraga <[email protected]>
Even though I raised concerns about the ext_int thing previously, this
is still a step forward, so:
Reviewed-by: Russell King (Oracle) <[email protected]>
Thanks!
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Hello:
This patch was applied to netdev/net.git (master)
by Jakub Kicinski <[email protected]>:
On Tue, 7 Jun 2022 20:46:24 +0200 you wrote:
> From: Alvin Šipraga <[email protected]>
>
> Since commit a18e6521a7d9 ("net: phylink: handle NA interface mode in
> phylink_fwnode_phy_connect()"), phylib defaults to GMII when no phy-mode
> or phy-connection-type property is specified in a DSA port node of the
> device tree. The same commit caused a regression in rtl8365mb whereby
> phylink would fail to connect, because the driver did not advertise
> support for GMII for ports with internal PHY.
>
> [...]
Here is the summary with links:
- [net,v2] net: dsa: realtek: rtl8365mb: fix GMII caps for ports with internal PHY
https://git.kernel.org/netdev/net/c/487994ff7588
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html