[PATCH net] net: dsa: fix older DSA drivers using phylink

[PATCH net] net: dsa: fix older DSA drivers using phylink

[Russell King (Oracle)]

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(-)

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

Re: [PATCH net] net: dsa: fix older DSA drivers using phylink

[Vladimir Oltean]

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
> 

Re: [PATCH net] net: dsa: fix older DSA drivers using phylink

[Florian Fainelli]

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>
-- 
Florian

Re: [PATCH net] net: dsa: fix older DSA drivers using phylink

[patchwork-bot+netdevbpf]

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!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net] net: dsa: fix older DSA drivers using phylink - manual merge

[Matthieu Baerts]

[-- Attachment #1: Type: text/plain, Size: 1841 bytes --]

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
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

[-- Attachment #2: 8cbf72d9be3a6a2cda95e3db9d1fa3cc4b5f3924.patch --]
[-- Type: text/x-patch, Size: 951 bytes --]

diff --cc net/dsa/port.c
index c63cbfbe6489,2f6195d7b741..24015e11255f
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@@ -1686,8 -1720,22 +1686,15 @@@ int dsa_port_phylink_create(struct dsa_
  	if (err)
  		mode = PHY_INTERFACE_MODE_NA;
  
- 	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);
+ 	}
  
  	pl = phylink_create(&dp->pl_config, of_fwnode_handle(dp->dn),
  			    mode, &dsa_port_phylink_mac_ops);

Re: [PATCH net] net: dsa: fix older DSA drivers using phylink - manual merge

[Russell King (Oracle)]

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.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net] net: dsa: fix older DSA drivers using phylink - manual merge

[Matthieu Baerts]

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
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net