Re: [PATCH net-next v7 05/12] net: phylink: support late PCS provider attach

[Maxime Chevallier <maxime.chevallier@bootlin.com>]

On 6/15/26 16:35, Christian Marangi wrote:
> On Mon, Jun 15, 2026 at 04:29:04PM +0200, Maxime Chevallier wrote:
>>
>>
>> On 6/15/26 16:10, Christian Marangi wrote:
>>> On Mon, Jun 15, 2026 at 04:07:03PM +0200, Maxime Chevallier wrote:
>>>> Hi Christian,
>>>>
>>>> On 6/15/26 14:29, Christian Marangi wrote:
>>>>> Add support for late PCS provider attachment to a phylink instance.
>>>>> This works by creating a global notifier for the PCS provider and
>>>>> making each phylink instance that makes use of fwnode subscribe to
>>>>> this notifier.
>>>>>
>>>>> The PCS notifier will emit the event FWNODE_PCS_PROVIDER_ADD every time
>>>>> a new PCS provider is added.
>>>>>
>>>>> phylink will then react to this event and will call the new function
>>>>> fwnode_phylink_pcs_get_from_fwnode() that will check if the PCS fwnode
>>>>> provided by the event is present in the pcs-handle property of the
>>>>> phylink instance.
>>>>>
>>>>> If a related PCS is found, then such PCS is added to the phylink
>>>>> instance PCS list.
>>>>>
>>>>> Then we link the PCS to the phylink instance and we refresh the supported
>>>>> interfaces of the phylink instance.
>>>>>
>>>>> Finally we check if we are in a major_config_failed scenario and trigger
>>>>> an interface reconfiguration in the next phylink resolve.
>>>>>
>>>>> In the example scenario where the link was previously torn down due to
>>>>> removal of PCS, the link will be established again as the PCS came back
>>>>> and is now available to phylink.
>>>>>
>>>>> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
>>>>> ---
>>>>
>>>> [...]
>>>>
>>>>> @@ -2151,6 +2204,10 @@ void phylink_destroy(struct phylink *pl)
>>>>>  	if (pl->link_gpio)
>>>>>  		gpiod_put(pl->link_gpio);
>>>>>  
>>>>> +	/* Unregister notifier for late PCS attach */
>>>>> +	if (pl->fwnode_pcs_nb.notifier_call)
>>>>> +		unregister_fwnode_pcs_notifier(&pl->fwnode_pcs_nb);
>>>>
>>>> I wanted to try this out, but I get :
>>>>
>>>> drivers/net/phy/phylink.c:2218:17: error: implicit declaration of function ‘unregister_fwnode_pcs_notifier’; did you mean ‘register_fwnode_pcs_notifier’? [-Werror=implicit-function-declaration]
>>>>  2218 |                 unregister_fwnode_pcs_notifier(&pl->fwnode_pcs_nb);
>>>>       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>>       |                 register_fwnode_pcs_notifier
>>>>
>>>> I guess you either need to stub this, or there's a missing Kconfig
>>>> dependency somewhere
>>>>
>>>
>>> Hi yes if you want toi test just enable CONFIG_FWNODE_PCS. I forgot to add
>>> the static declaration for unregister_fwnode_pcs_notifier. 
>>
>> I'll give it a go with this yeah, I have a few devices here I'd like to
>> try this on.
>>
>> Can you CC me for the next rounds ?
>>
> 
> Sure, if you can would be good to check also the other commit if everything
> is logically correct.

Absolutely, I'm planning on doing this :) This is quite the series though,
I'll try to give it some deeper look in the upcoming days, I expect net-next
to close soon anyways, so that's as good as it gets for reviewing this :)

Also, it's probably a good idea to have Sean in CC as well, I know a few people
interested in the Xilinx PCS his series brought, hopefully he can follow-up on
this series with the other new PCS drivers.

> 
> BTW by checking test from patchwork I can see also another fix is needed if
> you want to test this revision.
> 
> phylink_create()
> 
> -if (config->num_possible_pcs && pl->mac_ops->mac_select_pcs) {
> +if (config->num_possible_pcs && mac_ops->mac_select_pcs) {

Ack, I'll test it with this change, thanks :)

Maxime

> 
> Aside from these change I expect the current legacy code to be not
> affected by these change.

I'll probably have things to say about this "legacy" naming :)

Thanks for driving this work forwards anyway, that's 

>