diff mbox series

[net] driver core: auxiliary bus: Fix sysfs creation on bind

Message ID 20250317103254.573985-1-mbloch@nvidia.com (mailing list archive)
State New
Delegated to: Netdev Maintainers
Headers show
Series [net] driver core: auxiliary bus: Fix sysfs creation on bind | expand

Commit Message

Mark Bloch March 17, 2025, 10:32 a.m. UTC
From: Amir Tzin <amirtz@nvidia.com>

In case an auxiliary device with IRQs directory is unbinded, the
directory is released, but auxdev->sysfs.irq_dir_exists remains true.
This leads to a failure recreating the directory on bind.

Remove flag auxdev->sysfs.irq_dir_exists, add an API for updating
managed attributes group and use it to create the IRQs attribute group
as it does not fail if the group exists. Move initialization of the
sysfs xarray to auxiliary device probe.

Fixes: a808878308a8 ("driver core: auxiliary bus: show auxiliary device IRQs")
Signed-off-by: Amir Tzin <amirtz@nvidia.com>
Reviewed-by: Moshe Shemesh <moshe@nvidia.com>
Signed-off-by: Mark Bloch <mbloch@nvidia.com>
---
 drivers/base/auxiliary.c       | 20 +++++++++--
 drivers/base/auxiliary_sysfs.c | 13 +------
 drivers/base/core.c            | 65 +++++++++++++++++++++++++++-------
 include/linux/auxiliary_bus.h  |  2 --
 include/linux/device.h         |  2 ++
 5 files changed, 73 insertions(+), 29 deletions(-)

Comments

Greg KH March 17, 2025, 1:16 p.m. UTC | #1
On Mon, Mar 17, 2025 at 12:32:54PM +0200, Mark Bloch wrote:
> From: Amir Tzin <amirtz@nvidia.com>
> 
> In case an auxiliary device with IRQs directory is unbinded, the
> directory is released, but auxdev->sysfs.irq_dir_exists remains true.
> This leads to a failure recreating the directory on bind.
> 
> Remove flag auxdev->sysfs.irq_dir_exists, add an API for updating
> managed attributes group and use it to create the IRQs attribute group
> as it does not fail if the group exists. Move initialization of the
> sysfs xarray to auxiliary device probe.

This feels like a lot of different things all tied up into one patch,
why isn't this a series?

> Fixes: a808878308a8 ("driver core: auxiliary bus: show auxiliary device IRQs")
> Signed-off-by: Amir Tzin <amirtz@nvidia.com>
> Reviewed-by: Moshe Shemesh <moshe@nvidia.com>
> Signed-off-by: Mark Bloch <mbloch@nvidia.com>

Why the [net] on the subject line, this is not for the networking
tree...

> ---
>  drivers/base/auxiliary.c       | 20 +++++++++--
>  drivers/base/auxiliary_sysfs.c | 13 +------
>  drivers/base/core.c            | 65 +++++++++++++++++++++++++++-------
>  include/linux/auxiliary_bus.h  |  2 --
>  include/linux/device.h         |  2 ++
>  5 files changed, 73 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
> index afa4df4c5a3f..56a487fce053 100644
> --- a/drivers/base/auxiliary.c
> +++ b/drivers/base/auxiliary.c
> @@ -201,6 +201,18 @@ static const struct dev_pm_ops auxiliary_dev_pm_ops = {
>  	SET_SYSTEM_SLEEP_PM_OPS(pm_generic_suspend, pm_generic_resume)
>  };
>  
> +static void auxiliary_bus_sysfs_probe(struct auxiliary_device *auxdev)
> +{
> +	mutex_init(&auxdev->sysfs.lock);
> +	xa_init(&auxdev->sysfs.irqs);

You aren't adding anything to sysfs here, so why is this called
auxiliary_bus_sysfs_probe()?  Naming is hard, I know :(

> +}
> +
> +static void auxiliary_bus_sysfs_remove(struct auxiliary_device *auxdev)
> +{
> +	xa_destroy(&auxdev->sysfs.irqs);
> +	mutex_destroy(&auxdev->sysfs.lock);

Same here, you aren't removing anything from sysfs.

> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -2835,17 +2835,8 @@ static void devm_attr_group_remove(struct device *dev, void *res)
>  	sysfs_remove_group(&dev->kobj, group);
>  }
>  
> -/**
> - * devm_device_add_group - given a device, create a managed attribute group
> - * @dev:	The device to create the group for
> - * @grp:	The attribute group to create
> - *
> - * This function creates a group for the first time.  It will explicitly
> - * warn and error if any of the attribute files being created already exist.
> - *
> - * Returns 0 on success or error code on failure.
> - */
> -int devm_device_add_group(struct device *dev, const struct attribute_group *grp)
> +static int __devm_device_add_group(struct device *dev, const struct attribute_group *grp,
> +				   bool sysfs_update)
>  {
>  	union device_attr_group_devres *devres;
>  	int error;
> @@ -2855,7 +2846,8 @@ int devm_device_add_group(struct device *dev, const struct attribute_group *grp)
>  	if (!devres)
>  		return -ENOMEM;
>  
> -	error = sysfs_create_group(&dev->kobj, grp);
> +	error = sysfs_update ? sysfs_update_group(&dev->kobj, grp) :
> +			       sysfs_create_group(&dev->kobj, grp);

Add is really an update?  That feels broken.

>  	if (error) {
>  		devres_free(devres);
>  		return error;
> @@ -2865,8 +2857,57 @@ int devm_device_add_group(struct device *dev, const struct attribute_group *grp)
>  	devres_add(dev, devres);
>  	return 0;
>  }
> +
> +/**
> + * devm_device_add_group - given a device, create a managed attribute group
> + * @dev:	The device to create the group for
> + * @grp:	The attribute group to create
> + *
> + * This function creates a group for the first time.  It will explicitly
> + * warn and error if any of the attribute files being created already exist.
> + *
> + * Returns 0 on success or error code on failure.
> + */
> +int devm_device_add_group(struct device *dev, const struct attribute_group *grp)
> +{
> +	return __devm_device_add_group(dev, grp, false);
> +}
>  EXPORT_SYMBOL_GPL(devm_device_add_group);
>  
> +static int devm_device_group_match(struct device *dev, void *res, void *grp)
> +{
> +	union device_attr_group_devres *devres = res;
> +
> +	return devres->group == grp;
> +}
> +
> +/**
> + * devm_device_update_group - given a device, update managed attribute group
> + * @dev:	The device to update the group for
> + * @grp:	The attribute group to update
> + *
> + * This function updates a managed attribute group, create it if it does not
> + * exist and converts an unmanaged attributes group into a managed attributes
> + * group. Unlike devm_device_add_group it will explicitly not warn or error if
> + * any of the attribute files being created already exist. Furthermore, if the
> + * visibility of the files has changed through the is_visible() callback, it
> + * will update the permissions and add or remove the relevant files. Changing a
> + * group's name (subdirectory name under kobj's directory in sysfs) is not
> + * allowed.
> + *
> + * Returns 0 on success or error code on failure.
> + */
> +int devm_device_update_group(struct device *dev, const struct attribute_group *grp)
> +{
> +	union device_attr_group_devres *devres;
> +
> +	devres = devres_find(dev, devm_attr_group_remove, devm_device_group_match, (void *)grp);
> +
> +	return devres ? sysfs_update_group(&dev->kobj, grp) :
> +			__devm_device_add_group(dev, grp, true);
> +}
> +EXPORT_SYMBOL_GPL(devm_device_update_group);

Who is now using this new function?  I don't see it here in this patch,
so why is it included here?

> +
>  static int device_add_attrs(struct device *dev)
>  {
>  	const struct class *class = dev->class;
> diff --git a/include/linux/auxiliary_bus.h b/include/linux/auxiliary_bus.h
> index 65dd7f154374..d8684cbff54e 100644
> --- a/include/linux/auxiliary_bus.h
> +++ b/include/linux/auxiliary_bus.h
> @@ -146,7 +146,6 @@ struct auxiliary_device {
>  	struct {
>  		struct xarray irqs;
>  		struct mutex lock; /* Synchronize irq sysfs creation */
> -		bool irq_dir_exists;
>  	} sysfs;
>  };
>  
> @@ -238,7 +237,6 @@ auxiliary_device_sysfs_irq_remove(struct auxiliary_device *auxdev, int irq) {}
>  
>  static inline void auxiliary_device_uninit(struct auxiliary_device *auxdev)
>  {
> -	mutex_destroy(&auxdev->sysfs.lock);
>  	put_device(&auxdev->dev);
>  }
>  
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 80a5b3268986..faec7a3fab68 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -1273,6 +1273,8 @@ static inline void device_remove_group(struct device *dev,
>  
>  int __must_check devm_device_add_group(struct device *dev,
>  				       const struct attribute_group *grp);
> +int __must_check devm_device_update_group(struct device *dev,
> +					  const struct attribute_group *grp);

Oh no, please no.  I hate the devm_device_add_group() to start with (and
still think it is wrong and will break people's real use cases), I don't
want to mess with a update group thing as well.

Please fix this up and make this a patch series to make it more obvious
why all of this is needed, and that the change really is fixing a real
problem.  As it is, I can't take this, sorry.

greg k-h
Mark Bloch March 17, 2025, 1:22 p.m. UTC | #2
On 17/03/2025 15:16, Greg Kroah-Hartman wrote:
> On Mon, Mar 17, 2025 at 12:32:54PM +0200, Mark Bloch wrote:
>> From: Amir Tzin <amirtz@nvidia.com>
>>
>> In case an auxiliary device with IRQs directory is unbinded, the
>> directory is released, but auxdev->sysfs.irq_dir_exists remains true.
>> This leads to a failure recreating the directory on bind.
>>
>> Remove flag auxdev->sysfs.irq_dir_exists, add an API for updating
>> managed attributes group and use it to create the IRQs attribute group
>> as it does not fail if the group exists. Move initialization of the
>> sysfs xarray to auxiliary device probe.
> 
> This feels like a lot of different things all tied up into one patch,
> why isn't this a series?

Will make it one.

> 
>> Fixes: a808878308a8 ("driver core: auxiliary bus: show auxiliary device IRQs")
>> Signed-off-by: Amir Tzin <amirtz@nvidia.com>
>> Reviewed-by: Moshe Shemesh <moshe@nvidia.com>
>> Signed-off-by: Mark Bloch <mbloch@nvidia.com>
> 
> Why the [net] on the subject line, this is not for the networking
> tree...

We tested it internally over net tree as it affects SFs bind/unbind
on mlx5 driver and because of that my scripts got it wrong, sorry
for the noise.

> 
>> ---
>>  drivers/base/auxiliary.c       | 20 +++++++++--
>>  drivers/base/auxiliary_sysfs.c | 13 +------
>>  drivers/base/core.c            | 65 +++++++++++++++++++++++++++-------
>>  include/linux/auxiliary_bus.h  |  2 --
>>  include/linux/device.h         |  2 ++
>>  5 files changed, 73 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
>> index afa4df4c5a3f..56a487fce053 100644
>> --- a/drivers/base/auxiliary.c
>> +++ b/drivers/base/auxiliary.c
>> @@ -201,6 +201,18 @@ static const struct dev_pm_ops auxiliary_dev_pm_ops = {
>>  	SET_SYSTEM_SLEEP_PM_OPS(pm_generic_suspend, pm_generic_resume)
>>  };
>>  
>> +static void auxiliary_bus_sysfs_probe(struct auxiliary_device *auxdev)
>> +{
>> +	mutex_init(&auxdev->sysfs.lock);
>> +	xa_init(&auxdev->sysfs.irqs);
> 
> You aren't adding anything to sysfs here, so why is this called
> auxiliary_bus_sysfs_probe()?  Naming is hard, I know :(
> 
>> +}
>> +
>> +static void auxiliary_bus_sysfs_remove(struct auxiliary_device *auxdev)
>> +{
>> +	xa_destroy(&auxdev->sysfs.irqs);
>> +	mutex_destroy(&auxdev->sysfs.lock);
> 
> Same here, you aren't removing anything from sysfs.
> 
>> --- a/drivers/base/core.c
>> +++ b/drivers/base/core.c
>> @@ -2835,17 +2835,8 @@ static void devm_attr_group_remove(struct device *dev, void *res)
>>  	sysfs_remove_group(&dev->kobj, group);
>>  }
>>  
>> -/**
>> - * devm_device_add_group - given a device, create a managed attribute group
>> - * @dev:	The device to create the group for
>> - * @grp:	The attribute group to create
>> - *
>> - * This function creates a group for the first time.  It will explicitly
>> - * warn and error if any of the attribute files being created already exist.
>> - *
>> - * Returns 0 on success or error code on failure.
>> - */
>> -int devm_device_add_group(struct device *dev, const struct attribute_group *grp)
>> +static int __devm_device_add_group(struct device *dev, const struct attribute_group *grp,
>> +				   bool sysfs_update)
>>  {
>>  	union device_attr_group_devres *devres;
>>  	int error;
>> @@ -2855,7 +2846,8 @@ int devm_device_add_group(struct device *dev, const struct attribute_group *grp)
>>  	if (!devres)
>>  		return -ENOMEM;
>>  
>> -	error = sysfs_create_group(&dev->kobj, grp);
>> +	error = sysfs_update ? sysfs_update_group(&dev->kobj, grp) :
>> +			       sysfs_create_group(&dev->kobj, grp);
> 
> Add is really an update?  That feels broken.
> 
>>  	if (error) {
>>  		devres_free(devres);
>>  		return error;
>> @@ -2865,8 +2857,57 @@ int devm_device_add_group(struct device *dev, const struct attribute_group *grp)
>>  	devres_add(dev, devres);
>>  	return 0;
>>  }
>> +
>> +/**
>> + * devm_device_add_group - given a device, create a managed attribute group
>> + * @dev:	The device to create the group for
>> + * @grp:	The attribute group to create
>> + *
>> + * This function creates a group for the first time.  It will explicitly
>> + * warn and error if any of the attribute files being created already exist.
>> + *
>> + * Returns 0 on success or error code on failure.
>> + */
>> +int devm_device_add_group(struct device *dev, const struct attribute_group *grp)
>> +{
>> +	return __devm_device_add_group(dev, grp, false);
>> +}
>>  EXPORT_SYMBOL_GPL(devm_device_add_group);
>>  
>> +static int devm_device_group_match(struct device *dev, void *res, void *grp)
>> +{
>> +	union device_attr_group_devres *devres = res;
>> +
>> +	return devres->group == grp;
>> +}
>> +
>> +/**
>> + * devm_device_update_group - given a device, update managed attribute group
>> + * @dev:	The device to update the group for
>> + * @grp:	The attribute group to update
>> + *
>> + * This function updates a managed attribute group, create it if it does not
>> + * exist and converts an unmanaged attributes group into a managed attributes
>> + * group. Unlike devm_device_add_group it will explicitly not warn or error if
>> + * any of the attribute files being created already exist. Furthermore, if the
>> + * visibility of the files has changed through the is_visible() callback, it
>> + * will update the permissions and add or remove the relevant files. Changing a
>> + * group's name (subdirectory name under kobj's directory in sysfs) is not
>> + * allowed.
>> + *
>> + * Returns 0 on success or error code on failure.
>> + */
>> +int devm_device_update_group(struct device *dev, const struct attribute_group *grp)
>> +{
>> +	union device_attr_group_devres *devres;
>> +
>> +	devres = devres_find(dev, devm_attr_group_remove, devm_device_group_match, (void *)grp);
>> +
>> +	return devres ? sysfs_update_group(&dev->kobj, grp) :
>> +			__devm_device_add_group(dev, grp, true);
>> +}
>> +EXPORT_SYMBOL_GPL(devm_device_update_group);
> 
> Who is now using this new function?  I don't see it here in this patch,
> so why is it included here?
> 
>> +
>>  static int device_add_attrs(struct device *dev)
>>  {
>>  	const struct class *class = dev->class;
>> diff --git a/include/linux/auxiliary_bus.h b/include/linux/auxiliary_bus.h
>> index 65dd7f154374..d8684cbff54e 100644
>> --- a/include/linux/auxiliary_bus.h
>> +++ b/include/linux/auxiliary_bus.h
>> @@ -146,7 +146,6 @@ struct auxiliary_device {
>>  	struct {
>>  		struct xarray irqs;
>>  		struct mutex lock; /* Synchronize irq sysfs creation */
>> -		bool irq_dir_exists;
>>  	} sysfs;
>>  };
>>  
>> @@ -238,7 +237,6 @@ auxiliary_device_sysfs_irq_remove(struct auxiliary_device *auxdev, int irq) {}
>>  
>>  static inline void auxiliary_device_uninit(struct auxiliary_device *auxdev)
>>  {
>> -	mutex_destroy(&auxdev->sysfs.lock);
>>  	put_device(&auxdev->dev);
>>  }
>>  
>> diff --git a/include/linux/device.h b/include/linux/device.h
>> index 80a5b3268986..faec7a3fab68 100644
>> --- a/include/linux/device.h
>> +++ b/include/linux/device.h
>> @@ -1273,6 +1273,8 @@ static inline void device_remove_group(struct device *dev,
>>  
>>  int __must_check devm_device_add_group(struct device *dev,
>>  				       const struct attribute_group *grp);
>> +int __must_check devm_device_update_group(struct device *dev,
>> +					  const struct attribute_group *grp);
> 
> Oh no, please no.  I hate the devm_device_add_group() to start with (and
> still think it is wrong and will break people's real use cases), I don't
> want to mess with a update group thing as well.
> 
> Please fix this up and make this a patch series to make it more obvious
> why all of this is needed, and that the change really is fixing a real
> problem.  As it is, I can't take this, sorry.

ACK to all comments, I'll take it with the author and will make it a
proper series.

Mark

> 
> greg k-h
diff mbox series

Patch

diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
index afa4df4c5a3f..56a487fce053 100644
--- a/drivers/base/auxiliary.c
+++ b/drivers/base/auxiliary.c
@@ -201,6 +201,18 @@  static const struct dev_pm_ops auxiliary_dev_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(pm_generic_suspend, pm_generic_resume)
 };
 
+static void auxiliary_bus_sysfs_probe(struct auxiliary_device *auxdev)
+{
+	mutex_init(&auxdev->sysfs.lock);
+	xa_init(&auxdev->sysfs.irqs);
+}
+
+static void auxiliary_bus_sysfs_remove(struct auxiliary_device *auxdev)
+{
+	xa_destroy(&auxdev->sysfs.irqs);
+	mutex_destroy(&auxdev->sysfs.lock);
+}
+
 static int auxiliary_bus_probe(struct device *dev)
 {
 	const struct auxiliary_driver *auxdrv = to_auxiliary_drv(dev->driver);
@@ -213,10 +225,12 @@  static int auxiliary_bus_probe(struct device *dev)
 		return ret;
 	}
 
+	auxiliary_bus_sysfs_probe(auxdev);
 	ret = auxdrv->probe(auxdev, auxiliary_match_id(auxdrv->id_table, auxdev));
-	if (ret)
+	if (ret) {
+		auxiliary_bus_sysfs_remove(auxdev);
 		dev_pm_domain_detach(dev, true);
-
+	}
 	return ret;
 }
 
@@ -227,6 +241,7 @@  static void auxiliary_bus_remove(struct device *dev)
 
 	if (auxdrv->remove)
 		auxdrv->remove(auxdev);
+	auxiliary_bus_sysfs_remove(auxdev);
 	dev_pm_domain_detach(dev, true);
 }
 
@@ -287,7 +302,6 @@  int auxiliary_device_init(struct auxiliary_device *auxdev)
 
 	dev->bus = &auxiliary_bus_type;
 	device_initialize(&auxdev->dev);
-	mutex_init(&auxdev->sysfs.lock);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(auxiliary_device_init);
diff --git a/drivers/base/auxiliary_sysfs.c b/drivers/base/auxiliary_sysfs.c
index 754f21730afd..fa0eb4009f4d 100644
--- a/drivers/base/auxiliary_sysfs.c
+++ b/drivers/base/auxiliary_sysfs.c
@@ -24,19 +24,8 @@  static const struct attribute_group auxiliary_irqs_group = {
 
 static int auxiliary_irq_dir_prepare(struct auxiliary_device *auxdev)
 {
-	int ret = 0;
-
 	guard(mutex)(&auxdev->sysfs.lock);
-	if (auxdev->sysfs.irq_dir_exists)
-		return 0;
-
-	ret = devm_device_add_group(&auxdev->dev, &auxiliary_irqs_group);
-	if (ret)
-		return ret;
-
-	auxdev->sysfs.irq_dir_exists = true;
-	xa_init(&auxdev->sysfs.irqs);
-	return 0;
+	return devm_device_update_group(&auxdev->dev, &auxiliary_irqs_group);
 }
 
 /**
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 2fde698430df..5cc89528ffd2 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -2835,17 +2835,8 @@  static void devm_attr_group_remove(struct device *dev, void *res)
 	sysfs_remove_group(&dev->kobj, group);
 }
 
-/**
- * devm_device_add_group - given a device, create a managed attribute group
- * @dev:	The device to create the group for
- * @grp:	The attribute group to create
- *
- * This function creates a group for the first time.  It will explicitly
- * warn and error if any of the attribute files being created already exist.
- *
- * Returns 0 on success or error code on failure.
- */
-int devm_device_add_group(struct device *dev, const struct attribute_group *grp)
+static int __devm_device_add_group(struct device *dev, const struct attribute_group *grp,
+				   bool sysfs_update)
 {
 	union device_attr_group_devres *devres;
 	int error;
@@ -2855,7 +2846,8 @@  int devm_device_add_group(struct device *dev, const struct attribute_group *grp)
 	if (!devres)
 		return -ENOMEM;
 
-	error = sysfs_create_group(&dev->kobj, grp);
+	error = sysfs_update ? sysfs_update_group(&dev->kobj, grp) :
+			       sysfs_create_group(&dev->kobj, grp);
 	if (error) {
 		devres_free(devres);
 		return error;
@@ -2865,8 +2857,57 @@  int devm_device_add_group(struct device *dev, const struct attribute_group *grp)
 	devres_add(dev, devres);
 	return 0;
 }
+
+/**
+ * devm_device_add_group - given a device, create a managed attribute group
+ * @dev:	The device to create the group for
+ * @grp:	The attribute group to create
+ *
+ * This function creates a group for the first time.  It will explicitly
+ * warn and error if any of the attribute files being created already exist.
+ *
+ * Returns 0 on success or error code on failure.
+ */
+int devm_device_add_group(struct device *dev, const struct attribute_group *grp)
+{
+	return __devm_device_add_group(dev, grp, false);
+}
 EXPORT_SYMBOL_GPL(devm_device_add_group);
 
+static int devm_device_group_match(struct device *dev, void *res, void *grp)
+{
+	union device_attr_group_devres *devres = res;
+
+	return devres->group == grp;
+}
+
+/**
+ * devm_device_update_group - given a device, update managed attribute group
+ * @dev:	The device to update the group for
+ * @grp:	The attribute group to update
+ *
+ * This function updates a managed attribute group, create it if it does not
+ * exist and converts an unmanaged attributes group into a managed attributes
+ * group. Unlike devm_device_add_group it will explicitly not warn or error if
+ * any of the attribute files being created already exist. Furthermore, if the
+ * visibility of the files has changed through the is_visible() callback, it
+ * will update the permissions and add or remove the relevant files. Changing a
+ * group's name (subdirectory name under kobj's directory in sysfs) is not
+ * allowed.
+ *
+ * Returns 0 on success or error code on failure.
+ */
+int devm_device_update_group(struct device *dev, const struct attribute_group *grp)
+{
+	union device_attr_group_devres *devres;
+
+	devres = devres_find(dev, devm_attr_group_remove, devm_device_group_match, (void *)grp);
+
+	return devres ? sysfs_update_group(&dev->kobj, grp) :
+			__devm_device_add_group(dev, grp, true);
+}
+EXPORT_SYMBOL_GPL(devm_device_update_group);
+
 static int device_add_attrs(struct device *dev)
 {
 	const struct class *class = dev->class;
diff --git a/include/linux/auxiliary_bus.h b/include/linux/auxiliary_bus.h
index 65dd7f154374..d8684cbff54e 100644
--- a/include/linux/auxiliary_bus.h
+++ b/include/linux/auxiliary_bus.h
@@ -146,7 +146,6 @@  struct auxiliary_device {
 	struct {
 		struct xarray irqs;
 		struct mutex lock; /* Synchronize irq sysfs creation */
-		bool irq_dir_exists;
 	} sysfs;
 };
 
@@ -238,7 +237,6 @@  auxiliary_device_sysfs_irq_remove(struct auxiliary_device *auxdev, int irq) {}
 
 static inline void auxiliary_device_uninit(struct auxiliary_device *auxdev)
 {
-	mutex_destroy(&auxdev->sysfs.lock);
 	put_device(&auxdev->dev);
 }
 
diff --git a/include/linux/device.h b/include/linux/device.h
index 80a5b3268986..faec7a3fab68 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1273,6 +1273,8 @@  static inline void device_remove_group(struct device *dev,
 
 int __must_check devm_device_add_group(struct device *dev,
 				       const struct attribute_group *grp);
+int __must_check devm_device_update_group(struct device *dev,
+					  const struct attribute_group *grp);
 
 /*
  * get_device - atomically increment the reference count for the device.