Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1332972 > unrolled thread

[PATCH v2 8/9] serial: 8250: add acpi_match

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 8/9] serial: 8250: add acpi_match Aleksey Makarov <aleksey.makarov@linaro.org> - 2016-02-12 18:50 +0100
    Re: [PATCH v2 8/9] serial: 8250: add acpi_match Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-12 20:00 +0100

#1332972 — [PATCH v2 8/9] serial: 8250: add acpi_match

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;
+}
+
 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,
-- 
2.7.0

[toc] | [next] | [standalone]

#1333044

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web