diff mbox series

[net-next,v9,1/2] driver core: auxiliary bus: show auxiliary device IRQs

Message ID 20240703073858.932299-2-shayd@nvidia.com (mailing list archive)
State Superseded
Headers show
Series Introduce auxiliary bus IRQs sysfs | expand

Commit Message

Shay Drori July 3, 2024, 7:38 a.m. UTC 
PCI subfunctions (SF) are anchored on the auxiliary bus. PCI physical
and virtual functions are anchored on the PCI bus. The irq information
of each such function is visible to users via sysfs directory "msi_irqs"
containing files for each irq entry. However, for PCI SFs such
information is unavailable. Due to this users have no visibility on IRQs
used by the SFs.
Secondly, an SF can be multi function device supporting rdma, netdevice
and more. Without irq information at the bus level, the user is unable
to view or use the affinity of the SF IRQs.

Hence to match to the equivalent PCI PFs and VFs, add "irqs" directory,
for supporting auxiliary devices, containing file for each irq entry.

For example:
$ ls /sys/bus/auxiliary/devices/mlx5_core.sf.1/irqs/
50  51  52  53  54  55  56  57  58

Cc: Simon Horman <horms@kernel.org>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Reviewed-by: Parav Pandit <parav@nvidia.com>
Signed-off-by: Shay Drory <shayd@nvidia.com>

---
v8-v9:
- add Przemek RB
- use guard() in auxiliary_irq_dir_prepare (Paolo)
v7-v8:
- use cleanup.h for info and name fields (Greg)
- correct error flow in auxiliary_irq_dir_prepare (Przemek)
- add documentation for new fields of auxiliary_device (Simon)
v6-v7:
- dynamically creating irqs directory when first irq file created (Greg)
- removed irqs flag and simplified the dev_add() API (Greg)
- move sysfs related new code to a new auxiliary_sysfs.c file (Greg)
v5-v6:
- removed concept of shared and exclusive and hence global xarray (Greg)
v4-v5:
- restore global mutex and replace refcount_t with simple integer (Greg)
v3->4:
- remove global mutex (Przemek)
v2->v3:
- fix function declaration in case SYSFS isn't defined
v1->v2:
- move #ifdefs from drivers/base/auxiliary.c to
  include/linux/auxiliary_bus.h (Greg)
- use EXPORT_SYMBOL_GPL instead of EXPORT_SYMBOL (Greg)
- Fix kzalloc(ref) to kzalloc(*ref) (Simon)
- Add return description in auxiliary_device_sysfs_irq_add() kdoc (Simon)
- Fix auxiliary_irq_mode_show doc (kernel test boot)
---
 Documentation/ABI/testing/sysfs-bus-auxiliary |   9 ++
 drivers/base/Makefile                         |   1 +
 drivers/base/auxiliary.c                      |   1 +
 drivers/base/auxiliary_sysfs.c                | 111 ++++++++++++++++++
 include/linux/auxiliary_bus.h                 |  22 ++++
 5 files changed, 144 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-auxiliary
 create mode 100644 drivers/base/auxiliary_sysfs.c

Comments

Greg KH July 4, 2024, 10:41 a.m. UTC | #1 
On Wed, Jul 03, 2024 at 10:38:57AM +0300, Shay Drory wrote:
> +/**
> + * auxiliary_device_sysfs_irq_add - add a sysfs entry for the given IRQ
> + * @auxdev: auxiliary bus device to add the sysfs entry.
> + * @irq: The associated interrupt number.
> + *
> + * This function should be called after auxiliary device have successfully
> + * received the irq.
> + * The driver is responsible to add a unique irq for the auxiliary device. The
> + * driver can invoke this function from multiple thread context safely for
> + * unique irqs of the auxiliary devices. The driver must not invoke this API
> + * multiple times if the irq is already added previously.
> + *
> + * Return: zero on success or an error code on failure.
> + */
> +int auxiliary_device_sysfs_irq_add(struct auxiliary_device *auxdev, int irq)
> +{
> +	struct auxiliary_irq_info *info __free(kfree) = NULL;
> +	struct device *dev = &auxdev->dev;
> +	char *name __free(kfree) = NULL;
> +	int ret;
> +
> +	ret = auxiliary_irq_dir_prepare(auxdev);
> +	if (ret)
> +		return ret;
> +
> +	info = kzalloc(sizeof(*info), GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	sysfs_attr_init(&info->sysfs_attr.attr);
> +	name = kasprintf(GFP_KERNEL, "%d", irq);
> +	if (!name)
> +		return -ENOMEM;
> +
> +	ret = xa_insert(&auxdev->irqs, irq, info, GFP_KERNEL);
> +	if (ret)
> +		return ret;
> +
> +	info->sysfs_attr.attr.name = name;
> +	ret = sysfs_add_file_to_group(&dev->kobj, &info->sysfs_attr.attr,
> +				      auxiliary_irqs_group.name);
> +	if (ret)
> +		goto sysfs_add_err;
> +
> +	info->sysfs_attr.attr.name = no_free_ptr(name);

This assignment of a name AFTER it has been created is odd.  I think I
know why you are doing this, but please make it obvious and perhaps
solve it in a cleaner way.  Assigning this "deep" in a sysfs structure
is not ok.


> +	xa_store(&auxdev->irqs, irq, no_free_ptr(info), GFP_KERNEL);
> +	return 0;
> +
> +sysfs_add_err:
> +	xa_erase(&auxdev->irqs, irq);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(auxiliary_device_sysfs_irq_add);
> +
> +/**
> + * auxiliary_device_sysfs_irq_remove - remove a sysfs entry for the given IRQ
> + * @auxdev: auxiliary bus device to add the sysfs entry.
> + * @irq: the IRQ to remove.
> + *
> + * This function should be called to remove an IRQ sysfs entry.
> + * The driver must invoke this API when IRQ is released by the device.
> + */
> +void auxiliary_device_sysfs_irq_remove(struct auxiliary_device *auxdev, int irq)
> +{
> +	struct auxiliary_irq_info *info __free(kfree) =	xa_load(&auxdev->irqs, irq);

No verification that this is an actual entry before you dereferenced it?
Bold move...

greg k-h
Shay Drori July 5, 2024, 5:35 a.m. UTC | #2 
On 04/07/2024 13:41, Greg KH wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Wed, Jul 03, 2024 at 10:38:57AM +0300, Shay Drory wrote:
>> +/**
>> + * auxiliary_device_sysfs_irq_add - add a sysfs entry for the given IRQ
>> + * @auxdev: auxiliary bus device to add the sysfs entry.
>> + * @irq: The associated interrupt number.
>> + *
>> + * This function should be called after auxiliary device have successfully
>> + * received the irq.
>> + * The driver is responsible to add a unique irq for the auxiliary device. The
>> + * driver can invoke this function from multiple thread context safely for
>> + * unique irqs of the auxiliary devices. The driver must not invoke this API
>> + * multiple times if the irq is already added previously.
>> + *
>> + * Return: zero on success or an error code on failure.
>> + */
>> +int auxiliary_device_sysfs_irq_add(struct auxiliary_device *auxdev, int irq)
>> +{
>> +     struct auxiliary_irq_info *info __free(kfree) = NULL;
>> +     struct device *dev = &auxdev->dev;
>> +     char *name __free(kfree) = NULL;
>> +     int ret;
>> +
>> +     ret = auxiliary_irq_dir_prepare(auxdev);
>> +     if (ret)
>> +             return ret;
>> +
>> +     info = kzalloc(sizeof(*info), GFP_KERNEL);
>> +     if (!info)
>> +             return -ENOMEM;
>> +
>> +     sysfs_attr_init(&info->sysfs_attr.attr);
>> +     name = kasprintf(GFP_KERNEL, "%d", irq);
>> +     if (!name)
>> +             return -ENOMEM;
>> +
>> +     ret = xa_insert(&auxdev->irqs, irq, info, GFP_KERNEL);
>> +     if (ret)
>> +             return ret;
>> +
>> +     info->sysfs_attr.attr.name = name;
>> +     ret = sysfs_add_file_to_group(&dev->kobj, &info->sysfs_attr.attr,
>> +                                   auxiliary_irqs_group.name);
>> +     if (ret)
>> +             goto sysfs_add_err;
>> +
>> +     info->sysfs_attr.attr.name = no_free_ptr(name);
> 
> This assignment of a name AFTER it has been created is odd.  I think I
> know why you are doing this, but please make it obvious and perhaps
> solve it in a cleaner way. 

I am doing it since I want the name memory to be freed in case of
sysfs_add_file_to_group() fails.
I don’t see a cleaner way available with cleanup.h.

> Assigning this "deep" in a sysfs structure is not ok.

when creating sysfs dynamically, there isn't a cleaner way to assign the
name memory.
The closest and exact same use case for pci irq sysfs which uses dynamic
sysfs is msi_sysfs_populate_desc().
It does not use cleanup.h but still has to assign.
I Don’t have any other ideas on how to implement it any more elegantly
with cleanup.h.
Do you prefer to assign it before sysfs_add_file_to_group() similar to
msi_sysfs_populate_desc() and avoid cleanup.h for now?


> 
> 
>> +     xa_store(&auxdev->irqs, irq, no_free_ptr(info), GFP_KERNEL);
>> +     return 0;
>> +
>> +sysfs_add_err:
>> +     xa_erase(&auxdev->irqs, irq);
>> +     return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(auxiliary_device_sysfs_irq_add);
>> +
>> +/**
>> + * auxiliary_device_sysfs_irq_remove - remove a sysfs entry for the given IRQ
>> + * @auxdev: auxiliary bus device to add the sysfs entry.
>> + * @irq: the IRQ to remove.
>> + *
>> + * This function should be called to remove an IRQ sysfs entry.
>> + * The driver must invoke this API when IRQ is released by the device.
>> + */
>> +void auxiliary_device_sysfs_irq_remove(struct auxiliary_device *auxdev, int irq)
>> +{
>> +     struct auxiliary_irq_info *info __free(kfree) = xa_load(&auxdev->irqs, irq);
> 
> No verification that this is an actual entry before you dereferenced it?
> Bold move...

Driver must do this for allocated irq. So xa_load cannot fail.
In previous versions we had WARN_ON to catch driver bugs, but you didn’t
like it.
I think this is fine the way it is in v9.

> 
> greg k-h
Greg KH July 5, 2024, 5:53 a.m. UTC | #3 
On Fri, Jul 05, 2024 at 08:35:33AM +0300, Shay Drori wrote:
> 
> 
> On 04/07/2024 13:41, Greg KH wrote:
> > External email: Use caution opening links or attachments
> > 
> > 
> > On Wed, Jul 03, 2024 at 10:38:57AM +0300, Shay Drory wrote:
> > > +/**
> > > + * auxiliary_device_sysfs_irq_add - add a sysfs entry for the given IRQ
> > > + * @auxdev: auxiliary bus device to add the sysfs entry.
> > > + * @irq: The associated interrupt number.
> > > + *
> > > + * This function should be called after auxiliary device have successfully
> > > + * received the irq.
> > > + * The driver is responsible to add a unique irq for the auxiliary device. The
> > > + * driver can invoke this function from multiple thread context safely for
> > > + * unique irqs of the auxiliary devices. The driver must not invoke this API
> > > + * multiple times if the irq is already added previously.
> > > + *
> > > + * Return: zero on success or an error code on failure.
> > > + */
> > > +int auxiliary_device_sysfs_irq_add(struct auxiliary_device *auxdev, int irq)
> > > +{
> > > +     struct auxiliary_irq_info *info __free(kfree) = NULL;
> > > +     struct device *dev = &auxdev->dev;
> > > +     char *name __free(kfree) = NULL;
> > > +     int ret;
> > > +
> > > +     ret = auxiliary_irq_dir_prepare(auxdev);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     info = kzalloc(sizeof(*info), GFP_KERNEL);
> > > +     if (!info)
> > > +             return -ENOMEM;
> > > +
> > > +     sysfs_attr_init(&info->sysfs_attr.attr);
> > > +     name = kasprintf(GFP_KERNEL, "%d", irq);
> > > +     if (!name)
> > > +             return -ENOMEM;
> > > +
> > > +     ret = xa_insert(&auxdev->irqs, irq, info, GFP_KERNEL);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     info->sysfs_attr.attr.name = name;
> > > +     ret = sysfs_add_file_to_group(&dev->kobj, &info->sysfs_attr.attr,
> > > +                                   auxiliary_irqs_group.name);
> > > +     if (ret)
> > > +             goto sysfs_add_err;
> > > +
> > > +     info->sysfs_attr.attr.name = no_free_ptr(name);
> > 
> > This assignment of a name AFTER it has been created is odd.  I think I
> > know why you are doing this, but please make it obvious and perhaps
> > solve it in a cleaner way.
> 
> I am doing it since I want the name memory to be freed in case of
> sysfs_add_file_to_group() fails.
> I don’t see a cleaner way available with cleanup.h.
> 
> > Assigning this "deep" in a sysfs structure is not ok.
> 
> when creating sysfs dynamically, there isn't a cleaner way to assign the
> name memory.
> The closest and exact same use case for pci irq sysfs which uses dynamic
> sysfs is msi_sysfs_populate_desc().
> It does not use cleanup.h but still has to assign.
> I Don’t have any other ideas on how to implement it any more elegantly
> with cleanup.h.
> Do you prefer to assign it before sysfs_add_file_to_group() similar to
> msi_sysfs_populate_desc() and avoid cleanup.h for now?

No, what msi_sysfs_populate_desc() does is not good, the only objection
here is the assignment after-the-fact you are doing just to work around
cleanup.h.  Surely there's a better way to tell it not to free the
pointer at this point in time other than this.

> > > +     xa_store(&auxdev->irqs, irq, no_free_ptr(info), GFP_KERNEL);
> > > +     return 0;
> > > +
> > > +sysfs_add_err:
> > > +     xa_erase(&auxdev->irqs, irq);
> > > +     return ret;
> > > +}
> > > +EXPORT_SYMBOL_GPL(auxiliary_device_sysfs_irq_add);
> > > +
> > > +/**
> > > + * auxiliary_device_sysfs_irq_remove - remove a sysfs entry for the given IRQ
> > > + * @auxdev: auxiliary bus device to add the sysfs entry.
> > > + * @irq: the IRQ to remove.
> > > + *
> > > + * This function should be called to remove an IRQ sysfs entry.
> > > + * The driver must invoke this API when IRQ is released by the device.
> > > + */
> > > +void auxiliary_device_sysfs_irq_remove(struct auxiliary_device *auxdev, int irq)
> > > +{
> > > +     struct auxiliary_irq_info *info __free(kfree) = xa_load(&auxdev->irqs, irq);
> > 
> > No verification that this is an actual entry before you dereferenced it?
> > Bold move...
> 
> Driver must do this for allocated irq. So xa_load cannot fail.
> In previous versions we had WARN_ON to catch driver bugs, but you didn’t
> like it.

Yes, because if something can happen, you handle the error properly, you
don't reboot a machine.

> I think this is fine the way it is in v9.

No, you are now causing a NULL dereference (or close to it) if something
went wrong.  Properly check this and handle it correctly.

thanks,

greg k-h
Przemek Kitszel July 5, 2024, 6:27 a.m. UTC | #4 
On 7/5/24 07:35, Shay Drori wrote:
> 
> 
> On 04/07/2024 13:41, Greg KH wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On Wed, Jul 03, 2024 at 10:38:57AM +0300, Shay Drory wrote:
>>> +/**
>>> + * auxiliary_device_sysfs_irq_add - add a sysfs entry for the given IRQ
>>> + * @auxdev: auxiliary bus device to add the sysfs entry.
>>> + * @irq: The associated interrupt number.
>>> + *
>>> + * This function should be called after auxiliary device have 
>>> successfully
>>> + * received the irq.
>>> + * The driver is responsible to add a unique irq for the auxiliary 
>>> device. The
>>> + * driver can invoke this function from multiple thread context 
>>> safely for
>>> + * unique irqs of the auxiliary devices. The driver must not invoke 
>>> this API
>>> + * multiple times if the irq is already added previously.
>>> + *
>>> + * Return: zero on success or an error code on failure.
>>> + */
>>> +int auxiliary_device_sysfs_irq_add(struct auxiliary_device *auxdev, 
>>> int irq)
>>> +{
>>> +     struct auxiliary_irq_info *info __free(kfree) = NULL;
>>> +     struct device *dev = &auxdev->dev;
>>> +     char *name __free(kfree) = NULL;
>>> +     int ret;
>>> +
>>> +     ret = auxiliary_irq_dir_prepare(auxdev);
>>> +     if (ret)
>>> +             return ret;
>>> +
>>> +     info = kzalloc(sizeof(*info), GFP_KERNEL);
>>> +     if (!info)
>>> +             return -ENOMEM;
>>> +
>>> +     sysfs_attr_init(&info->sysfs_attr.attr);
>>> +     name = kasprintf(GFP_KERNEL, "%d", irq);
>>> +     if (!name)
>>> +             return -ENOMEM;
>>> +
>>> +     ret = xa_insert(&auxdev->irqs, irq, info, GFP_KERNEL);
>>> +     if (ret)
>>> +             return ret;
>>> +
>>> +     info->sysfs_attr.attr.name = name;
>>> +     ret = sysfs_add_file_to_group(&dev->kobj, &info->sysfs_attr.attr,
>>> +                                   auxiliary_irqs_group.name);
>>> +     if (ret)
>>> +             goto sysfs_add_err;
>>> +
>>> +     info->sysfs_attr.attr.name = no_free_ptr(name);
>>
>> This assignment of a name AFTER it has been created is odd.  I think I
>> know why you are doing this, but please make it obvious and perhaps
>> solve it in a cleaner way. 
> 
> I am doing it since I want the name memory to be freed in case of
> sysfs_add_file_to_group() fails.
> I don’t see a cleaner way available with cleanup.h.
> 
>> Assigning this "deep" in a sysfs structure is not ok.
> 
> when creating sysfs dynamically, there isn't a cleaner way to assign the
> name memory.
> The closest and exact same use case for pci irq sysfs which uses dynamic
> sysfs is msi_sysfs_populate_desc().
> It does not use cleanup.h but still has to assign.
> I Don’t have any other ideas on how to implement it any more elegantly
> with cleanup.h.
> Do you prefer to assign it before sysfs_add_file_to_group() similar to
> msi_sysfs_populate_desc() and avoid cleanup.h for now?

I've overlooked it earlier, sorry.

easiest solution for "general" case would be:
	info->sysfs_attr.attr.name = no_free_ptr(name);
	ret = sysfs_add_file_to_group(&dev->kobj,
			&info->sysfs_attr.attr,
			auxiliary_irqs_group.name);
	if (ret) {
		/* freeing manualy since auto cleanup was
		 * disabled by no_free_ptr() */
		kfree(info->sysfs_attr.attr.name);
		goto sysfs_add_err;
	}

but in your case it will be cleaner to alloc the space for name
together with struct auxiliary_irq_info, by placing a char array
there, either with static size or a flex one (if such case would
be generic)

going one step further would be be to reorder struct device_attribute
and struct attribute fields to have @name as the last one and make it
a flex array - but it is perhaps for another series ;)

>>> +void auxiliary_device_sysfs_irq_remove(struct auxiliary_device *auxdev, int irq)
>>> +{
>>> +     struct auxiliary_irq_info *info __free(kfree) = xa_load(&auxdev->irqs, irq);
>>
>> No verification that this is an actual entry before you dereferenced it?
>> Bold move...
> 
> Driver must do this for allocated irq. So xa_load cannot fail.
> In previous versions we had WARN_ON to catch driver bugs, but you didn’t
> like it.
> I think this is fine the way it is in v9.
> 
>>
>> greg k-h 

Perhaps this is more about trust boundaries?,
I would like to learn something from this case :)
diff mbox series

Patch

diff --git a/Documentation/ABI/testing/sysfs-bus-auxiliary b/Documentation/ABI/testing/sysfs-bus-auxiliary
new file mode 100644
index 000000000000..cc856079690f
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-auxiliary
@@ -0,0 +1,9 @@ 
+What:		/sys/bus/auxiliary/devices/.../irqs/
+Date:		April, 2024
+Contact:	Shay Drory <shayd@nvidia.com>
+Description:
+		The /sys/devices/.../irqs directory contains a variable set of
+		files, with each file is named as irq number similar to PCI PF
+		or VF's irq number located in msi_irqs directory.
+		These irq files are added and removed dynamically when an IRQ
+		is requested and freed respectively for the PCI SF.
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 3079bfe53d04..7fb21768ca36 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -16,6 +16,7 @@  obj-$(CONFIG_NUMA)	+= node.o
 obj-$(CONFIG_MEMORY_HOTPLUG) += memory.o
 ifeq ($(CONFIG_SYSFS),y)
 obj-$(CONFIG_MODULES)	+= module.o
+obj-$(CONFIG_AUXILIARY_BUS) += auxiliary_sysfs.o
 endif
 obj-$(CONFIG_SYS_HYPERVISOR) += hypervisor.o
 obj-$(CONFIG_REGMAP)	+= regmap/
diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
index d3a2c40c2f12..55bde375150f 100644
--- a/drivers/base/auxiliary.c
+++ b/drivers/base/auxiliary.c
@@ -287,6 +287,7 @@  int auxiliary_device_init(struct auxiliary_device *auxdev)
 
 	dev->bus = &auxiliary_bus_type;
 	device_initialize(&auxdev->dev);
+	mutex_init(&auxdev->lock);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(auxiliary_device_init);
diff --git a/drivers/base/auxiliary_sysfs.c b/drivers/base/auxiliary_sysfs.c
new file mode 100644
index 000000000000..f4e267971d70
--- /dev/null
+++ b/drivers/base/auxiliary_sysfs.c
@@ -0,0 +1,111 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2024, NVIDIA CORPORATION & AFFILIATES
+ */
+
+#include <linux/auxiliary_bus.h>
+#include <linux/slab.h>
+
+struct auxiliary_irq_info {
+	struct device_attribute sysfs_attr;
+};
+
+static struct attribute *auxiliary_irq_attrs[] = {
+	NULL
+};
+
+static const struct attribute_group auxiliary_irqs_group = {
+	.name = "irqs",
+	.attrs = auxiliary_irq_attrs,
+};
+
+static int auxiliary_irq_dir_prepare(struct auxiliary_device *auxdev)
+{
+	int ret = 0;
+
+	guard(mutex)(&auxdev->lock);
+	if (auxdev->irq_dir_exists)
+		return 0;
+
+	ret = devm_device_add_group(&auxdev->dev, &auxiliary_irqs_group);
+	if (ret)
+		return ret;
+
+	auxdev->irq_dir_exists = true;
+	xa_init(&auxdev->irqs);
+	return 0;
+}
+
+/**
+ * auxiliary_device_sysfs_irq_add - add a sysfs entry for the given IRQ
+ * @auxdev: auxiliary bus device to add the sysfs entry.
+ * @irq: The associated interrupt number.
+ *
+ * This function should be called after auxiliary device have successfully
+ * received the irq.
+ * The driver is responsible to add a unique irq for the auxiliary device. The
+ * driver can invoke this function from multiple thread context safely for
+ * unique irqs of the auxiliary devices. The driver must not invoke this API
+ * multiple times if the irq is already added previously.
+ *
+ * Return: zero on success or an error code on failure.
+ */
+int auxiliary_device_sysfs_irq_add(struct auxiliary_device *auxdev, int irq)
+{
+	struct auxiliary_irq_info *info __free(kfree) = NULL;
+	struct device *dev = &auxdev->dev;
+	char *name __free(kfree) = NULL;
+	int ret;
+
+	ret = auxiliary_irq_dir_prepare(auxdev);
+	if (ret)
+		return ret;
+
+	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	sysfs_attr_init(&info->sysfs_attr.attr);
+	name = kasprintf(GFP_KERNEL, "%d", irq);
+	if (!name)
+		return -ENOMEM;
+
+	ret = xa_insert(&auxdev->irqs, irq, info, GFP_KERNEL);
+	if (ret)
+		return ret;
+
+	info->sysfs_attr.attr.name = name;
+	ret = sysfs_add_file_to_group(&dev->kobj, &info->sysfs_attr.attr,
+				      auxiliary_irqs_group.name);
+	if (ret)
+		goto sysfs_add_err;
+
+	info->sysfs_attr.attr.name = no_free_ptr(name);
+	xa_store(&auxdev->irqs, irq, no_free_ptr(info), GFP_KERNEL);
+	return 0;
+
+sysfs_add_err:
+	xa_erase(&auxdev->irqs, irq);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(auxiliary_device_sysfs_irq_add);
+
+/**
+ * auxiliary_device_sysfs_irq_remove - remove a sysfs entry for the given IRQ
+ * @auxdev: auxiliary bus device to add the sysfs entry.
+ * @irq: the IRQ to remove.
+ *
+ * This function should be called to remove an IRQ sysfs entry.
+ * The driver must invoke this API when IRQ is released by the device.
+ */
+void auxiliary_device_sysfs_irq_remove(struct auxiliary_device *auxdev, int irq)
+{
+	struct auxiliary_irq_info *info __free(kfree) =	xa_load(&auxdev->irqs, irq);
+	const char *name __free(kfree) = info->sysfs_attr.attr.name;
+	struct device *dev = &auxdev->dev;
+
+	sysfs_remove_file_from_group(&dev->kobj, &info->sysfs_attr.attr,
+				     auxiliary_irqs_group.name);
+	xa_erase(&auxdev->irqs, irq);
+}
+EXPORT_SYMBOL_GPL(auxiliary_device_sysfs_irq_remove);
diff --git a/include/linux/auxiliary_bus.h b/include/linux/auxiliary_bus.h
index de21d9d24a95..ee738379d5f2 100644
--- a/include/linux/auxiliary_bus.h
+++ b/include/linux/auxiliary_bus.h
@@ -58,6 +58,9 @@ 
  *       in
  * @name: Match name found by the auxiliary device driver,
  * @id: unique identitier if multiple devices of the same name are exported,
+ * @irqs: irqs xarray contains irq indices which are used by the device,
+ * @lock: Synchronize irq sysfs creation,
+ * @irq_dir_exists: whether "irqs" directory exists,
  *
  * An auxiliary_device represents a part of its parent device's functionality.
  * It is given a name that, combined with the registering drivers
@@ -138,7 +141,10 @@ 
 struct auxiliary_device {
 	struct device dev;
 	const char *name;
+	struct xarray irqs;
+	struct mutex lock; /* Synchronize irq sysfs creation */
 	u32 id;
+	bool irq_dir_exists;
 };
 
 /**
@@ -212,8 +218,24 @@  int auxiliary_device_init(struct auxiliary_device *auxdev);
 int __auxiliary_device_add(struct auxiliary_device *auxdev, const char *modname);
 #define auxiliary_device_add(auxdev) __auxiliary_device_add(auxdev, KBUILD_MODNAME)
 
+#ifdef CONFIG_SYSFS
+int auxiliary_device_sysfs_irq_add(struct auxiliary_device *auxdev, int irq);
+void auxiliary_device_sysfs_irq_remove(struct auxiliary_device *auxdev,
+				       int irq);
+#else /* CONFIG_SYSFS */
+static inline int
+auxiliary_device_sysfs_irq_add(struct auxiliary_device *auxdev, int irq)
+{
+	return 0;
+}
+
+static inline void
+auxiliary_device_sysfs_irq_remove(struct auxiliary_device *auxdev, int irq) {}
+#endif
+
 static inline void auxiliary_device_uninit(struct auxiliary_device *auxdev)
 {
+	mutex_destroy(&auxdev->lock);
 	put_device(&auxdev->dev);
 }