Re: [PATCH v2 1/3] acpi/apei: Add devm_ghes_register_vendor_record_notifier()

[Jonathan Cameron]

On Thu, 19 Mar 2026 19:13:07 +0800
Kai-Heng Feng <kaihengf@xxxxxxxxxx> wrote:

> Add a device-managed wrapper around ghes_register_vendor_record_notifier()
> so drivers can avoid manual cleanup on device removal or probe failure.
> 
> Cc: Jonathan Cameron <jonathan.cameron@xxxxxxxxxx>
> Signed-off-by: Kai-Heng Feng <kaihengf@xxxxxxxxxx>
Hi,

My only comment is about following local style. I think that
means moving the docs to the header.  Unfortunately whether things
are in the header or the c file is a subsystem specific thing.

My preference is in the c file, but local style overrides that!
Better to have all the docs in the same place.

Jonathan

> ---
> v2:
>  - New patch.
> 
>  drivers/acpi/apei/ghes.c | 25 +++++++++++++++++++++++++
>  include/acpi/ghes.h      |  3 +++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 8acd2742bb27..d31a70a05538 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/
> @@ -689,6 +689,31 @@ void ghes_unregister_vendor_record_notifier(struct notifier_block *nb)
>  }
>  EXPORT_SYMBOL_GPL(ghes_unregister_vendor_record_notifier);
>  
> +static void ghes_vendor_record_notifier_destroy(void *nb)
> +{
> +	ghes_unregister_vendor_record_notifier(nb);
> +}
> +
> +/**
> + * devm_ghes_register_vendor_record_notifier - device-managed vendor record notifier registration

There is also quite a bit of kernel doc in header.  So I guess
local convention is put it there not in the C code?

Hence I would move the docs there.


> + * @dev: device that owns the notifier lifetime
> + * @nb: pointer to the notifier_block structure of the vendor record handler
> + *
> + * Return: 0 on success, negative errno on failure.
> + */
> +int devm_ghes_register_vendor_record_notifier(struct device *ev,
> +					      struct notifier_block *nb)
> +{
> +	int ret;
> +
> +	ret = ghes_register_vendor_record_notifier(nb);
> +	if (ret)
> +		return ret;
> +
> +	return devm_add_action_or_reset(dev, ghes_vendor_record_notifier_destroy, nb);
> +}
> +EXPORT_SYMBOL_GPL(devm_ghes_register_vendor_record_notifier);
> +
>  static void ghes_vendor_record_work_func(struct work_struct *work)
>  {
>  	struct ghes_vendor_record_entry *entry;
> diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
> index 7bea522c0657..ca3ace828c1c 100644
> --- a/include/acpi/ghes.h
> +++ b/include/acpi/ghes.h
> @@ -71,6 +71,9 @@ int ghes_register_vendor_record_notifier(struct notifier_block *nb);
>   */
>  void ghes_unregister_vendor_record_notifier(struct notifier_block *nb);
>  
> +int devm_ghes_register_vendor_record_notifier(struct device *dev,
> +					      struct notifier_block *nb);
> +
>  struct list_head *ghes_get_devices(void);
>  
>  void ghes_estatus_pool_region_free(unsigned long addr, u32 size);