Skip to contentSkip to conversation

serdev: bus-code clean ups

PATCH Superseded Johan Hovold 2 participants 7 messages Jan 8, 2018 – Jan 9, 2018

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover serdev: bus-code clean ups

As noted by Hans, we currently fail to generate uevents for ACPI serdev controller as they do not have any ACPI companions from which ACPI modaliases are constructed.

In fact, we should not have been generating modaliases for controllers in the first place as controllers are not bound to drivers.

This series applies on top of Hans's minimal fix which suppresses the uevent errors for ACPI controllers (even though it could replace it entirely if preferred).

Johan

Johan Hovold (2):
serdev: do not generate modaliases for controllers
serdev: only match serdev devices

 drivers/tty/serdev/core.c | 80 +++++++++++++++++++++++++----------------------
 1 file changed, 42 insertions(+), 38 deletions(-)
Signature · 1 line
2.15.1
#1 lore raw

PATCH 1/2 serdev: do not generate modaliases for controllers

Serdev controllers are not bound to any drivers and it therefore makes no sense to generate modaliases for them.

This has already been fixed separately for ACPI controllers for which uevent errors were also being logged during probe due to the missing ACPI companions (from which ACPI modaliases are generated).

This patch moves the modalias handling from the bus type to the client device type. Specifically, this means that only serdev devices (a.k.a. clients or slaves) will have have MODALIAS fields in their uevent environments and corresponding modalias sysfs attributes.

---
 drivers/tty/serdev/core.c | 72 ++++++++++++++++++++++-------------------------
 1 file changed, 34 insertions(+), 38 deletions(-)
drivers/tty/serdev/core.c +34 -38
index 5dc88f61f506..61c85e49e178 100644
@@ -19,6 +19,38 @@
 static bool is_registered;
 static DEFINE_IDA(ctrl_ida);
 
+static ssize_t modalias_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	int len;
+
+	len = acpi_device_modalias(dev, buf, PAGE_SIZE - 1);
+	if (len != -ENODEV)
+		return len;
+
+	return of_device_modalias(dev, buf, PAGE_SIZE);
+}
+DEVICE_ATTR_RO(modalias);
+
+static struct attribute *serdev_device_attrs[] = {
+	&dev_attr_modalias.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(serdev_device);
+
+static int serdev_device_uevent(struct device *dev, struct kobj_uevent_env *env)
+{
+	int rc;
+
+	/* TODO: platform modalias */
+
+	rc = acpi_device_uevent_modalias(dev, env);
+	if (rc != -ENODEV)
+		return rc;
+
+	return of_device_uevent_modalias(dev, env);
+}
+
 static void serdev_device_release(struct device *dev)
 {
 	struct serdev_device *serdev = to_serdev_device(dev);
@@ -26,6 +58,8 @@ static void serdev_device_release(struct device *dev)
 }
 
 static const struct device_type serdev_device_type = {
+	.groups		= serdev_device_groups,
+	.uevent		= serdev_device_uevent,
 	.release	= serdev_device_release,
 };
 
@@ -49,23 +83,6 @@ static int serdev_device_match(struct device *dev, struct device_driver *drv)
 	return of_driver_match_device(dev, drv);
 }
 
-static int serdev_uevent(struct device *dev, struct kobj_uevent_env *env)
-{
-	int rc;
-
-	/* TODO: platform modalias */
-
-	/* ACPI enumerated controllers do not have a modalias */
-	if (!dev->of_node && dev->type == &serdev_ctrl_type)
-		return 0;
-
-	rc = acpi_device_uevent_modalias(dev, env);
-	if (rc != -ENODEV)
-		return rc;
-
-	return of_device_uevent_modalias(dev, env);
-}
-
 /**
  * serdev_device_add() - add a device previously constructed via serdev_device_alloc()
  * @serdev:	serdev_device to be added
@@ -305,32 +322,11 @@ static int serdev_drv_remove(struct device *dev)
 	return 0;
 }
 
-static ssize_t modalias_show(struct device *dev,
-			     struct device_attribute *attr, char *buf)
-{
-	int len;
-
-	len = acpi_device_modalias(dev, buf, PAGE_SIZE - 1);
-	if (len != -ENODEV)
-		return len;
-
-	return of_device_modalias(dev, buf, PAGE_SIZE);
-}
-DEVICE_ATTR_RO(modalias);
-
-static struct attribute *serdev_device_attrs[] = {
-	&dev_attr_modalias.attr,
-	NULL,
-};
-ATTRIBUTE_GROUPS(serdev_device);
-
 static struct bus_type serdev_bus_type = {
 	.name		= "serial",
 	.match		= serdev_device_match,
 	.probe		= serdev_drv_probe,
 	.remove		= serdev_drv_remove,
-	.uevent		= serdev_uevent,
-	.dev_groups	= serdev_device_groups,
 };
 
 /**
Signature · 1 line
2.15.1
#2 lore raw

Re: [PATCH 1/2] serdev: do not generate modaliases for controllers

Quoted 28 lines · Johan Hovold

Serdev controllers are not bound to any drivers and it therefore makes no sense to generate modaliases for them.

This has already been fixed separately for ACPI controllers for which uevent errors were also being logged during probe due to the missing ACPI companions (from which ACPI modaliases are generated).

This patch moves the modalias handling from the bus type to the client device type. Specifically, this means that only serdev devices (a.k.a. clients or slaves) will have have MODALIAS fields in their uevent environments and corresponding modalias sysfs attributes.

---
 drivers/tty/serdev/core.c | 72 ++++++++++++++++++++++-------------------------
 1 file changed, 34 insertions(+), 38 deletions(-)
drivers/tty/serdev/core.c +2 -0
index 5dc88f61f506..61c85e49e178 100644
@@ -19,6 +19,38 @@
 static bool is_registered;
 static DEFINE_IDA(ctrl_ida);
 
+static ssize_t modalias_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)

drivers/tty/serdev/core.c line 33

Quoted 10 lines
drivers/tty/serdev/core.c +10 -0
+{
+	int len;
+
+	len = acpi_device_modalias(dev, buf, PAGE_SIZE - 1);
+	if (len != -ENODEV)
+		return len;
+
+	return of_device_modalias(dev, buf, PAGE_SIZE);
+}
+DEVICE_ATTR_RO(modalias);

static?

Sorry, minor nit :(

#3 lore raw

Re: [PATCH 1/2] serdev: do not generate modaliases for controllers

Quoted 42 lines · Greg Kroah-Hartman
Quoted 28 lines · Johan Hovold

Serdev controllers are not bound to any drivers and it therefore makes no sense to generate modaliases for them.

This has already been fixed separately for ACPI controllers for which uevent errors were also being logged during probe due to the missing ACPI companions (from which ACPI modaliases are generated).

This patch moves the modalias handling from the bus type to the client device type. Specifically, this means that only serdev devices (a.k.a. clients or slaves) will have have MODALIAS fields in their uevent environments and corresponding modalias sysfs attributes.

---
 drivers/tty/serdev/core.c | 72 ++++++++++++++++++++++-------------------------
 1 file changed, 34 insertions(+), 38 deletions(-)
drivers/tty/serdev/core.c +2 -0
index 5dc88f61f506..61c85e49e178 100644
@@ -19,6 +19,38 @@
 static bool is_registered;
 static DEFINE_IDA(ctrl_ida);
 
+static ssize_t modalias_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)

drivers/tty/serdev/core.c line 33

Quoted 10 lines
drivers/tty/serdev/core.c +10 -0
+{
+	int len;
+
+	len = acpi_device_modalias(dev, buf, PAGE_SIZE - 1);
+	if (len != -ENODEV)
+		return len;
+
+	return of_device_modalias(dev, buf, PAGE_SIZE);
+}
+DEVICE_ATTR_RO(modalias);

static?

Quoted 1 line

Sorry, minor nit :(

Heh, no worries. I didn't notice that as I was just moving code around here, but I'll make sure to add that missing static in a v2.

Thanks,
Johan

#4 lore raw

PATCH 2/2 serdev: only match serdev devices

In reply to Johan Hovold

Only serdev devices (a.k.a. clients or slaves) are bound to drivers so bail out early from match() in case the device is not a serdev device (i.e. if it's a serdev controller).

---
 drivers/tty/serdev/core.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/tty/serdev/core.c +8 -0
index 61c85e49e178..a30e237eb98c 100644
@@ -63,6 +63,11 @@ static const struct device_type serdev_device_type = {
 	.release	= serdev_device_release,
 };
 
+static bool is_serdev_device(const struct device *dev)
+{
+	return dev->type == &serdev_device_type;
+}
+
 static void serdev_ctrl_release(struct device *dev)
 {
 	struct serdev_controller *ctrl = to_serdev_controller(dev);
@@ -76,6 +81,9 @@ static const struct device_type serdev_ctrl_type = {
 
 static int serdev_device_match(struct device *dev, struct device_driver *drv)
 {
+	if (!is_serdev_device(dev))
+		return 0;
+
 	/* TODO: platform matching */
 	if (acpi_driver_match_device(dev, drv))
 		return 1;
Signature · 1 line
2.15.1
#5 lore raw
In reply to Johan Hovold
Quoted 10 lines · Johan Hovold

As noted by Hans, we currently fail to generate uevents for ACPI serdev controller as they do not have any ACPI companions from which ACPI modaliases are constructed.

In fact, we should not have been generating modaliases for controllers in the first place as controllers are not bound to drivers.

This series applies on top of Hans's minimal fix which suppresses the uevent errors for ACPI controllers (even though it could replace it entirely if preferred).

I somehow forgot to CC Hans. Sorry about that. This series can be found here:

	https://lkml.kernel.org/r/[email protected]
Quoted 6 lines

Johan Hovold (2):
serdev: do not generate modaliases for controllers
serdev: only match serdev devices

 drivers/tty/serdev/core.c | 80 +++++++++++++++++++++++++----------------------
 1 file changed, 42 insertions(+), 38 deletions(-)

Johan

#6 lore raw
Quoted 8 lines · Johan Hovold
Quoted 7 lines · Johan Hovold

As noted by Hans, we currently fail to generate uevents for ACPI serdev controller as they do not have any ACPI companions from which ACPI modaliases are constructed.

In fact, we should not have been generating modaliases for controllers in the first place as controllers are not bound to drivers.

Quoted 8 lines
Quoted 3 lines

This series applies on top of Hans's minimal fix which suppresses the uevent errors for ACPI controllers (even though it could replace it entirely if preferred).

I somehow forgot to CC Hans. Sorry about that. This series can be found here:

	https://lkml.kernel.org/r/[email protected]

Minor issue found with patch 1, can you fix that up and resend and cc:
him this time? :)

thanks,

greg k-h