Re: [PATCH V2] driver core: auxiliary bus: Fix sysfs creation on bind

["Danilo Krummrich" <dakr@kernel.org>]

On Thu Feb 19, 2026 at 10:04 PM CET, Tariq Toukan wrote:
> +/**
> + * auxiliary_device_sysfs_irq_dir_init - initialize the IRQ sysfs directory
> + * @auxdev: auxiliary bus device to initialize the sysfs directory.
> + *
> + * This function should be called by drivers to initialize the IRQ directory
> + * before adding any IRQ sysfs entries. The driver is responsible for ensuring
> + * this function is called only once and for handling any concurrency control
> + * if needed.
> + *
> + * Drivers must call auxiliary_device_sysfs_irq_dir_destroy() to clean up when
> + * done.
> + *
> + * Return: zero on success or an error code on failure.
> + */
> +int auxiliary_device_sysfs_irq_dir_init(struct auxiliary_device *auxdev)
>  {
> -	int ret = 0;
> -
> -	guard(mutex)(&auxdev->sysfs.lock);
> -	if (auxdev->sysfs.irq_dir_exists)
> -		return 0;
> +	int ret;
>  
> -	ret = devm_device_add_group(&auxdev->dev, &auxiliary_irqs_group);
> +	ret = sysfs_create_group(&auxdev->dev.kobj, &auxiliary_irqs_group);
>  	if (ret)
>  		return ret;
>  
> -	auxdev->sysfs.irq_dir_exists = true;
>  	xa_init(&auxdev->sysfs.irqs);
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(auxiliary_device_sysfs_irq_dir_init);
> +
> +/**
> + * auxiliary_device_sysfs_irq_dir_destroy - destroy the IRQ sysfs directory
> + * @auxdev: auxiliary bus device to destroy the sysfs directory.
> + *
> + * This function should be called by drivers to clean up the IRQ directory
> + * after all IRQ sysfs entries have been removed. The driver is responsible
> + * for ensuring all IRQs are removed before calling this function.
> + */
> +void auxiliary_device_sysfs_irq_dir_destroy(struct auxiliary_device *auxdev)
> +{
> +	xa_destroy(&auxdev->sysfs.irqs);
> +	sysfs_remove_group(&auxdev->dev.kobj, &auxiliary_irqs_group);
> +}
> +EXPORT_SYMBOL_GPL(auxiliary_device_sysfs_irq_dir_destroy);
>  
>  /**
>   * auxiliary_device_sysfs_irq_add - add a sysfs entry for the given IRQ
> @@ -45,7 +70,8 @@ static int auxiliary_irq_dir_prepare(struct auxiliary_device *auxdev)
>   * @irq: The associated interrupt number.
>   *
>   * This function should be called after auxiliary device have successfully
> - * received the irq.
> + * received the irq. The driver must call auxiliary_device_sysfs_irq_dir_init()
> + * before calling this function for the first time.

I'm not convinced by this approach. This adds two new sources of bugs for
drivers.

  1. Drivers can now forget to call auxiliary_device_sysfs_irq_dir_init()
     *before* auxiliary_device_sysfs_irq_add().

  2. Drivers can forget to call auxiliary_device_sysfs_irq_dir_destroy().

Instead, I suggest to keep the current approach and just replace
devm_device_add_group() with devm_auxiliary_device_add_group(), which in its
devres callback additionally clears auxdev->sysfs.irq_dir_exists.

In terms of the auxdev->sysfs.lock, I think this can still be removed, as it
wasn't needed in the first place.

auxiliary_device_sysfs_irq_add() must only be called from a scope where the
auxiliary device is guaranteed to be bound, so there can't be a concurrent
unbind.

There may only be multiple concurrent calls to auxiliary_device_sysfs_irq_add()
itself, and in this case irq_dir_exists can just be an atomic.

Yes, we're still stuck with an atomic for irq_dir_exists, but the driver API
remains much simpler and less error prone.

- Danilo