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

[An Wu]

Hi Hans

> I see. Makes me wonder though how that -ENODEV is a problem,
> it is weird. But is it a problem?
>
> The v1 patch writes the following about this:
>
> "This makes the modalias sysfs read fail and suppresses the device
> add uevent, even though the device is registered and bound to its driver."
>
> Ok, but the backlight driver ultimately ends up adding a
> /sys/class/backlight device which I assume does get
> a uevent, or is the problem that one never getting a uevent
> because of its parent not getting one ?
>
> And the lenovo-yoga-tab2-pro-1380-fastcharger driver does all
> it needs to do fully in kernel and does not even export
> a userspace interface.

You are right: the backlight class device is registered independently, so
the missing add uevent for the serdev client does not prevent the
backlight from working. I would not consider the missing uevent itself a
user-visible problem.

However, as Markus pointed out, serial0-0 is an existing serdev device
whose modalias read fails with -ENODEV. That is inappropriate core
behavior.

BR
    An

On Thu, Oct 22, 2026 at 2:14 AM Markus Probst <markus.probst@xxxxxxxxx> wrote:
>
> On Wed, 2026-10-21 at 20:07 +0200, Hans de Goede wrote:
> > Hi,
> >
> > On 21-Oct-26 17:08, Markus Probst wrote:
> > > On Wed, 2026-10-21 at 16:32 +0200, Hans de Goede wrote:
> > > > Hi,
> > > >
> > > > On 21-Oct-26 11:21, 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@xxxxxxxxxxxxx>
> > > >
> > > > For v3 of this series please include a cover-letter and explain
> > > > why this series is an improvement over the` existing situation.
> > > >
> > > > The dell-uart-backlight and lenovo-yoga-tab2-pro-1380-fastcharger
> > > > drivers are both pretty special cases and here you are adding
> > > > a chunk of code to the serdev-core for those special cases,
> > > > so I wonder: why?
> > > >
> > > > The current manual attach approach taking in these 2 special
> > > > drivers works fine.
> > > >
> > > > What does this new approach gain us ?
> > >
> > > That was my review comment [1] that resulted in this.
> >
> > Thanks, that is useful info and the kinda info I would
> > expect in a cover-letter :)
> > > From what I can tell from revision 1, ChunAn Wu is trying to fix that
> > > accessing uevent and modalias of the device (via sysfs) results in a -
> > > ENODEV error.
> >
> > I see. Makes me wonder though how that -ENODEV is a problem,
> > it is weird. But is it a problem?
> >
> > The v1 patch writes the following about this:
> >
> > "This makes the modalias sysfs read fail and suppresses the device
> > add uevent, even though the device is registered and bound to its driver."
> >
> > Ok, but the backlight driver ultimately ends up adding a
> > /sys/class/backlight device which I assume does get
> > a uevent, or is the problem that one never getting a uevent
> > because of its parent not getting one ?
> >
> > And the lenovo-yoga-tab2-pro-1380-fastcharger driver does all
> > it needs to do fully in kernel and does not even export
> > a userspace interface.
> I am not sure which issue is solves precisely. Maybe udev doesn't like
> it?
>
> ChunAn Wu has to answer that.
>
> Returning -ENODEV on an existing device there is likely not intended.
>
>
> >
> > > As mentioned in my review:
> > >
> > > - "/* TODO: platform modalias */"
> > > - "/* TODO: platform matching */"
> > > still exist in the serdev codebase (drivers/tty/serdev/core.c) and are
> > > unresolved. Resolving those also fixes the issue mentioned above.
> > > Therefore I think this would be a better approach.
> >
> > With that said given that there are TODO comments for this
> > I guess addressing the TODO-s is ok.
> > > A cover letter is ofc still needed.
> >
> > Ack.
> >
> > > > Also if you are going to do this may I suggest looking at doing
> > > > some sort of of-compatible matching based on a compatible string
> > > > inside a swnode ? I do not know what that would look like but
> > > > that is something which seems like it might be generally useful
> > > > also outside these 2 special cases.
> > >
> > > I haven't used them before, but I don't think of-compatible string
> > > matching inside a swnode is done anywhere in the kernel (At least I
> > > haven't found a case).
> > >
> > > They can be added supplementary if needed.
> >
> > The idea was to make this a generic match mechanism / helper
> > function busses can call, like the current of and acpi
> > match (and modalias) helpers. And then have matching by
> > a compatible string instead of by name since matching by name
> > only works if there is only 1 instance.
> Please take a look at my previous review comment [1].
>
> In this I suggested adding a `id_table` field to `serdev_driver`.
> This allows to match multiple instances, including providing different
> private datas.
>
> software node is only relevant I think if you already use
> `fwnode_property_read_string` inside a driver next to acpi and of, and
> also want to handle devices without of or acpi.
>
> Thanks
> - Markus Probst
>
> [1]
> https://lore.kernel.org/linux-serial/0be5624e3778c3426f88daf2d67376435f474497.camel@xxxxxxxxx/
>
> >
> > Anyways this is just an idea and somewhat out of scope for
> > this patch-set.
> >
> > Regards,
> >
> > Hans
> >