[PATCH 1/3] w1: use match() callback instead of hardwiring dev->driver

[PATCH 1/3] w1: use match() callback instead of hardwiring dev->driver

[Danilo Krummrich]

w1 hardwires dev->driver before calling device_register(), relying on
the if (dev->driver) special case in __device_attach() to call
device_bind_driver() and skip normal matching. This is a legacy pattern
that should be replaced with proper match() logic.

Add device types for master and slave devices and implement a w1_match()
callback that routes each device type to its corresponding driver. This
lets the driver core bind w1 devices through the normal
__device_attach_driver() path.

Link: https://lore.kernel.org/driver-core/DKU2J83B6IX6.2CG1WOB66WC39@kernel.org/
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
Compile-tested only, no hardware available.
---
 drivers/w1/w1.c | 37 +++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 486f321eadc8..faa1c03f4807 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -58,11 +58,6 @@ MODULE_PARM_DESC(slave_ttl,
 DEFINE_MUTEX(w1_mlock);
 LIST_HEAD(w1_masters);
 
-static int w1_master_probe(struct device *dev)
-{
-	return -ENODEV;
-}
-
 static void w1_master_release(struct device *dev)
 {
 	struct w1_master *md = dev_to_w1_master(dev);
@@ -165,24 +160,42 @@ static struct w1_family w1_default_family = {
 	.fops = &w1_default_fops,
 };
 
+static const struct device_type w1_master_device_type = {
+	.name = "w1_master",
+};
+
+static const struct device_type w1_slave_device_type = {
+	.name = "w1_slave",
+};
+
 static int w1_uevent(const struct device *dev, struct kobj_uevent_env *env);
+static struct device_driver w1_slave_driver;
+
+static int w1_match(struct device *dev, const struct device_driver *drv)
+{
+	if (dev->type == &w1_master_device_type)
+		return drv == &w1_master_driver;
+	if (dev->type == &w1_slave_device_type)
+		return drv == &w1_slave_driver;
+	return 0;
+}
 
 static const struct bus_type w1_bus_type = {
 	.name = "w1",
+	.match = w1_match,
 	.uevent = w1_uevent,
 };
 
 struct device_driver w1_master_driver = {
 	.name = "w1_master_driver",
 	.bus = &w1_bus_type,
-	.probe = w1_master_probe,
 };
 
 struct device w1_master_device = {
 	.parent = NULL,
 	.bus = &w1_bus_type,
+	.type = &w1_master_device_type,
 	.init_name = "w1 bus master",
-	.driver = &w1_master_driver,
 	.release = &w1_master_release
 };
 
@@ -195,8 +208,8 @@ static struct device_driver w1_slave_driver = {
 struct device w1_slave_device = {
 	.parent = NULL,
 	.bus = &w1_bus_type,
+	.type = &w1_slave_device_type,
 	.init_name = "w1 bus slave",
-	.driver = &w1_slave_driver,
 	.release = &w1_slave_release
 };
 #endif  /*  0  */
@@ -574,11 +587,11 @@ static int w1_uevent(const struct device *dev, struct kobj_uevent_env *env)
 	const char *event_owner, *name;
 	int err = 0;
 
-	if (dev->driver == &w1_master_driver) {
+	if (dev->type == &w1_master_device_type) {
 		md = container_of(dev, struct w1_master, dev);
 		event_owner = "master";
 		name = md->name;
-	} else if (dev->driver == &w1_slave_driver) {
+	} else if (dev->type == &w1_slave_device_type) {
 		sl = container_of(dev, struct w1_slave, dev);
 		event_owner = "slave";
 		name = sl->name;
@@ -590,7 +603,7 @@ static int w1_uevent(const struct device *dev, struct kobj_uevent_env *env)
 	dev_dbg(dev, "Hotplug event for %s %s, bus_id=%s.\n",
 			event_owner, name, dev_name(dev));
 
-	if (dev->driver != &w1_slave_driver || !sl)
+	if (dev->type != &w1_slave_device_type || !sl)
 		goto end;
 
 	err = add_uevent_var(env, "W1_FID=%02X", sl->reg_num.family);
@@ -666,8 +679,8 @@ static int __w1_attach_slave_device(struct w1_slave *sl)
 	int err;
 
 	sl->dev.parent = &sl->master->dev;
-	sl->dev.driver = &w1_slave_driver;
 	sl->dev.bus = &w1_bus_type;
+	sl->dev.type = &w1_slave_device_type;
 	sl->dev.release = &w1_slave_release;
 	sl->dev.groups = w1_slave_groups;
 	sl->dev.of_node = of_find_matching_node(sl->master->dev.of_node,

base-commit: 073e62fd33fe9cec754cb89e60c0ebbab781a50a
-- 
2.55.0

[PATCH 2/3] w1: move slave uevent into w1_slave_device_type

[Danilo Krummrich]

The bus-level w1_uevent() only ever added uevent variables (W1_FID,
W1_SLAVE_ID) for slave devices.

Move the slave uevent logic into w1_slave_device_type.uevent where it
naturally belongs and remove the bus-level uevent callback entirely.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
Compile-tested only, no hardware available.

I think the dev_dbg() stuff wasn't really  useful, so I dropped it.
---
 drivers/w1/w1.c | 37 +++++++++----------------------------
 1 file changed, 9 insertions(+), 28 deletions(-)

diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index faa1c03f4807..b40f2d9fd29d 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -164,11 +164,14 @@ static const struct device_type w1_master_device_type = {
 	.name = "w1_master",
 };
 
+static int w1_slave_uevent(const struct device *dev,
+			   struct kobj_uevent_env *env);
+
 static const struct device_type w1_slave_device_type = {
 	.name = "w1_slave",
+	.uevent = w1_slave_uevent,
 };
 
-static int w1_uevent(const struct device *dev, struct kobj_uevent_env *env);
 static struct device_driver w1_slave_driver;
 
 static int w1_match(struct device *dev, const struct device_driver *drv)
@@ -183,7 +186,6 @@ static int w1_match(struct device *dev, const struct device_driver *drv)
 static const struct bus_type w1_bus_type = {
 	.name = "w1",
 	.match = w1_match,
-	.uevent = w1_uevent,
 };
 
 struct device_driver w1_master_driver = {
@@ -580,39 +582,18 @@ void w1_destroy_master_attributes(struct w1_master *master)
 	sysfs_remove_group(&master->dev.kobj, &w1_master_defattr_group);
 }
 
-static int w1_uevent(const struct device *dev, struct kobj_uevent_env *env)
+static int w1_slave_uevent(const struct device *dev,
+			   struct kobj_uevent_env *env)
 {
-	const struct w1_master *md = NULL;
-	const struct w1_slave *sl = NULL;
-	const char *event_owner, *name;
-	int err = 0;
-
-	if (dev->type == &w1_master_device_type) {
-		md = container_of(dev, struct w1_master, dev);
-		event_owner = "master";
-		name = md->name;
-	} else if (dev->type == &w1_slave_device_type) {
-		sl = container_of(dev, struct w1_slave, dev);
-		event_owner = "slave";
-		name = sl->name;
-	} else {
-		dev_dbg(dev, "Unknown event.\n");
-		return -EINVAL;
-	}
-
-	dev_dbg(dev, "Hotplug event for %s %s, bus_id=%s.\n",
-			event_owner, name, dev_name(dev));
-
-	if (dev->type != &w1_slave_device_type || !sl)
-		goto end;
+	const struct w1_slave *sl = container_of(dev, struct w1_slave, dev);
+	int err;
 
 	err = add_uevent_var(env, "W1_FID=%02X", sl->reg_num.family);
 	if (err)
-		goto end;
+		return err;
 
 	err = add_uevent_var(env, "W1_SLAVE_ID=%024LX",
 			     (unsigned long long)sl->reg_num.id);
-end:
 	return err;
 }
 
-- 
2.55.0

[PATCH 3/3] w1: stop using struct device template for master devices

[Danilo Krummrich]

w1_alloc_dev() uses memcpy() with a static struct device template
(w1_master_device) to initialize new master devices.

Replace the memcpy() with a w1_master_dev_init() helper that sets only
bus, type, and release.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
Compile-tested only, no hardware available.
---
 drivers/w1/w1.c          | 15 +++++++--------
 drivers/w1/w1_int.c      |  7 +++----
 drivers/w1/w1_internal.h |  2 +-
 3 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index b40f2d9fd29d..d71e2a1568bd 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -193,19 +193,18 @@ struct device_driver w1_master_driver = {
 	.bus = &w1_bus_type,
 };
 
-struct device w1_master_device = {
-	.parent = NULL,
-	.bus = &w1_bus_type,
-	.type = &w1_master_device_type,
-	.init_name = "w1 bus master",
-	.release = &w1_master_release
-};
-
 static struct device_driver w1_slave_driver = {
 	.name = "w1_slave_driver",
 	.bus = &w1_bus_type,
 };
 
+void w1_master_dev_init(struct device *dev)
+{
+	dev->bus = &w1_bus_type;
+	dev->type = &w1_master_device_type;
+	dev->release = &w1_master_release;
+}
+
 #if 0
 struct device w1_slave_device = {
 	.parent = NULL,
diff --git a/drivers/w1/w1_int.c b/drivers/w1/w1_int.c
index 19a0ea28e9f3..a59ca19471e0 100644
--- a/drivers/w1/w1_int.c
+++ b/drivers/w1/w1_int.c
@@ -22,8 +22,7 @@ static int w1_enable_pullup = 1;
 module_param_named(enable_pullup, w1_enable_pullup, int, 0);
 
 static struct w1_master *w1_alloc_dev(u32 id, int slave_count, int slave_ttl,
-				       struct device_driver *driver,
-				       struct device *device)
+				       struct device_driver *driver)
 {
 	struct w1_master *dev;
 	int err;
@@ -57,7 +56,7 @@ static struct w1_master *w1_alloc_dev(u32 id, int slave_count, int slave_ttl,
 	mutex_init(&dev->bus_mutex);
 	mutex_init(&dev->list_mutex);
 
-	memcpy(&dev->dev, device, sizeof(struct device));
+	w1_master_dev_init(&dev->dev);
 	dev_set_name(&dev->dev, "w1_bus_master%u", dev->id);
 	snprintf(dev->name, sizeof(dev->name), "w1_bus_master%u", dev->id);
 	dev->dev.init_name = dev->name;
@@ -116,7 +115,7 @@ int w1_add_master_device(struct w1_bus_master *master)
 	} while (found);
 
 	dev = w1_alloc_dev(id, w1_max_slave_count, w1_max_slave_ttl,
-		&w1_master_driver, &w1_master_device);
+		&w1_master_driver);
 	if (!dev) {
 		mutex_unlock(&w1_mlock);
 		return -ENOMEM;
diff --git a/drivers/w1/w1_internal.h b/drivers/w1/w1_internal.h
index cb5a74e9939a..ffe7c7e8c4ca 100644
--- a/drivers/w1/w1_internal.h
+++ b/drivers/w1/w1_internal.h
@@ -65,7 +65,7 @@ void __w1_family_get(struct w1_family *f);
 struct w1_family *w1_family_registered(u8 fid);
 
 extern struct device_driver w1_master_driver;
-extern struct device w1_master_device;
+void w1_master_dev_init(struct device *dev);
 extern int w1_max_slave_count;
 extern int w1_max_slave_ttl;
 extern struct list_head w1_masters;
-- 
2.55.0