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 > --- > 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:" 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 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 > #include > +#include > #include > +#include > #include > #include > #include > @@ -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) > {