Re: [PATCH] 8250: serial device file is out of order

2014-08-25 Thread Greg KH
On Tue, Aug 26, 2014 at 11:04:57AM +0800, Scott Yuan wrote:
> On x86 architecture, the configuration of serial device maybe get from ACPI
> DSDT, but the order of DSDT is not mandatory, result as array serial8250_ports
> is out of order. This situation is more obvious in multiple serial port
> mainboard. Sort it by unique id that in DSDT will fix it.

Who cares about the order, you should use the "persistant" names in
/dev/serial/ to connect to things that you really care about which is
which.

> 
> Signed-off-by: Scott Yuan 
> ---
>  drivers/pnp/pnpacpi/core.c  |  1 +
>  drivers/tty/serial/8250/8250_core.c | 40 
> -
>  drivers/tty/serial/8250/8250_pnp.c  | 12 +++
>  include/linux/pnp.h |  1 +
>  include/linux/serial_8250.h |  2 ++
>  5 files changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
> index a5c6cb7..84b518e 100644
> --- a/drivers/pnp/pnpacpi/core.c
> +++ b/drivers/pnp/pnpacpi/core.c
> @@ -254,6 +254,7 @@ static int __init pnpacpi_add_device(struct acpi_device 
> *device)
>   return -ENOMEM;
>  
>   dev->data = device;
> + dev->is_acpi = 1;
>   /* .enabled means the device can decode the resources */
>   dev->active = device->status.enabled;
>   if (acpi_has_method(device->handle, "_SRS"))
> diff --git a/drivers/tty/serial/8250/8250_core.c 
> b/drivers/tty/serial/8250/8250_core.c
> index 7a91c6d..0e5ece5 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -3221,6 +3221,41 @@ static struct uart_8250_port 
> *serial8250_find_match_or_unused(struct uart_port *
>   return NULL;
>  }
>  
> +static struct uart_8250_port *
> +serial8250_find_match_sorted_port(struct uart_port *port)
> +{
> + int i, j;
> + struct uart_8250_port *uart;
> +
> + uart = container_of(port, struct uart_8250_port, port);
> +
> + /*
> +  * We need sorted array serial8250_ports, it is sort by ACPI
> +  * device unique id, so find an apropriate position to insert.
> +  */
> + for (i = 0; i < nr_uarts; i++) {
> + if (serial8250_ports[i].uid == 0 ||
> + uart->uid < serial8250_ports[i].uid)
> + break;
> + }
> +
> + /*
> +  * If current uid that in serial8250_ports is big than port uid,
> +  * then move 8250 port data.
> +  */
> + if (i < nr_uarts - 1 &&
> + uart->uid < serial8250_ports[i].uid) {
> + memmove(_ports[i+1], _ports[i],
> + (nr_uarts - i - 1) * sizeof(struct uart_8250_port));

Heh, no, that's crazy...

> +
> + /* modify port.line, since data moved */
> + for (j = i + 1; j < nr_uarts; j++)
> + serial8250_ports[j].port.line += 1;
> + }
> + serial8250_ports[i].uid = uart->uid;
> + return _ports[i];
> +}

I really don't understand what you are trying to "solve" here.  What is
wrong with the current kernel code?  What does this patch change?

Why is acpi "special"?

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] 8250: serial device file is out of order

2014-08-25 Thread Scott Yuan
On x86 architecture, the configuration of serial device maybe get from ACPI
DSDT, but the order of DSDT is not mandatory, result as array serial8250_ports
is out of order. This situation is more obvious in multiple serial port
mainboard. Sort it by unique id that in DSDT will fix it.

Signed-off-by: Scott Yuan 
---
 drivers/pnp/pnpacpi/core.c  |  1 +
 drivers/tty/serial/8250/8250_core.c | 40 -
 drivers/tty/serial/8250/8250_pnp.c  | 12 +++
 include/linux/pnp.h |  1 +
 include/linux/serial_8250.h |  2 ++
 5 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
index a5c6cb7..84b518e 100644
--- a/drivers/pnp/pnpacpi/core.c
+++ b/drivers/pnp/pnpacpi/core.c
@@ -254,6 +254,7 @@ static int __init pnpacpi_add_device(struct acpi_device 
*device)
return -ENOMEM;
 
dev->data = device;
+   dev->is_acpi = 1;
/* .enabled means the device can decode the resources */
dev->active = device->status.enabled;
if (acpi_has_method(device->handle, "_SRS"))
diff --git a/drivers/tty/serial/8250/8250_core.c 
b/drivers/tty/serial/8250/8250_core.c
index 7a91c6d..0e5ece5 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -3221,6 +3221,41 @@ static struct uart_8250_port 
*serial8250_find_match_or_unused(struct uart_port *
return NULL;
 }
 
+static struct uart_8250_port *
+serial8250_find_match_sorted_port(struct uart_port *port)
+{
+   int i, j;
+   struct uart_8250_port *uart;
+
+   uart = container_of(port, struct uart_8250_port, port);
+
+   /*
+* We need sorted array serial8250_ports, it is sort by ACPI
+* device unique id, so find an apropriate position to insert.
+*/
+   for (i = 0; i < nr_uarts; i++) {
+   if (serial8250_ports[i].uid == 0 ||
+   uart->uid < serial8250_ports[i].uid)
+   break;
+   }
+
+   /*
+* If current uid that in serial8250_ports is big than port uid,
+* then move 8250 port data.
+*/
+   if (i < nr_uarts - 1 &&
+   uart->uid < serial8250_ports[i].uid) {
+   memmove(_ports[i+1], _ports[i],
+   (nr_uarts - i - 1) * sizeof(struct uart_8250_port));
+
+   /* modify port.line, since data moved */
+   for (j = i + 1; j < nr_uarts; j++)
+   serial8250_ports[j].port.line += 1;
+   }
+   serial8250_ports[i].uid = uart->uid;
+   return _ports[i];
+}
+
 /**
  * serial8250_register_8250_port - register a serial port
  * @up: serial port template
@@ -3244,7 +3279,10 @@ int serial8250_register_8250_port(struct uart_8250_port 
*up)
 
mutex_lock(_mutex);
 
-   uart = serial8250_find_match_or_unused(>port);
+   if (up->is_acpi)
+   uart = serial8250_find_match_sorted_port(>port);
+   else
+   uart = serial8250_find_match_or_unused(>port);
if (uart && uart->port.type != PORT_8250_CIR) {
if (uart->port.dev)
uart_remove_one_port(_reg, >port);
diff --git a/drivers/tty/serial/8250/8250_pnp.c 
b/drivers/tty/serial/8250/8250_pnp.c
index 682a2fb..c880ed3 100644
--- a/drivers/tty/serial/8250/8250_pnp.c
+++ b/drivers/tty/serial/8250/8250_pnp.c
@@ -15,6 +15,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -427,6 +429,8 @@ static int
 serial_pnp_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
 {
struct uart_8250_port uart;
+   struct acpi_device *acpi_dev;
+   char *serial_uid;
int ret, line, flags = dev_id->driver_data;
 
if (flags & UNKNOWN_DEV) {
@@ -451,6 +455,14 @@ serial_pnp_probe(struct pnp_dev *dev, const struct 
pnp_device_id *dev_id)
} else
return -ENODEV;
 
+   if (dev->is_acpi) {
+   acpi_dev = dev->data;
+   serial_uid = acpi_dev->pnp.unique_id;
+   uart.is_acpi = 1;
+   if (kstrtoint(serial_uid, 10, ))
+   return -EINVAL;
+   }
+
 #ifdef SERIAL_DEBUG_PNP
printk(KERN_DEBUG
"Setup PNP port: port %x, mem 0x%lx, irq %d, type %d\n",
diff --git a/include/linux/pnp.h b/include/linux/pnp.h
index 195aafc..d745f14 100644
--- a/include/linux/pnp.h
+++ b/include/linux/pnp.h
@@ -245,6 +245,7 @@ struct pnp_dev {
u64 dma_mask;
unsigned int number;/* used as an index, must be unique */
int status;
+   unsigned char is_acpi;  /* is acpi device */
 
struct list_head global_list;   /* node in global list of devices */
struct list_head protocol_list; /* node in list of device's protocol */
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index af47a8a..8f1e1bd 100644
--- 

Re: [PATCH] 8250: serial device file is out of order

2014-08-25 Thread Greg KH
On Mon, Aug 25, 2014 at 07:38:27PM +0800, Scott Yuan wrote:
> From ebb85a61456bf25f87aeb8a31c92c74540bdf306 Mon Sep 17 00:00:00 2001
> From: Scott Yuan 
> Date: Sun, 24 Aug 2014 13:46:56 +0800
> Subject: [PATCH] 8250: serial device file is out of order

Why is this here in the email?  Please use git send-email to not send
this.

> On x86 architecture, the configuration of serial device maybe get from ACPI
> DSDT, but the order of DSDT is not mandatory, result as array
> serial8250_ports
> is out of order. This situation is more obvious in multiple serial port
> mainboard. Sort it by unique id that in DSDT will fix it.
> 
> Signed-off-by: Scott Yuan 
> ---
>  drivers/pnp/pnpacpi/core.c  |  1 +
>  drivers/tty/serial/8250/8250_core.c | 40
> -
>  drivers/tty/serial/8250/8250_pnp.c  | 12 +++
>  include/linux/pnp.h |  1 +
>  include/linux/serial_8250.h |  2 ++
>  5 files changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
> index a5c6cb7..84b518e 100644
> --- a/drivers/pnp/pnpacpi/core.c
> +++ b/drivers/pnp/pnpacpi/core.c
> @@ -254,6 +254,7 @@ static int __init pnpacpi_add_device(struct acpi_device
> *device)

Your patch is line-wrapped and can not be applied :(

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] 8250: serial device file is out of order

2014-08-25 Thread Peter Hurley
Hi Scott,

On 08/25/2014 07:38 AM, Scott Yuan wrote:
> From ebb85a61456bf25f87aeb8a31c92c74540bdf306 Mon Sep 17 00:00:00 2001
> From: Scott Yuan 
> Date: Sun, 24 Aug 2014 13:46:56 +0800
> Subject: [PATCH] 8250: serial device file is out of order
> 
> On x86 architecture, the configuration of serial device maybe get from ACPI
> DSDT, but the order of DSDT is not mandatory, result as array serial8250_ports
> is out of order. This situation is more obvious in multiple serial port
> mainboard. Sort it by unique id that in DSDT will fix it.

Why does this matter?


> 
> Signed-off-by: Scott Yuan 
> ---
>  drivers/pnp/pnpacpi/core.c  |  1 +
>  drivers/tty/serial/8250/8250_core.c | 40 
> -
>  drivers/tty/serial/8250/8250_pnp.c  | 12 +++
>  include/linux/pnp.h |  1 +
>  include/linux/serial_8250.h |  2 ++
>  5 files changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
> index a5c6cb7..84b518e 100644
> --- a/drivers/pnp/pnpacpi/core.c
> +++ b/drivers/pnp/pnpacpi/core.c
> @@ -254,6 +254,7 @@ static int __init pnpacpi_add_device(struct acpi_device 
> *device)
>  return -ENOMEM;
>   dev->data = device;
> +dev->is_acpi = 1;
>  /* .enabled means the device can decode the resources */
>  dev->active = device->status.enabled;
>  if (acpi_has_method(device->handle, "_SRS"))
> diff --git a/drivers/tty/serial/8250/8250_core.c 
> b/drivers/tty/serial/8250/8250_core.c
> index 7a91c6d..0e5ece5 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -3221,6 +3221,41 @@ static struct uart_8250_port 
> *serial8250_find_match_or_unused(struct uart_port *
>  return NULL;
>  }
>  +static struct uart_8250_port *
> +serial8250_find_match_sorted_port(struct uart_port *port)
> +{
> +int i, j;
> +struct uart_8250_port *uart;
> +
> +uart = container_of(port, struct uart_8250_port, port);
> +
> +/*
> + * We need sorted array serial8250_ports, it is sort by ACPI
> + * device unique id, so find an apropriate position to insert.
> + */
> +for (i = 0; i < nr_uarts; i++) {
> +if (serial8250_ports[i].uid == 0 ||
> +uart->uid < serial8250_ports[i].uid)
> +break;
> +}
> +
> +/*
> + * If current uid that in serial8250_ports is big than port uid,
> + * then move 8250 port data.
> + */

The port index cannot be changed after the port is registered with the
tty core. That's because the tty core associates the index at registration
time with a specific tty.

Also, this is thread-unsafe: the 8250 driver uses serial_mutex to
protect concurrent add/remove to the serial8250_ports but otherwise
does not claim that mutex to dereference that table once a given port
index has been assigned.


> +if (i < nr_uarts - 1 &&
> +uart->uid < serial8250_ports[i].uid) {
> +memmove(_ports[i+1], _ports[i],
> +(nr_uarts - i - 1) * sizeof(struct uart_8250_port));
> +
> +/* modify port.line, since data moved */
> +for (j = i + 1; j < nr_uarts; j++)
> +serial8250_ports[j].port.line += 1;
> +}
> +serial8250_ports[i].uid = uart->uid;
> +return _ports[i];
> +}
> +
>  /**
>   *serial8250_register_8250_port - register a serial port
>   *@up: serial port template
> @@ -3244,7 +3279,10 @@ int serial8250_register_8250_port(struct 
> uart_8250_port *up)
>   mutex_lock(_mutex);
>  -uart = serial8250_find_match_or_unused(>port);
> +if (up->is_acpi)
> +uart = serial8250_find_match_sorted_port(>port);
> +else
> +uart = serial8250_find_match_or_unused(>port);
>  if (uart && uart->port.type != PORT_8250_CIR) {
>  if (uart->port.dev)
>  uart_remove_one_port(_reg, >port);
> diff --git a/drivers/tty/serial/8250/8250_pnp.c 
> b/drivers/tty/serial/8250/8250_pnp.c
> index 682a2fb..c880ed3 100644
> --- a/drivers/tty/serial/8250/8250_pnp.c
> +++ b/drivers/tty/serial/8250/8250_pnp.c
> @@ -15,6 +15,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -427,6 +429,8 @@ static int
>  serial_pnp_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
>  {
>  struct uart_8250_port uart;
> +struct acpi_device *acpi_dev;
> +char *serial_uid;
>  int ret, line, flags = dev_id->driver_data;
>   if (flags & UNKNOWN_DEV) {
> @@ -451,6 +455,14 @@ serial_pnp_probe(struct pnp_de

[PATCH] 8250: serial device file is out of order

2014-08-25 Thread Scott Yuan

From ebb85a61456bf25f87aeb8a31c92c74540bdf306 Mon Sep 17 00:00:00 2001
From: Scott Yuan 
Date: Sun, 24 Aug 2014 13:46:56 +0800
Subject: [PATCH] 8250: serial device file is out of order

On x86 architecture, the configuration of serial device maybe get from ACPI
DSDT, but the order of DSDT is not mandatory, result as array 
serial8250_ports

is out of order. This situation is more obvious in multiple serial port
mainboard. Sort it by unique id that in DSDT will fix it.

Signed-off-by: Scott Yuan 
---
 drivers/pnp/pnpacpi/core.c  |  1 +
 drivers/tty/serial/8250/8250_core.c | 40 
-

 drivers/tty/serial/8250/8250_pnp.c  | 12 +++
 include/linux/pnp.h |  1 +
 include/linux/serial_8250.h |  2 ++
 5 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
index a5c6cb7..84b518e 100644
--- a/drivers/pnp/pnpacpi/core.c
+++ b/drivers/pnp/pnpacpi/core.c
@@ -254,6 +254,7 @@ static int __init pnpacpi_add_device(struct 
acpi_device *device)

return -ENOMEM;
dev->data = device;
+   dev->is_acpi = 1;
/* .enabled means the device can decode the resources */
dev->active = device->status.enabled;
if (acpi_has_method(device->handle, "_SRS"))
diff --git a/drivers/tty/serial/8250/8250_core.c 
b/drivers/tty/serial/8250/8250_core.c

index 7a91c6d..0e5ece5 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -3221,6 +3221,41 @@ static struct uart_8250_port 
*serial8250_find_match_or_unused(struct uart_port *

return NULL;
 }
 +static struct uart_8250_port *
+serial8250_find_match_sorted_port(struct uart_port *port)
+{
+   int i, j;
+   struct uart_8250_port *uart;
+
+   uart = container_of(port, struct uart_8250_port, port);
+
+   /*
+* We need sorted array serial8250_ports, it is sort by ACPI
+* device unique id, so find an apropriate position to insert.
+*/
+   for (i = 0; i < nr_uarts; i++) {
+   if (serial8250_ports[i].uid == 0 ||
+   uart->uid < serial8250_ports[i].uid)
+   break;
+   }
+
+   /*
+* If current uid that in serial8250_ports is big than port uid,
+* then move 8250 port data.
+*/
+   if (i < nr_uarts - 1 &&
+   uart->uid < serial8250_ports[i].uid) {
+   memmove(_ports[i+1], _ports[i],
+   (nr_uarts - i - 1) * sizeof(struct uart_8250_port));
+
+   /* modify port.line, since data moved */
+   for (j = i + 1; j < nr_uarts; j++)
+   serial8250_ports[j].port.line += 1;
+   }
+   serial8250_ports[i].uid = uart->uid;
+   return _ports[i];
+}
+
 /**
  * serial8250_register_8250_port - register a serial port
  * @up: serial port template
@@ -3244,7 +3279,10 @@ int serial8250_register_8250_port(struct 
uart_8250_port *up)

mutex_lock(_mutex);
 -  uart = serial8250_find_match_or_unused(>port);
+   if (up->is_acpi)
+   uart = serial8250_find_match_sorted_port(>port);
+   else
+   uart = serial8250_find_match_or_unused(>port);
if (uart && uart->port.type != PORT_8250_CIR) {
if (uart->port.dev)
uart_remove_one_port(_reg, >port);
diff --git a/drivers/tty/serial/8250/8250_pnp.c 
b/drivers/tty/serial/8250/8250_pnp.c

index 682a2fb..c880ed3 100644
--- a/drivers/tty/serial/8250/8250_pnp.c
+++ b/drivers/tty/serial/8250/8250_pnp.c
@@ -15,6 +15,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -427,6 +429,8 @@ static int
 serial_pnp_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
 {
struct uart_8250_port uart;
+   struct acpi_device *acpi_dev;
+   char *serial_uid;
int ret, line, flags = dev_id->driver_data;
if (flags & UNKNOWN_DEV) {
@@ -451,6 +455,14 @@ serial_pnp_probe(struct pnp_dev *dev, const struct 
pnp_device_id *dev_id)

} else
return -ENODEV;
 +  if (dev->is_acpi) {
+   acpi_dev = dev->data;
+   serial_uid = acpi_dev->pnp.unique_id;
+   uart.is_acpi = 1;
+   if (kstrtoint(serial_uid, 10, ))
+   return -EINVAL;
+   }
+
 #ifdef SERIAL_DEBUG_PNP
printk(KERN_DEBUG
"Setup PNP port: port %x, mem 0x%lx, irq %d, type %d\n",
diff --git a/include/linux/pnp.h b/include/linux/pnp.h
index 195aafc..d745f14 100644
--- a/include/linux/pnp.h
+++ b/include/linux/pnp.h
@@ -245,6 +245,7 @@ struct pnp_dev {
u64 dma_mask;
unsigned int number;/* used as an index, must be unique */
int status;
+   unsigned char 

[PATCH] 8250: serial device file is out of order

2014-08-25 Thread Scott Yuan

From ebb85a61456bf25f87aeb8a31c92c74540bdf306 Mon Sep 17 00:00:00 2001
From: Scott Yuan scottze...@gmail.com
Date: Sun, 24 Aug 2014 13:46:56 +0800
Subject: [PATCH] 8250: serial device file is out of order

On x86 architecture, the configuration of serial device maybe get from ACPI
DSDT, but the order of DSDT is not mandatory, result as array 
serial8250_ports

is out of order. This situation is more obvious in multiple serial port
mainboard. Sort it by unique id that in DSDT will fix it.

Signed-off-by: Scott Yuan scottze...@gmail.com
---
 drivers/pnp/pnpacpi/core.c  |  1 +
 drivers/tty/serial/8250/8250_core.c | 40 
-

 drivers/tty/serial/8250/8250_pnp.c  | 12 +++
 include/linux/pnp.h |  1 +
 include/linux/serial_8250.h |  2 ++
 5 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
index a5c6cb7..84b518e 100644
--- a/drivers/pnp/pnpacpi/core.c
+++ b/drivers/pnp/pnpacpi/core.c
@@ -254,6 +254,7 @@ static int __init pnpacpi_add_device(struct 
acpi_device *device)

return -ENOMEM;
dev-data = device;
+   dev-is_acpi = 1;
/* .enabled means the device can decode the resources */
dev-active = device-status.enabled;
if (acpi_has_method(device-handle, _SRS))
diff --git a/drivers/tty/serial/8250/8250_core.c 
b/drivers/tty/serial/8250/8250_core.c

index 7a91c6d..0e5ece5 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -3221,6 +3221,41 @@ static struct uart_8250_port 
*serial8250_find_match_or_unused(struct uart_port *

return NULL;
 }
 +static struct uart_8250_port *
+serial8250_find_match_sorted_port(struct uart_port *port)
+{
+   int i, j;
+   struct uart_8250_port *uart;
+
+   uart = container_of(port, struct uart_8250_port, port);
+
+   /*
+* We need sorted array serial8250_ports, it is sort by ACPI
+* device unique id, so find an apropriate position to insert.
+*/
+   for (i = 0; i  nr_uarts; i++) {
+   if (serial8250_ports[i].uid == 0 ||
+   uart-uid  serial8250_ports[i].uid)
+   break;
+   }
+
+   /*
+* If current uid that in serial8250_ports is big than port uid,
+* then move 8250 port data.
+*/
+   if (i  nr_uarts - 1 
+   uart-uid  serial8250_ports[i].uid) {
+   memmove(serial8250_ports[i+1], serial8250_ports[i],
+   (nr_uarts - i - 1) * sizeof(struct uart_8250_port));
+
+   /* modify port.line, since data moved */
+   for (j = i + 1; j  nr_uarts; j++)
+   serial8250_ports[j].port.line += 1;
+   }
+   serial8250_ports[i].uid = uart-uid;
+   return serial8250_ports[i];
+}
+
 /**
  * serial8250_register_8250_port - register a serial port
  * @up: serial port template
@@ -3244,7 +3279,10 @@ int serial8250_register_8250_port(struct 
uart_8250_port *up)

mutex_lock(serial_mutex);
 -  uart = serial8250_find_match_or_unused(up-port);
+   if (up-is_acpi)
+   uart = serial8250_find_match_sorted_port(up-port);
+   else
+   uart = serial8250_find_match_or_unused(up-port);
if (uart  uart-port.type != PORT_8250_CIR) {
if (uart-port.dev)
uart_remove_one_port(serial8250_reg, uart-port);
diff --git a/drivers/tty/serial/8250/8250_pnp.c 
b/drivers/tty/serial/8250/8250_pnp.c

index 682a2fb..c880ed3 100644
--- a/drivers/tty/serial/8250/8250_pnp.c
+++ b/drivers/tty/serial/8250/8250_pnp.c
@@ -15,6 +15,8 @@
 #include linux/pci.h
 #include linux/pnp.h
 #include linux/string.h
+#include acpi/acpi.h
+#include acpi/acpi_bus.h
 #include linux/kernel.h
 #include linux/serial_core.h
 #include linux/bitops.h
@@ -427,6 +429,8 @@ static int
 serial_pnp_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
 {
struct uart_8250_port uart;
+   struct acpi_device *acpi_dev;
+   char *serial_uid;
int ret, line, flags = dev_id-driver_data;
if (flags  UNKNOWN_DEV) {
@@ -451,6 +455,14 @@ serial_pnp_probe(struct pnp_dev *dev, const struct 
pnp_device_id *dev_id)

} else
return -ENODEV;
 +  if (dev-is_acpi) {
+   acpi_dev = dev-data;
+   serial_uid = acpi_dev-pnp.unique_id;
+   uart.is_acpi = 1;
+   if (kstrtoint(serial_uid, 10, uart.uid))
+   return -EINVAL;
+   }
+
 #ifdef SERIAL_DEBUG_PNP
printk(KERN_DEBUG
Setup PNP port: port %x, mem 0x%lx, irq %d, type %d\n,
diff --git a/include/linux/pnp.h b/include/linux/pnp.h
index 195aafc..d745f14 100644
--- a/include/linux/pnp.h
+++ b/include/linux/pnp.h
@@ -245,6 +245,7 @@ struct pnp_dev {
u64 dma_mask;
unsigned int number;/* used as an index, must

Re: [PATCH] 8250: serial device file is out of order

2014-08-25 Thread Peter Hurley
Hi Scott,

On 08/25/2014 07:38 AM, Scott Yuan wrote:
 From ebb85a61456bf25f87aeb8a31c92c74540bdf306 Mon Sep 17 00:00:00 2001
 From: Scott Yuan scottze...@gmail.com
 Date: Sun, 24 Aug 2014 13:46:56 +0800
 Subject: [PATCH] 8250: serial device file is out of order
 
 On x86 architecture, the configuration of serial device maybe get from ACPI
 DSDT, but the order of DSDT is not mandatory, result as array serial8250_ports
 is out of order. This situation is more obvious in multiple serial port
 mainboard. Sort it by unique id that in DSDT will fix it.

Why does this matter?


 
 Signed-off-by: Scott Yuan scottze...@gmail.com
 ---
  drivers/pnp/pnpacpi/core.c  |  1 +
  drivers/tty/serial/8250/8250_core.c | 40 
 -
  drivers/tty/serial/8250/8250_pnp.c  | 12 +++
  include/linux/pnp.h |  1 +
  include/linux/serial_8250.h |  2 ++
  5 files changed, 55 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
 index a5c6cb7..84b518e 100644
 --- a/drivers/pnp/pnpacpi/core.c
 +++ b/drivers/pnp/pnpacpi/core.c
 @@ -254,6 +254,7 @@ static int __init pnpacpi_add_device(struct acpi_device 
 *device)
  return -ENOMEM;
   dev-data = device;
 +dev-is_acpi = 1;
  /* .enabled means the device can decode the resources */
  dev-active = device-status.enabled;
  if (acpi_has_method(device-handle, _SRS))
 diff --git a/drivers/tty/serial/8250/8250_core.c 
 b/drivers/tty/serial/8250/8250_core.c
 index 7a91c6d..0e5ece5 100644
 --- a/drivers/tty/serial/8250/8250_core.c
 +++ b/drivers/tty/serial/8250/8250_core.c
 @@ -3221,6 +3221,41 @@ static struct uart_8250_port 
 *serial8250_find_match_or_unused(struct uart_port *
  return NULL;
  }
  +static struct uart_8250_port *
 +serial8250_find_match_sorted_port(struct uart_port *port)
 +{
 +int i, j;
 +struct uart_8250_port *uart;
 +
 +uart = container_of(port, struct uart_8250_port, port);
 +
 +/*
 + * We need sorted array serial8250_ports, it is sort by ACPI
 + * device unique id, so find an apropriate position to insert.
 + */
 +for (i = 0; i  nr_uarts; i++) {
 +if (serial8250_ports[i].uid == 0 ||
 +uart-uid  serial8250_ports[i].uid)
 +break;
 +}
 +
 +/*
 + * If current uid that in serial8250_ports is big than port uid,
 + * then move 8250 port data.
 + */

The port index cannot be changed after the port is registered with the
tty core. That's because the tty core associates the index at registration
time with a specific tty.

Also, this is thread-unsafe: the 8250 driver uses serial_mutex to
protect concurrent add/remove to the serial8250_ports but otherwise
does not claim that mutex to dereference that table once a given port
index has been assigned.


 +if (i  nr_uarts - 1 
 +uart-uid  serial8250_ports[i].uid) {
 +memmove(serial8250_ports[i+1], serial8250_ports[i],
 +(nr_uarts - i - 1) * sizeof(struct uart_8250_port));
 +
 +/* modify port.line, since data moved */
 +for (j = i + 1; j  nr_uarts; j++)
 +serial8250_ports[j].port.line += 1;
 +}
 +serial8250_ports[i].uid = uart-uid;
 +return serial8250_ports[i];
 +}
 +
  /**
   *serial8250_register_8250_port - register a serial port
   *@up: serial port template
 @@ -3244,7 +3279,10 @@ int serial8250_register_8250_port(struct 
 uart_8250_port *up)
   mutex_lock(serial_mutex);
  -uart = serial8250_find_match_or_unused(up-port);
 +if (up-is_acpi)
 +uart = serial8250_find_match_sorted_port(up-port);
 +else
 +uart = serial8250_find_match_or_unused(up-port);
  if (uart  uart-port.type != PORT_8250_CIR) {
  if (uart-port.dev)
  uart_remove_one_port(serial8250_reg, uart-port);
 diff --git a/drivers/tty/serial/8250/8250_pnp.c 
 b/drivers/tty/serial/8250/8250_pnp.c
 index 682a2fb..c880ed3 100644
 --- a/drivers/tty/serial/8250/8250_pnp.c
 +++ b/drivers/tty/serial/8250/8250_pnp.c
 @@ -15,6 +15,8 @@
  #include linux/pci.h
  #include linux/pnp.h
  #include linux/string.h
 +#include acpi/acpi.h
 +#include acpi/acpi_bus.h
  #include linux/kernel.h
  #include linux/serial_core.h
  #include linux/bitops.h
 @@ -427,6 +429,8 @@ static int
  serial_pnp_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
  {
  struct uart_8250_port uart;
 +struct acpi_device *acpi_dev;
 +char *serial_uid;
  int ret, line, flags = dev_id-driver_data;
   if (flags  UNKNOWN_DEV) {
 @@ -451,6 +455,14 @@ serial_pnp_probe(struct pnp_dev *dev, const struct 
 pnp_device_id *dev_id)
  } else
  return -ENODEV;
  +if (dev-is_acpi) {
 +acpi_dev = dev-data;
 +serial_uid = acpi_dev-pnp.unique_id;
 +uart.is_acpi = 1;
 +if (kstrtoint(serial_uid, 10, uart.uid))
 +return -EINVAL;
 +}
 +
  #ifdef SERIAL_DEBUG_PNP
  printk(KERN_DEBUG

Re: [PATCH] 8250: serial device file is out of order

2014-08-25 Thread Greg KH
On Mon, Aug 25, 2014 at 07:38:27PM +0800, Scott Yuan wrote:
 From ebb85a61456bf25f87aeb8a31c92c74540bdf306 Mon Sep 17 00:00:00 2001
 From: Scott Yuan scottze...@gmail.com
 Date: Sun, 24 Aug 2014 13:46:56 +0800
 Subject: [PATCH] 8250: serial device file is out of order

Why is this here in the email?  Please use git send-email to not send
this.

 On x86 architecture, the configuration of serial device maybe get from ACPI
 DSDT, but the order of DSDT is not mandatory, result as array
 serial8250_ports
 is out of order. This situation is more obvious in multiple serial port
 mainboard. Sort it by unique id that in DSDT will fix it.
 
 Signed-off-by: Scott Yuan scottze...@gmail.com
 ---
  drivers/pnp/pnpacpi/core.c  |  1 +
  drivers/tty/serial/8250/8250_core.c | 40
 -
  drivers/tty/serial/8250/8250_pnp.c  | 12 +++
  include/linux/pnp.h |  1 +
  include/linux/serial_8250.h |  2 ++
  5 files changed, 55 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
 index a5c6cb7..84b518e 100644
 --- a/drivers/pnp/pnpacpi/core.c
 +++ b/drivers/pnp/pnpacpi/core.c
 @@ -254,6 +254,7 @@ static int __init pnpacpi_add_device(struct acpi_device
 *device)

Your patch is line-wrapped and can not be applied :(

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] 8250: serial device file is out of order

2014-08-25 Thread Scott Yuan
On x86 architecture, the configuration of serial device maybe get from ACPI
DSDT, but the order of DSDT is not mandatory, result as array serial8250_ports
is out of order. This situation is more obvious in multiple serial port
mainboard. Sort it by unique id that in DSDT will fix it.

Signed-off-by: Scott Yuan scottze...@gmail.com
---
 drivers/pnp/pnpacpi/core.c  |  1 +
 drivers/tty/serial/8250/8250_core.c | 40 -
 drivers/tty/serial/8250/8250_pnp.c  | 12 +++
 include/linux/pnp.h |  1 +
 include/linux/serial_8250.h |  2 ++
 5 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
index a5c6cb7..84b518e 100644
--- a/drivers/pnp/pnpacpi/core.c
+++ b/drivers/pnp/pnpacpi/core.c
@@ -254,6 +254,7 @@ static int __init pnpacpi_add_device(struct acpi_device 
*device)
return -ENOMEM;
 
dev-data = device;
+   dev-is_acpi = 1;
/* .enabled means the device can decode the resources */
dev-active = device-status.enabled;
if (acpi_has_method(device-handle, _SRS))
diff --git a/drivers/tty/serial/8250/8250_core.c 
b/drivers/tty/serial/8250/8250_core.c
index 7a91c6d..0e5ece5 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -3221,6 +3221,41 @@ static struct uart_8250_port 
*serial8250_find_match_or_unused(struct uart_port *
return NULL;
 }
 
+static struct uart_8250_port *
+serial8250_find_match_sorted_port(struct uart_port *port)
+{
+   int i, j;
+   struct uart_8250_port *uart;
+
+   uart = container_of(port, struct uart_8250_port, port);
+
+   /*
+* We need sorted array serial8250_ports, it is sort by ACPI
+* device unique id, so find an apropriate position to insert.
+*/
+   for (i = 0; i  nr_uarts; i++) {
+   if (serial8250_ports[i].uid == 0 ||
+   uart-uid  serial8250_ports[i].uid)
+   break;
+   }
+
+   /*
+* If current uid that in serial8250_ports is big than port uid,
+* then move 8250 port data.
+*/
+   if (i  nr_uarts - 1 
+   uart-uid  serial8250_ports[i].uid) {
+   memmove(serial8250_ports[i+1], serial8250_ports[i],
+   (nr_uarts - i - 1) * sizeof(struct uart_8250_port));
+
+   /* modify port.line, since data moved */
+   for (j = i + 1; j  nr_uarts; j++)
+   serial8250_ports[j].port.line += 1;
+   }
+   serial8250_ports[i].uid = uart-uid;
+   return serial8250_ports[i];
+}
+
 /**
  * serial8250_register_8250_port - register a serial port
  * @up: serial port template
@@ -3244,7 +3279,10 @@ int serial8250_register_8250_port(struct uart_8250_port 
*up)
 
mutex_lock(serial_mutex);
 
-   uart = serial8250_find_match_or_unused(up-port);
+   if (up-is_acpi)
+   uart = serial8250_find_match_sorted_port(up-port);
+   else
+   uart = serial8250_find_match_or_unused(up-port);
if (uart  uart-port.type != PORT_8250_CIR) {
if (uart-port.dev)
uart_remove_one_port(serial8250_reg, uart-port);
diff --git a/drivers/tty/serial/8250/8250_pnp.c 
b/drivers/tty/serial/8250/8250_pnp.c
index 682a2fb..c880ed3 100644
--- a/drivers/tty/serial/8250/8250_pnp.c
+++ b/drivers/tty/serial/8250/8250_pnp.c
@@ -15,6 +15,8 @@
 #include linux/pci.h
 #include linux/pnp.h
 #include linux/string.h
+#include acpi/acpi.h
+#include acpi/acpi_bus.h
 #include linux/kernel.h
 #include linux/serial_core.h
 #include linux/bitops.h
@@ -427,6 +429,8 @@ static int
 serial_pnp_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
 {
struct uart_8250_port uart;
+   struct acpi_device *acpi_dev;
+   char *serial_uid;
int ret, line, flags = dev_id-driver_data;
 
if (flags  UNKNOWN_DEV) {
@@ -451,6 +455,14 @@ serial_pnp_probe(struct pnp_dev *dev, const struct 
pnp_device_id *dev_id)
} else
return -ENODEV;
 
+   if (dev-is_acpi) {
+   acpi_dev = dev-data;
+   serial_uid = acpi_dev-pnp.unique_id;
+   uart.is_acpi = 1;
+   if (kstrtoint(serial_uid, 10, uart.uid))
+   return -EINVAL;
+   }
+
 #ifdef SERIAL_DEBUG_PNP
printk(KERN_DEBUG
Setup PNP port: port %x, mem 0x%lx, irq %d, type %d\n,
diff --git a/include/linux/pnp.h b/include/linux/pnp.h
index 195aafc..d745f14 100644
--- a/include/linux/pnp.h
+++ b/include/linux/pnp.h
@@ -245,6 +245,7 @@ struct pnp_dev {
u64 dma_mask;
unsigned int number;/* used as an index, must be unique */
int status;
+   unsigned char is_acpi;  /* is acpi device */
 
struct list_head global_list;   /* node in global list of devices */
struct list_head 

Re: [PATCH] 8250: serial device file is out of order

2014-08-25 Thread Greg KH
On Tue, Aug 26, 2014 at 11:04:57AM +0800, Scott Yuan wrote:
 On x86 architecture, the configuration of serial device maybe get from ACPI
 DSDT, but the order of DSDT is not mandatory, result as array serial8250_ports
 is out of order. This situation is more obvious in multiple serial port
 mainboard. Sort it by unique id that in DSDT will fix it.

Who cares about the order, you should use the persistant names in
/dev/serial/ to connect to things that you really care about which is
which.

 
 Signed-off-by: Scott Yuan scottze...@gmail.com
 ---
  drivers/pnp/pnpacpi/core.c  |  1 +
  drivers/tty/serial/8250/8250_core.c | 40 
 -
  drivers/tty/serial/8250/8250_pnp.c  | 12 +++
  include/linux/pnp.h |  1 +
  include/linux/serial_8250.h |  2 ++
  5 files changed, 55 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
 index a5c6cb7..84b518e 100644
 --- a/drivers/pnp/pnpacpi/core.c
 +++ b/drivers/pnp/pnpacpi/core.c
 @@ -254,6 +254,7 @@ static int __init pnpacpi_add_device(struct acpi_device 
 *device)
   return -ENOMEM;
  
   dev-data = device;
 + dev-is_acpi = 1;
   /* .enabled means the device can decode the resources */
   dev-active = device-status.enabled;
   if (acpi_has_method(device-handle, _SRS))
 diff --git a/drivers/tty/serial/8250/8250_core.c 
 b/drivers/tty/serial/8250/8250_core.c
 index 7a91c6d..0e5ece5 100644
 --- a/drivers/tty/serial/8250/8250_core.c
 +++ b/drivers/tty/serial/8250/8250_core.c
 @@ -3221,6 +3221,41 @@ static struct uart_8250_port 
 *serial8250_find_match_or_unused(struct uart_port *
   return NULL;
  }
  
 +static struct uart_8250_port *
 +serial8250_find_match_sorted_port(struct uart_port *port)
 +{
 + int i, j;
 + struct uart_8250_port *uart;
 +
 + uart = container_of(port, struct uart_8250_port, port);
 +
 + /*
 +  * We need sorted array serial8250_ports, it is sort by ACPI
 +  * device unique id, so find an apropriate position to insert.
 +  */
 + for (i = 0; i  nr_uarts; i++) {
 + if (serial8250_ports[i].uid == 0 ||
 + uart-uid  serial8250_ports[i].uid)
 + break;
 + }
 +
 + /*
 +  * If current uid that in serial8250_ports is big than port uid,
 +  * then move 8250 port data.
 +  */
 + if (i  nr_uarts - 1 
 + uart-uid  serial8250_ports[i].uid) {
 + memmove(serial8250_ports[i+1], serial8250_ports[i],
 + (nr_uarts - i - 1) * sizeof(struct uart_8250_port));

Heh, no, that's crazy...

 +
 + /* modify port.line, since data moved */
 + for (j = i + 1; j  nr_uarts; j++)
 + serial8250_ports[j].port.line += 1;
 + }
 + serial8250_ports[i].uid = uart-uid;
 + return serial8250_ports[i];
 +}

I really don't understand what you are trying to solve here.  What is
wrong with the current kernel code?  What does this patch change?

Why is acpi special?

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/