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

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

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

On Sat, 2026-10-24 at 16:40 +0800, An Wu wrote:
> Hi Makus
>     According to sashiko:
> '''
> In drivers/tty/serdev/core.c, serdev_controller_alloc() places controller
> devices on serdev_bus_type:
> 
>     device_initialize(&ctrl->dev);
>     ctrl->dev.type = &serdev_ctrl_type;
>     ctrl->dev.bus = &serdev_bus_type;
> 
> When yt2_1380_fc_module_init() calls serdev_device_driver_register() for
> yt2_1380_fc_serdev_driver (or when a new serdev_controller is added), the
> driver core runs serdev_device_match() against every device on
> serdev_bus_type, including struct serdev_controller instances:
> 
> static int serdev_device_match(struct device *dev, const struct
> device_driver *drv)
> {
>         const struct serdev_device *serdev = to_serdev_device(dev);
>         const struct serdev_device_driver *sdrv = to_serdev_device_driver(drv);
> 
>         if (acpi_driver_match_device(dev, drv))
>                 return 1;
> 
>         if (of_driver_match_device(dev, drv))
>                 return 1;
> 
>         return !!serdev_match_id(sdrv->id_table, serdev->modalias);
> }
> 
> Because serdev_device_match() does not check is_serdev_device(dev) before
> casting dev with to_serdev_device(dev), and yt2_1380_fc_serdev_driver has no
> ACPI or OF match table, serdev_device_match() falls through to
> serdev_match_id(sdrv->id_table, serdev->modalias) on struct serdev_controller
> devices.
> 
> In struct serdev_device, modalias[SERDEV_NAME_SIZE] starts at offset
> sizeof(struct device) + 4, which in struct serdev_controller overlaps the
> struct device *host pointer, ctrl->nr, ctrl->serdev, ctrl->ops, and extends 12
> bytes past sizeof(struct serdev_controller). When the overlapping byte at
> modalias[0] is non-zero, serdev_match_id() passes that non-string memory and
> trailing out-of-bounds bytes to strcmp().
> 
> Should serdev_device_match() verify is_serdev_device(dev) before casting dev
> to struct serdev_device?
> '''
> 
> If we remove is_serdev_device could cause some problem due to the
> serdev_controller could also
> run into this function and "serdev_match_id(sdrv->id_table,
> serdev->modalias);" can be a potential risk.
> 
> I will add the "is_serdev_device" back and send v5 out.
Your right, I missed that.

Thanks
- Markus Probst

> 
> BR
>     An
> 
> 
> On Sat, Oct 24, 2026 at 10:29 AM ChunAn Wu <an.wu@canonical.com> wrote:
> > 
> > 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 letting the creator
> > pass a modalias to serdev_device_add() and giving the serdev driver 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.
> > 
> > Changes in v4:
> > - Patch 1: drop the dead is_serdev_device() guard from the match
> >   callback, and dereference serdev once at the top
> > - Patch 1: drop the "large enough" comment on SERDEV_NAME_SIZE
> > - Patch 1: in struct serdev_device_driver, place id_table right under
> >   driver, next to driver->of_match_table and driver->acpi_match_table
> > - Patch 1: merge serdev_device_set_modalias() into serdev_device_add()
> >   so the modalias is set at the only point where forgetting it would
> >   matter; internal firmware callers pass NULL
> > - All three patches: tightened commit messages and moved the v2/v3
> >   changelog narrative out of Patch 1 into this cover letter
> > 
> > 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
> > 
> > v3: https://lore.kernel.org/lkml/20261023052717.2577234-1-an.wu@canonical.com/
> > v2: https://lore.kernel.org/lkml/20261022032842.2492572-1-an.wu@canonical.com/
> > v1: https://lore.kernel.org/lkml/20261021092124.2306053-1-an.wu@canonical.com/
> > 
> > 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   | 33 ++++------
> >  .../lenovo/yoga-tab2-pro-1380-fastcharger.c   | 25 +++----
> >  .../platform/x86/x86-android-tablets/core.c   |  2 +-
> >  drivers/power/sequencing/pwrseq-pcie-m2.c     |  2 +-
> >  drivers/tty/serdev/core.c                     | 65 ++++++++++++++-----
> >  include/linux/device-id/serdev.h              | 19 ++++++
> >  include/linux/serdev.h                        | 12 +++-
> >  8 files changed, 105 insertions(+), 54 deletions(-)
> >  create mode 100644 include/linux/device-id/serdev.h
> > 
> > 
> > base-commit: 9e947d96915af63e28896caae85a81b74855621b
> > --
> > 2.34.1
> > 

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