On Fri, Feb 20, 2026 at 12:59:45AM +0100, Danilo Krummrich wrote:

On Thu Feb 19, 2026 at 10:04 PM CET, Tariq Toukan wrote:

quoted

+/**
+ * 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().

This init->add->remove->destroy pattern follows standard Linux kernel practice.
I expect all current review tools to flag any missing function call
among these three.

Drivers already call auxiliary_device_sysfs_irq_add() and
auxiliary_device_sysfs_irq_remove(). It seems unlikely that they would
intentionally omit half of the required callbacks.

<...>

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.

It is not, atomic is not a replacement for locking and this hunk is
going to be racy as hell:

   25 static int auxiliary_irq_dir_prepare(struct auxiliary_device *auxdev)
   26 {
   ...
   30         if (auxdev->sysfs.irq_dir_exists)
   31                 return 0;
   32
   33         ret = devm_device_add_group(&auxdev->dev, &auxiliary_irqs_group);
   34         if (ret)
   35                 return ret;
   36
   37         auxdev->sysfs.irq_dir_exists = true;

In the proposed patch, locking is handled by the driver, which understands the
flow far better than the driver core.

Thanks

- Danilo