Re: [PATCH net-next v6 1/5] net: pse-pd: add notifier chain for controller lifecycle events

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1

- [Low] Claim: the patch adds a blocking notifier chain and two
  EXPORT_SYMBOL_GPL() entry points…

--

net: pse-pd: add notifier chain for controller lifecycle events

This adds a blocking notifier chain to pse_core along with
pse_register_notifier() and pse_unregister_notifier() as the subscriber
interface, plus enum pse_controller_event describing PSE_REGISTERED and
PSE_UNREGISTERED. Nothing fires the chain and nothing subscribes to it in
this patch.

No Link: tags present in the commit header.

> diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
> index 69dbdbde9d71a..3c4d09f1d6e4f 100644
> --- a/drivers/net/pse-pd/pse_core.c
> +++ b/drivers/net/pse-pd/pse_core.c

[ ... ]

> @@ -23,6 +24,39 @@ static LIST_HEAD(pse_controller_list);
>  static DEFINE_XARRAY_ALLOC(pse_pw_d_map);
>  static DEFINE_MUTEX(pse_pw_d_mutex);
>  
> +static BLOCKING_NOTIFIER_HEAD(pse_controller_notifier);
> +
> +/**
> + * pse_register_notifier - register a callback for PSE controller events
> + * @nb: notifier block to register
> + *
> + * See enum pse_controller_event for events fired and their subscriber
> + * contract. Callbacks run in process context; they may sleep, take
> + * rtnl, and call of_pse_control_get(). The chain fires synchronously,
> + * so a PSE controller driver's probe/unbind path must not hold any
> + * such lock when calling pse_controller_register() or
> + * pse_controller_unregister().
> + *
> + * Return: 0 on success, negative error code otherwise.
> + */
> +int pse_register_notifier(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_register(&pse_controller_notifier, nb);
> +}
> +EXPORT_SYMBOL_GPL(pse_register_notifier);
> +
> +/**
> + * pse_unregister_notifier - unregister a previously registered callback
> + * @nb: notifier block previously passed to pse_register_notifier()
> + *
> + * Return: 0 on success, negative error code otherwise.
> + */
> +int pse_unregister_notifier(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_unregister(&pse_controller_notifier, nb);
> +}
> +EXPORT_SYMBOL_GPL(pse_unregister_notifier);

[Severity: Low]
This isn't a bug, just a note on something that stood out while reading the
patch on its own.

At this commit, pse_register_notifier() and pse_unregister_notifier() in
drivers/net/pse-pd/pse_core.c are exported with no in-tree caller, and the
event-data contract documented for enum pse_controller_event has no producer
yet, so the documented semantics cannot be checked from this patch alone.

Following the series forward, the producer side shows up in the next commit,
with pse_controller_register() and pse_controller_unregister() doing:

	blocking_notifier_call_chain(&pse_controller_notifier,
				     PSE_REGISTERED, pcdev);

	blocking_notifier_call_chain(&pse_controller_notifier,
				     PSE_UNREGISTERED, pcdev);

and the first subscriber lands in phy_init() in drivers/net/phy/phy_device.c:

	rc = pse_register_notifier(&phy_pse_notifier);

Both call sites pass pcdev, which matches what the kerneldoc for enum
pse_controller_event promises, and the commit message states outright that
nothing fires or subscribes yet.

So nothing to fix here, this is the usual "add the API, then use it" split.

[ ... ]

> diff --git a/include/linux/pse-pd/pse.h b/include/linux/pse-pd/pse.h
> index 4e5696cfade7c..78fe3a2b1ea84 100644
> --- a/include/linux/pse-pd/pse.h
> +++ b/include/linux/pse-pd/pse.h

[ ... ]

> @@ -366,6 +385,9 @@ int pse_ethtool_set_prio(struct pse_control *psec,
>  bool pse_has_podl(struct pse_control *psec);
>  bool pse_has_c33(struct pse_control *psec);
>  
> +int pse_register_notifier(struct notifier_block *nb);
> +int pse_unregister_notifier(struct notifier_block *nb);
> +
>  #else

[ ... ]

No other regressions found.  The CONFIG_PSE_CONTROLLER=n stubs returning 0
match the existing stub style in include/linux/pse-pd/pse.h, and placing
enum pse_controller_event outside the IS_ENABLED() guard is consistent with
the stated goal of letting subscribers reference the values in dead-code
paths.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260906153102.959217-1-github%40szelinsky.de