| Message ID | 2025020424-retrain-recharger-407c@gregkh (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Driver core: Add faux bus devices | expand |
On Tue, Feb 04, 2025 at 12:09:13PM +0100, Greg Kroah-Hartman wrote: > Many drivers abuse the platform driver/bus system as it provides a > simple way to create and bind a device to a driver-specific set of > probe/release functions. Instead of doing that, and wasting all of the > memory associated with a platform device, here is a "faux" bus that > can be used instead. > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > v2: - renamed bus and root device to just "faux" thanks to Thomas > - removed the one-driver-per-device and now just have one driver > entirely thanks to Danilo > - kerneldoc fixups and additions and string handling bounds checks > hanks to Andy > - coding style fix thanks to Jonathan > - tested that the destroy path actually works > > drivers/base/Makefile | 2 +- > drivers/base/base.h | 1 + > drivers/base/faux.c | 196 ++++++++++++++++++++++++++++++++++++ > drivers/base/init.c | 1 + > include/linux/device/faux.h | 31 ++++++ > 5 files changed, 230 insertions(+), 1 deletion(-) > create mode 100644 drivers/base/faux.c > create mode 100644 include/linux/device/faux.h I really like it, it's as simply as it can be. Please find one nit below, otherwise Reviewed-by: Danilo Krummrich <dakr@kernel.org> > > +/** > + * faux_device_destroy - destroy a faux device > + * @faux_dev: faux device to destroy > + * > + * Unregister and free all memory associated with a faux device that was > + * previously created with a call to faux_device_create(). Can we really claim that this frees all memory? Someone can still have a reference to the underlying struct device, right? > + */ > +void faux_device_destroy(struct faux_device *faux_dev) > +{ > + struct device *dev = &faux_dev->dev; > + > + if (IS_ERR_OR_NULL(faux_dev)) > + return; > + > + device_del(dev); > + > + /* The final put_device() will clean up the driver we created for this device. */ > + put_device(dev); Same here, how do we know it's the final one? I also think the "clean up the driver we created for this device" part isn't true any longer. > +} > +EXPORT_SYMBOL_GPL(faux_device_destroy);
On Tue, Feb 04, 2025 at 12:44:03PM +0100, Danilo Krummrich wrote: > On Tue, Feb 04, 2025 at 12:09:13PM +0100, Greg Kroah-Hartman wrote: > > Many drivers abuse the platform driver/bus system as it provides a > > simple way to create and bind a device to a driver-specific set of > > probe/release functions. Instead of doing that, and wasting all of the > > memory associated with a platform device, here is a "faux" bus that > > can be used instead. > > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > --- > > v2: - renamed bus and root device to just "faux" thanks to Thomas > > - removed the one-driver-per-device and now just have one driver > > entirely thanks to Danilo > > - kerneldoc fixups and additions and string handling bounds checks > > hanks to Andy > > - coding style fix thanks to Jonathan > > - tested that the destroy path actually works > > > > drivers/base/Makefile | 2 +- > > drivers/base/base.h | 1 + > > drivers/base/faux.c | 196 ++++++++++++++++++++++++++++++++++++ > > drivers/base/init.c | 1 + > > include/linux/device/faux.h | 31 ++++++ > > 5 files changed, 230 insertions(+), 1 deletion(-) > > create mode 100644 drivers/base/faux.c > > create mode 100644 include/linux/device/faux.h > > I really like it, it's as simply as it can be. > > Please find one nit below, otherwise > > Reviewed-by: Danilo Krummrich <dakr@kernel.org> > > > > > +/** > > + * faux_device_destroy - destroy a faux device > > + * @faux_dev: faux device to destroy > > + * > > + * Unregister and free all memory associated with a faux device that was > > + * previously created with a call to faux_device_create(). > > Can we really claim that this frees all memory? Someone can still have a > reference to the underlying struct device, right? That "someone" is the person that had the original device pointer passed to it, so if that person then calls faux_device_destroy(), yes, that should all be properly cleaned up. But even if it isn't, the device is destroyed and gone from sysfs, and whenever that final final put_device() is called, the memory will then be freed by the driver core itself. So all should be ok unless I'm missing something in some codepath here. > > + */ > > +void faux_device_destroy(struct faux_device *faux_dev) > > +{ > > + struct device *dev = &faux_dev->dev; > > + > > + if (IS_ERR_OR_NULL(faux_dev)) > > + return; > > + > > + device_del(dev); > > + > > + /* The final put_device() will clean up the driver we created for this device. */ > > + put_device(dev); > > Same here, how do we know it's the final one? I also think the "clean up the > driver we created for this device" part isn't true any longer. Oops, missed that comment, will go fix that. And same response as above, it should all be cleaned up as best as we can by now, any other external references is up to them be dropped by that owner. thanks for the review, greg k-h
On Tue, Feb 04, 2025 at 12:52:34PM +0100, Greg Kroah-Hartman wrote: > On Tue, Feb 04, 2025 at 12:44:03PM +0100, Danilo Krummrich wrote: > > On Tue, Feb 04, 2025 at 12:09:13PM +0100, Greg Kroah-Hartman wrote: > > > Many drivers abuse the platform driver/bus system as it provides a > > > simple way to create and bind a device to a driver-specific set of > > > probe/release functions. Instead of doing that, and wasting all of the > > > memory associated with a platform device, here is a "faux" bus that > > > can be used instead. > > > > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > --- > > > v2: - renamed bus and root device to just "faux" thanks to Thomas > > > - removed the one-driver-per-device and now just have one driver > > > entirely thanks to Danilo > > > - kerneldoc fixups and additions and string handling bounds checks > > > hanks to Andy > > > - coding style fix thanks to Jonathan > > > - tested that the destroy path actually works > > > > > > drivers/base/Makefile | 2 +- > > > drivers/base/base.h | 1 + > > > drivers/base/faux.c | 196 ++++++++++++++++++++++++++++++++++++ > > > drivers/base/init.c | 1 + > > > include/linux/device/faux.h | 31 ++++++ > > > 5 files changed, 230 insertions(+), 1 deletion(-) > > > create mode 100644 drivers/base/faux.c > > > create mode 100644 include/linux/device/faux.h > > > > I really like it, it's as simply as it can be. > > > > Please find one nit below, otherwise > > > > Reviewed-by: Danilo Krummrich <dakr@kernel.org> > > > > > > > > +/** > > > + * faux_device_destroy - destroy a faux device > > > + * @faux_dev: faux device to destroy > > > + * > > > + * Unregister and free all memory associated with a faux device that was > > > + * previously created with a call to faux_device_create(). > > > > Can we really claim that this frees all memory? Someone can still have a > > reference to the underlying struct device, right? > > That "someone" is the person that had the original device pointer passed > to it, so if that person then calls faux_device_destroy(), yes, that > should all be properly cleaned up. > > But even if it isn't, the device is destroyed and gone from sysfs, and > whenever that final final put_device() is called, the memory will then > be freed by the driver core itself. Oh indeed, the code here is perfectly fine. I just wanted to say that calling faux_device_destroy() is not a guarantee that "all memory associated with a faux device" is actually freed, as the kernel-doc comment above says (or at least implies). So, the concern only was that the comment could be confusing, as in "How can faux_device_destroy() free the memory, if I still have a separate reference to this thing?" (which it clearly would not).
On Tue, 4 Feb 2025 12:09:13 +0100 Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > Many drivers abuse the platform driver/bus system as it provides a > simple way to create and bind a device to a driver-specific set of > probe/release functions. Instead of doing that, and wasting all of the > memory associated with a platform device, here is a "faux" bus that > can be used instead. > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> FWIW LGTM Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
On Tue, Feb 04, 2025 at 01:04:13PM +0100, Danilo Krummrich wrote: > On Tue, Feb 04, 2025 at 12:52:34PM +0100, Greg Kroah-Hartman wrote: > > On Tue, Feb 04, 2025 at 12:44:03PM +0100, Danilo Krummrich wrote: > > > On Tue, Feb 04, 2025 at 12:09:13PM +0100, Greg Kroah-Hartman wrote: > > > > Many drivers abuse the platform driver/bus system as it provides a > > > > simple way to create and bind a device to a driver-specific set of > > > > probe/release functions. Instead of doing that, and wasting all of the > > > > memory associated with a platform device, here is a "faux" bus that > > > > can be used instead. > > > > > > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > > --- > > > > v2: - renamed bus and root device to just "faux" thanks to Thomas > > > > - removed the one-driver-per-device and now just have one driver > > > > entirely thanks to Danilo > > > > - kerneldoc fixups and additions and string handling bounds checks > > > > hanks to Andy > > > > - coding style fix thanks to Jonathan > > > > - tested that the destroy path actually works > > > > > > > > drivers/base/Makefile | 2 +- > > > > drivers/base/base.h | 1 + > > > > drivers/base/faux.c | 196 ++++++++++++++++++++++++++++++++++++ > > > > drivers/base/init.c | 1 + > > > > include/linux/device/faux.h | 31 ++++++ > > > > 5 files changed, 230 insertions(+), 1 deletion(-) > > > > create mode 100644 drivers/base/faux.c > > > > create mode 100644 include/linux/device/faux.h > > > > > > I really like it, it's as simply as it can be. > > > > > > Please find one nit below, otherwise > > > > > > Reviewed-by: Danilo Krummrich <dakr@kernel.org> > > > > > > > > > > > +/** > > > > + * faux_device_destroy - destroy a faux device > > > > + * @faux_dev: faux device to destroy > > > > + * > > > > + * Unregister and free all memory associated with a faux device that was > > > > + * previously created with a call to faux_device_create(). > > > > > > Can we really claim that this frees all memory? Someone can still have a > > > reference to the underlying struct device, right? > > > > That "someone" is the person that had the original device pointer passed > > to it, so if that person then calls faux_device_destroy(), yes, that > > should all be properly cleaned up. > > > > But even if it isn't, the device is destroyed and gone from sysfs, and > > whenever that final final put_device() is called, the memory will then > > be freed by the driver core itself. > > Oh indeed, the code here is perfectly fine. I just wanted to say that calling > faux_device_destroy() is not a guarantee that "all memory associated with a > faux device" is actually freed, as the kernel-doc comment above says (or at > least implies). > > So, the concern only was that the comment could be confusing, as in "How can > faux_device_destroy() free the memory, if I still have a separate reference to > this thing?" (which it clearly would not). Documentation is hard :) Can you think of some wording here that would explain this better? Something like "after you call this, you can't touch the pointer you passed into here" is what I'm going for. I guess the documentation for device_destroy() would work here as well, which says: * This call unregisters and cleans up a device that was created with a * call to device_create(). Would that be better? thanks, greg k-h
On Tue, Feb 04, 2025 at 01:55:54PM +0100, Greg Kroah-Hartman wrote: > On Tue, Feb 04, 2025 at 01:04:13PM +0100, Danilo Krummrich wrote: > > On Tue, Feb 04, 2025 at 12:52:34PM +0100, Greg Kroah-Hartman wrote: > > > On Tue, Feb 04, 2025 at 12:44:03PM +0100, Danilo Krummrich wrote: > > > > On Tue, Feb 04, 2025 at 12:09:13PM +0100, Greg Kroah-Hartman wrote: > > > > > Many drivers abuse the platform driver/bus system as it provides a > > > > > simple way to create and bind a device to a driver-specific set of > > > > > probe/release functions. Instead of doing that, and wasting all of the > > > > > memory associated with a platform device, here is a "faux" bus that > > > > > can be used instead. > > > > > > > > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > > > --- > > > > > v2: - renamed bus and root device to just "faux" thanks to Thomas > > > > > - removed the one-driver-per-device and now just have one driver > > > > > entirely thanks to Danilo > > > > > - kerneldoc fixups and additions and string handling bounds checks > > > > > hanks to Andy > > > > > - coding style fix thanks to Jonathan > > > > > - tested that the destroy path actually works > > > > > > > > > > drivers/base/Makefile | 2 +- > > > > > drivers/base/base.h | 1 + > > > > > drivers/base/faux.c | 196 ++++++++++++++++++++++++++++++++++++ > > > > > drivers/base/init.c | 1 + > > > > > include/linux/device/faux.h | 31 ++++++ > > > > > 5 files changed, 230 insertions(+), 1 deletion(-) > > > > > create mode 100644 drivers/base/faux.c > > > > > create mode 100644 include/linux/device/faux.h > > > > > > > > I really like it, it's as simply as it can be. > > > > > > > > Please find one nit below, otherwise > > > > > > > > Reviewed-by: Danilo Krummrich <dakr@kernel.org> > > > > > > > > > > > > > > +/** > > > > > + * faux_device_destroy - destroy a faux device > > > > > + * @faux_dev: faux device to destroy > > > > > + * > > > > > + * Unregister and free all memory associated with a faux device that was > > > > > + * previously created with a call to faux_device_create(). > > > > > > > > Can we really claim that this frees all memory? Someone can still have a > > > > reference to the underlying struct device, right? > > > > > > That "someone" is the person that had the original device pointer passed > > > to it, so if that person then calls faux_device_destroy(), yes, that > > > should all be properly cleaned up. > > > > > > But even if it isn't, the device is destroyed and gone from sysfs, and > > > whenever that final final put_device() is called, the memory will then > > > be freed by the driver core itself. > > > > Oh indeed, the code here is perfectly fine. I just wanted to say that calling > > faux_device_destroy() is not a guarantee that "all memory associated with a > > faux device" is actually freed, as the kernel-doc comment above says (or at > > least implies). > > > > So, the concern only was that the comment could be confusing, as in "How can > > faux_device_destroy() free the memory, if I still have a separate reference to > > this thing?" (which it clearly would not). > > Documentation is hard :) > > Can you think of some wording here that would explain this better? > Something like "after you call this, you can't touch the pointer you > passed into here" is what I'm going for. I would probably just say that it drops the initial reference created by faux_device_create(), e.g.: "Unregister a faux device and drop the initial reference obtained through faux_device_create()." -- From the formal side of things: The thing with "can't touch the pointer you passed into here" is that it depends on the conditions and on the definition of what it means for a pointer to be valid. Let's say I have another pointer (B) to the device with (of course) a separate reference. Now, when I call faux_device_destroy(A), and given that I know for sure that the reference of (B) does out-live this operation, I could also argue that I downgraded my strong reference to a weak reference. So, technically I could still touch the pointer, but formally it probably wouldn't be valid anymore.
On Tue, Feb 04, 2025 at 12:09:13PM +0100, Greg Kroah-Hartman wrote: > Many drivers abuse the platform driver/bus system as it provides a > simple way to create and bind a device to a driver-specific set of > probe/release functions. Instead of doing that, and wasting all of the > memory associated with a platform device, here is a "faux" bus that > can be used instead. > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > +/** > + * faux_device_create - create and register a faux device and driver > + * @name: name of the device and driver we are adding > + * @faux_ops: struct faux_driver_ops that the new device will call back into, can be NULL > + * > + * Create a new faux device and driver, both with the same name, and > + * register them in the driver core properly. Along the same lines as Danilo's comment, this routine does not create a new driver any more. Alan Stern
On Tue, Feb 04, 2025 at 12:09:13PM +0100, Greg Kroah-Hartman wrote: > Many drivers abuse the platform driver/bus system as it provides a > simple way to create and bind a device to a driver-specific set of > probe/release functions. Instead of doing that, and wasting all of the > memory associated with a platform device, here is a "faux" bus that > can be used instead. > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > v2: - renamed bus and root device to just "faux" thanks to Thomas > - removed the one-driver-per-device and now just have one driver > entirely thanks to Danilo > - kerneldoc fixups and additions and string handling bounds checks > hanks to Andy > - coding style fix thanks to Jonathan > - tested that the destroy path actually works > > drivers/base/Makefile | 2 +- > drivers/base/base.h | 1 + > drivers/base/faux.c | 196 ++++++++++++++++++++++++++++++++++++ > drivers/base/init.c | 1 + > include/linux/device/faux.h | 31 ++++++ > 5 files changed, 230 insertions(+), 1 deletion(-) > create mode 100644 drivers/base/faux.c > create mode 100644 include/linux/device/faux.h > > diff --git a/drivers/base/Makefile b/drivers/base/Makefile > index 7fb21768ca36..8074a10183dc 100644 > --- a/drivers/base/Makefile > +++ b/drivers/base/Makefile > @@ -6,7 +6,7 @@ obj-y := component.o core.o bus.o dd.o syscore.o \ > cpu.o firmware.o init.o map.o devres.o \ > attribute_container.o transport_class.o \ > topology.o container.o property.o cacheinfo.o \ > - swnode.o > + swnode.o faux.o > obj-$(CONFIG_AUXILIARY_BUS) += auxiliary.o > obj-$(CONFIG_DEVTMPFS) += devtmpfs.o > obj-y += power/ > diff --git a/drivers/base/base.h b/drivers/base/base.h > index 8cf04a557bdb..0042e4774b0c 100644 > --- a/drivers/base/base.h > +++ b/drivers/base/base.h > @@ -137,6 +137,7 @@ int hypervisor_init(void); > static inline int hypervisor_init(void) { return 0; } > #endif > int platform_bus_init(void); > +int faux_bus_init(void); > void cpu_dev_init(void); > void container_dev_init(void); > #ifdef CONFIG_AUXILIARY_BUS > diff --git a/drivers/base/faux.c b/drivers/base/faux.c > new file mode 100644 > index 000000000000..9b28643afc45 > --- /dev/null > +++ b/drivers/base/faux.c > @@ -0,0 +1,196 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2025 Greg Kroah-Hartman <gregkh@linuxfoundation.org> > + * Copyright (c) 2025 The Linux Foundation > + * > + * A "simple" faux bus that allows devices to be created and added > + * automatically to it. This is to be used whenever you need to create a > + * device that is not associated with any "real" system resources, and do > + * not want to have to deal with a bus/driver binding logic. It is > + * intended to be very simple, with only a create and a destroy function > + * available. > + */ > +#include <linux/err.h> > +#include <linux/init.h> > +#include <linux/slab.h> > +#include <linux/string.h> > +#include <linux/container_of.h> > +#include <linux/device/faux.h> > +#include "base.h" > + > +#define MAX_NAME_SIZE 256 /* Max size of a faux_device name */ > + > +/* > + * Internal wrapper structure so we can hold the memory > + * for the driver and the name string of the faux device. > + */ > +struct faux_object { > + struct faux_device faux_dev; > + const struct faux_driver_ops *faux_ops; > + char name[]; > +}; > +#define to_faux_object(dev) container_of_const(dev, struct faux_object, faux_dev.dev) > + > +static struct device faux_bus_root = { > + .init_name = "faux", > +}; > + > +static int faux_match(struct device *dev, const struct device_driver *drv) > +{ > + /* Match always succeeds, we only have one driver */ > + return 1; > +} > + > +static int faux_probe(struct device *dev) > +{ > + struct faux_object *faux_obj = to_faux_object(dev); > + struct faux_device *faux_dev = &faux_obj->faux_dev; > + const struct faux_driver_ops *faux_ops = faux_obj->faux_ops; > + int ret = 0; > + > + if (faux_ops && faux_ops->probe) Is there any use for faux_ops being NULL (or probe being NULL for that matter)? I can't think of one. So faux_device_create should check that and fail instead of checking here. > + ret = faux_ops->probe(faux_dev); > + > + return ret; > +} > + > +static void faux_remove(struct device *dev) > +{ > + struct faux_object *faux_obj = to_faux_object(dev); > + struct faux_device *faux_dev = &faux_obj->faux_dev; > + const struct faux_driver_ops *faux_ops = faux_obj->faux_ops; > + > + if (faux_ops && faux_ops->remove) > + faux_ops->remove(faux_dev); > +} > + > +static const struct bus_type faux_bus_type = { > + .name = "faux", > + .match = faux_match, > + .probe = faux_probe, > + .remove = faux_remove, > +}; > + > +static struct device_driver faux_driver = { > + .name = "faux_driver", > + .bus = &faux_bus_type, > + .probe_type = PROBE_PREFER_ASYNCHRONOUS, > +}; > + > +static void faux_device_release(struct device *dev) > +{ > + struct faux_object *faux_obj = to_faux_object(dev); > + > + kfree(faux_obj); > +} > + > +/** > + * faux_device_create - create and register a faux device and driver > + * @name: name of the device and driver we are adding > + * @faux_ops: struct faux_driver_ops that the new device will call back into, can be NULL > + * > + * Create a new faux device and driver, both with the same name, and > + * register them in the driver core properly. The probe() callback of > + * @faux_ops will be called with the new device that is created for the > + * caller to do something with. > + * > + * Note, when this function is called, the functions specified in struct > + * faux_ops will be called before the function returns, so be prepared for > + * everything to be properly initialized before that point in time. > + * > + * Return: > + * * NULL if an error happened with creating the device > + * * pointer to a valid struct faux_device that is registered with sysfs > + */ > +struct faux_device *faux_device_create(const char *name, struct faux_driver_ops *faux_ops) const struct faux_driver_ops Rob
On Tue, Feb 04, 2025 at 10:46:50AM -0600, Rob Herring wrote: > On Tue, Feb 04, 2025 at 12:09:13PM +0100, Greg Kroah-Hartman wrote: > > Many drivers abuse the platform driver/bus system as it provides a > > simple way to create and bind a device to a driver-specific set of > > probe/release functions. Instead of doing that, and wasting all of the > > memory associated with a platform device, here is a "faux" bus that > > can be used instead. > > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > --- > > v2: - renamed bus and root device to just "faux" thanks to Thomas > > - removed the one-driver-per-device and now just have one driver > > entirely thanks to Danilo > > - kerneldoc fixups and additions and string handling bounds checks > > hanks to Andy > > - coding style fix thanks to Jonathan > > - tested that the destroy path actually works > > > > drivers/base/Makefile | 2 +- > > drivers/base/base.h | 1 + > > drivers/base/faux.c | 196 ++++++++++++++++++++++++++++++++++++ > > drivers/base/init.c | 1 + > > include/linux/device/faux.h | 31 ++++++ > > 5 files changed, 230 insertions(+), 1 deletion(-) > > create mode 100644 drivers/base/faux.c > > create mode 100644 include/linux/device/faux.h > > > > diff --git a/drivers/base/Makefile b/drivers/base/Makefile > > index 7fb21768ca36..8074a10183dc 100644 > > --- a/drivers/base/Makefile > > +++ b/drivers/base/Makefile > > @@ -6,7 +6,7 @@ obj-y := component.o core.o bus.o dd.o syscore.o \ > > cpu.o firmware.o init.o map.o devres.o \ > > attribute_container.o transport_class.o \ > > topology.o container.o property.o cacheinfo.o \ > > - swnode.o > > + swnode.o faux.o > > obj-$(CONFIG_AUXILIARY_BUS) += auxiliary.o > > obj-$(CONFIG_DEVTMPFS) += devtmpfs.o > > obj-y += power/ > > diff --git a/drivers/base/base.h b/drivers/base/base.h > > index 8cf04a557bdb..0042e4774b0c 100644 > > --- a/drivers/base/base.h > > +++ b/drivers/base/base.h > > @@ -137,6 +137,7 @@ int hypervisor_init(void); > > static inline int hypervisor_init(void) { return 0; } > > #endif > > int platform_bus_init(void); > > +int faux_bus_init(void); > > void cpu_dev_init(void); > > void container_dev_init(void); > > #ifdef CONFIG_AUXILIARY_BUS > > diff --git a/drivers/base/faux.c b/drivers/base/faux.c > > new file mode 100644 > > index 000000000000..9b28643afc45 > > --- /dev/null > > +++ b/drivers/base/faux.c > > @@ -0,0 +1,196 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Copyright (c) 2025 Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > + * Copyright (c) 2025 The Linux Foundation > > + * > > + * A "simple" faux bus that allows devices to be created and added > > + * automatically to it. This is to be used whenever you need to create a > > + * device that is not associated with any "real" system resources, and do > > + * not want to have to deal with a bus/driver binding logic. It is > > + * intended to be very simple, with only a create and a destroy function > > + * available. > > + */ > > +#include <linux/err.h> > > +#include <linux/init.h> > > +#include <linux/slab.h> > > +#include <linux/string.h> > > +#include <linux/container_of.h> > > +#include <linux/device/faux.h> > > +#include "base.h" > > + > > +#define MAX_NAME_SIZE 256 /* Max size of a faux_device name */ > > + > > +/* > > + * Internal wrapper structure so we can hold the memory > > + * for the driver and the name string of the faux device. > > + */ > > +struct faux_object { > > + struct faux_device faux_dev; > > + const struct faux_driver_ops *faux_ops; > > + char name[]; > > +}; > > +#define to_faux_object(dev) container_of_const(dev, struct faux_object, faux_dev.dev) > > + > > +static struct device faux_bus_root = { > > + .init_name = "faux", > > +}; > > + > > +static int faux_match(struct device *dev, const struct device_driver *drv) > > +{ > > + /* Match always succeeds, we only have one driver */ > > + return 1; > > +} > > + > > +static int faux_probe(struct device *dev) > > +{ > > + struct faux_object *faux_obj = to_faux_object(dev); > > + struct faux_device *faux_dev = &faux_obj->faux_dev; > > + const struct faux_driver_ops *faux_ops = faux_obj->faux_ops; > > + int ret = 0; > > + > > + if (faux_ops && faux_ops->probe) > > Is there any use for faux_ops being NULL (or probe being NULL for that > matter)? I can't think of one. So faux_device_create should check that > and fail instead of checking here. NM, I see your converted cases do just that. Weird. I suppose you could still say if faux_ops is not NULL, then probe must not be NULL. Rob
I am currently writing up bindings for this in rust now (shouldn't take very long), but after reading through this patch: Reviewed-by: Lyude Paul <lyude@redhat.com> Once I send out bindings for this I can also write up some conversion patches for vkms and vgem, thank you a ton for the help so far greg! On Tue, 2025-02-04 at 12:09 +0100, Greg Kroah-Hartman wrote: > Many drivers abuse the platform driver/bus system as it provides a > simple way to create and bind a device to a driver-specific set of > probe/release functions. Instead of doing that, and wasting all of the > memory associated with a platform device, here is a "faux" bus that > can be used instead. > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > v2: - renamed bus and root device to just "faux" thanks to Thomas > - removed the one-driver-per-device and now just have one driver > entirely thanks to Danilo > - kerneldoc fixups and additions and string handling bounds checks > hanks to Andy > - coding style fix thanks to Jonathan > - tested that the destroy path actually works > > drivers/base/Makefile | 2 +- > drivers/base/base.h | 1 + > drivers/base/faux.c | 196 ++++++++++++++++++++++++++++++++++++ > drivers/base/init.c | 1 + > include/linux/device/faux.h | 31 ++++++ > 5 files changed, 230 insertions(+), 1 deletion(-) > create mode 100644 drivers/base/faux.c > create mode 100644 include/linux/device/faux.h > > diff --git a/drivers/base/Makefile b/drivers/base/Makefile > index 7fb21768ca36..8074a10183dc 100644 > --- a/drivers/base/Makefile > +++ b/drivers/base/Makefile > @@ -6,7 +6,7 @@ obj-y := component.o core.o bus.o dd.o syscore.o \ > cpu.o firmware.o init.o map.o devres.o \ > attribute_container.o transport_class.o \ > topology.o container.o property.o cacheinfo.o \ > - swnode.o > + swnode.o faux.o > obj-$(CONFIG_AUXILIARY_BUS) += auxiliary.o > obj-$(CONFIG_DEVTMPFS) += devtmpfs.o > obj-y += power/ > diff --git a/drivers/base/base.h b/drivers/base/base.h > index 8cf04a557bdb..0042e4774b0c 100644 > --- a/drivers/base/base.h > +++ b/drivers/base/base.h > @@ -137,6 +137,7 @@ int hypervisor_init(void); > static inline int hypervisor_init(void) { return 0; } > #endif > int platform_bus_init(void); > +int faux_bus_init(void); > void cpu_dev_init(void); > void container_dev_init(void); > #ifdef CONFIG_AUXILIARY_BUS > diff --git a/drivers/base/faux.c b/drivers/base/faux.c > new file mode 100644 > index 000000000000..9b28643afc45 > --- /dev/null > +++ b/drivers/base/faux.c > @@ -0,0 +1,196 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2025 Greg Kroah-Hartman <gregkh@linuxfoundation.org> > + * Copyright (c) 2025 The Linux Foundation > + * > + * A "simple" faux bus that allows devices to be created and added > + * automatically to it. This is to be used whenever you need to create a > + * device that is not associated with any "real" system resources, and do > + * not want to have to deal with a bus/driver binding logic. It is > + * intended to be very simple, with only a create and a destroy function > + * available. > + */ > +#include <linux/err.h> > +#include <linux/init.h> > +#include <linux/slab.h> > +#include <linux/string.h> > +#include <linux/container_of.h> > +#include <linux/device/faux.h> > +#include "base.h" > + > +#define MAX_NAME_SIZE 256 /* Max size of a faux_device name */ > + > +/* > + * Internal wrapper structure so we can hold the memory > + * for the driver and the name string of the faux device. > + */ > +struct faux_object { > + struct faux_device faux_dev; > + const struct faux_driver_ops *faux_ops; > + char name[]; > +}; > +#define to_faux_object(dev) container_of_const(dev, struct faux_object, faux_dev.dev) > + > +static struct device faux_bus_root = { > + .init_name = "faux", > +}; > + > +static int faux_match(struct device *dev, const struct device_driver *drv) > +{ > + /* Match always succeeds, we only have one driver */ > + return 1; > +} > + > +static int faux_probe(struct device *dev) > +{ > + struct faux_object *faux_obj = to_faux_object(dev); > + struct faux_device *faux_dev = &faux_obj->faux_dev; > + const struct faux_driver_ops *faux_ops = faux_obj->faux_ops; > + int ret = 0; > + > + if (faux_ops && faux_ops->probe) > + ret = faux_ops->probe(faux_dev); > + > + return ret; > +} > + > +static void faux_remove(struct device *dev) > +{ > + struct faux_object *faux_obj = to_faux_object(dev); > + struct faux_device *faux_dev = &faux_obj->faux_dev; > + const struct faux_driver_ops *faux_ops = faux_obj->faux_ops; > + > + if (faux_ops && faux_ops->remove) > + faux_ops->remove(faux_dev); > +} > + > +static const struct bus_type faux_bus_type = { > + .name = "faux", > + .match = faux_match, > + .probe = faux_probe, > + .remove = faux_remove, > +}; > + > +static struct device_driver faux_driver = { > + .name = "faux_driver", > + .bus = &faux_bus_type, > + .probe_type = PROBE_PREFER_ASYNCHRONOUS, > +}; > + > +static void faux_device_release(struct device *dev) > +{ > + struct faux_object *faux_obj = to_faux_object(dev); > + > + kfree(faux_obj); > +} > + > +/** > + * faux_device_create - create and register a faux device and driver > + * @name: name of the device and driver we are adding > + * @faux_ops: struct faux_driver_ops that the new device will call back into, can be NULL > + * > + * Create a new faux device and driver, both with the same name, and > + * register them in the driver core properly. The probe() callback of > + * @faux_ops will be called with the new device that is created for the > + * caller to do something with. > + * > + * Note, when this function is called, the functions specified in struct > + * faux_ops will be called before the function returns, so be prepared for > + * everything to be properly initialized before that point in time. > + * > + * Return: > + * * NULL if an error happened with creating the device > + * * pointer to a valid struct faux_device that is registered with sysfs > + */ > +struct faux_device *faux_device_create(const char *name, struct faux_driver_ops *faux_ops) > +{ > + struct device *dev; > + struct faux_object *faux_obj; > + struct faux_device *faux_dev; > + int name_size; > + int ret; > + > + name_size = strlen(name); > + if (name_size > MAX_NAME_SIZE) > + return NULL; > + > + faux_obj = kzalloc(sizeof(*faux_obj) + name_size + 1, GFP_KERNEL); > + if (!faux_obj) > + return NULL; > + > + /* Save off the name of the object into local memory */ > + memcpy(faux_obj->name, name, name_size); > + > + /* Save off the callbacks so we can use them in the future */ > + faux_obj->faux_ops = faux_ops; > + > + /* Initialize the device portion and register it with the driver core */ > + faux_dev = &faux_obj->faux_dev; > + dev = &faux_dev->dev; > + > + device_initialize(dev); > + dev->release = faux_device_release; > + dev->parent = &faux_bus_root; > + dev->bus = &faux_bus_type; > + dev_set_name(dev, "%s", name); > + > + ret = device_add(dev); > + if (ret) { > + pr_err("%s: device_add for %s faux device failed with %d\n", > + __func__, name, ret); > + put_device(dev); > + return NULL; > + } > + > + return faux_dev; > +} > +EXPORT_SYMBOL_GPL(faux_device_create); > + > +/** > + * faux_device_destroy - destroy a faux device > + * @faux_dev: faux device to destroy > + * > + * Unregister and free all memory associated with a faux device that was > + * previously created with a call to faux_device_create(). > + */ > +void faux_device_destroy(struct faux_device *faux_dev) > +{ > + struct device *dev = &faux_dev->dev; > + > + if (IS_ERR_OR_NULL(faux_dev)) > + return; > + > + device_del(dev); > + > + /* The final put_device() will clean up the driver we created for this device. */ > + put_device(dev); > +} > +EXPORT_SYMBOL_GPL(faux_device_destroy); > + > +int __init faux_bus_init(void) > +{ > + int ret; > + > + ret = device_register(&faux_bus_root); > + if (ret) { > + put_device(&faux_bus_root); > + return ret; > + } > + > + ret = bus_register(&faux_bus_type); > + if (ret) > + goto error_bus; > + > + ret = driver_register(&faux_driver); > + if (ret) > + goto error_driver; > + > + return ret; > + > +error_driver: > + bus_unregister(&faux_bus_type); > + > +error_bus: > + device_unregister(&faux_bus_root); > + return ret; > +} > diff --git a/drivers/base/init.c b/drivers/base/init.c > index c4954835128c..9d2b06d65dfc 100644 > --- a/drivers/base/init.c > +++ b/drivers/base/init.c > @@ -32,6 +32,7 @@ void __init driver_init(void) > /* These are also core pieces, but must come after the > * core core pieces. > */ > + faux_bus_init(); > of_core_init(); > platform_bus_init(); > auxiliary_bus_init(); > diff --git a/include/linux/device/faux.h b/include/linux/device/faux.h > new file mode 100644 > index 000000000000..2c8ae5bd7ae8 > --- /dev/null > +++ b/include/linux/device/faux.h > @@ -0,0 +1,31 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright (c) 2025 Greg Kroah-Hartman <gregkh@linuxfoundation.org> > + * Copyright (c) 2025 The Linux Foundation > + * > + * A "simple" faux bus that allows devices to be created and added > + * automatically to it. This is to be used whenever you need to create a > + * device that is not associated with any "real" system resources, and do > + * not want to have to deal with a bus/driver binding logic. It is > + * intended to be very simple, with only a create and a destroy function > + * available. > + */ > +#ifndef _FAUX_DEVICE_H_ > +#define _FAUX_DEVICE_H_ > + > +#include <linux/device.h> > + > +struct faux_device { > + struct device dev; > +}; > +#define to_faux_device(x) container_of_const((x), struct faux_device, dev) > + > +struct faux_driver_ops { > + int (*probe)(struct faux_device *faux_dev); > + void (*remove)(struct faux_device *faux_dev); > +}; > + > +struct faux_device *faux_device_create(const char *name, struct faux_driver_ops *faux_ops); > +void faux_device_destroy(struct faux_device *faux_dev); > + > +#endif /* _FAUX_DEVICE_H_ */
Oops! I actually caught one small nitpick I didn't notice before when writing up the bindings: On Tue, 2025-02-04 at 12:09 +0100, Greg Kroah-Hartman wrote: > +/** > + * faux_device_create - create and register a faux device and driver > + * @name: name of the device and driver we are adding > + * @faux_ops: struct faux_driver_ops that the new device will call back into, can be NULL > + * > + * Create a new faux device and driver, both with the same name, and > + * register them in the driver core properly. The probe() callback of > + * @faux_ops will be called with the new device that is created for the > + * caller to do something with. > + * > + * Note, when this function is called, the functions specified in struct > + * faux_ops will be called before the function returns, so be prepared for > + * everything to be properly initialized before that point in time. > + * > + * Return: > + * * NULL if an error happened with creating the device > + * * pointer to a valid struct faux_device that is registered with sysfs > + */ > +struct faux_device *faux_device_create(const char *name, struct faux_driver_ops *faux_ops) ^ Why not const struct faux_driver_ops? Doesn't seem like there's any need to mutate faux_ops. > +{ > + struct device *dev; > + struct faux_object *faux_obj; > + struct faux_device *faux_dev; > + int name_size; > + int ret; > +
OK I definitely should have waited to write the actual bindings before review - sorry! There was one other small thing I ended up noticing: On Tue, 2025-02-04 at 12:09 +0100, Greg Kroah-Hartman wrote: > diff --git a/include/linux/device/faux.h b/include/linux/device/faux.h > new file mode 100644 > index 000000000000..2c8ae5bd7ae8 > --- /dev/null > +++ b/include/linux/device/faux.h > @@ -0,0 +1,31 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright (c) 2025 Greg Kroah-Hartman <gregkh@linuxfoundation.org> > + * Copyright (c) 2025 The Linux Foundation > + * > + * A "simple" faux bus that allows devices to be created and added > + * automatically to it. This is to be used whenever you need to create a > + * device that is not associated with any "real" system resources, and do > + * not want to have to deal with a bus/driver binding logic. It is > + * intended to be very simple, with only a create and a destroy function > + * available. > + */ > +#ifndef _FAUX_DEVICE_H_ > +#define _FAUX_DEVICE_H_ > + > +#include <linux/device.h> > + > +struct faux_device { > + struct device dev; > +}; > +#define to_faux_device(x) container_of_const((x), struct faux_device, dev) > + > +struct faux_driver_ops { > + int (*probe)(struct faux_device *faux_dev); > + void (*remove)(struct faux_device *faux_dev); > +}; > + > +struct faux_device *faux_device_create(const char *name, struct faux_driver_ops *faux_ops); > +void faux_device_destroy(struct faux_device *faux_dev); Should we add faux_get_drvdata()/faux_set_drvdata() since we've got a probe/remove function? Doesn't really look like the platform driver equivalent does mcuh, but I assume just having an inline function for this would make things a little less confusing for users. > + > +#endif /* _FAUX_DEVICE_H_ */
On Tue, Feb 04, 2025 at 05:51:25PM -0500, Lyude Paul wrote: > Oops! I actually caught one small nitpick I didn't notice before when writing > up the bindings: > > On Tue, 2025-02-04 at 12:09 +0100, Greg Kroah-Hartman wrote: > > +/** > > + * faux_device_create - create and register a faux device and driver > > + * @name: name of the device and driver we are adding > > + * @faux_ops: struct faux_driver_ops that the new device will call back into, can be NULL > > + * > > + * Create a new faux device and driver, both with the same name, and > > + * register them in the driver core properly. The probe() callback of > > + * @faux_ops will be called with the new device that is created for the > > + * caller to do something with. > > + * > > + * Note, when this function is called, the functions specified in struct > > + * faux_ops will be called before the function returns, so be prepared for > > + * everything to be properly initialized before that point in time. > > + * > > + * Return: > > + * * NULL if an error happened with creating the device > > + * * pointer to a valid struct faux_device that is registered with sysfs > > + */ > > +struct faux_device *faux_device_create(const char *name, struct faux_driver_ops *faux_ops) > > ^ Why not const struct faux_driver_ops? Doesn't seem like there's any need to > mutate faux_ops. Yes, Rob also pointed this out, I'll make this change, and the documentation updates, later today. thanks, greg k-h
On Tue, Feb 04, 2025 at 06:10:36PM -0500, Lyude Paul wrote: > OK I definitely should have waited to write the actual bindings before review > - sorry! There was one other small thing I ended up noticing: > > On Tue, 2025-02-04 at 12:09 +0100, Greg Kroah-Hartman wrote: > > diff --git a/include/linux/device/faux.h b/include/linux/device/faux.h > > new file mode 100644 > > index 000000000000..2c8ae5bd7ae8 > > --- /dev/null > > +++ b/include/linux/device/faux.h > > @@ -0,0 +1,31 @@ > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > +/* > > + * Copyright (c) 2025 Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > + * Copyright (c) 2025 The Linux Foundation > > + * > > + * A "simple" faux bus that allows devices to be created and added > > + * automatically to it. This is to be used whenever you need to create a > > + * device that is not associated with any "real" system resources, and do > > + * not want to have to deal with a bus/driver binding logic. It is > > + * intended to be very simple, with only a create and a destroy function > > + * available. > > + */ > > +#ifndef _FAUX_DEVICE_H_ > > +#define _FAUX_DEVICE_H_ > > + > > +#include <linux/device.h> > > + > > +struct faux_device { > > + struct device dev; > > +}; > > +#define to_faux_device(x) container_of_const((x), struct faux_device, dev) > > + > > +struct faux_driver_ops { > > + int (*probe)(struct faux_device *faux_dev); > > + void (*remove)(struct faux_device *faux_dev); > > +}; > > + > > +struct faux_device *faux_device_create(const char *name, struct faux_driver_ops *faux_ops); > > +void faux_device_destroy(struct faux_device *faux_dev); > > Should we add faux_get_drvdata()/faux_set_drvdata() since we've got a > probe/remove function? Doesn't really look like the platform driver equivalent > does mcuh, but I assume just having an inline function for this would make > things a little less confusing for users. You already have a reference counted object returned to you, why do you need to increment/decrement it again? All of the users I've found in the kernel so far didn't need that, do you have a specific example where it would be useful? I'll be glad to add it, I just didn't think anyone would ever call it. thanks, greg k-h
On Wed, Feb 05, 2025 at 06:53:09AM +0100, Greg Kroah-Hartman wrote: > On Tue, Feb 04, 2025 at 06:10:36PM -0500, Lyude Paul wrote: > > On Tue, 2025-02-04 at 12:09 +0100, Greg Kroah-Hartman wrote: ... > > Should we add faux_get_drvdata()/faux_set_drvdata() since we've got a > > probe/remove function? Doesn't really look like the platform driver equivalent > > does mcuh, but I assume just having an inline function for this would make > > things a little less confusing for users. > > You already have a reference counted object returned to you, why do you > need to increment/decrement it again? All of the users I've found in > the kernel so far didn't need that, do you have a specific example where > it would be useful? It's about getter and setter for the .driver_data field, I don't see how reference counting can help with this. > I'll be glad to add it, I just didn't think anyone would ever call it.
On Wed, Feb 05, 2025 at 09:57:32AM +0200, Andy Shevchenko wrote: > On Wed, Feb 05, 2025 at 06:53:09AM +0100, Greg Kroah-Hartman wrote: > > On Tue, Feb 04, 2025 at 06:10:36PM -0500, Lyude Paul wrote: > > > On Tue, 2025-02-04 at 12:09 +0100, Greg Kroah-Hartman wrote: > > ... > > > > Should we add faux_get_drvdata()/faux_set_drvdata() since we've got a > > > probe/remove function? Doesn't really look like the platform driver equivalent > > > does mcuh, but I assume just having an inline function for this would make > > > things a little less confusing for users. > > > > You already have a reference counted object returned to you, why do you > > need to increment/decrement it again? All of the users I've found in > > the kernel so far didn't need that, do you have a specific example where > > it would be useful? > > It's about getter and setter for the .driver_data field, I don't see how > reference counting can help with this. {sigh} I saw get and thought get_device() as my coffee hadn't kicked in, my fault. You both are right, I'll go add these wrappers. thanks, greg k-h
diff --git a/drivers/base/Makefile b/drivers/base/Makefile index 7fb21768ca36..8074a10183dc 100644 --- a/drivers/base/Makefile +++ b/drivers/base/Makefile @@ -6,7 +6,7 @@ obj-y := component.o core.o bus.o dd.o syscore.o \ cpu.o firmware.o init.o map.o devres.o \ attribute_container.o transport_class.o \ topology.o container.o property.o cacheinfo.o \ - swnode.o + swnode.o faux.o obj-$(CONFIG_AUXILIARY_BUS) += auxiliary.o obj-$(CONFIG_DEVTMPFS) += devtmpfs.o obj-y += power/ diff --git a/drivers/base/base.h b/drivers/base/base.h index 8cf04a557bdb..0042e4774b0c 100644 --- a/drivers/base/base.h +++ b/drivers/base/base.h @@ -137,6 +137,7 @@ int hypervisor_init(void); static inline int hypervisor_init(void) { return 0; } #endif int platform_bus_init(void); +int faux_bus_init(void); void cpu_dev_init(void); void container_dev_init(void); #ifdef CONFIG_AUXILIARY_BUS diff --git a/drivers/base/faux.c b/drivers/base/faux.c new file mode 100644 index 000000000000..9b28643afc45 --- /dev/null +++ b/drivers/base/faux.c @@ -0,0 +1,196 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2025 Greg Kroah-Hartman <gregkh@linuxfoundation.org> + * Copyright (c) 2025 The Linux Foundation + * + * A "simple" faux bus that allows devices to be created and added + * automatically to it. This is to be used whenever you need to create a + * device that is not associated with any "real" system resources, and do + * not want to have to deal with a bus/driver binding logic. It is + * intended to be very simple, with only a create and a destroy function + * available. + */ +#include <linux/err.h> +#include <linux/init.h> +#include <linux/slab.h> +#include <linux/string.h> +#include <linux/container_of.h> +#include <linux/device/faux.h> +#include "base.h" + +#define MAX_NAME_SIZE 256 /* Max size of a faux_device name */ + +/* + * Internal wrapper structure so we can hold the memory + * for the driver and the name string of the faux device. + */ +struct faux_object { + struct faux_device faux_dev; + const struct faux_driver_ops *faux_ops; + char name[]; +}; +#define to_faux_object(dev) container_of_const(dev, struct faux_object, faux_dev.dev) + +static struct device faux_bus_root = { + .init_name = "faux", +}; + +static int faux_match(struct device *dev, const struct device_driver *drv) +{ + /* Match always succeeds, we only have one driver */ + return 1; +} + +static int faux_probe(struct device *dev) +{ + struct faux_object *faux_obj = to_faux_object(dev); + struct faux_device *faux_dev = &faux_obj->faux_dev; + const struct faux_driver_ops *faux_ops = faux_obj->faux_ops; + int ret = 0; + + if (faux_ops && faux_ops->probe) + ret = faux_ops->probe(faux_dev); + + return ret; +} + +static void faux_remove(struct device *dev) +{ + struct faux_object *faux_obj = to_faux_object(dev); + struct faux_device *faux_dev = &faux_obj->faux_dev; + const struct faux_driver_ops *faux_ops = faux_obj->faux_ops; + + if (faux_ops && faux_ops->remove) + faux_ops->remove(faux_dev); +} + +static const struct bus_type faux_bus_type = { + .name = "faux", + .match = faux_match, + .probe = faux_probe, + .remove = faux_remove, +}; + +static struct device_driver faux_driver = { + .name = "faux_driver", + .bus = &faux_bus_type, + .probe_type = PROBE_PREFER_ASYNCHRONOUS, +}; + +static void faux_device_release(struct device *dev) +{ + struct faux_object *faux_obj = to_faux_object(dev); + + kfree(faux_obj); +} + +/** + * faux_device_create - create and register a faux device and driver + * @name: name of the device and driver we are adding + * @faux_ops: struct faux_driver_ops that the new device will call back into, can be NULL + * + * Create a new faux device and driver, both with the same name, and + * register them in the driver core properly. The probe() callback of + * @faux_ops will be called with the new device that is created for the + * caller to do something with. + * + * Note, when this function is called, the functions specified in struct + * faux_ops will be called before the function returns, so be prepared for + * everything to be properly initialized before that point in time. + * + * Return: + * * NULL if an error happened with creating the device + * * pointer to a valid struct faux_device that is registered with sysfs + */ +struct faux_device *faux_device_create(const char *name, struct faux_driver_ops *faux_ops) +{ + struct device *dev; + struct faux_object *faux_obj; + struct faux_device *faux_dev; + int name_size; + int ret; + + name_size = strlen(name); + if (name_size > MAX_NAME_SIZE) + return NULL; + + faux_obj = kzalloc(sizeof(*faux_obj) + name_size + 1, GFP_KERNEL); + if (!faux_obj) + return NULL; + + /* Save off the name of the object into local memory */ + memcpy(faux_obj->name, name, name_size); + + /* Save off the callbacks so we can use them in the future */ + faux_obj->faux_ops = faux_ops; + + /* Initialize the device portion and register it with the driver core */ + faux_dev = &faux_obj->faux_dev; + dev = &faux_dev->dev; + + device_initialize(dev); + dev->release = faux_device_release; + dev->parent = &faux_bus_root; + dev->bus = &faux_bus_type; + dev_set_name(dev, "%s", name); + + ret = device_add(dev); + if (ret) { + pr_err("%s: device_add for %s faux device failed with %d\n", + __func__, name, ret); + put_device(dev); + return NULL; + } + + return faux_dev; +} +EXPORT_SYMBOL_GPL(faux_device_create); + +/** + * faux_device_destroy - destroy a faux device + * @faux_dev: faux device to destroy + * + * Unregister and free all memory associated with a faux device that was + * previously created with a call to faux_device_create(). + */ +void faux_device_destroy(struct faux_device *faux_dev) +{ + struct device *dev = &faux_dev->dev; + + if (IS_ERR_OR_NULL(faux_dev)) + return; + + device_del(dev); + + /* The final put_device() will clean up the driver we created for this device. */ + put_device(dev); +} +EXPORT_SYMBOL_GPL(faux_device_destroy); + +int __init faux_bus_init(void) +{ + int ret; + + ret = device_register(&faux_bus_root); + if (ret) { + put_device(&faux_bus_root); + return ret; + } + + ret = bus_register(&faux_bus_type); + if (ret) + goto error_bus; + + ret = driver_register(&faux_driver); + if (ret) + goto error_driver; + + return ret; + +error_driver: + bus_unregister(&faux_bus_type); + +error_bus: + device_unregister(&faux_bus_root); + return ret; +} diff --git a/drivers/base/init.c b/drivers/base/init.c index c4954835128c..9d2b06d65dfc 100644 --- a/drivers/base/init.c +++ b/drivers/base/init.c @@ -32,6 +32,7 @@ void __init driver_init(void) /* These are also core pieces, but must come after the * core core pieces. */ + faux_bus_init(); of_core_init(); platform_bus_init(); auxiliary_bus_init(); diff --git a/include/linux/device/faux.h b/include/linux/device/faux.h new file mode 100644 index 000000000000..2c8ae5bd7ae8 --- /dev/null +++ b/include/linux/device/faux.h @@ -0,0 +1,31 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (c) 2025 Greg Kroah-Hartman <gregkh@linuxfoundation.org> + * Copyright (c) 2025 The Linux Foundation + * + * A "simple" faux bus that allows devices to be created and added + * automatically to it. This is to be used whenever you need to create a + * device that is not associated with any "real" system resources, and do + * not want to have to deal with a bus/driver binding logic. It is + * intended to be very simple, with only a create and a destroy function + * available. + */ +#ifndef _FAUX_DEVICE_H_ +#define _FAUX_DEVICE_H_ + +#include <linux/device.h> + +struct faux_device { + struct device dev; +}; +#define to_faux_device(x) container_of_const((x), struct faux_device, dev) + +struct faux_driver_ops { + int (*probe)(struct faux_device *faux_dev); + void (*remove)(struct faux_device *faux_dev); +}; + +struct faux_device *faux_device_create(const char *name, struct faux_driver_ops *faux_ops); +void faux_device_destroy(struct faux_device *faux_dev); + +#endif /* _FAUX_DEVICE_H_ */
Many drivers abuse the platform driver/bus system as it provides a simple way to create and bind a device to a driver-specific set of probe/release functions. Instead of doing that, and wasting all of the memory associated with a platform device, here is a "faux" bus that can be used instead. Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- v2: - renamed bus and root device to just "faux" thanks to Thomas - removed the one-driver-per-device and now just have one driver entirely thanks to Danilo - kerneldoc fixups and additions and string handling bounds checks hanks to Andy - coding style fix thanks to Jonathan - tested that the destroy path actually works drivers/base/Makefile | 2 +- drivers/base/base.h | 1 + drivers/base/faux.c | 196 ++++++++++++++++++++++++++++++++++++ drivers/base/init.c | 1 + include/linux/device/faux.h | 31 ++++++ 5 files changed, 230 insertions(+), 1 deletion(-) create mode 100644 drivers/base/faux.c create mode 100644 include/linux/device/faux.h