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

[Hans de Goede <hansg@kernel.org>]

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@canonical.com>
>>
>> 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.

> 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.

Anyways this is just an idea and somewhat out of scope for
this patch-set.

Regards,

Hans