[CFT 0/8] rework phylink interface for split MAC/PCS support

[Florian Fainelli]

Le 2020-06-21 à 13:02, Russell King - ARM Linux admin a écrit :
> On Sun, Jun 21, 2020 at 10:37:43PM +0300, Vladimir Oltean wrote:
>> Hi Russell,
>>
>> On Sun, 21 Jun 2020 at 17:34, Russell King - ARM Linux admin
>> <linux at armlinux.org.uk> wrote:
>>>
>>> All,
>>>
>>> This is now almost four months old, but I see that I didn't copy the
>>> message to everyone who should've been, especially for the five
>>> remaining drivers.
>>>
>>> I had asked for input from maintainers to help me convert their
>>> phylink-using drivers to the new style where mac_link_up() performs
>>> the speed, duplex and pause setup rather than mac_config(). So far,
>>> I have had very little assistance with this, and it is now standing
>>> in the way of further changes to phylink, particularly with proper
>>> PCS support. You are effectively blocking this work; I can't break
>>> your code as that will cause a kernel regression.
>>>
>>> This is one of the reasons why there were not many phylink patches
>>> merged for the last merge window.
>>>
>>> The following drivers in current net-next remain unconverted:
>>>
>>> drivers/net/ethernet/mediatek/mtk_eth_soc.c
>>> drivers/net/dsa/ocelot/felix.c
>>> drivers/net/dsa/qca/ar9331.c
>>> drivers/net/dsa/bcm_sf2.c
>>> drivers/net/dsa/b53/b53_common.c
>>>
>>> These can be easily identified by grepping for conditionals where the
>>> expression matches the "MLO_PAUSE_.X" regexp.
>>>
>>> I have an untested patch that I will be sending out today for
>>> mtk_eth_soc.c, but the four DSA ones definitely require their authors
>>> or maintainers to either make the changes, or assist with that since
>>> their code is not straight forward.
>>>
>>> Essentially, if you are listed in this email's To: header, then you
>>> are listed as a maintainer for one of the affected drivers, and I am
>>> requesting assistance from you for this task please.
>>>
>>> Thanks.
>>>
>>> Russell.
>>>
>>
>> If forcing MAC speed is to be moved in mac_link_up(), and if (as you
>> requested in the mdio-lynx-pcs thread) configuring the PCS is to be
>> moved in pcs_link_up() and pcs_config() respectively, then what
>> remains to be done in mac_config()?
> 
> Hopefully very little, but I suspect there will still be a need for
> some kind of interface to configure the MAC interface type at the MAC.
> 
> Note that I have said many many many times that using state->{speed,
> duplex,pause} in mac_config() when in in-band mode is unreliable, yet
> still people insist on using them.  There _are_ and always _have been_
> paths in phylink where these members will be passed with an unresolved
> state, and they will corrupt the link settings when that happens.
> 
> I know that phylink was deficient in its handling of a split PCS, but
> I have worked to correct that.  That job still is not complete, because
> because I'm held up by these drivers that have not yet converted.  I've
> already waited a kernel cycle, despite having the next series of
> phylink patches ready and waiting since early February.
> 
> I'm getting to the point of wishing that phylink did not have users
> except my own.

You have to realize that most people are not capable of working at your
pace either because they just do not have your intellect or because
their day job is not supporting a switch driver 100% of the time. The
other thing to realize is that PHYLINK has seen a fair amount of churn
since its introduction which makes it really hard to follow what is
needed, what was bogus, what ended up working when it should not etc.
You have clearly worked on improving both code and documentation, there
is however inertia for people to catch up.

So please stop with this attitude, just send the patches if it breaks it
will take many cycles for people to realize it and that is on them, not
you, they do not get to complain at all and should have been more
reactive since you warned them.

Thank you
-- 
Florian