Re: [PATCH v2 1/5] driver core: add a faux bus for use when a simple device/bus is needed

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

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.

probe and/or faux_ops can be NULL, that's fine.  And if a driver only
wants to be notified when remove() gets called, that's fine too, I'm not
going to object too hard.

So this should be ok for now, unless it starts to get abused in odd
ways.

> > +/**
> > + * 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

Good catch, will fix up.  Thanks for the review.

greg k-h