[PATCH 0/3] Driver core: faux bus

[PATCH 0/3] Driver core: faux bus

[Greg Kroah-Hartman]

For years/decades now, I've been complaining when I see people use
platform devices for things that are obviously NOT platform devices.
To finally fix this up, here is a "faux bus" that should be used instead
of a platform device for these tiny and "fake" devices that people
create all over the place.

The api is even simpler than the normal platform device api, just two
functions, one to create a device and one to remove it.  When a device
is created, a driver is also created and automatically bound to it,
which allows for the probe/release callbacks to work like expected.
When finished with the device, just destroy it and all should be good.

This simple api should also hopefully provide for a simple rust binding
to it given the simple rules and lifecycle of the pointer passed back
from the creation function (i.e. it is alive and valid for as long as
you have not called destroy on it.)

I've also converted two different examples of platform device abuse, the
dummy regulator driver, and the USB phy code, to use this api.  In both
cases, the logic either was identical, or became simpler, than before, a
good sign (side note, a bug was fixed in the usb phy code that no one
ever noticed before).


Greg Kroah-Hartman (3):
  driver core: add a faux bus for use when a simple device/bus is needed
  regulator: dummy: convert to use the faux bus
  USB: phy: convert usb_phy_generic logic to use a faux device

 drivers/base/Makefile               |   2 +-
 drivers/base/base.h                 |   1 +
 drivers/base/faux.c                 | 189 ++++++++++++++++++++++++++++
 drivers/base/init.c                 |   1 +
 drivers/regulator/dummy.c           |  37 ++----
 drivers/usb/chipidea/ci_hdrc_pci.c  |   2 +-
 drivers/usb/dwc2/pci.c              |   4 +-
 drivers/usb/musb/mediatek.c         |   4 +-
 drivers/usb/musb/mpfs.c             |   4 +-
 drivers/usb/musb/tusb6010.c         |   2 +-
 drivers/usb/phy/phy-generic.c       |   9 +-
 include/linux/device/faux.h         |  33 +++++
 include/linux/usb/usb_phy_generic.h |   9 +-
 13 files changed, 251 insertions(+), 46 deletions(-)
 create mode 100644 drivers/base/faux.c
 create mode 100644 include/linux/device/faux.h

-- 
2.48.1

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

[Greg Kroah-Hartman]

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>
---
 drivers/base/Makefile       |   2 +-
 drivers/base/base.h         |   1 +
 drivers/base/faux.c         | 189 ++++++++++++++++++++++++++++++++++++
 drivers/base/init.c         |   1 +
 include/linux/device/faux.h |  33 +++++++
 5 files changed, 225 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..0eba89a5cd57
--- /dev/null
+++ b/drivers/base/faux.c
@@ -0,0 +1,189 @@
+// 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.  Whenever you need a device that is not "real",
+ * use this interface instead of even thinking of using a platform device.
+ *
+ */
+#include <linux/device/faux.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include "base.h"
+
+/*
+ * 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;
+	struct device_driver driver;
+	const struct faux_driver_ops *faux_ops;
+	char name[];
+};
+#define to_faux_object(x) container_of_const(dev, struct faux_object, faux_dev.dev);
+
+static struct device faux_bus_root = {
+	.init_name	= "faux_bus",
+};
+
+static int faux_match(struct device *dev, const struct device_driver *drv)
+{
+	struct faux_object *faux_obj = to_faux_object(dev);
+
+	/* Match is simple, strcmp()! */
+	return (strcmp(faux_obj->name, drv->name) == 0);
+}
+
+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_bus",
+	.match		= faux_match,
+	.probe		= faux_probe,
+	.remove		= faux_remove,
+};
+
+static void faux_device_release(struct device *dev)
+{
+	struct faux_object *faux_obj = to_faux_object(dev);
+	struct device_driver *drv = &faux_obj->driver;
+
+	/*
+	 * Now that the device is going away, it has been unbound from the
+	 * driver we created for it, so it is safe to unregister the driver from
+	 * the system.
+	 */
+	driver_unregister(drv);
+
+	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
+ * @owner: module owner of the device/driver
+ *
+ * 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.
+ */
+struct faux_device *__faux_device_create(const char *name,
+					       struct faux_driver_ops *faux_ops,
+					       struct module *owner)
+{
+	struct device_driver *drv;
+	struct device *dev;
+	struct faux_object *faux_obj;
+	struct faux_device *faux_dev;
+	int ret;
+
+	faux_obj = kzalloc(sizeof(*faux_obj) + strlen(name) + 1, GFP_KERNEL);
+	if (!faux_obj)
+		return NULL;
+
+	/* Save off the name of the object into local memory */
+	strcpy(faux_obj->name, name);
+
+	/* Initialize the driver portion and register it with the driver core */
+	faux_obj->faux_ops = faux_ops;
+	drv = &faux_obj->driver;
+
+	drv->owner = owner;
+	drv->name = faux_obj->name;
+	drv->bus = &faux_bus_type;
+	drv->probe_type = PROBE_PREFER_ASYNCHRONOUS;
+
+	ret = driver_register(drv);
+	if (ret) {
+		pr_err("%s: driver_register for %s faux driver failed with %d\n",
+		       __func__, name, ret);
+		kfree(faux_obj);
+		return NULL;
+	}
+
+	/* 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.
+ */
+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 error;
+
+	error = device_register(&faux_bus_root);
+	if (error) {
+		put_device(&faux_bus_root);
+		return error;
+	}
+
+	error =  bus_register(&faux_bus_type);
+	if (error)
+		device_unregister(&faux_bus_root);
+
+	return error;
+}
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..b1a51fbb6f39
--- /dev/null
+++ b/include/linux/device/faux.h
@@ -0,0 +1,33 @@
+/* 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.  Whenever you need a device that is not "real",
+ * use this interface instead of even thinking of using a platform device.
+ *
+ */
+#ifndef _FAUX_DEVICE_H_
+#define _FAUX_DEVICE_H_
+
+#include <linux/module.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);
+};
+
+#define faux_device_create(name, faux_ops) __faux_device_create(name, faux_ops, THIS_MODULE)
+struct faux_device *__faux_device_create(const char *name,
+					       struct faux_driver_ops *faux_ops,
+					       struct module *module);
+void faux_device_destroy(struct faux_device *faux_dev);
+
+#endif /* _FAUX_DEVICE_H_ */
-- 
2.48.1

[PATCH 2/3] regulator: dummy: convert to use the faux bus

[Greg Kroah-Hartman]

The dummy regulator driver does not need to create a platform device, it
only did so because it was simple to do.  Change it over to use the
faux bus instead as this is NOT a real platform device, and it makes
the code even smaller than before.

Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/regulator/dummy.c | 37 +++++++++----------------------------
 1 file changed, 9 insertions(+), 28 deletions(-)

diff --git a/drivers/regulator/dummy.c b/drivers/regulator/dummy.c
index 5b9b9e4e762d..163b47e25291 100644
--- a/drivers/regulator/dummy.c
+++ b/drivers/regulator/dummy.c
@@ -13,7 +13,7 @@
 
 #include <linux/err.h>
 #include <linux/export.h>
-#include <linux/platform_device.h>
+#include <linux/device/faux.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/machine.h>
 
@@ -37,15 +37,15 @@ static const struct regulator_desc dummy_desc = {
 	.ops = &dummy_ops,
 };
 
-static int dummy_regulator_probe(struct platform_device *pdev)
+static int dummy_regulator_probe(struct faux_device *vdev)
 {
 	struct regulator_config config = { };
 	int ret;
 
-	config.dev = &pdev->dev;
+	config.dev = &vdev->dev;
 	config.init_data = &dummy_initdata;
 
-	dummy_regulator_rdev = devm_regulator_register(&pdev->dev, &dummy_desc,
+	dummy_regulator_rdev = devm_regulator_register(&vdev->dev, &dummy_desc,
 						       &config);
 	if (IS_ERR(dummy_regulator_rdev)) {
 		ret = PTR_ERR(dummy_regulator_rdev);
@@ -56,36 +56,17 @@ static int dummy_regulator_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static struct platform_driver dummy_regulator_driver = {
-	.probe		= dummy_regulator_probe,
-	.driver		= {
-		.name		= "reg-dummy",
-		.probe_type	= PROBE_PREFER_ASYNCHRONOUS,
-	},
+struct faux_driver_ops dummy_regulator_driver = {
+	.probe = dummy_regulator_probe,
 };
 
-static struct platform_device *dummy_pdev;
+static struct faux_device *dummy_fdev;
 
 void __init regulator_dummy_init(void)
 {
-	int ret;
-
-	dummy_pdev = platform_device_alloc("reg-dummy", -1);
-	if (!dummy_pdev) {
+	dummy_fdev = faux_device_create("reg-dummy", &dummy_regulator_driver);
+	if (!dummy_fdev) {
 		pr_err("Failed to allocate dummy regulator device\n");
 		return;
 	}
-
-	ret = platform_device_add(dummy_pdev);
-	if (ret != 0) {
-		pr_err("Failed to register dummy regulator device: %d\n", ret);
-		platform_device_put(dummy_pdev);
-		return;
-	}
-
-	ret = platform_driver_register(&dummy_regulator_driver);
-	if (ret != 0) {
-		pr_err("Failed to register dummy regulator driver: %d\n", ret);
-		platform_device_unregister(dummy_pdev);
-	}
 }
-- 
2.48.1

[PATCH 3/3] USB: phy: convert usb_phy_generic logic to use a faux device

[Greg Kroah-Hartman]

The usb_phy_generic code was creating a "fake" platform device to pass
around in different places.  Instead of doing that, use the faux bus
instead as that is what is really wanted here.

Site note, this fixes a bug in the mpfs driver where the incorrect
pointer was being passed to usb_phy_generic_unregister(), odd that no
one ever hit this in the past.

Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/usb/chipidea/ci_hdrc_pci.c  | 2 +-
 drivers/usb/dwc2/pci.c              | 4 ++--
 drivers/usb/musb/mediatek.c         | 4 ++--
 drivers/usb/musb/mpfs.c             | 4 ++--
 drivers/usb/musb/tusb6010.c         | 2 +-
 drivers/usb/phy/phy-generic.c       | 9 ++++-----
 include/linux/usb/usb_phy_generic.h | 9 +++++----
 7 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/chipidea/ci_hdrc_pci.c b/drivers/usb/chipidea/ci_hdrc_pci.c
index d63479e1ad10..63511ea30d6e 100644
--- a/drivers/usb/chipidea/ci_hdrc_pci.c
+++ b/drivers/usb/chipidea/ci_hdrc_pci.c
@@ -20,7 +20,7 @@
 
 struct ci_hdrc_pci {
 	struct platform_device	*ci;
-	struct platform_device	*phy;
+	struct faux_device	*phy;
 };
 
 /******************************************************************************
diff --git a/drivers/usb/dwc2/pci.c b/drivers/usb/dwc2/pci.c
index f3a1e4232a31..8cb50620aa55 100644
--- a/drivers/usb/dwc2/pci.c
+++ b/drivers/usb/dwc2/pci.c
@@ -30,7 +30,7 @@ static const char dwc2_driver_name[] = "dwc2-pci";
 
 struct dwc2_pci_glue {
 	struct platform_device *dwc2;
-	struct platform_device *phy;
+	struct faux_device *phy;
 };
 
 /**
@@ -53,7 +53,7 @@ static int dwc2_pci_probe(struct pci_dev *pci,
 {
 	struct resource		res[2];
 	struct platform_device	*dwc2;
-	struct platform_device	*phy;
+	struct faux_device	*phy;
 	int			ret;
 	struct device		*dev = &pci->dev;
 	struct dwc2_pci_glue	*glue;
diff --git a/drivers/usb/musb/mediatek.c b/drivers/usb/musb/mediatek.c
index aa988d74b58d..995bab88506a 100644
--- a/drivers/usb/musb/mediatek.c
+++ b/drivers/usb/musb/mediatek.c
@@ -43,7 +43,7 @@ struct mtk_glue {
 	struct device *dev;
 	struct musb *musb;
 	struct platform_device *musb_pdev;
-	struct platform_device *usb_phy;
+	struct faux_device *usb_phy;
 	struct phy *phy;
 	struct usb_phy *xceiv;
 	enum phy_mode phy_mode;
@@ -507,7 +507,7 @@ static int mtk_musb_probe(struct platform_device *pdev)
 static void mtk_musb_remove(struct platform_device *pdev)
 {
 	struct mtk_glue *glue = platform_get_drvdata(pdev);
-	struct platform_device *usb_phy = glue->usb_phy;
+	struct faux_device *usb_phy = glue->usb_phy;
 
 	platform_device_unregister(glue->musb_pdev);
 	usb_phy_generic_unregister(usb_phy);
diff --git a/drivers/usb/musb/mpfs.c b/drivers/usb/musb/mpfs.c
index 7edc8429b274..ef20794aee12 100644
--- a/drivers/usb/musb/mpfs.c
+++ b/drivers/usb/musb/mpfs.c
@@ -25,7 +25,7 @@
 struct mpfs_glue {
 	struct device *dev;
 	struct platform_device *musb;
-	struct platform_device *phy;
+	struct faux_device *phy;
 	struct clk *clk;
 };
 
@@ -356,7 +356,7 @@ static void mpfs_remove(struct platform_device *pdev)
 
 	clk_disable_unprepare(glue->clk);
 	platform_device_unregister(glue->musb);
-	usb_phy_generic_unregister(pdev);
+	usb_phy_generic_unregister(glue->phy);
 }
 
 #ifdef CONFIG_OF
diff --git a/drivers/usb/musb/tusb6010.c b/drivers/usb/musb/tusb6010.c
index 90b760a95e4e..92609f9d20ff 100644
--- a/drivers/usb/musb/tusb6010.c
+++ b/drivers/usb/musb/tusb6010.c
@@ -32,7 +32,7 @@
 struct tusb6010_glue {
 	struct device		*dev;
 	struct platform_device	*musb;
-	struct platform_device	*phy;
+	struct faux_device	*phy;
 	struct gpio_desc	*enable;
 	struct gpio_desc	*intpin;
 };
diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index 6c3ececf9137..a6cece75d0f8 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -30,16 +30,15 @@
 	(IRQF_SHARED | IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | \
 		IRQF_ONESHOT)
 
-struct platform_device *usb_phy_generic_register(void)
+struct faux_device *usb_phy_generic_register(void)
 {
-	return platform_device_register_simple("usb_phy_generic",
-			PLATFORM_DEVID_AUTO, NULL, 0);
+	return faux_device_create("usb_phy_generic", NULL);
 }
 EXPORT_SYMBOL_GPL(usb_phy_generic_register);
 
-void usb_phy_generic_unregister(struct platform_device *pdev)
+void usb_phy_generic_unregister(struct faux_device *fdev)
 {
-	platform_device_unregister(pdev);
+	faux_device_destroy(fdev);
 }
 EXPORT_SYMBOL_GPL(usb_phy_generic_unregister);
 
diff --git a/include/linux/usb/usb_phy_generic.h b/include/linux/usb/usb_phy_generic.h
index cd9e70a552a0..856db2bacc07 100644
--- a/include/linux/usb/usb_phy_generic.h
+++ b/include/linux/usb/usb_phy_generic.h
@@ -3,18 +3,19 @@
 #define __LINUX_USB_NOP_XCEIV_H
 
 #include <linux/usb/otg.h>
+#include <linux/device/faux.h>
 
 #if IS_ENABLED(CONFIG_NOP_USB_XCEIV)
 /* sometimes transceivers are accessed only through e.g. ULPI */
-extern struct platform_device *usb_phy_generic_register(void);
-extern void usb_phy_generic_unregister(struct platform_device *);
+struct faux_device *usb_phy_generic_register(void);
+void usb_phy_generic_unregister(struct faux_device *);
 #else
-static inline struct platform_device *usb_phy_generic_register(void)
+static inline struct faux_device *usb_phy_generic_register(void)
 {
 	return NULL;
 }
 
-static inline void usb_phy_generic_unregister(struct platform_device *pdev)
+static inline void usb_phy_generic_unregister(struct faux_device *fdev)
 {
 }
 #endif
-- 
2.48.1

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

[Andy Shevchenko]

On Mon, Feb 03, 2025 at 03:25:17PM +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.

...

> +#include <linux/device/faux.h>

I would rather think that this goes after generic inclusions...

> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>

...somewhere here.

But looking into organisation of device.h and device/*.h,
I would rather think of the linux/faux_device.h.

> +#include "base.h"

I don't remember by heart what it does include, I would go with IWYU principle
and list above all what we use.

container_of.h
device.h
export.h
printk.h
types.h

...

> +static int faux_match(struct device *dev, const struct device_driver *drv)
> +{
> +	struct faux_object *faux_obj = to_faux_object(dev);
> +
> +	/* Match is simple, strcmp()! */
> +	return (strcmp(faux_obj->name, drv->name) == 0);

Outer parentheses are not needed.

> +}

...

> +/**
> + * __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
> + * @owner: module owner of the device/driver
> + *
> + * 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.

The kernel-doc will complain on missing Return: section.

> + */
> +struct faux_device *__faux_device_create(const char *name,
> +					       struct faux_driver_ops *faux_ops,
> +					       struct module *owner)
> +{
> +	struct device_driver *drv;
> +	struct device *dev;
> +	struct faux_object *faux_obj;
> +	struct faux_device *faux_dev;
> +	int ret;

> +	faux_obj = kzalloc(sizeof(*faux_obj) + strlen(name) + 1, GFP_KERNEL);

Potential overflow. To avoid one may use struct_size() from overflow.h.

> +	if (!faux_obj)
> +		return NULL;
> +
> +	/* Save off the name of the object into local memory */
> +	strcpy(faux_obj->name, name);
> +
> +	/* Initialize the driver portion and register it with the driver core */
> +	faux_obj->faux_ops = faux_ops;
> +	drv = &faux_obj->driver;
> +
> +	drv->owner = owner;
> +	drv->name = faux_obj->name;
> +	drv->bus = &faux_bus_type;
> +	drv->probe_type = PROBE_PREFER_ASYNCHRONOUS;
> +
> +	ret = driver_register(drv);
> +	if (ret) {
> +		pr_err("%s: driver_register for %s faux driver failed with %d\n",
> +		       __func__, name, ret);
> +		kfree(faux_obj);
> +		return NULL;
> +	}
> +
> +	/* 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);

...

> +#ifndef _FAUX_DEVICE_H_
> +#define _FAUX_DEVICE_H_

> +#include <linux/module.h>

+ container_of.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);
> +};
> +
> +#define faux_device_create(name, faux_ops) __faux_device_create(name, faux_ops, THIS_MODULE)
> +struct faux_device *__faux_device_create(const char *name,
> +					       struct faux_driver_ops *faux_ops,
> +					       struct module *module);
> +void faux_device_destroy(struct faux_device *faux_dev);
> +
> +#endif /* _FAUX_DEVICE_H_ */

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Mon, Feb 03, 2025 at 05:11:03PM +0200, Andy Shevchenko wrote:
> On Mon, Feb 03, 2025 at 03:25:17PM +0100, Greg Kroah-Hartman wrote:

...

> I don't remember by heart what it does include, I would go with IWYU principle
> and list above all what we use.
> 
> container_of.h
> device.h
> export.h
> printk.h

> types.h

Probably types.h is too much and stddef.h would suffice (as it provides NULL
pointer definition).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/3] USB: phy: convert usb_phy_generic logic to use a faux device

[Andy Shevchenko]

On Mon, Feb 03, 2025 at 03:25:19PM +0100, Greg Kroah-Hartman wrote:
> The usb_phy_generic code was creating a "fake" platform device to pass
> around in different places.  Instead of doing that, use the faux bus
> instead as that is what is really wanted here.

> Site note, this fixes a bug in the mpfs driver where the incorrect
> pointer was being passed to usb_phy_generic_unregister(), odd that no
> one ever hit this in the past.

Seems nobody ever removed (unbind'ed) the module at run-time.

-- 
With Best Regards,
Andy Shevchenko

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

[Greg Kroah-Hartman]

On Mon, Feb 03, 2025 at 05:11:03PM +0200, Andy Shevchenko wrote:
> On Mon, Feb 03, 2025 at 03:25:17PM +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.
> 
> ...
> 
> > +#include <linux/device/faux.h>
> 
> I would rather think that this goes after generic inclusions...
> 
> > +#include <linux/err.h>
> > +#include <linux/init.h>
> > +#include <linux/slab.h>
> > +#include <linux/string.h>
> 
> ...somewhere here.
> 
> But looking into organisation of device.h and device/*.h,
> I would rather think of the linux/faux_device.h.

It can go anywhere, there is no need to sort things :)

> > +#include "base.h"
> 
> I don't remember by heart what it does include, I would go with IWYU principle
> and list above all what we use.
> 
> container_of.h
> device.h
> export.h
> printk.h
> types.h

That's not what the driver core ever did, so no need to worry about it,
thanks.


> 
> ...
> 
> > +static int faux_match(struct device *dev, const struct device_driver *drv)
> > +{
> > +	struct faux_object *faux_obj = to_faux_object(dev);
> > +
> > +	/* Match is simple, strcmp()! */
> > +	return (strcmp(faux_obj->name, drv->name) == 0);
> 
> Outer parentheses are not needed.

Makes me feel good as it is an assignment test, and that's what
platform.c has for the past few decades.


> > +/**
> > + * __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
> > + * @owner: module owner of the device/driver
> > + *
> > + * 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.
> 
> The kernel-doc will complain on missing Return: section.

Is that new?  Does that mean platform.c has lots of complaints in it as
well?  What does platform_find_device_by_driver() give you for a
documentation issue?

And as I didn't hook this up to the kernel documentation build yet, it
shouldn't produce any warnings anywhere :)

> > + */
> > +struct faux_device *__faux_device_create(const char *name,
> > +					       struct faux_driver_ops *faux_ops,
> > +					       struct module *owner)
> > +{
> > +	struct device_driver *drv;
> > +	struct device *dev;
> > +	struct faux_object *faux_obj;
> > +	struct faux_device *faux_dev;
> > +	int ret;
> 
> > +	faux_obj = kzalloc(sizeof(*faux_obj) + strlen(name) + 1, GFP_KERNEL);
> 
> Potential overflow. To avoid one may use struct_size() from overflow.h.

Users should not be providing the string here.  Again, this comes from
platform.c.

> > +#ifndef _FAUX_DEVICE_H_
> > +#define _FAUX_DEVICE_H_
> 
> > +#include <linux/module.h>
> 
> + container_of.h

Not needed to compile this file, only if someone uses the #define in it.

thanks,

greg k-h

Re: [PATCH 2/3] regulator: dummy: convert to use the faux bus

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 713 bytes --]

On Mon, Feb 03, 2025 at 03:25:18PM +0100, Greg Kroah-Hartman wrote:
> The dummy regulator driver does not need to create a platform device, it
> only did so because it was simple to do.  Change it over to use the
> faux bus instead as this is NOT a real platform device, and it makes
> the code even smaller than before.

No, they did this because you explicitly asked that this be done....

> -static int dummy_regulator_probe(struct platform_device *pdev)
> +static int dummy_regulator_probe(struct faux_device *vdev)

Just dev or fdev - we could just pass a struct device in here, we don't
actually care that it's a platform device at this point.  Having the
abbreviation mismatch with the device type is odd.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/3] regulator: dummy: convert to use the faux bus

[Greg Kroah-Hartman]

On Mon, Feb 03, 2025 at 03:39:06PM +0000, Mark Brown wrote:
> On Mon, Feb 03, 2025 at 03:25:18PM +0100, Greg Kroah-Hartman wrote:
> > The dummy regulator driver does not need to create a platform device, it
> > only did so because it was simple to do.  Change it over to use the
> > faux bus instead as this is NOT a real platform device, and it makes
> > the code even smaller than before.
> 
> No, they did this because you explicitly asked that this be done....

I did?  What was it attempting to be before this?  I don't remember that
at all, sorry.

> > -static int dummy_regulator_probe(struct platform_device *pdev)
> > +static int dummy_regulator_probe(struct faux_device *vdev)
> 
> Just dev or fdev - we could just pass a struct device in here, we don't
> actually care that it's a platform device at this point.  Having the
> abbreviation mismatch with the device type is odd.

Ah, that's a mistake from my first pass when this was a "struct
virtual_device" and I called this "vdev".  I'll go fix that up, thanks.

greg k-h

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

[Greg Kroah-Hartman]

On Mon, Feb 03, 2025 at 04:35:45PM +0100, Greg Kroah-Hartman wrote:
> > > + */
> > > +struct faux_device *__faux_device_create(const char *name,
> > > +					       struct faux_driver_ops *faux_ops,
> > > +					       struct module *owner)
> > > +{
> > > +	struct device_driver *drv;
> > > +	struct device *dev;
> > > +	struct faux_object *faux_obj;
> > > +	struct faux_device *faux_dev;
> > > +	int ret;
> > 
> > > +	faux_obj = kzalloc(sizeof(*faux_obj) + strlen(name) + 1, GFP_KERNEL);
> > 
> > Potential overflow. To avoid one may use struct_size() from overflow.h.
> 
> Users should not be providing the string here.  Again, this comes from
> platform.c.

Sima just proved me wrong, I'll go check for this now, thanks for
pointing it out.

greg k-h

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

[Andy Shevchenko]

On Mon, Feb 03, 2025 at 04:35:45PM +0100, Greg Kroah-Hartman wrote:
> On Mon, Feb 03, 2025 at 05:11:03PM +0200, Andy Shevchenko wrote:
> > On Mon, Feb 03, 2025 at 03:25:17PM +0100, Greg Kroah-Hartman wrote:

...

> > > +#include <linux/device/faux.h>
> > 
> > I would rather think that this goes after generic inclusions...
> > 
> > > +#include <linux/err.h>
> > > +#include <linux/init.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/string.h>
> > 
> > ...somewhere here.
> > 
> > But looking into organisation of device.h and device/*.h,
> > I would rather think of the linux/faux_device.h.
> 
> It can go anywhere, there is no need to sort things :)

It's not about sorting, it's about grouping from more generic to less generic.

> > > +#include "base.h"
> > 
> > I don't remember by heart what it does include, I would go with IWYU principle
> > and list above all what we use.
> > 
> > container_of.h
> > device.h
> > export.h
> > printk.h
> > types.h
> 
> That's not what the driver core ever did, so no need to worry about it,
> thanks.

But it doesn't mean that driver code does its best. No big worries, of course.

...

> > > +	return (strcmp(faux_obj->name, drv->name) == 0);
> > 
> > Outer parentheses are not needed.
> 
> Makes me feel good as it is an assignment test, and that's what
> platform.c has for the past few decades.

Sure, it also can be written as

	return !strcmp(faux_obj->name, drv->name);

that makes the same without explicit comparing to 0. But it's matter of taste.

...

> > > +/**
> > > + * __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
> > > + * @owner: module owner of the device/driver
> > > + *
> > > + * 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.
> > 
> > The kernel-doc will complain on missing Return: section.
> 
> Is that new?  Does that mean platform.c has lots of complaints in it as
> well?  What does platform_find_device_by_driver() give you for a
> documentation issue?
> 
> And as I didn't hook this up to the kernel documentation build yet, it
> shouldn't produce any warnings anywhere :)

I would rather say it's old.

Run

	kernel-doc -Wall -none -v ...your file...

and find the warning. During the kernel builds this is moved to extra warnings.

> > > + */

...

> > > +	faux_obj = kzalloc(sizeof(*faux_obj) + strlen(name) + 1, GFP_KERNEL);
> > 
> > Potential overflow. To avoid one may use struct_size() from overflow.h.
> 
> Users should not be providing the string here.  Again, this comes from
> platform.c.

I'm not sure I follow. The name parameter is not limited anyhow, so one may
provide non-terminated string and strlen() will return an arbitrary number.
Potentially this can lead to big numbers and become an overflow when gets
to a parameter for kmalloc(). This most likely never happen in real life,
but still the overflow is possible.

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Mon, Feb 03, 2025 at 04:46:56PM +0100, Greg Kroah-Hartman wrote:
> On Mon, Feb 03, 2025 at 04:35:45PM +0100, Greg Kroah-Hartman wrote:
> > > > + */
> > > > +struct faux_device *__faux_device_create(const char *name,
> > > > +					       struct faux_driver_ops *faux_ops,
> > > > +					       struct module *owner)
> > > > +{
> > > > +	struct device_driver *drv;
> > > > +	struct device *dev;
> > > > +	struct faux_object *faux_obj;
> > > > +	struct faux_device *faux_dev;
> > > > +	int ret;
> > > 
> > > > +	faux_obj = kzalloc(sizeof(*faux_obj) + strlen(name) + 1, GFP_KERNEL);
> > > 
> > > Potential overflow. To avoid one may use struct_size() from overflow.h.
> > 
> > Users should not be providing the string here.  Again, this comes from
> > platform.c.
> 
> Sima just proved me wrong, I'll go check for this now, thanks for
> pointing it out.

Ah, you are welcome!

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Mon, Feb 03, 2025 at 06:05:20PM +0200, Andy Shevchenko wrote:
> On Mon, Feb 03, 2025 at 04:35:45PM +0100, Greg Kroah-Hartman wrote:
> > On Mon, Feb 03, 2025 at 05:11:03PM +0200, Andy Shevchenko wrote:
> > > On Mon, Feb 03, 2025 at 03:25:17PM +0100, Greg Kroah-Hartman wrote:

...

> > > > +	faux_obj = kzalloc(sizeof(*faux_obj) + strlen(name) + 1, GFP_KERNEL);
> > > 
> > > Potential overflow. To avoid one may use struct_size() from overflow.h.
> > 
> > Users should not be providing the string here.  Again, this comes from
> > platform.c.
> 
> I'm not sure I follow. The name parameter is not limited anyhow, so one may
> provide non-terminated string and strlen() will return an arbitrary number.
> Potentially this can lead to big numbers and become an overflow when gets
> to a parameter for kmalloc(). This most likely never happen in real life,
> but still the overflow is possible.

After reading your other messages I got what you are talking about here.
Now it's all clear.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/3] regulator: dummy: convert to use the faux bus

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 859 bytes --]

On Mon, Feb 03, 2025 at 04:46:02PM +0100, Greg Kroah-Hartman wrote:
> On Mon, Feb 03, 2025 at 03:39:06PM +0000, Mark Brown wrote:
> > On Mon, Feb 03, 2025 at 03:25:18PM +0100, Greg Kroah-Hartman wrote:

> > > The dummy regulator driver does not need to create a platform device, it
> > > only did so because it was simple to do.  Change it over to use the
> > > faux bus instead as this is NOT a real platform device, and it makes
> > > the code even smaller than before.

> > No, they did this because you explicitly asked that this be done....

> I did?  What was it attempting to be before this?  I don't remember that
> at all, sorry.

Yeah, there were some things where people were creating custom buses for
internal uses like this which you pushed back on due to code duplication
- you said to just use platform bus since the bus code looked identical.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Greg Kroah-Hartman]

On Mon, Feb 03, 2025 at 06:05:19PM +0200, Andy Shevchenko 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
> > > > + * @owner: module owner of the device/driver
> > > > + *
> > > > + * 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.
> > > 
> > > The kernel-doc will complain on missing Return: section.
> > 
> > Is that new?  Does that mean platform.c has lots of complaints in it as
> > well?  What does platform_find_device_by_driver() give you for a
> > documentation issue?
> > 
> > And as I didn't hook this up to the kernel documentation build yet, it
> > shouldn't produce any warnings anywhere :)
> 
> I would rather say it's old.
> 
> Run
> 
> 	kernel-doc -Wall -none -v ...your file...
> 
> and find the warning. During the kernel builds this is moved to extra warnings.

Ah, nice, didn't know about that.  Now fixed up.

> > > > + */
> 
> ...
> 
> > > > +	faux_obj = kzalloc(sizeof(*faux_obj) + strlen(name) + 1, GFP_KERNEL);
> > > 
> > > Potential overflow. To avoid one may use struct_size() from overflow.h.
> > 
> > Users should not be providing the string here.  Again, this comes from
> > platform.c.
> 
> I'm not sure I follow. The name parameter is not limited anyhow, so one may
> provide non-terminated string and strlen() will return an arbitrary number.
> Potentially this can lead to big numbers and become an overflow when gets
> to a parameter for kmalloc(). This most likely never happen in real life,
> but still the overflow is possible.

I've now bounded at 256, because really, who needs a bigger name for a
device than that :)

thanks,

greg k-h

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

[Andy Shevchenko]

On Mon, Feb 03, 2025 at 05:13:28PM +0100, Greg Kroah-Hartman wrote:
> On Mon, Feb 03, 2025 at 06:05:19PM +0200, Andy Shevchenko wrote:

...

> > > > > +	faux_obj = kzalloc(sizeof(*faux_obj) + strlen(name) + 1, GFP_KERNEL);
> > > > 
> > > > Potential overflow. To avoid one may use struct_size() from overflow.h.
> > > 
> > > Users should not be providing the string here.  Again, this comes from
> > > platform.c.
> > 
> > I'm not sure I follow. The name parameter is not limited anyhow, so one may
> > provide non-terminated string and strlen() will return an arbitrary number.
> > Potentially this can lead to big numbers and become an overflow when gets
> > to a parameter for kmalloc(). This most likely never happen in real life,
> > but still the overflow is possible.
> 
> I've now bounded at 256, because really, who needs a bigger name for a
> device than that :)

Works for me! With printable ASCII characters it can be estimated as up to
64^256 combinations, which "ought to be enough for everybody" (of course
it will be much less if we count only human-readable strings). :-)

-- 
With Best Regards,
Andy Shevchenko

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

[Jonathan Cameron]

On Mon,  3 Feb 2025 15:25:17 +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>

Nice. One trivial thing inline.


> diff --git a/drivers/base/faux.c b/drivers/base/faux.c
> new file mode 100644
> index 000000000000..0eba89a5cd57
> --- /dev/null
> +++ b/drivers/base/faux.c
> @@ -0,0 +1,189 @@


> +int __init faux_bus_init(void)
> +{
> +	int error;
> +
> +	error = device_register(&faux_bus_root);
> +	if (error) {
> +		put_device(&faux_bus_root);
> +		return error;
> +	}
> +
> +	error =  bus_register(&faux_bus_type);

Odd bonus space after = 

> +	if (error)
> +		device_unregister(&faux_bus_root);
> +
> +	return error;
> +}

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

[Thomas Weißschuh]

On 2025-02-03 15:25:17+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>
> ---
>  drivers/base/Makefile       |   2 +-
>  drivers/base/base.h         |   1 +
>  drivers/base/faux.c         | 189 ++++++++++++++++++++++++++++++++++++
>  drivers/base/init.c         |   1 +
>  include/linux/device/faux.h |  33 +++++++
>  5 files changed, 225 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/base/faux.c
>  create mode 100644 include/linux/device/faux.h

<snip>

> diff --git a/drivers/base/faux.c b/drivers/base/faux.c
> new file mode 100644
> index 000000000000..0eba89a5cd57
> --- /dev/null
> +++ b/drivers/base/faux.c
> @@ -0,0 +1,189 @@
> +// 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.  Whenever you need a device that is not "real",
> + * use this interface instead of even thinking of using a platform device.
> + *
> + */
> +#include <linux/device/faux.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +#include "base.h"
> +
> +/*
> + * 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;
> +	struct device_driver driver;
> +	const struct faux_driver_ops *faux_ops;
> +	char name[];
> +};
> +#define to_faux_object(x) container_of_const(dev, struct faux_object, faux_dev.dev);
> +
> +static struct device faux_bus_root = {
> +	.init_name	= "faux_bus",
> +};
> +
> +static int faux_match(struct device *dev, const struct device_driver *drv)
> +{
> +	struct faux_object *faux_obj = to_faux_object(dev);
> +
> +	/* Match is simple, strcmp()! */
> +	return (strcmp(faux_obj->name, drv->name) == 0);
> +}
> +
> +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_bus",

Is the _bus suffix intentional?
Other busses don't have it.

> +	.match		= faux_match,
> +	.probe		= faux_probe,
> +	.remove		= faux_remove,
> +};
> +
> +static void faux_device_release(struct device *dev)
> +{
> +	struct faux_object *faux_obj = to_faux_object(dev);
> +	struct device_driver *drv = &faux_obj->driver;
> +
> +	/*
> +	 * Now that the device is going away, it has been unbound from the
> +	 * driver we created for it, so it is safe to unregister the driver from
> +	 * the system.
> +	 */
> +	driver_unregister(drv);
> +
> +	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
> + * @owner: module owner of the device/driver
> + *
> + * 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.
> + */
> +struct faux_device *__faux_device_create(const char *name,
> +					       struct faux_driver_ops *faux_ops,

const

> +					       struct module *owner)

What about attributes?

> +{
> +	struct device_driver *drv;
> +	struct device *dev;
> +	struct faux_object *faux_obj;
> +	struct faux_device *faux_dev;
> +	int ret;
> +
> +	faux_obj = kzalloc(sizeof(*faux_obj) + strlen(name) + 1, GFP_KERNEL);
> +	if (!faux_obj)
> +		return NULL;
> +
> +	/* Save off the name of the object into local memory */
> +	strcpy(faux_obj->name, name);
> +
> +	/* Initialize the driver portion and register it with the driver core */
> +	faux_obj->faux_ops = faux_ops;
> +	drv = &faux_obj->driver;
> +
> +	drv->owner = owner;
> +	drv->name = faux_obj->name;

Assuming most names are constant, this would be better with kstrdup_const().
Which is also used by dev_set_name() under the hood.

> +	drv->bus = &faux_bus_type;
> +	drv->probe_type = PROBE_PREFER_ASYNCHRONOUS;
> +
> +	ret = driver_register(drv);
> +	if (ret) {
> +		pr_err("%s: driver_register for %s faux driver failed with %d\n",
> +		       __func__, name, ret);
> +		kfree(faux_obj);
> +		return NULL;
> +	}
> +
> +	/* 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);

<snip>

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

[Greg Kroah-Hartman]

On Tue, Feb 04, 2025 at 09:25:32AM +0000, Jonathan Cameron wrote:
> On Mon,  3 Feb 2025 15:25:17 +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>
> 
> Nice. One trivial thing inline.
> 
> 
> > diff --git a/drivers/base/faux.c b/drivers/base/faux.c
> > new file mode 100644
> > index 000000000000..0eba89a5cd57
> > --- /dev/null
> > +++ b/drivers/base/faux.c
> > @@ -0,0 +1,189 @@
> 
> 
> > +int __init faux_bus_init(void)
> > +{
> > +	int error;
> > +
> > +	error = device_register(&faux_bus_root);
> > +	if (error) {
> > +		put_device(&faux_bus_root);
> > +		return error;
> > +	}
> > +
> > +	error =  bus_register(&faux_bus_type);
> 
> Odd bonus space after = 

Thanks!  I caught that when I just rewrote this function, and I finally
got around to running checkpatch which found another error in it that I
missed.  It's sometimes easier to notice changes in other's code than
your own :)

greg k-h

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

[Greg Kroah-Hartman]

On Tue, Feb 04, 2025 at 11:08:11AM +0100, Thomas Weißschuh wrote:
> On 2025-02-03 15:25:17+0100, Greg Kroah-Hartman wrote:
> > +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_bus",
> 
> Is the _bus suffix intentional?

It was intentional.

> Other busses don't have it.

True.  Naming is hard.  I guess /sys/bus/faux/ makes sense, I will go
rename it.

But for the "root" device, does /sys/devices/faux_bus/ make sense, or
should it be /sys/devices/faux/ as well?  I'm now leaning toward the
latter...

> > +	.match		= faux_match,
> > +	.probe		= faux_probe,
> > +	.remove		= faux_remove,
> > +};
> > +
> > +static void faux_device_release(struct device *dev)
> > +{
> > +	struct faux_object *faux_obj = to_faux_object(dev);
> > +	struct device_driver *drv = &faux_obj->driver;
> > +
> > +	/*
> > +	 * Now that the device is going away, it has been unbound from the
> > +	 * driver we created for it, so it is safe to unregister the driver from
> > +	 * the system.
> > +	 */
> > +	driver_unregister(drv);
> > +
> > +	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
> > + * @owner: module owner of the device/driver
> > + *
> > + * 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.
> > + */
> > +struct faux_device *__faux_device_create(const char *name,
> > +					       struct faux_driver_ops *faux_ops,
> 
> const
> 
> > +					       struct module *owner)
> 
> What about attributes?

What in-kernel user of this wants an attribute for such a device?

And again, if we find one, we can make a faux_device_create_groups()
call that takes a pointer to an attribute group structure if it's really
needed.


> 
> > +{
> > +	struct device_driver *drv;
> > +	struct device *dev;
> > +	struct faux_object *faux_obj;
> > +	struct faux_device *faux_dev;
> > +	int ret;
> > +
> > +	faux_obj = kzalloc(sizeof(*faux_obj) + strlen(name) + 1, GFP_KERNEL);
> > +	if (!faux_obj)
> > +		return NULL;
> > +
> > +	/* Save off the name of the object into local memory */
> > +	strcpy(faux_obj->name, name);
> > +
> > +	/* Initialize the driver portion and register it with the driver core */
> > +	faux_obj->faux_ops = faux_ops;
> > +	drv = &faux_obj->driver;
> > +
> > +	drv->owner = owner;
> > +	drv->name = faux_obj->name;
> 
> Assuming most names are constant, this would be better with kstrdup_const().
> Which is also used by dev_set_name() under the hood.

I've now removed the additional driver, but note that this is just a
pointer assignment, which is fine to do here as the lifespan of
faux_obj->name outlived the driver structure's lifespan.

thanks for the review, much appreciated!

greg k-h

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

[Thomas Weißschuh]

On 2025-02-04 11:20:43+0100, Greg Kroah-Hartman wrote:
> On Tue, Feb 04, 2025 at 11:08:11AM +0100, Thomas Weißschuh wrote:
> > On 2025-02-03 15:25:17+0100, Greg Kroah-Hartman wrote:
> > > +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_bus",
> > 
> > Is the _bus suffix intentional?
> 
> It was intentional.
> 
> > Other busses don't have it.
> 
> True.  Naming is hard.  I guess /sys/bus/faux/ makes sense, I will go
> rename it.
> 
> But for the "root" device, does /sys/devices/faux_bus/ make sense, or
> should it be /sys/devices/faux/ as well?  I'm now leaning toward the
> latter...

I'm leaning slightly towards the former.
But my naming skills are beyond limited.

> > > +	.match		= faux_match,
> > > +	.probe		= faux_probe,
> > > +	.remove		= faux_remove,
> > > +};
> > > +
> > > +static void faux_device_release(struct device *dev)
> > > +{
> > > +	struct faux_object *faux_obj = to_faux_object(dev);
> > > +	struct device_driver *drv = &faux_obj->driver;
> > > +
> > > +	/*
> > > +	 * Now that the device is going away, it has been unbound from the
> > > +	 * driver we created for it, so it is safe to unregister the driver from
> > > +	 * the system.
> > > +	 */
> > > +	driver_unregister(drv);
> > > +
> > > +	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
> > > + * @owner: module owner of the device/driver
> > > + *
> > > + * 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.
> > > + */
> > > +struct faux_device *__faux_device_create(const char *name,
> > > +					       struct faux_driver_ops *faux_ops,
> > 
> > const
> > 
> > > +					       struct module *owner)
> > 
> > What about attributes?
> 
> What in-kernel user of this wants an attribute for such a device?

It was mostly a guess.
However drivers/video/fbdev/gbefb.c seems to be an example.

> And again, if we find one, we can make a faux_device_create_groups()
> call that takes a pointer to an attribute group structure if it's really
> needed.

Fair enough.

> > > +{
> > > +	struct device_driver *drv;
> > > +	struct device *dev;
> > > +	struct faux_object *faux_obj;
> > > +	struct faux_device *faux_dev;
> > > +	int ret;
> > > +
> > > +	faux_obj = kzalloc(sizeof(*faux_obj) + strlen(name) + 1, GFP_KERNEL);
> > > +	if (!faux_obj)
> > > +		return NULL;
> > > +
> > > +	/* Save off the name of the object into local memory */
> > > +	strcpy(faux_obj->name, name);
> > > +
> > > +	/* Initialize the driver portion and register it with the driver core */
> > > +	faux_obj->faux_ops = faux_ops;
> > > +	drv = &faux_obj->driver;
> > > +
> > > +	drv->owner = owner;
> > > +	drv->name = faux_obj->name;
> > 
> > Assuming most names are constant, this would be better with kstrdup_const().
> > Which is also used by dev_set_name() under the hood.
> 
> I've now removed the additional driver, but note that this is just a
> pointer assignment, which is fine to do here as the lifespan of
> faux_obj->name outlived the driver structure's lifespan.

It outlives it because there is extra space allocated for it in faux_obj.
With kstrdup_const() that space would not be needed.
In the end it shouldn't really matter one way or another.


Thomas

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

[Greg Kroah-Hartman]

On Tue, Feb 04, 2025 at 11:44:41AM +0100, Thomas Weißschuh wrote:
> On 2025-02-04 11:20:43+0100, Greg Kroah-Hartman wrote:
> > On Tue, Feb 04, 2025 at 11:08:11AM +0100, Thomas Weißschuh wrote:
> > > On 2025-02-03 15:25:17+0100, Greg Kroah-Hartman wrote:
> > > > +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_bus",
> > > 
> > > Is the _bus suffix intentional?
> > 
> > It was intentional.
> > 
> > > Other busses don't have it.
> > 
> > True.  Naming is hard.  I guess /sys/bus/faux/ makes sense, I will go
> > rename it.
> > 
> > But for the "root" device, does /sys/devices/faux_bus/ make sense, or
> > should it be /sys/devices/faux/ as well?  I'm now leaning toward the
> > latter...
> 
> I'm leaning slightly towards the former.
> But my naming skills are beyond limited.
> 
> > > > +	.match		= faux_match,
> > > > +	.probe		= faux_probe,
> > > > +	.remove		= faux_remove,
> > > > +};
> > > > +
> > > > +static void faux_device_release(struct device *dev)
> > > > +{
> > > > +	struct faux_object *faux_obj = to_faux_object(dev);
> > > > +	struct device_driver *drv = &faux_obj->driver;
> > > > +
> > > > +	/*
> > > > +	 * Now that the device is going away, it has been unbound from the
> > > > +	 * driver we created for it, so it is safe to unregister the driver from
> > > > +	 * the system.
> > > > +	 */
> > > > +	driver_unregister(drv);
> > > > +
> > > > +	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
> > > > + * @owner: module owner of the device/driver
> > > > + *
> > > > + * 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.
> > > > + */
> > > > +struct faux_device *__faux_device_create(const char *name,
> > > > +					       struct faux_driver_ops *faux_ops,
> > > 
> > > const
> > > 
> > > > +					       struct module *owner)
> > > 
> > > What about attributes?
> > 
> > What in-kernel user of this wants an attribute for such a device?
> 
> It was mostly a guess.
> However drivers/video/fbdev/gbefb.c seems to be an example.

As that driver is allocateing io memory and the like, it really looks
like a "real" platform driver/device to me.  Do you think it should not
be one?

Also, that driver should be converted to use an attribute group instead
of manually adding those sysfs files, if you wanted to touch it in the
future :)

thanks,

greg k-h