Message-ID: <506eca29-9785-e580-91d2-9b7f8f26cdfc@tessares.net>
Date: Fri, 28 Jul 2023 13:23:10 +0200
From: Matthieu Baerts <matthieu.baerts@...sares.net>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Andrew Lunn <andrew@...n.ch>, Florian Fainelli <f.fainelli@...il.com>,
 Vladimir Oltean <olteanv@...il.com>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 netdev@...r.kernel.org, Stephen Rothwell <sfr@...b.auug.org.au>
Subject: Re: [PATCH net] net: dsa: fix older DSA drivers using phylink -
 manual merge

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@...il.com>
>>> Fixes: de5c9bf40c45 ("net: phylink: require supported_interfaces to be filled")
>>> Signed-off-by: Russell King (Oracle) <rmk+kernel@...linux.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

Powered by blists - more mailing lists