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

[Markus Probst <markus.probst@posteo.de>]

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

On Wed, 2026-10-21 at 17:21 +0800, ChunAn Wu wrote:
> Driver-created serdev clients may lack an ACPI companion or device tree
> node.
> 
> Add a creator-supplied name for these clients. Use it as a serial
> modalias fallback and to match a serdev driver of the same name. Keep
> returning -ENODEV for unnamed clients.
> 
> Signed-off-by: ChunAn Wu <an.wu@canonical.com>
> ---
>  drivers/tty/serdev/core.c | 36 ++++++++++++++++++++++++++++++------
>  include/linux/serdev.h    | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 61 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index 7500efcdfc21..8354a4688fd8 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->name[0])
> +		return -ENODEV;
> +
> +	return sysfs_emit(buf, "%s%s\n", SERDEV_MODULE_PREFIX, serdev->name);
>  }
>  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->name[0])
> +		return -ENODEV;
> +
> +	add_uevent_var(env, "MODALIAS=%s%s", SERDEV_MODULE_PREFIX, serdev->name);
> +	return 0;
>  }
>  
>  static void serdev_device_release(struct device *dev)
> @@ -88,14 +103,23 @@ static const struct device_type serdev_ctrl_type = {
>  
>  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;
> +
> +	/* Fall back to the name set by the creator of the device */
> +	serdev = to_serdev_device(dev);
> +	if (!serdev->name[0])
> +		return 0;
> +
> +	return strcmp(serdev->name, drv->name) == 0;
>  }
>  
>  /**
> diff --git a/include/linux/serdev.h b/include/linux/serdev.h
> index b6c3d957ec15..26baa0321eef 100644
> --- a/include/linux/serdev.h
> +++ b/include/linux/serdev.h
> @@ -8,6 +8,7 @@
>  #include <linux/types.h>
>  #include <linux/device.h>
>  #include <linux/iopoll.h>
> +#include <linux/string.h>
>  #include <linux/uaccess.h>
>  #include <linux/termios.h>
>  #include <linux/delay.h>
> @@ -19,6 +20,10 @@ struct serdev_device;
>   * serdev device structures
>   */
>  
> +/* Large enough to hold the KBUILD_MODNAME of any serdev client driver */
> +#define SERDEV_NAME_SIZE	40
> +#define SERDEV_MODULE_PREFIX	"serial:"
Move into include/linux/device-id/, see below first.

> +
>  /**
>   * struct serdev_device_ops - Callback operations for a serdev device
>   * @receive_buf:	Function called with data received from device;
> @@ -35,6 +40,9 @@ 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.
> + * @name:	Name of the device, set by its creator, empty for devices
> + *		which are enumerated from firmware, see
> + *		serdev_device_set_name().
>   * @ctrl:	serdev controller managing this device.
>   * @ops:	Device operations.
>   * @write_comp:	Completion used by serdev_device_write() internally
> @@ -43,6 +51,7 @@ struct serdev_device_ops {
>  struct serdev_device {
>  	struct device dev;
>  	int nr;
> +	char name[SERDEV_NAME_SIZE];
>  	struct serdev_controller *ctrl;
>  	const struct serdev_device_ops *ops;
>  	struct completion write_comp;
> @@ -135,6 +144,28 @@ static inline void serdev_device_set_client_ops(struct serdev_device *serdev,
>  	serdev->ops = ops;
>  }
>  
> +/**
> + * serdev_device_set_name() - set the name of a serdev device
> + * @serdev:	serdev device.
> + * @name:	non-empty name 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 name 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 name before serdev_device_add() to be
> + * identifiable. Doing so gives them a SERDEV_MODULE_PREFIX modalias and
> + * makes them match a serdev device driver of the same name.
> + *
> + * Devices enumerated from firmware have no name and keep deriving their
> + * modalias and their driver match from firmware only.
> + */
> +static inline void serdev_device_set_name(struct serdev_device *serdev,
> +					  const char *name)
> +{
> +	strscpy(serdev->name, name);
> +}
> +
If you set a name via `serdev_device_set_name()`, the actual device
name will still be "serial0-0" or similar.

I don't think we should change the actual device name though, as it is
a potentially breaking change.

Instead we should name the field "modalias" like in the spi bus. So its
clear it doesn't represent the device name.


Also add

`const struct serdev_device_id *id_table;`

to `struct serdev_device_driver`.
This makes it more consistent with other bus devices. It makes it also
possible to handle multiple serial devices (without fwnode), including
storing different private data.

You will have to define serdev_device_id first inside
`include/linux/device-id/`.

Thanks
- Markus Probst


>  static inline
>  void *serdev_controller_get_drvdata(const struct serdev_controller *ctrl)
>  {
> 
> base-commit: 9e947d96915af63e28896caae85a81b74855621b

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