Re: [PATCH v2 1/1] serial: core: Start managing serial controllers to enable runtime PM

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux