| Message ID | E1qOflM-001AEz-D3@rmk-PC.armlinux.org.uk (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9945c1fb03a3c9f7e0dcf9aa17041a70e551387a |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: dsa: fix older DSA drivers using phylink | expand |
On Wed, Jul 26, 2023 at 03:45:16PM +0100, Russell King (Oracle) wrote: > Older DSA drivers that do not provide an dsa_ops adjust_link method end > up using phylink. Unfortunately, a recent phylink change that requires > its supported_interfaces bitmap to be filled breaks these drivers > because the bitmap remains empty. > > Rather than fixing each driver individually, fix it in the core code so > we have a sensible set of defaults. > > Reported-by: Sergei Antonov <saproj@gmail.com> > Fixes: de5c9bf40c45 ("net: phylink: require supported_interfaces to be filled") > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > --- Reviewed-by: Vladimir Oltean <olteanv@gmail.com> Tested-by: Vladimir Oltean <olteanv@gmail.com> # dsa_loop Thanks! > net/dsa/port.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/net/dsa/port.c b/net/dsa/port.c > index 0ce8fd311c78..2f6195d7b741 100644 > --- a/net/dsa/port.c > +++ b/net/dsa/port.c > @@ -1727,8 +1727,15 @@ int dsa_port_phylink_create(struct dsa_port *dp) > ds->ops->phylink_mac_an_restart) > dp->pl_config.legacy_pre_march2020 = true; > > - if (ds->ops->phylink_get_caps) > + if (ds->ops->phylink_get_caps) { > ds->ops->phylink_get_caps(ds, dp->index, &dp->pl_config); > + } else { > + /* For legacy drivers */ > + __set_bit(PHY_INTERFACE_MODE_INTERNAL, > + dp->pl_config.supported_interfaces); > + __set_bit(PHY_INTERFACE_MODE_GMII, > + dp->pl_config.supported_interfaces); > + } > > pl = phylink_create(&dp->pl_config, of_fwnode_handle(dp->dn), > mode, &dsa_port_phylink_mac_ops); > -- > 2.30.2 >
On 7/26/23 07:45, Russell King (Oracle) wrote: > Older DSA drivers that do not provide an dsa_ops adjust_link method end > up using phylink. Unfortunately, a recent phylink change that requires > its supported_interfaces bitmap to be filled breaks these drivers > because the bitmap remains empty. > > Rather than fixing each driver individually, fix it in the core code so > we have a sensible set of defaults. > > Reported-by: Sergei Antonov <saproj@gmail.com> > Fixes: de5c9bf40c45 ("net: phylink: require supported_interfaces to be filled") > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Wed, 26 Jul 2023 15:45:16 +0100 you wrote: > Older DSA drivers that do not provide an dsa_ops adjust_link method end > up using phylink. Unfortunately, a recent phylink change that requires > its supported_interfaces bitmap to be filled breaks these drivers > because the bitmap remains empty. > > Rather than fixing each driver individually, fix it in the core code so > we have a sensible set of defaults. > > [...] Here is the summary with links: - [net] net: dsa: fix older DSA drivers using phylink https://git.kernel.org/netdev/net/c/9945c1fb03a3 You are awesome, thank you!
Hi Russell, On 26/07/2023 16:45, Russell King (Oracle) wrote: > Older DSA drivers that do not provide an dsa_ops adjust_link method end > up using phylink. Unfortunately, a recent phylink change that requires > its supported_interfaces bitmap to be filled breaks these drivers > because the bitmap remains empty. > > Rather than fixing each driver individually, fix it in the core code so > we have a sensible set of defaults. > > Reported-by: Sergei Antonov <saproj@gmail.com> > Fixes: de5c9bf40c45 ("net: phylink: require supported_interfaces to be filled") > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> FYI, we got a small conflict when merging 'net' in 'net-next' in the MPTCP tree due to this patch applied in 'net': 9945c1fb03a3 ("net: dsa: fix older DSA drivers using phylink") and this one from 'net-next': a88dd7538461 ("net: dsa: remove legacy_pre_march2020 detection") ----- Generic Message ----- The best is to avoid conflicts between 'net' and 'net-next' trees but if they cannot be avoided when preparing patches, a note about how to fix them is much appreciated. The conflict has been resolved on our side[1] and the resolution we suggest is attached to this email. Please report any issues linked to this conflict resolution as it might be used by others. If you worked on the mentioned patches, don't hesitate to ACK this conflict resolution. --------------------------- Regarding this conflict, the two commits modify lines in the same context but they don't modify the same ones. I then took the modifications from both side. Rerere cache is available in [2]. Cheers, Matt [1] https://github.com/multipath-tcp/mptcp_net-next/commit/8cbf72d9be3a [2] https://github.com/multipath-tcp/mptcp-upstream-rr-cache/commit/b0f1
On Fri, Jul 28, 2023 at 12:23:25PM +0200, Matthieu Baerts wrote: > Hi Russell, > > On 26/07/2023 16:45, Russell King (Oracle) wrote: > > Older DSA drivers that do not provide an dsa_ops adjust_link method end > > up using phylink. Unfortunately, a recent phylink change that requires > > its supported_interfaces bitmap to be filled breaks these drivers > > because the bitmap remains empty. > > > > Rather than fixing each driver individually, fix it in the core code so > > we have a sensible set of defaults. > > > > Reported-by: Sergei Antonov <saproj@gmail.com> > > Fixes: de5c9bf40c45 ("net: phylink: require supported_interfaces to be filled") > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > > FYI, we got a small conflict when merging 'net' in 'net-next' in the > MPTCP tree due to this patch applied in 'net': > > 9945c1fb03a3 ("net: dsa: fix older DSA drivers using phylink") > > and this one from 'net-next': > > a88dd7538461 ("net: dsa: remove legacy_pre_march2020 detection") It was unavoidable. > ----- Generic Message ----- > The best is to avoid conflicts between 'net' and 'net-next' trees but if > they cannot be avoided when preparing patches, a note about how to fix > them is much appreciated. Given that this is a trivial context-based conflict, it wasn't worth it. If it was a conflict that actually involved two changes touching the same lines of code, then yes, that would be sensible. Note that I don't get these messages from the netdev maintainers when they update net-next (as they did last night.) > > - if (ds->ops->phylink_get_caps) > - /* Presence of phylink_mac_link_state or phylink_mac_an_restart is > - * an indicator of a legacy phylink driver. > - */ > - if (ds->ops->phylink_mac_link_state || > - ds->ops->phylink_mac_an_restart) > - dp->pl_config.legacy_pre_march2020 = true; > - > + if (ds->ops->phylink_get_caps) { > ds->ops->phylink_get_caps(ds, dp->index, &dp->pl_config); > + } else { > + /* For legacy drivers */ > + __set_bit(PHY_INTERFACE_MODE_INTERNAL, > + dp->pl_config.supported_interfaces); > + __set_bit(PHY_INTERFACE_MODE_GMII, > + dp->pl_config.supported_interfaces); > + } Of course, being a purely context-based conflict, that is correct.
Hi Russell, On 28/07/2023 12:37, Russell King (Oracle) wrote: > On Fri, Jul 28, 2023 at 12:23:25PM +0200, Matthieu Baerts wrote: >> Hi Russell, >> >> On 26/07/2023 16:45, Russell King (Oracle) wrote: >>> Older DSA drivers that do not provide an dsa_ops adjust_link method end >>> up using phylink. Unfortunately, a recent phylink change that requires >>> its supported_interfaces bitmap to be filled breaks these drivers >>> because the bitmap remains empty. >>> >>> Rather than fixing each driver individually, fix it in the core code so >>> we have a sensible set of defaults. >>> >>> Reported-by: Sergei Antonov <saproj@gmail.com> >>> Fixes: de5c9bf40c45 ("net: phylink: require supported_interfaces to be filled") >>> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> >> >> FYI, we got a small conflict when merging 'net' in 'net-next' in the >> MPTCP tree due to this patch applied in 'net': >> >> 9945c1fb03a3 ("net: dsa: fix older DSA drivers using phylink") >> >> and this one from 'net-next': >> >> a88dd7538461 ("net: dsa: remove legacy_pre_march2020 detection") > > It was unavoidable. > >> ----- Generic Message ----- >> The best is to avoid conflicts between 'net' and 'net-next' trees but if >> they cannot be avoided when preparing patches, a note about how to fix >> them is much appreciated. > > Given that this is a trivial context-based conflict, it wasn't worth it. > If it was a conflict that actually involved two changes touching the > same lines of code, then yes, that would be sensible. Sorry, it was a generic message from a template I used, mainly for occasional devs reading this, not for you then. I didn't know you were not mentioning anything for trivial patches. Noted now. > Note that I don't get these messages from the netdev maintainers when > they update net-next (as they did last night.) Your patch is not in net-next from what I can see, nor in linux-next. That's why I sent this message because usually it helps Net maintainers (and maybe Stephen). I thought it would be helpful to share that even with trivial conflicts because it requires a manual operation, looking at the different patches causing conflicts, etc. but if such message does the opposite than helping, I don't mind not sending them when the conflicts are "trivial". >> - if (ds->ops->phylink_get_caps) >> - /* Presence of phylink_mac_link_state or phylink_mac_an_restart is >> - * an indicator of a legacy phylink driver. >> - */ >> - if (ds->ops->phylink_mac_link_state || >> - ds->ops->phylink_mac_an_restart) >> - dp->pl_config.legacy_pre_march2020 = true; >> - >> + if (ds->ops->phylink_get_caps) { >> ds->ops->phylink_get_caps(ds, dp->index, &dp->pl_config); >> + } else { >> + /* For legacy drivers */ >> + __set_bit(PHY_INTERFACE_MODE_INTERNAL, >> + dp->pl_config.supported_interfaces); >> + __set_bit(PHY_INTERFACE_MODE_GMII, >> + dp->pl_config.supported_interfaces); >> + } > > Of course, being a purely context-based conflict, that is correct. Thank you for having checked and again sorry for having taken some of your time for that. Matt
diff --git a/net/dsa/port.c b/net/dsa/port.c index 0ce8fd311c78..2f6195d7b741 100644 --- a/net/dsa/port.c +++ b/net/dsa/port.c @@ -1727,8 +1727,15 @@ int dsa_port_phylink_create(struct dsa_port *dp) ds->ops->phylink_mac_an_restart) dp->pl_config.legacy_pre_march2020 = true; - if (ds->ops->phylink_get_caps) + if (ds->ops->phylink_get_caps) { ds->ops->phylink_get_caps(ds, dp->index, &dp->pl_config); + } else { + /* For legacy drivers */ + __set_bit(PHY_INTERFACE_MODE_INTERNAL, + dp->pl_config.supported_interfaces); + __set_bit(PHY_INTERFACE_MODE_GMII, + dp->pl_config.supported_interfaces); + } pl = phylink_create(&dp->pl_config, of_fwnode_handle(dp->dn), mode, &dsa_port_phylink_mac_ops);
Older DSA drivers that do not provide an dsa_ops adjust_link method end up using phylink. Unfortunately, a recent phylink change that requires its supported_interfaces bitmap to be filled breaks these drivers because the bitmap remains empty. Rather than fixing each driver individually, fix it in the core code so we have a sensible set of defaults. Reported-by: Sergei Antonov <saproj@gmail.com> Fixes: de5c9bf40c45 ("net: phylink: require supported_interfaces to be filled") Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> --- net/dsa/port.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)