Subject: Re: [PATCH v9 net-next 13/14] pds_core: publish events to the clients

From: Leon Romanovsky <[email-masked]>
Date: Sun, 9 Apr 2023 20:11:43 +0300

On Thu, Apr 06, 2023 at 04:41:42PM -0700, Shannon Nelson wrote:
> When the Core device gets an event from the device, or notices
> the device FW to be up or down, it needs to send those events
> on to the clients that have an event handler.  Add the code to
> pass along the events to the clients.
> 
> The entry points pdsc_register_notify() and pdsc_unregister_notify()
> are EXPORTed for other drivers that want to listen for these events.
> 
> Signed-off-by: Shannon Nelson <[email-masked]>
> ---
>  drivers/net/ethernet/amd/pds_core/adminq.c |  2 ++
>  drivers/net/ethernet/amd/pds_core/core.c   | 32 ++++++++++++++++++++++
>  drivers/net/ethernet/amd/pds_core/core.h   |  3 ++
>  include/linux/pds/pds_common.h             |  2 ++
>  4 files changed, 39 insertions(+)
> 
> diff --git a/drivers/net/ethernet/amd/pds_core/adminq.c b/drivers/net/ethernet/amd/pds_core/adminq.c
> index 25c7dd0d37e5..bb18ac1aabab 100644
> --- a/drivers/net/ethernet/amd/pds_core/adminq.c
> +++ b/drivers/net/ethernet/amd/pds_core/adminq.c
> @@ -27,11 +27,13 @@ static int pdsc_process_notifyq(struct pdsc_qcq *qcq)
>  		case PDS_EVENT_LINK_CHANGE:
>  			dev_info(pdsc->dev, "NotifyQ LINK_CHANGE ecode %d eid %lld\n",
>  				 ecode, eid);
> +			pdsc_notify(PDS_EVENT_LINK_CHANGE, comp);

Aren't you "resending" standard netdev event?
It will be better to send only custom, specific to pds_core events,
while leaving general ones to netdev.

>  			break;
>  
>  		case PDS_EVENT_RESET:
>  			dev_info(pdsc->dev, "NotifyQ RESET ecode %d eid %lld\n",
>  				 ecode, eid);
> +			pdsc_notify(PDS_EVENT_RESET, comp);

We can argue if clients should get this event. Once reset is detected,
the pds_core should close devices by deleting aux drivers.

Thanks

>  			break;
>  
>  		case PDS_EVENT_XCVR:
> diff --git a/drivers/net/ethernet/amd/pds_core/core.c b/drivers/net/ethernet/amd/pds_core/core.c
> index ec088d490d34..b2790be0fc46 100644
> --- a/drivers/net/ethernet/amd/pds_core/core.c
> +++ b/drivers/net/ethernet/amd/pds_core/core.c
> @@ -6,6 +6,25 @@
>  
>  #include "core.h"
>  
> +static BLOCKING_NOTIFIER_HEAD(pds_notify_chain);
> +
> +int pdsc_register_notify(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_register(&pds_notify_chain, nb);
> +}
> +EXPORT_SYMBOL_GPL(pdsc_register_notify);
> +
> +void pdsc_unregister_notify(struct notifier_block *nb)
> +{
> +	blocking_notifier_chain_unregister(&pds_notify_chain, nb);
> +}
> +EXPORT_SYMBOL_GPL(pdsc_unregister_notify);
> +
> +void pdsc_notify(unsigned long event, void *data)
> +{
> +	blocking_notifier_call_chain(&pds_notify_chain, event, data);
> +}
> +
>  void pdsc_intr_free(struct pdsc *pdsc, int index)
>  {
>  	struct pdsc_intr_info *intr_info;
> @@ -513,12 +532,19 @@ void pdsc_stop(struct pdsc *pdsc)
>  
>  static void pdsc_fw_down(struct pdsc *pdsc)
>  {
> +	union pds_core_notifyq_comp reset_event = {
> +		.reset.ecode = cpu_to_le16(PDS_EVENT_RESET),
> +		.reset.state = 0,
> +	};
> +
>  	if (test_and_set_bit(PDSC_S_FW_DEAD, &pdsc->state)) {
>  		dev_err(pdsc->dev, "%s: already happening\n", __func__);
>  		return;
>  	}
>  
> +	/* Notify clients of fw_down */
>  	devlink_health_report(pdsc->fw_reporter, "FW down reported", pdsc);
> +	pdsc_notify(PDS_EVENT_RESET, &reset_event);
>  
>  	pdsc_mask_interrupts(pdsc);
>  	pdsc_teardown(pdsc, PDSC_TEARDOWN_RECOVERY);
> @@ -526,6 +552,10 @@ static void pdsc_fw_down(struct pdsc *pdsc)
>  
>  static void pdsc_fw_up(struct pdsc *pdsc)
>  {
> +	union pds_core_notifyq_comp reset_event = {
> +		.reset.ecode = cpu_to_le16(PDS_EVENT_RESET),
> +		.reset.state = 1,
> +	};
>  	int err;
>  
>  	if (!test_bit(PDSC_S_FW_DEAD, &pdsc->state)) {
> @@ -541,9 +571,11 @@ static void pdsc_fw_up(struct pdsc *pdsc)
>  	if (err)
>  		goto err_out;
>  
> +	/* Notify clients of fw_up */
>  	pdsc->fw_recoveries++;
>  	devlink_health_reporter_state_update(pdsc->fw_reporter,
>  					     DEVLINK_HEALTH_REPORTER_STATE_HEALTHY);
> +	pdsc_notify(PDS_EVENT_RESET, &reset_event);
>  
>  	return;
>  
> diff --git a/drivers/net/ethernet/amd/pds_core/core.h b/drivers/net/ethernet/amd/pds_core/core.h
> index aab4986007b9..2215e4915e6a 100644
> --- a/drivers/net/ethernet/amd/pds_core/core.h
> +++ b/drivers/net/ethernet/amd/pds_core/core.h
> @@ -310,6 +310,9 @@ int pdsc_start(struct pdsc *pdsc);
>  void pdsc_stop(struct pdsc *pdsc);
>  void pdsc_health_thread(struct work_struct *work);
>  
> +int pdsc_register_notify(struct notifier_block *nb);
> +void pdsc_unregister_notify(struct notifier_block *nb);
> +void pdsc_notify(unsigned long event, void *data);
>  int pdsc_auxbus_dev_add_vf(struct pdsc *vf, struct pdsc *pf);
>  int pdsc_auxbus_dev_del_vf(struct pdsc *vf, struct pdsc *pf);
>  
> diff --git a/include/linux/pds/pds_common.h b/include/linux/pds/pds_common.h
> index 898f3c7b14b7..17708a142349 100644
> --- a/include/linux/pds/pds_common.h
> +++ b/include/linux/pds/pds_common.h
> @@ -91,5 +91,7 @@ enum pds_core_logical_qtype {
>  	PDS_CORE_QTYPE_MAX     = 16   /* don't change - used in struct size */
>  };
>  
> +int pdsc_register_notify(struct notifier_block *nb);
> +void pdsc_unregister_notify(struct notifier_block *nb);
>  void *pdsc_get_pf_struct(struct pci_dev *vf_pdev);
>  #endif /* _PDS_COMMON_H_ */
> -- 
> 2.17.1
>