| Message ID | 1455299022-11641-9-git-send-email-aleksey.makarov@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Feb 12, 2016 at 08:43:39PM +0300, Aleksey Makarov wrote: > From: Mark Salter <msalter@redhat.com> > > Add an implementation of acpi_match() to the 8250 driver. > It allows console to check if it matches the console specified by > ACPI SPCR table. > > Signed-off-by: Mark Salter <msalter@redhat.com> > Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org> > --- > drivers/tty/serial/8250/8250_core.c | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c > index 7775221..059338a 100644 > --- a/drivers/tty/serial/8250/8250_core.c > +++ b/drivers/tty/serial/8250/8250_core.c > @@ -40,6 +40,7 @@ > #ifdef CONFIG_SPARC > #include <linux/sunserialcore.h> > #endif > +#include <linux/acpi.h> > > #include <asm/irq.h> > > @@ -669,12 +670,34 @@ static int univ8250_console_match(struct console *co, char *name, int idx, > return -ENODEV; > } > > +static int __init univ8250_console_acpi_match(struct console *co, > + struct acpi_table_spcr *spcr) > +{ > + int index = co->index >= 0 ? co->index : 0; > + struct uart_port *port = &serial8250_ports[index].port; > + > + if (spcr->interface_type != ACPI_DBG2_16550_SUBSET && > + spcr->interface_type != ACPI_DBG2_16550_COMPATIBLE) > + return -ENODEV; > + > + if (spcr->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY && > + spcr->serial_port.address == (u64)port->mapbase) > + return 0; > + > + if (spcr->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_IO && > + spcr->serial_port.address == (u64)port->iobase) > + return 0; > + > + return -ENODEV; > +} Ick, I don't like this, you are doing almost the same thing that univ8250_console_match does, yet you have to know the internals of the acpi_table_spcr structure, which is insane (again, do you want to do this for every firmware type?) Isn't this something that resources are supposed to handle for us? What happened to getting this type of information from the firmware in a non-firmware-specific way to pass to the drivers? Please work on that instead, then you don't have to modify each and every single driver you want to work with your system, they will all "just work" automagically. Please redo this series, as it is, I don't like it at all. thanks, greg k-h
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index 7775221..059338a 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -40,6 +40,7 @@ #ifdef CONFIG_SPARC #include <linux/sunserialcore.h> #endif +#include <linux/acpi.h> #include <asm/irq.h> @@ -669,12 +670,34 @@ static int univ8250_console_match(struct console *co, char *name, int idx, return -ENODEV; } +static int __init univ8250_console_acpi_match(struct console *co, + struct acpi_table_spcr *spcr) +{ + int index = co->index >= 0 ? co->index : 0; + struct uart_port *port = &serial8250_ports[index].port; + + if (spcr->interface_type != ACPI_DBG2_16550_SUBSET && + spcr->interface_type != ACPI_DBG2_16550_COMPATIBLE) + return -ENODEV; + + if (spcr->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY && + spcr->serial_port.address == (u64)port->mapbase) + return 0; + + if (spcr->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_IO && + spcr->serial_port.address == (u64)port->iobase) + return 0; + + return -ENODEV; +} + static struct console univ8250_console = { .name = "ttyS", .write = univ8250_console_write, .device = uart_console_device, .setup = univ8250_console_setup, .match = univ8250_console_match, + .acpi_match = univ8250_console_acpi_match, .flags = CON_PRINTBUFFER | CON_ANYTIME, .index = -1, .data = &serial8250_reg,