drivers/base/auxiliary.c | 89 +++++++++++++++++++++++++++++++++++++++++++ include/linux/auxiliary_bus.h | 4 ++ 2 files changed, 93 insertions(+)
Add an function helper to create a device on the auxiliary bus.
This should avoid having the same code repeated in the different drivers
registering auxiliary devices.
Suggested-by: Stephen Boyd <sboyd@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
The suggestion for this change was initially discussed here: [1]
I was not sure if the managed variant should return the auxiliary device or
just the error. This initial version returns the auxiliary device, allowing
it to be further (ab)used. Please let me know if you prefer to just return
the error code instead.
Also the non managed variant of the helper is not exported but it could
easily be, if necessary.
[1]: https://lore.kernel.org/linux-clk/df0a53ee859e450d84e81547099f5f36.sboyd@kernel.org
---
drivers/base/auxiliary.c | 89 +++++++++++++++++++++++++++++++++++++++++++
include/linux/auxiliary_bus.h | 4 ++
2 files changed, 93 insertions(+)
diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
index afa4df4c5a3f371b91d8dd8c4325495d32ad1291..60ca3f0da329fb7f8e69ecdf703b505e7cf5085c 100644
--- a/drivers/base/auxiliary.c
+++ b/drivers/base/auxiliary.c
@@ -385,6 +385,95 @@ void auxiliary_driver_unregister(struct auxiliary_driver *auxdrv)
}
EXPORT_SYMBOL_GPL(auxiliary_driver_unregister);
+static DEFINE_IDA(auxiliary_device_ida);
+
+static void auxiliary_device_release(struct device *dev)
+{
+ struct auxiliary_device *auxdev = to_auxiliary_dev(dev);
+
+ ida_free(&auxiliary_device_ida, auxdev->id);
+ kfree(auxdev);
+}
+
+static struct auxiliary_device *auxiliary_device_create(struct device *dev,
+ const char *name,
+ void *platform_data)
+{
+ struct auxiliary_device *auxdev;
+ int ret;
+
+ auxdev = kzalloc(sizeof(*auxdev), GFP_KERNEL);
+ if (!auxdev)
+ return ERR_PTR(-ENOMEM);
+
+ ret = ida_alloc(&auxiliary_device_ida, GFP_KERNEL);
+ if (ret < 0)
+ goto auxdev_free;
+
+ auxdev->id = ret;
+ auxdev->name = name;
+ auxdev->dev.parent = dev;
+ auxdev->dev.platform_data = platform_data;
+ auxdev->dev.release = auxiliary_device_release;
+ device_set_of_node_from_dev(&auxdev->dev, dev);
+
+ ret = auxiliary_device_init(auxdev);
+ if (ret)
+ goto ida_free;
+
+ ret = __auxiliary_device_add(auxdev, dev->driver->name);
+ if (ret) {
+ auxiliary_device_uninit(auxdev);
+ return ERR_PTR(ret);
+ }
+
+ return auxdev;
+
+ida_free:
+ ida_free(&auxiliary_device_ida, auxdev->id);
+auxdev_free:
+ kfree(auxdev);
+ return ERR_PTR(ret);
+}
+
+static void auxiliary_device_destroy(void *_auxdev)
+{
+ struct auxiliary_device *auxdev = _auxdev;
+
+ auxiliary_device_delete(auxdev);
+ auxiliary_device_uninit(auxdev);
+}
+
+/**
+ * devm_auxiliary_device_create - create a device on the auxiliary bus
+ * @dev: parent device
+ * @name: auxiliary bus driver name
+ * @platform_data: auxiliary bus device platform data
+ *
+ * Device managed helper to create an auxiliary bus device.
+ * The parent device KBUILD_MODNAME is automatically inserted before the
+ * provided name for the modname parameter of the auxiliary device created.
+ */
+struct auxiliary_device *devm_auxiliary_device_create(struct device *dev,
+ const char *name,
+ void *platform_data)
+{
+ struct auxiliary_device *auxdev;
+ int ret;
+
+ auxdev = auxiliary_device_create(dev, name, platform_data);
+ if (IS_ERR(auxdev))
+ return auxdev;
+
+ ret = devm_add_action_or_reset(dev, auxiliary_device_destroy,
+ auxdev);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return auxdev;
+}
+EXPORT_SYMBOL_GPL(devm_auxiliary_device_create);
+
void __init auxiliary_bus_init(void)
{
WARN_ON(bus_register(&auxiliary_bus_type));
diff --git a/include/linux/auxiliary_bus.h b/include/linux/auxiliary_bus.h
index 65dd7f15437474468acf0e28f6932a7ff2cfff2c..3ba11b3658833917a225916a508bddbd291bb545 100644
--- a/include/linux/auxiliary_bus.h
+++ b/include/linux/auxiliary_bus.h
@@ -254,6 +254,10 @@ int __auxiliary_driver_register(struct auxiliary_driver *auxdrv, struct module *
void auxiliary_driver_unregister(struct auxiliary_driver *auxdrv);
+struct auxiliary_device *devm_auxiliary_device_create(struct device *dev,
+ const char *name,
+ void *platform_data);
+
/**
* module_auxiliary_driver() - Helper macro for registering an auxiliary driver
* @__auxiliary_driver: auxiliary driver struct
---
base-commit: 40384c840ea1944d7c5a392e8975ed088ecf0b37
change-id: 20241210-aux-device-create-helper-93141524e523
Best regards,
--
JeromeOn Tue, Dec 10, 2024 at 02:43:12PM +0100, Jerome Brunet wrote:
> Add an function helper to create a device on the auxiliary bus.
> This should avoid having the same code repeated in the different drivers
> registering auxiliary devices.
>
> Suggested-by: Stephen Boyd <sboyd@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
> The suggestion for this change was initially discussed here: [1]
>
> I was not sure if the managed variant should return the auxiliary device or
> just the error. This initial version returns the auxiliary device, allowing
> it to be further (ab)used. Please let me know if you prefer to just return
> the error code instead.
>
> Also the non managed variant of the helper is not exported but it could
> easily be, if necessary.
>
> [1]: https://lore.kernel.org/linux-clk/df0a53ee859e450d84e81547099f5f36.sboyd@kernel.org
> ---
> drivers/base/auxiliary.c | 89 +++++++++++++++++++++++++++++++++++++++++++
> include/linux/auxiliary_bus.h | 4 ++
> 2 files changed, 93 insertions(+)
We can't add new functions like this without a real user of it. Please
submit that at the same time.
And are you ok with sharing the id range with multiple aux bus
implementations?
>
> diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
> index afa4df4c5a3f371b91d8dd8c4325495d32ad1291..60ca3f0da329fb7f8e69ecdf703b505e7cf5085c 100644
> --- a/drivers/base/auxiliary.c
> +++ b/drivers/base/auxiliary.c
> @@ -385,6 +385,95 @@ void auxiliary_driver_unregister(struct auxiliary_driver *auxdrv)
> }
> EXPORT_SYMBOL_GPL(auxiliary_driver_unregister);
>
> +static DEFINE_IDA(auxiliary_device_ida);
> +
> +static void auxiliary_device_release(struct device *dev)
> +{
> + struct auxiliary_device *auxdev = to_auxiliary_dev(dev);
> +
> + ida_free(&auxiliary_device_ida, auxdev->id);
> + kfree(auxdev);
> +}
> +
> +static struct auxiliary_device *auxiliary_device_create(struct device *dev,
> + const char *name,
> + void *platform_data)
> +{
> + struct auxiliary_device *auxdev;
> + int ret;
> +
> + auxdev = kzalloc(sizeof(*auxdev), GFP_KERNEL);
> + if (!auxdev)
> + return ERR_PTR(-ENOMEM);
> +
> + ret = ida_alloc(&auxiliary_device_ida, GFP_KERNEL);
> + if (ret < 0)
> + goto auxdev_free;
> +
> + auxdev->id = ret;
> + auxdev->name = name;
> + auxdev->dev.parent = dev;
> + auxdev->dev.platform_data = platform_data;
> + auxdev->dev.release = auxiliary_device_release;
> + device_set_of_node_from_dev(&auxdev->dev, dev);
> +
> + ret = auxiliary_device_init(auxdev);
> + if (ret)
> + goto ida_free;
> +
> + ret = __auxiliary_device_add(auxdev, dev->driver->name);
> + if (ret) {
> + auxiliary_device_uninit(auxdev);
> + return ERR_PTR(ret);
> + }
> +
> + return auxdev;
> +
> +ida_free:
> + ida_free(&auxiliary_device_ida, auxdev->id);
> +auxdev_free:
> + kfree(auxdev);
> + return ERR_PTR(ret);
> +}
> +
> +static void auxiliary_device_destroy(void *_auxdev)
> +{
> + struct auxiliary_device *auxdev = _auxdev;
> +
> + auxiliary_device_delete(auxdev);
> + auxiliary_device_uninit(auxdev);
> +}
> +
> +/**
> + * devm_auxiliary_device_create - create a device on the auxiliary bus
> + * @dev: parent device
> + * @name: auxiliary bus driver name
> + * @platform_data: auxiliary bus device platform data
> + *
> + * Device managed helper to create an auxiliary bus device.
> + * The parent device KBUILD_MODNAME is automatically inserted before the
KBUILD_MODNAME doesn't make sense here, as that's the aux bus file.
thanks,
greg k-h
On Tue 10 Dec 2024 at 15:05, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> On Tue, Dec 10, 2024 at 02:43:12PM +0100, Jerome Brunet wrote:
>> Add an function helper to create a device on the auxiliary bus.
>> This should avoid having the same code repeated in the different drivers
>> registering auxiliary devices.
>>
>> Suggested-by: Stephen Boyd <sboyd@kernel.org>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>> ---
>> The suggestion for this change was initially discussed here: [1]
>>
>> I was not sure if the managed variant should return the auxiliary device or
>> just the error. This initial version returns the auxiliary device, allowing
>> it to be further (ab)used. Please let me know if you prefer to just return
>> the error code instead.
>>
>> Also the non managed variant of the helper is not exported but it could
>> easily be, if necessary.
>>
>> [1]: https://lore.kernel.org/linux-clk/df0a53ee859e450d84e81547099f5f36.sboyd@kernel.org
>> ---
>> drivers/base/auxiliary.c | 89 +++++++++++++++++++++++++++++++++++++++++++
>> include/linux/auxiliary_bus.h | 4 ++
>> 2 files changed, 93 insertions(+)
>
> We can't add new functions like this without a real user of it. Please
> submit that at the same time.
Sure. There is some prep work ongoing in the user. It will get used once
that's done. I'll resubmit once this is ready, assuming the rest is fine.
>
> And are you ok with sharing the id range with multiple aux bus
> implementations?
>
In the initial discussion, a global id was thought to sufficient [2]
It also helps to make things simpler on the user side, which is good I think.
Do you think we've overlooked something ?
[2]: https://lore.kernel.org/linux-clk/c9556de589e289cb1d278d41014791a6.sboyd@kernel.org
>
>>
>> diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
>> index afa4df4c5a3f371b91d8dd8c4325495d32ad1291..60ca3f0da329fb7f8e69ecdf703b505e7cf5085c 100644
>> --- a/drivers/base/auxiliary.c
>> +++ b/drivers/base/auxiliary.c
>> @@ -385,6 +385,95 @@ void auxiliary_driver_unregister(struct auxiliary_driver *auxdrv)
>> }
>> EXPORT_SYMBOL_GPL(auxiliary_driver_unregister);
>>
>> +static DEFINE_IDA(auxiliary_device_ida);
>> +
>> +static void auxiliary_device_release(struct device *dev)
>> +{
>> + struct auxiliary_device *auxdev = to_auxiliary_dev(dev);
>> +
>> + ida_free(&auxiliary_device_ida, auxdev->id);
>> + kfree(auxdev);
>> +}
>> +
>> +static struct auxiliary_device *auxiliary_device_create(struct device *dev,
>> + const char *name,
>> + void *platform_data)
>> +{
>> + struct auxiliary_device *auxdev;
>> + int ret;
>> +
>> + auxdev = kzalloc(sizeof(*auxdev), GFP_KERNEL);
>> + if (!auxdev)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + ret = ida_alloc(&auxiliary_device_ida, GFP_KERNEL);
>> + if (ret < 0)
>> + goto auxdev_free;
>> +
>> + auxdev->id = ret;
>> + auxdev->name = name;
>> + auxdev->dev.parent = dev;
>> + auxdev->dev.platform_data = platform_data;
>> + auxdev->dev.release = auxiliary_device_release;
>> + device_set_of_node_from_dev(&auxdev->dev, dev);
>> +
>> + ret = auxiliary_device_init(auxdev);
>> + if (ret)
>> + goto ida_free;
>> +
>> + ret = __auxiliary_device_add(auxdev, dev->driver->name);
>> + if (ret) {
>> + auxiliary_device_uninit(auxdev);
>> + return ERR_PTR(ret);
>> + }
>> +
>> + return auxdev;
>> +
>> +ida_free:
>> + ida_free(&auxiliary_device_ida, auxdev->id);
>> +auxdev_free:
>> + kfree(auxdev);
>> + return ERR_PTR(ret);
>> +}
>> +
>> +static void auxiliary_device_destroy(void *_auxdev)
>> +{
>> + struct auxiliary_device *auxdev = _auxdev;
>> +
>> + auxiliary_device_delete(auxdev);
>> + auxiliary_device_uninit(auxdev);
>> +}
>> +
>> +/**
>> + * devm_auxiliary_device_create - create a device on the auxiliary bus
>> + * @dev: parent device
>> + * @name: auxiliary bus driver name
>> + * @platform_data: auxiliary bus device platform data
>> + *
>> + * Device managed helper to create an auxiliary bus device.
>> + * The parent device KBUILD_MODNAME is automatically inserted before the
>
> KBUILD_MODNAME doesn't make sense here, as that's the aux bus file.
I'll change to "parent device driver name"
>
> thanks,
>
> greg k-h
--
Jerome
On Tue, Dec 10, 2024 at 03:34:17PM +0100, Jerome Brunet wrote: > On Tue 10 Dec 2024 at 15:05, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > > On Tue, Dec 10, 2024 at 02:43:12PM +0100, Jerome Brunet wrote: > >> Add an function helper to create a device on the auxiliary bus. > >> This should avoid having the same code repeated in the different drivers > >> registering auxiliary devices. > >> > >> Suggested-by: Stephen Boyd <sboyd@kernel.org> > >> Cc: Arnd Bergmann <arnd@arndb.de> > >> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > >> --- > >> The suggestion for this change was initially discussed here: [1] > >> > >> I was not sure if the managed variant should return the auxiliary device or > >> just the error. This initial version returns the auxiliary device, allowing > >> it to be further (ab)used. Please let me know if you prefer to just return > >> the error code instead. > >> > >> Also the non managed variant of the helper is not exported but it could > >> easily be, if necessary. > >> > >> [1]: https://lore.kernel.org/linux-clk/df0a53ee859e450d84e81547099f5f36.sboyd@kernel.org > >> --- > >> drivers/base/auxiliary.c | 89 +++++++++++++++++++++++++++++++++++++++++++ > >> include/linux/auxiliary_bus.h | 4 ++ > >> 2 files changed, 93 insertions(+) > > > > We can't add new functions like this without a real user of it. Please > > submit that at the same time. > > Sure. There is some prep work ongoing in the user. It will get used once > that's done. I'll resubmit once this is ready, assuming the rest is fine. > > > > > And are you ok with sharing the id range with multiple aux bus > > implementations? > > > > In the initial discussion, a global id was thought to sufficient [2] > It also helps to make things simpler on the user side, which is good I think. > > Do you think we've overlooked something ? > > [2]: https://lore.kernel.org/linux-clk/c9556de589e289cb1d278d41014791a6.sboyd@kernel.org No, it is ok, you just don't document it as such, so it might look a bit odd for many users. thanks, greg k-h
© 2016 - 2025 Red Hat, Inc.