lkml.org 
[lkml]   [2022]   [Nov]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 1/1] serial: core: Start managing serial controllers to enable runtime PM
Hi,

I am returning to v2, as I managed to read only v3 and only now. But
here was already the point below.

On 27. 06. 22, 15:48, Tony Lindgren wrote:
>>> Considering the above, let's improve the serial core layer so we can
>>> manage the serial port controllers better. Let's register the controllers
>>> with the serial core layer in addition to the serial ports.
>>
>> Why can't controllers be a device as well?
>
> The controllers are devices already probed by the serial port drivers.
> What's missing is mapping the ports (as devices based on the comments
> above) to the controller devices. I don't think we need another struct
> device for the serial controller in addition to the serial port driver
> device and it's child port devices.

To be honest, I don't like the patch (even v3). We have uart_state which
I already hate and now we have another structure holding *some* other
info about a serial device (apart from uart_port). It's mess already and
hard to follow, esp. to newcomers.

AFAIU, what Greg suggests would be:

PCI/platform/acpi/whatever struct dev
-> serial controller 1 struct dev
-> serial port 1 struct dev (tty_port instance exists for this)
-> serial port 2 struct dev (tty_port instance exists for this)
-> ...
-> serial controller 2 struct dev
-> serial port 1 struct dev (tty_port instance exists for this)
-> serial port 2 struct dev (tty_port instance exists for this)
-> ...

And you are objecting that mostly (or in all cases?), there will never
be "serial controller 2"?

But given your description, I believe you need it anyway -- side note:
does really the PM layer/or you need it or would you be fine with
"serial port N" dev children? But provided you don't have the
controller, you work around it by struct serial_controller. So what's
actually the point of the workaround instead of sticking to proper
driver model? With the workaround you seem you have to implement all the
binding, lookup and such yourself anyway. And that renders the serial
even worse :P. Let's do the reverse instead.

The only thing I am not sure about, whether tty_port should be struct
dev too -- and if it should have serial port 1 as a parent. But likely
so. And then with pure tty (i.e. tty_driver's, not uart_driver's), it
would have PCI/platform/acpi/whatever as a parent directly.

In sum, the above structure makes perfect sense to me. There has only
been noone to do the real work yet. And having tty_port was a hard
prerequisite for this to happen. And that happened long time ago. All
this would need a lot of work initially¹⁾, but it paid off a lot in long
term.

¹⁾I know what I am writing about -- I converted HID. After all, the core
was only 1000 lines patch (cf 85cdaf524b7d) + patches to convert all the
drivers incrementally (like 8c19a51591).

thanks,
--
js
suse labs

\
 
 \ /
  Last update: 2022-11-24 07:54    [W:0.181 / U:0.524 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site