[PATCH v3 0/3] serdev: handle devices without an ACPI or device tree

[PATCH v3 0/3] serdev: handle devices without an ACPI or device tree

[ChunAn Wu]

Driver-created serdev clients without an ACPI companion or device tree
node currently have no modalias and cannot be matched to a driver:
their modalias sysfs read returns -ENODEV and their creators force a
bind with device_driver_attach().

Backlight function on dell-uart-backlight is not affected: that class
device is registered independently and gets its own add uevent. The
user-visible breakage is the failing modalias read on an existing
serdev child -- Checkbox dkms_info_attachment aborts with
"OSError: [Errno 19] No such device".

Resolve the two long-standing serdev-core TODOs by adding a creator-set
modalias and an id_table, modelled on the SPI bus. ACPI and OF match
first and are unchanged. Convert dell-uart-backlight and the Lenovo
Yoga Tab 2 Pro fastcharger to this path and drop their force-attach
workarounds.

ChunAn Wu (3):
  serdev: handle devices without an ACPI or device tree node
  platform/x86: dell-uart-backlight: Bind through normal serdev matching
  platform/x86: lenovo-yoga-tab2-pro-1380-fastcharger: Bind through
    normal serdev matching

 MAINTAINERS                                   |  1 +
 .../platform/x86/dell/dell-uart-backlight.c   | 18 +++----
 .../lenovo/yoga-tab2-pro-1380-fastcharger.c   | 27 +++++-----
 drivers/tty/serdev/core.c                     | 54 ++++++++++++++++---
 include/linux/device-id/serdev.h              | 16 ++++++
 include/linux/serdev.h                        | 34 ++++++++++++
 6 files changed, 119 insertions(+), 31 deletions(-)
 create mode 100644 include/linux/device-id/serdev.h


base-commit: 9e947d96915af63e28896caae85a81b74855621b
-- 
2.34.1

[PATCH v3 1/3] serdev: handle devices without an ACPI or device tree node

[ChunAn Wu]

Serdev clients instantiated by a driver have no ACPI companion and no
device tree node. Their modalias read returns -ENODEV, their add uevent
is suppressed, and they cannot be driver-matched, so creators force a
bind with device_driver_attach().

Resolve both TODOs in the serdev core by modelling matching on the SPI
bus: add a creator-set modalias to struct serdev_device and an id_table
to struct serdev_device_driver. ACPI and OF are still tried first;
clients without their own modalias keep returning -ENODEV. The device
name, derived from the controller, is unchanged.

Signed-off-by: ChunAn Wu <an.wu@canonical.com>
---
Changes in v3:
- Rename struct serdev_device::name to ::modalias; dev_name() is unchanged
- Model matching on the SPI bus: add struct serdev_device_id and
  id_table; drop driver->name matching
- Add serdev_device_set_modalias() helper and SERDEV_MODULE_PREFIX
- No MODULE_DEVICE_TABLE(serdev, ...) support: both in-tree firmware-less
  clients are created by the same module that provides their driver, so
  udev has nothing to autoload

Changes in v2:
- Instead of silently returning success with no modalias, use a
  creator-supplied name as a "serial:<name>" modalias fallback and as
  the matching key, so clients can bind through normal serdev matching

 MAINTAINERS                      |  1 +
 drivers/tty/serdev/core.c        | 54 ++++++++++++++++++++++++++++----
 include/linux/device-id/serdev.h | 16 ++++++++++
 include/linux/serdev.h           | 34 ++++++++++++++++++++
 4 files changed, 99 insertions(+), 6 deletions(-)
 create mode 100644 include/linux/device-id/serdev.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 917f3f72e31b..06e67aa5d9b0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -24905,6 +24905,7 @@ L:	linux-serial@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/serial/serial.yaml
 F:	drivers/tty/serdev/
+F:	include/linux/device-id/serdev.h
 F:	include/linux/serdev.h
 F:	rust/helpers/serdev.c
 F:	rust/kernel/serdev.rs
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index 7500efcdfc21..e178dc57d31b 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -29,13 +29,21 @@ static DEFINE_IDA(ctrl_ida);
 static ssize_t modalias_show(struct device *dev,
 			     struct device_attribute *attr, char *buf)
 {
+	const struct serdev_device *serdev = to_serdev_device(dev);
 	int len;
 
 	len = acpi_device_modalias(dev, buf, PAGE_SIZE - 1);
 	if (len != -ENODEV)
 		return len;
 
-	return of_device_modalias(dev, buf, PAGE_SIZE);
+	len = of_device_modalias(dev, buf, PAGE_SIZE);
+	if (len != -ENODEV)
+		return len;
+
+	if (!serdev->modalias[0])
+		return -ENODEV;
+
+	return sysfs_emit(buf, "%s%s\n", SERDEV_MODULE_PREFIX, serdev->modalias);
 }
 static DEVICE_ATTR_RO(modalias);
 
@@ -47,15 +55,22 @@ ATTRIBUTE_GROUPS(serdev_device);
 
 static int serdev_device_uevent(const struct device *dev, struct kobj_uevent_env *env)
 {
+	const struct serdev_device *serdev = to_serdev_device(dev);
 	int rc;
 
-	/* TODO: platform modalias */
-
 	rc = acpi_device_uevent_modalias(dev, env);
 	if (rc != -ENODEV)
 		return rc;
 
-	return of_device_uevent_modalias(dev, env);
+	rc = of_device_uevent_modalias(dev, env);
+	if (rc != -ENODEV)
+		return rc;
+
+	if (!serdev->modalias[0])
+		return -ENODEV;
+
+	return add_uevent_var(env, "MODALIAS=%s%s", SERDEV_MODULE_PREFIX,
+			      serdev->modalias);
 }
 
 static void serdev_device_release(struct device *dev)
@@ -86,16 +101,43 @@ static const struct device_type serdev_ctrl_type = {
 	.release	= serdev_ctrl_release,
 };
 
+static const struct serdev_device_id *serdev_match_id(const struct serdev_device_id *id,
+						      const char *modalias)
+{
+	if (!id || !modalias[0])
+		return NULL;
+
+	while (id->name[0]) {
+		if (!strcmp(modalias, id->name))
+			return id;
+		id++;
+	}
+
+	return NULL;
+}
+
 static int serdev_device_match(struct device *dev, const struct device_driver *drv)
 {
+	const struct serdev_device *serdev;
+
 	if (!is_serdev_device(dev))
 		return 0;
 
-	/* TODO: platform matching */
 	if (acpi_driver_match_device(dev, drv))
 		return 1;
 
-	return of_driver_match_device(dev, drv);
+	if (of_driver_match_device(dev, drv))
+		return 1;
+
+	/*
+	 * Devices which are instantiated by a driver instead of by firmware
+	 * have no ACPI companion and no device tree node, match them by the
+	 * modalias their creator gave them.
+	 */
+	serdev = to_serdev_device(dev);
+
+	return !!serdev_match_id(to_serdev_device_driver(drv)->id_table,
+				 serdev->modalias);
 }
 
 /**
diff --git a/include/linux/device-id/serdev.h b/include/linux/device-id/serdev.h
new file mode 100644
index 000000000000..1c3b03b7d501
--- /dev/null
+++ b/include/linux/device-id/serdev.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef LINUX_DEVICE_ID_SERDEV_H
+#define LINUX_DEVICE_ID_SERDEV_H
+
+/* serdev */
+
+/* Large enough to hold the KBUILD_MODNAME of any serdev client driver */
+#define SERDEV_NAME_SIZE	40
+#define SERDEV_MODULE_PREFIX	"serial:"
+
+struct serdev_device_id {
+	char name[SERDEV_NAME_SIZE];
+	unsigned long driver_data;	/* Data private to the driver */
+};
+
+#endif /* ifndef LINUX_DEVICE_ID_SERDEV_H */
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index b6c3d957ec15..9f91aa3e875d 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -7,7 +7,9 @@
 
 #include <linux/types.h>
 #include <linux/device.h>
+#include <linux/device-id/serdev.h>
 #include <linux/iopoll.h>
+#include <linux/string.h>
 #include <linux/uaccess.h>
 #include <linux/termios.h>
 #include <linux/delay.h>
@@ -35,6 +37,10 @@ struct serdev_device_ops {
  * struct serdev_device - Basic representation of an serdev device
  * @dev:	Driver model representation of the device.
  * @nr:		Device number on serdev bus.
+ * @modalias:	Name of the driver to use with this device, or the alias
+ *		for it. Empty for devices enumerated from firmware, which
+ *		are identified through their ACPI companion or their device
+ *		tree node instead. See serdev_device_set_modalias().
  * @ctrl:	serdev controller managing this device.
  * @ops:	Device operations.
  * @write_comp:	Completion used by serdev_device_write() internally
@@ -43,6 +49,7 @@ struct serdev_device_ops {
 struct serdev_device {
 	struct device dev;
 	int nr;
+	char modalias[SERDEV_NAME_SIZE];
 	struct serdev_controller *ctrl;
 	const struct serdev_device_ops *ops;
 	struct completion write_comp;
@@ -58,12 +65,15 @@ struct serdev_device {
  * @probe:	binds this driver to a serdev device.
  * @remove:	unbinds this driver from the serdev device.
  * @shutdown:	shut down this serdev device.
+ * @id_table:	List of serdev devices supported by this driver, used to
+ *		match devices which are not enumerated from firmware.
  */
 struct serdev_device_driver {
 	struct device_driver driver;
 	int	(*probe)(struct serdev_device *);
 	void	(*remove)(struct serdev_device *);
 	void	(*shutdown)(struct serdev_device *);
+	const struct serdev_device_id *id_table;
 };
 
 #define to_serdev_device_driver(d) container_of_const(d, struct serdev_device_driver, driver)
@@ -135,6 +145,30 @@ static inline void serdev_device_set_client_ops(struct serdev_device *serdev,
 	serdev->ops = ops;
 }
 
+/**
+ * serdev_device_set_modalias() - set the modalias of a serdev device
+ * @serdev:	serdev device.
+ * @modalias:	non-empty modalias to use, typically KBUILD_MODNAME of the
+ *		client driver.
+ *
+ * A serdev device is identified by its ACPI companion, by its device tree
+ * node, or by a modalias set by its creator. Serdev devices which are
+ * instantiated by a driver rather than by firmware have neither of the
+ * former, so they must set a modalias before serdev_device_add() to be
+ * identifiable. Doing so reports a SERDEV_MODULE_PREFIX modalias to
+ * userspace and makes the device match an id_table entry of a serdev
+ * device driver.
+ *
+ * Devices enumerated from firmware have no modalias of their own and keep
+ * deriving their modalias and their driver match from firmware only. The
+ * device name is not affected either way.
+ */
+static inline void serdev_device_set_modalias(struct serdev_device *serdev,
+					      const char *modalias)
+{
+	strscpy(serdev->modalias, modalias);
+}
+
 static inline
 void *serdev_controller_get_drvdata(const struct serdev_controller *ctrl)
 {
-- 
2.34.1

Re: [PATCH v3 1/3] serdev: handle devices without an ACPI or device tree node

[Markus Probst]

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

On Fri, 2026-10-23 at 13:27 +0800, ChunAn Wu wrote:
> Serdev clients instantiated by a driver have no ACPI companion and no
> device tree node. Their modalias read returns -ENODEV, their add uevent
> is suppressed, and they cannot be driver-matched, so creators force a
> bind with device_driver_attach().
> 
> Resolve both TODOs in the serdev core by modelling matching on the SPI
> bus: add a creator-set modalias to struct serdev_device and an id_table
> to struct serdev_device_driver. ACPI and OF are still tried first;
> clients without their own modalias keep returning -ENODEV. The device
> name, derived from the controller, is unchanged.
> 
> Signed-off-by: ChunAn Wu <an.wu@canonical.com>
> ---
> Changes in v3:
> - Rename struct serdev_device::name to ::modalias; dev_name() is unchanged
> - Model matching on the SPI bus: add struct serdev_device_id and
>   id_table; drop driver->name matching
> - Add serdev_device_set_modalias() helper and SERDEV_MODULE_PREFIX
> - No MODULE_DEVICE_TABLE(serdev, ...) support: both in-tree firmware-less
>   clients are created by the same module that provides their driver, so
>   udev has nothing to autoload
> 
> Changes in v2:
> - Instead of silently returning success with no modalias, use a
>   creator-supplied name as a "serial:<name>" modalias fallback and as
>   the matching key, so clients can bind through normal serdev matching
> 
Thats something I would expect on the cover letter.
>  MAINTAINERS                      |  1 +
>  drivers/tty/serdev/core.c        | 54 ++++++++++++++++++++++++++++----
>  include/linux/device-id/serdev.h | 16 ++++++++++
>  include/linux/serdev.h           | 34 ++++++++++++++++++++
>  4 files changed, 99 insertions(+), 6 deletions(-)
>  create mode 100644 include/linux/device-id/serdev.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 917f3f72e31b..06e67aa5d9b0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -24905,6 +24905,7 @@ L:	linux-serial@vger.kernel.org
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/serial/serial.yaml
>  F:	drivers/tty/serdev/
> +F:	include/linux/device-id/serdev.h
>  F:	include/linux/serdev.h
>  F:	rust/helpers/serdev.c
>  F:	rust/kernel/serdev.rs
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index 7500efcdfc21..e178dc57d31b 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -29,13 +29,21 @@ static DEFINE_IDA(ctrl_ida);
>  static ssize_t modalias_show(struct device *dev,
>  			     struct device_attribute *attr, char *buf)
>  {
> +	const struct serdev_device *serdev = to_serdev_device(dev);
>  	int len;
>  
>  	len = acpi_device_modalias(dev, buf, PAGE_SIZE - 1);
>  	if (len != -ENODEV)
>  		return len;
>  
> -	return of_device_modalias(dev, buf, PAGE_SIZE);
> +	len = of_device_modalias(dev, buf, PAGE_SIZE);
> +	if (len != -ENODEV)
> +		return len;
> +
> +	if (!serdev->modalias[0])
> +		return -ENODEV;
> +
> +	return sysfs_emit(buf, "%s%s\n", SERDEV_MODULE_PREFIX, serdev->modalias);
>  }
>  static DEVICE_ATTR_RO(modalias);
>  
> @@ -47,15 +55,22 @@ ATTRIBUTE_GROUPS(serdev_device);
>  
>  static int serdev_device_uevent(const struct device *dev, struct kobj_uevent_env *env)
>  {
> +	const struct serdev_device *serdev = to_serdev_device(dev);
>  	int rc;
>  
> -	/* TODO: platform modalias */
> -
>  	rc = acpi_device_uevent_modalias(dev, env);
>  	if (rc != -ENODEV)
>  		return rc;
>  
> -	return of_device_uevent_modalias(dev, env);
> +	rc = of_device_uevent_modalias(dev, env);
> +	if (rc != -ENODEV)
> +		return rc;
> +
> +	if (!serdev->modalias[0])
> +		return -ENODEV;
> +
> +	return add_uevent_var(env, "MODALIAS=%s%s", SERDEV_MODULE_PREFIX,
> +			      serdev->modalias);
>  }
>  
>  static void serdev_device_release(struct device *dev)
> @@ -86,16 +101,43 @@ static const struct device_type serdev_ctrl_type = {
>  	.release	= serdev_ctrl_release,
>  };
>  
> +static const struct serdev_device_id *serdev_match_id(const struct serdev_device_id *id,
> +						      const char *modalias)
> +{
> +	if (!id || !modalias[0])
> +		return NULL;
> +
> +	while (id->name[0]) {
> +		if (!strcmp(modalias, id->name))
> +			return id;
> +		id++;
> +	}
> +
> +	return NULL;
> +}
> +
>  static int serdev_device_match(struct device *dev, const struct device_driver *drv)
>  {
> +	const struct serdev_device *serdev;
> +
>  	if (!is_serdev_device(dev))
>  		return 0;
>  
> -	/* TODO: platform matching */
>  	if (acpi_driver_match_device(dev, drv))
>  		return 1;
>  
> -	return of_driver_match_device(dev, drv);
> +	if (of_driver_match_device(dev, drv))
> +		return 1;
> +
> +	/*
> +	 * Devices which are instantiated by a driver instead of by firmware
> +	 * have no ACPI companion and no device tree node, match them by the
> +	 * modalias their creator gave them.
> +	 */
Drop comment.

> +	serdev = to_serdev_device(dev);
> +
> +	return !!serdev_match_id(to_serdev_device_driver(drv)->id_table,
> +				 serdev->modalias);
I think you can drop the `is_serdev_device` check and use

```
const struct serdev_device *serdev = to_serdev_device(dev);
const struct serdev_device_driver *sdrv = to_serdev_device_driver(drv);
```

in the top.

The function `serdev_device_match` is only used inside the match
callback on the bus type definition. Other bus devices also do not have
this check. It should be safe.

Alternatively `to_serdev_device` in the same expression as the function
call (like with `to_serdev_device_driver`). It should be consistent
(either both as variables or none).

>  }
>  
>  /**
> diff --git a/include/linux/device-id/serdev.h b/include/linux/device-id/serdev.h
> new file mode 100644
> index 000000000000..1c3b03b7d501
> --- /dev/null
> +++ b/include/linux/device-id/serdev.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef LINUX_DEVICE_ID_SERDEV_H
> +#define LINUX_DEVICE_ID_SERDEV_H
> +
> +/* serdev */
> +
> +/* Large enough to hold the KBUILD_MODNAME of any serdev client driver */
Is this relevant for anyone reading the code? If not, drop the comment.

> +#define SERDEV_NAME_SIZE	40
> +#define SERDEV_MODULE_PREFIX	"serial:"
> +
> +struct serdev_device_id {
> +	char name[SERDEV_NAME_SIZE];
> +	unsigned long driver_data;	/* Data private to the driver */
> +};
In almost any other include/linux/device-id/*.h file, there is a

```
#ifdef __KERNEL__
#include <linux/types.h>
typedef unsigned long kernel_ulong_t;
#endif
```

statement and `kernel_ulong_t` is used.

I don't know why, but there is likely a reason for it. Thus do it the
same way, unless you can explain why this isn't needed here.

> +
> +#endif /* ifndef LINUX_DEVICE_ID_SERDEV_H */
> diff --git a/include/linux/serdev.h b/include/linux/serdev.h
> index b6c3d957ec15..9f91aa3e875d 100644
> --- a/include/linux/serdev.h
> +++ b/include/linux/serdev.h
> @@ -7,7 +7,9 @@
>  
>  #include <linux/types.h>
>  #include <linux/device.h>
> +#include <linux/device-id/serdev.h>
>  #include <linux/iopoll.h>
> +#include <linux/string.h>
>  #include <linux/uaccess.h>
>  #include <linux/termios.h>
>  #include <linux/delay.h>
> @@ -35,6 +37,10 @@ struct serdev_device_ops {
>   * struct serdev_device - Basic representation of an serdev device
>   * @dev:	Driver model representation of the device.
>   * @nr:		Device number on serdev bus.
> + * @modalias:	Name of the driver to use with this device, or the alias
> + *		for it. Empty for devices enumerated from firmware, which
> + *		are identified through their ACPI companion or their device
> + *		tree node instead. See serdev_device_set_modalias().
>   * @ctrl:	serdev controller managing this device.
>   * @ops:	Device operations.
>   * @write_comp:	Completion used by serdev_device_write() internally
> @@ -43,6 +49,7 @@ struct serdev_device_ops {
>  struct serdev_device {
>  	struct device dev;
>  	int nr;
> +	char modalias[SERDEV_NAME_SIZE];
>  	struct serdev_controller *ctrl;
>  	const struct serdev_device_ops *ops;
>  	struct completion write_comp;
> @@ -58,12 +65,15 @@ struct serdev_device {
>   * @probe:	binds this driver to a serdev device.
>   * @remove:	unbinds this driver from the serdev device.
>   * @shutdown:	shut down this serdev device.
> + * @id_table:	List of serdev devices supported by this driver, used to
> + *		match devices which are not enumerated from firmware.
>   */
>  struct serdev_device_driver {
>  	struct device_driver driver;
>  	int	(*probe)(struct serdev_device *);
>  	void	(*remove)(struct serdev_device *);
>  	void	(*shutdown)(struct serdev_device *);
> +	const struct serdev_device_id *id_table;
Just a suggestion:
Move it directly under driver, so the fields (driver->of_match_table,
driver->acpi_match_table, id_table) are next to each other. It has no
real runtime difference, so I won't complain if you leave it there.

>  };
>  
>  #define to_serdev_device_driver(d) container_of_const(d, struct serdev_device_driver, driver)
> @@ -135,6 +145,30 @@ static inline void serdev_device_set_client_ops(struct serdev_device *serdev,
>  	serdev->ops = ops;
>  }
>  
> +/**
> + * serdev_device_set_modalias() - set the modalias of a serdev device
> + * @serdev:	serdev device.
> + * @modalias:	non-empty modalias to use, typically KBUILD_MODNAME of the
> + *		client driver.
> + *
> + * A serdev device is identified by its ACPI companion, by its device tree
> + * node, or by a modalias set by its creator. Serdev devices which are
> + * instantiated by a driver rather than by firmware have neither of the
> + * former, so they must set a modalias before serdev_device_add() to be
> + * identifiable. Doing so reports a SERDEV_MODULE_PREFIX modalias to
> + * userspace and makes the device match an id_table entry of a serdev
> + * device driver.
> + *
> + * Devices enumerated from firmware have no modalias of their own and keep
> + * deriving their modalias and their driver match from firmware only. The
> + * device name is not affected either way.
> + */
> +static inline void serdev_device_set_modalias(struct serdev_device *serdev,
> +					      const char *modalias)
> +{
> +	strscpy(serdev->modalias, modalias);
> +}
Drop the function.
Add `const char *modalias` as parameter to `serdev_device_add`. It
should be clear that any driver calling this function manually should
provide a modalias. Also, keep documentation simple (unless there is
need).

Either you add `__serdev_device_add` for internal use (of or acpi), or
it will use NULL. Your choice.


Overall, looks pretty good so far.

Thanks
- Markus Probst

> +
>  static inline
>  void *serdev_controller_get_drvdata(const struct serdev_controller *ctrl)
>  {

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]

Re: [PATCH v3 1/3] serdev: handle devices without an ACPI or device tree node

[An Wu]

Hi Markus
    Thank you for your review.
For this,
```
#ifdef __KERNEL__
#include <linux/types.h>
typedef unsigned long kernel_ulong_t;
#endif
```

I originally implemented it this way, but checkpatch.pl reported a warning.
Since it's only a warning, I'll go ahead and update it accordingly.
I really appreciate your patience and guidance. Thank you!

BR
    An

On Fri, Oct 23, 2026 at 8:26 PM Markus Probst <markus.probst@posteo.de> wrote:
>
> On Fri, 2026-10-23 at 13:27 +0800, ChunAn Wu wrote:
> > Serdev clients instantiated by a driver have no ACPI companion and no
> > device tree node. Their modalias read returns -ENODEV, their add uevent
> > is suppressed, and they cannot be driver-matched, so creators force a
> > bind with device_driver_attach().
> >
> > Resolve both TODOs in the serdev core by modelling matching on the SPI
> > bus: add a creator-set modalias to struct serdev_device and an id_table
> > to struct serdev_device_driver. ACPI and OF are still tried first;
> > clients without their own modalias keep returning -ENODEV. The device
> > name, derived from the controller, is unchanged.
> >
> > Signed-off-by: ChunAn Wu <an.wu@canonical.com>
> > ---
> > Changes in v3:
> > - Rename struct serdev_device::name to ::modalias; dev_name() is unchanged
> > - Model matching on the SPI bus: add struct serdev_device_id and
> >   id_table; drop driver->name matching
> > - Add serdev_device_set_modalias() helper and SERDEV_MODULE_PREFIX
> > - No MODULE_DEVICE_TABLE(serdev, ...) support: both in-tree firmware-less
> >   clients are created by the same module that provides their driver, so
> >   udev has nothing to autoload
> >
> > Changes in v2:
> > - Instead of silently returning success with no modalias, use a
> >   creator-supplied name as a "serial:<name>" modalias fallback and as
> >   the matching key, so clients can bind through normal serdev matching
> >
> Thats something I would expect on the cover letter.
> >  MAINTAINERS                      |  1 +
> >  drivers/tty/serdev/core.c        | 54 ++++++++++++++++++++++++++++----
> >  include/linux/device-id/serdev.h | 16 ++++++++++
> >  include/linux/serdev.h           | 34 ++++++++++++++++++++
> >  4 files changed, 99 insertions(+), 6 deletions(-)
> >  create mode 100644 include/linux/device-id/serdev.h
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 917f3f72e31b..06e67aa5d9b0 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -24905,6 +24905,7 @@ L:    linux-serial@vger.kernel.org
> >  S:   Maintained
> >  F:   Documentation/devicetree/bindings/serial/serial.yaml
> >  F:   drivers/tty/serdev/
> > +F:   include/linux/device-id/serdev.h
> >  F:   include/linux/serdev.h
> >  F:   rust/helpers/serdev.c
> >  F:   rust/kernel/serdev.rs
> > diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> > index 7500efcdfc21..e178dc57d31b 100644
> > --- a/drivers/tty/serdev/core.c
> > +++ b/drivers/tty/serdev/core.c
> > @@ -29,13 +29,21 @@ static DEFINE_IDA(ctrl_ida);
> >  static ssize_t modalias_show(struct device *dev,
> >                            struct device_attribute *attr, char *buf)
> >  {
> > +     const struct serdev_device *serdev = to_serdev_device(dev);
> >       int len;
> >
> >       len = acpi_device_modalias(dev, buf, PAGE_SIZE - 1);
> >       if (len != -ENODEV)
> >               return len;
> >
> > -     return of_device_modalias(dev, buf, PAGE_SIZE);
> > +     len = of_device_modalias(dev, buf, PAGE_SIZE);
> > +     if (len != -ENODEV)
> > +             return len;
> > +
> > +     if (!serdev->modalias[0])
> > +             return -ENODEV;
> > +
> > +     return sysfs_emit(buf, "%s%s\n", SERDEV_MODULE_PREFIX, serdev->modalias);
> >  }
> >  static DEVICE_ATTR_RO(modalias);
> >
> > @@ -47,15 +55,22 @@ ATTRIBUTE_GROUPS(serdev_device);
> >
> >  static int serdev_device_uevent(const struct device *dev, struct kobj_uevent_env *env)
> >  {
> > +     const struct serdev_device *serdev = to_serdev_device(dev);
> >       int rc;
> >
> > -     /* TODO: platform modalias */
> > -
> >       rc = acpi_device_uevent_modalias(dev, env);
> >       if (rc != -ENODEV)
> >               return rc;
> >
> > -     return of_device_uevent_modalias(dev, env);
> > +     rc = of_device_uevent_modalias(dev, env);
> > +     if (rc != -ENODEV)
> > +             return rc;
> > +
> > +     if (!serdev->modalias[0])
> > +             return -ENODEV;
> > +
> > +     return add_uevent_var(env, "MODALIAS=%s%s", SERDEV_MODULE_PREFIX,
> > +                           serdev->modalias);
> >  }
> >
> >  static void serdev_device_release(struct device *dev)
> > @@ -86,16 +101,43 @@ static const struct device_type serdev_ctrl_type = {
> >       .release        = serdev_ctrl_release,
> >  };
> >
> > +static const struct serdev_device_id *serdev_match_id(const struct serdev_device_id *id,
> > +                                                   const char *modalias)
> > +{
> > +     if (!id || !modalias[0])
> > +             return NULL;
> > +
> > +     while (id->name[0]) {
> > +             if (!strcmp(modalias, id->name))
> > +                     return id;
> > +             id++;
> > +     }
> > +
> > +     return NULL;
> > +}
> > +
> >  static int serdev_device_match(struct device *dev, const struct device_driver *drv)
> >  {
> > +     const struct serdev_device *serdev;
> > +
> >       if (!is_serdev_device(dev))
> >               return 0;
> >
> > -     /* TODO: platform matching */
> >       if (acpi_driver_match_device(dev, drv))
> >               return 1;
> >
> > -     return of_driver_match_device(dev, drv);
> > +     if (of_driver_match_device(dev, drv))
> > +             return 1;
> > +
> > +     /*
> > +      * Devices which are instantiated by a driver instead of by firmware
> > +      * have no ACPI companion and no device tree node, match them by the
> > +      * modalias their creator gave them.
> > +      */
> Drop comment.
>
> > +     serdev = to_serdev_device(dev);
> > +
> > +     return !!serdev_match_id(to_serdev_device_driver(drv)->id_table,
> > +                              serdev->modalias);
> I think you can drop the `is_serdev_device` check and use
>
> ```
> const struct serdev_device *serdev = to_serdev_device(dev);
> const struct serdev_device_driver *sdrv = to_serdev_device_driver(drv);
> ```
>
> in the top.
>
> The function `serdev_device_match` is only used inside the match
> callback on the bus type definition. Other bus devices also do not have
> this check. It should be safe.
>
> Alternatively `to_serdev_device` in the same expression as the function
> call (like with `to_serdev_device_driver`). It should be consistent
> (either both as variables or none).
>
> >  }
> >
> >  /**
> > diff --git a/include/linux/device-id/serdev.h b/include/linux/device-id/serdev.h
> > new file mode 100644
> > index 000000000000..1c3b03b7d501
> > --- /dev/null
> > +++ b/include/linux/device-id/serdev.h
> > @@ -0,0 +1,16 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef LINUX_DEVICE_ID_SERDEV_H
> > +#define LINUX_DEVICE_ID_SERDEV_H
> > +
> > +/* serdev */
> > +
> > +/* Large enough to hold the KBUILD_MODNAME of any serdev client driver */
> Is this relevant for anyone reading the code? If not, drop the comment.
>
> > +#define SERDEV_NAME_SIZE     40
> > +#define SERDEV_MODULE_PREFIX "serial:"
> > +
> > +struct serdev_device_id {
> > +     char name[SERDEV_NAME_SIZE];
> > +     unsigned long driver_data;      /* Data private to the driver */
> > +};
> In almost any other include/linux/device-id/*.h file, there is a
>
> ```
> #ifdef __KERNEL__
> #include <linux/types.h>
> typedef unsigned long kernel_ulong_t;
> #endif
> ```
>
> statement and `kernel_ulong_t` is used.
>
> I don't know why, but there is likely a reason for it. Thus do it the
> same way, unless you can explain why this isn't needed here.
>
> > +
> > +#endif /* ifndef LINUX_DEVICE_ID_SERDEV_H */
> > diff --git a/include/linux/serdev.h b/include/linux/serdev.h
> > index b6c3d957ec15..9f91aa3e875d 100644
> > --- a/include/linux/serdev.h
> > +++ b/include/linux/serdev.h
> > @@ -7,7 +7,9 @@
> >
> >  #include <linux/types.h>
> >  #include <linux/device.h>
> > +#include <linux/device-id/serdev.h>
> >  #include <linux/iopoll.h>
> > +#include <linux/string.h>
> >  #include <linux/uaccess.h>
> >  #include <linux/termios.h>
> >  #include <linux/delay.h>
> > @@ -35,6 +37,10 @@ struct serdev_device_ops {
> >   * struct serdev_device - Basic representation of an serdev device
> >   * @dev:     Driver model representation of the device.
> >   * @nr:              Device number on serdev bus.
> > + * @modalias:        Name of the driver to use with this device, or the alias
> > + *           for it. Empty for devices enumerated from firmware, which
> > + *           are identified through their ACPI companion or their device
> > + *           tree node instead. See serdev_device_set_modalias().
> >   * @ctrl:    serdev controller managing this device.
> >   * @ops:     Device operations.
> >   * @write_comp:      Completion used by serdev_device_write() internally
> > @@ -43,6 +49,7 @@ struct serdev_device_ops {
> >  struct serdev_device {
> >       struct device dev;
> >       int nr;
> > +     char modalias[SERDEV_NAME_SIZE];
> >       struct serdev_controller *ctrl;
> >       const struct serdev_device_ops *ops;
> >       struct completion write_comp;
> > @@ -58,12 +65,15 @@ struct serdev_device {
> >   * @probe:   binds this driver to a serdev device.
> >   * @remove:  unbinds this driver from the serdev device.
> >   * @shutdown:        shut down this serdev device.
> > + * @id_table:        List of serdev devices supported by this driver, used to
> > + *           match devices which are not enumerated from firmware.
> >   */
> >  struct serdev_device_driver {
> >       struct device_driver driver;
> >       int     (*probe)(struct serdev_device *);
> >       void    (*remove)(struct serdev_device *);
> >       void    (*shutdown)(struct serdev_device *);
> > +     const struct serdev_device_id *id_table;
> Just a suggestion:
> Move it directly under driver, so the fields (driver->of_match_table,
> driver->acpi_match_table, id_table) are next to each other. It has no
> real runtime difference, so I won't complain if you leave it there.
>
> >  };
> >
> >  #define to_serdev_device_driver(d) container_of_const(d, struct serdev_device_driver, driver)
> > @@ -135,6 +145,30 @@ static inline void serdev_device_set_client_ops(struct serdev_device *serdev,
> >       serdev->ops = ops;
> >  }
> >
> > +/**
> > + * serdev_device_set_modalias() - set the modalias of a serdev device
> > + * @serdev:  serdev device.
> > + * @modalias:        non-empty modalias to use, typically KBUILD_MODNAME of the
> > + *           client driver.
> > + *
> > + * A serdev device is identified by its ACPI companion, by its device tree
> > + * node, or by a modalias set by its creator. Serdev devices which are
> > + * instantiated by a driver rather than by firmware have neither of the
> > + * former, so they must set a modalias before serdev_device_add() to be
> > + * identifiable. Doing so reports a SERDEV_MODULE_PREFIX modalias to
> > + * userspace and makes the device match an id_table entry of a serdev
> > + * device driver.
> > + *
> > + * Devices enumerated from firmware have no modalias of their own and keep
> > + * deriving their modalias and their driver match from firmware only. The
> > + * device name is not affected either way.
> > + */
> > +static inline void serdev_device_set_modalias(struct serdev_device *serdev,
> > +                                           const char *modalias)
> > +{
> > +     strscpy(serdev->modalias, modalias);
> > +}
> Drop the function.
> Add `const char *modalias` as parameter to `serdev_device_add`. It
> should be clear that any driver calling this function manually should
> provide a modalias. Also, keep documentation simple (unless there is
> need).
>
> Either you add `__serdev_device_add` for internal use (of or acpi), or
> it will use NULL. Your choice.
>
>
> Overall, looks pretty good so far.
>
> Thanks
> - Markus Probst
>
> > +
> >  static inline
> >  void *serdev_controller_get_drvdata(const struct serdev_controller *ctrl)
> >  {

[PATCH v3 2/3] platform/x86: dell-uart-backlight: Bind through normal serdev matching

[ChunAn Wu]

The driver creates a serdev client without firmware metadata and
force-attaches its driver to it.

Set the modalias of the client before adding it and give the serdev
driver a matching id_table, so that the serdev core binds the two
through normal matching, then drop the manual attachment.

The client now also reports a "serial:dell_uart_backlight" modalias to
userspace instead of failing with -ENODEV. A failing serdev probe no
longer propagates out of the platform probe; as for any other bus, the
client is simply left unbound.

Signed-off-by: ChunAn Wu <an.wu@canonical.com>
---
Changes in v3:
- Use serdev_device_set_modalias() instead of ::name; add an id_table
  to the serdev driver

Changes in v2:
- New in v2

 .../platform/x86/dell/dell-uart-backlight.c    | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-uart-backlight.c b/drivers/platform/x86/dell/dell-uart-backlight.c
index f323a667dc2d..13515ebda3dc 100644
--- a/drivers/platform/x86/dell/dell-uart-backlight.c
+++ b/drivers/platform/x86/dell/dell-uart-backlight.c
@@ -325,11 +325,17 @@ static int dell_uart_bl_serdev_probe(struct serdev_device *serdev)
 	return PTR_ERR_OR_ZERO(dell_bl->bl);
 }
 
+static const struct serdev_device_id dell_uart_bl_serdev_id_table[] = {
+	{ KBUILD_MODNAME },
+	{ }
+};
+
 static struct serdev_device_driver dell_uart_bl_serdev_driver = {
 	.probe = dell_uart_bl_serdev_probe,
 	.driver = {
 		.name = KBUILD_MODNAME,
 	},
+	.id_table = dell_uart_bl_serdev_id_table,
 };
 
 static int dell_uart_bl_pdev_probe(struct platform_device *pdev)
@@ -354,6 +360,8 @@ static int dell_uart_bl_pdev_probe(struct platform_device *pdev)
 	if (!serdev)
 		return -ENOMEM;
 
+	serdev_device_set_modalias(serdev, KBUILD_MODNAME);
+
 	ret = serdev_device_add(serdev);
 	if (ret) {
 		dev_err(&pdev->dev, "error %d adding serdev\n", ret);
@@ -365,20 +373,10 @@ static int dell_uart_bl_pdev_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_remove_serdev;
 
-	/*
-	 * serdev device <-> driver matching relies on OF or ACPI matches and
-	 * neither is available here, manually bind the driver.
-	 */
-	ret = device_driver_attach(&dell_uart_bl_serdev_driver.driver, &serdev->dev);
-	if (ret)
-		goto err_unregister_serdev_driver;
-
 	/* So that dell_uart_bl_pdev_remove() can remove the serdev */
 	platform_set_drvdata(pdev, serdev);
 	return 0;
 
-err_unregister_serdev_driver:
-	serdev_device_driver_unregister(&dell_uart_bl_serdev_driver);
 err_remove_serdev:
 	serdev_device_remove(serdev);
 	return ret;
-- 
2.34.1

[PATCH v3 3/3] platform/x86: lenovo-yoga-tab2-pro-1380-fastcharger: Bind through normal serdev matching

[ChunAn Wu]

The software-node-backed serdev client cannot be matched through ACPI or
device tree and is force-attached to its driver.

Set the modalias of the client before adding it and give the serdev
driver a matching id_table, so that the serdev core binds the two
through normal matching. Deferred probing is then handled by the driver
core, so the -EAGAIN translation is no longer needed.

A failing serdev probe no longer propagates out of the platform probe;
as for any other bus, the client is simply left unbound.

Signed-off-by: ChunAn Wu <an.wu@canonical.com>
---
Changes in v3:
- Use serdev_device_set_modalias() instead of ::name; add an id_table
  to the serdev driver

Changes in v2:
- New in v2

 .../lenovo/yoga-tab2-pro-1380-fastcharger.c   | 27 +++++++++----------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/platform/x86/lenovo/yoga-tab2-pro-1380-fastcharger.c b/drivers/platform/x86/lenovo/yoga-tab2-pro-1380-fastcharger.c
index 8551ab4d2c7d..31780c4aebe0 100644
--- a/drivers/platform/x86/lenovo/yoga-tab2-pro-1380-fastcharger.c
+++ b/drivers/platform/x86/lenovo/yoga-tab2-pro-1380-fastcharger.c
@@ -219,11 +219,17 @@ static int yt2_1380_fc_serdev_probe(struct serdev_device *serdev)
 	return 0;
 }
 
+static const struct serdev_device_id yt2_1380_fc_serdev_id_table[] = {
+	{ KBUILD_MODNAME },
+	{ }
+};
+
 static struct serdev_device_driver yt2_1380_fc_serdev_driver = {
 	.probe = yt2_1380_fc_serdev_probe,
 	.driver = {
 		.name = KBUILD_MODNAME,
 	},
+	.id_table = yt2_1380_fc_serdev_id_table,
 };
 
 static const struct pinctrl_map yt2_1380_fc_pinctrl_map[] = {
@@ -260,25 +266,14 @@ static int yt2_1380_fc_pdev_probe(struct platform_device *pdev)
 	/* The fwnode is a managed node, so it will be auto-put on serdev_device_put() */
 	fwnode_handle_get(dev_fwnode(&serdev->dev));
 
+	serdev_device_set_modalias(serdev, KBUILD_MODNAME);
+
 	ret = serdev_device_add(serdev);
 	if (ret) {
 		serdev_device_put(serdev);
 		return dev_err_probe(&pdev->dev, ret, "adding serdev\n");
 	}
 
-	/*
-	 * serdev device <-> driver matching relies on OF or ACPI matches and
-	 * neither is available here, manually bind the driver.
-	 */
-	ret = device_driver_attach(&yt2_1380_fc_serdev_driver.driver, &serdev->dev);
-	if (ret) {
-		/* device_driver_attach() maps EPROBE_DEFER to EAGAIN, map it back */
-		serdev_device_remove(serdev);
-		return dev_err_probe(&pdev->dev,
-				     (ret == -EAGAIN) ? -EPROBE_DEFER : ret,
-				     "attaching serdev driver\n");
-	}
-
 	/* So that yt2_1380_fc_pdev_remove() can remove the serdev */
 	platform_set_drvdata(pdev, serdev);
 	return 0;
@@ -305,8 +300,10 @@ static int __init yt2_1380_fc_module_init(void)
 	int ret;
 
 	/*
-	 * serdev driver MUST be registered first because pdev driver calls
-	 * device_driver_attach() on the serdev, serdev-driver pair.
+	 * Register the serdev driver first, so that the serdev device which
+	 * the pdev driver's probe() adds is matched as soon as it is added.
+	 * Registering it afterwards would work as well, since driver_register()
+	 * also walks the devices which are already on the bus.
 	 */
 	ret = serdev_device_driver_register(&yt2_1380_fc_serdev_driver);
 	if (ret)
-- 
2.34.1