> > > > Looking at the comment for serial8250_get_port, it sounds like the serial driver > > doesn't really want other drivers using this function outside of suspend/resume > callback. > > > > /** > > * serial8250_get_port - retrieve struct uart_8250_port > > * @line: serial line number > > * > > * This function retrieves struct uart_8250_port for the specific line. > > * This struct *must* *not* be used to perform a 8250 or serial core operation > > * which is not accessible otherwise. Its only purpose is to make the struct > > * accessible to the runtime-pm callbacks for context suspend/restore. > > * The lock assumption made here is none because runtime-pm suspend/resume > > * callbacks should not be invoked if there is any operation performed on the > > * port. > > */ > > http://elixir.free- > electrons.com/linux/v4.13/source/drivers/tty/serial/8250/8250_core.c#L409 > > > > If serial core doesn't provide an API, have you already discussed with serial > maintainer > > to review your approach? It might make sense to work with serial maintainer to > provide > > an API for this type of usecase if it doesn't exist. > Yes, I know the pointer returned from serial8250_get_port() becomes > unavailable after S3. > So, I provide a suspend/resume function try to workaround it. > But just like the comment described, it's risky to perform serial > operations by using the structure, > and there is a new interface to deal with this kind of requirement, serdev. > But serdev is introduced after v4.12, to make it work before serdev, I > can't find other good solutions for it. Right, I know you have a need for this to work on older kernel too, so I think this interface as you did it makes sense on older kernel, but please do use serdev on newer kernel and as this is upstreamed. > > >> + /* unregister acpi backlight interface */ > >> + acpi_video_set_dmi_backlight_type(acpi_backlight_vendor); > > > > Since you change this when the driver is loaded, you should cache the old > > value and restore it when the driver is unloaded too. > > In this case, acpi backlight doesn't work, all the backlight > operations would go through UART commands. > So, it safe to remove it. OK. I think it would be good to include a command about this so that folks who look at this later understand this. Thanks,
