* [PATCH] serial: earlycon: Extend earlycon command line option to support 64-bit addresses @ 2016-09-02 11:20 Alexander Sverdlin 2016-09-11 0:45 ` Guenter Roeck 0 siblings, 1 reply; 4+ messages in thread From: Alexander Sverdlin @ 2016-09-02 11:20 UTC (permalink / raw) To: linux-kernel, linux-serial; +Cc: Jiri Slaby, Greg Kroah-Hartman earlycon implementation used "unsigned long" internally, but there are systems (ARM with LPAE) where sizeof(unsigned long) == 4 and uart is mapped beyond 4GiB address range. Switch to resource_size_t internally and replace obsoleted simple_strtoul() with kstrtoull(). Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com> --- drivers/tty/serial/8250/8250_core.c | 2 +- drivers/tty/serial/earlycon.c | 7 +++---- drivers/tty/serial/serial_core.c | 12 +++++++++--- include/linux/serial_core.h | 2 +- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index 13ad5c3..f64d6cd 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -639,7 +639,7 @@ static int univ8250_console_match(struct console *co, char *name, int idx, { char match[] = "uart"; /* 8250-specific earlycon name */ unsigned char iotype; - unsigned long addr; + resource_size_t addr; int i; if (strncmp(name, match, 4) != 0) diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c index 067783f..3940280 100644 --- a/drivers/tty/serial/earlycon.c +++ b/drivers/tty/serial/earlycon.c @@ -38,7 +38,7 @@ static struct earlycon_device early_console_dev = { .con = &early_con, }; -static void __iomem * __init earlycon_map(unsigned long paddr, size_t size) +static void __iomem * __init earlycon_map(resource_size_t paddr, size_t size) { void __iomem *base; #ifdef CONFIG_FIX_EARLYCON_MEM @@ -49,8 +49,7 @@ static void __iomem * __init earlycon_map(unsigned long paddr, size_t size) base = ioremap(paddr, size); #endif if (!base) - pr_err("%s: Couldn't map 0x%llx\n", __func__, - (unsigned long long)paddr); + pr_err("%s: Couldn't map %pa\n", __func__, &paddr); return base; } @@ -92,7 +91,7 @@ static int __init parse_options(struct earlycon_device *device, char *options) { struct uart_port *port = &device->port; int length; - unsigned long addr; + resource_size_t addr; if (uart_parse_earlycon(options, &port->iotype, &addr, &options)) return -EINVAL; diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 9fc1533..89f5d6a 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -1938,11 +1938,14 @@ uart_get_console(struct uart_port *ports, int nr, struct console *co) * console=<name>,0x<addr>,<options> * is also accepted; the returned @iotype will be UPIO_MEM. * - * Returns 0 on success or -EINVAL on failure + * Returns 0 on success, -EINVAL or -ERANGE on failure */ -int uart_parse_earlycon(char *p, unsigned char *iotype, unsigned long *addr, +int uart_parse_earlycon(char *p, unsigned char *iotype, resource_size_t *addr, char **options) { + int ret; + unsigned long long tmp; + if (strncmp(p, "mmio,", 5) == 0) { *iotype = UPIO_MEM; p += 5; @@ -1968,7 +1971,10 @@ int uart_parse_earlycon(char *p, unsigned char *iotype, unsigned long *addr, return -EINVAL; } - *addr = simple_strtoul(p, NULL, 0); + ret = kstrtoull(p, 0, &tmp); + if (ret) + return ret; + *addr = tmp; p = strchr(p, ','); if (p) p++; diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h index 2f44e20..cdba6f1 100644 --- a/include/linux/serial_core.h +++ b/include/linux/serial_core.h @@ -374,7 +374,7 @@ extern int of_setup_earlycon(const struct earlycon_id *match, struct uart_port *uart_get_console(struct uart_port *ports, int nr, struct console *c); -int uart_parse_earlycon(char *p, unsigned char *iotype, unsigned long *addr, +int uart_parse_earlycon(char *p, unsigned char *iotype, resource_size_t *addr, char **options); void uart_parse_options(char *options, int *baud, int *parity, int *bits, int *flow); ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: serial: earlycon: Extend earlycon command line option to support 64-bit addresses 2016-09-02 11:20 [PATCH] serial: earlycon: Extend earlycon command line option to support 64-bit addresses Alexander Sverdlin @ 2016-09-11 0:45 ` Guenter Roeck 2016-09-12 11:29 ` [PATCH] serial: core: Fix handling of options after MMIO address Alexander Sverdlin 0 siblings, 1 reply; 4+ messages in thread From: Guenter Roeck @ 2016-09-11 0:45 UTC (permalink / raw) To: Alexander Sverdlin Cc: linux-kernel, linux-serial, Jiri Slaby, Greg Kroah-Hartman Hi, On Fri, Sep 02, 2016 at 01:20:21PM +0200, Alexander Sverdlin wrote: > earlycon implementation used "unsigned long" internally, but there are systems > (ARM with LPAE) where sizeof(unsigned long) == 4 and uart is mapped beyond 4GiB > address range. > > Switch to resource_size_t internally and replace obsoleted simple_strtoul() with > kstrtoull(). > This patch breaks a qemu emulation using console=uart,mmio,0x90000000,115200 as command line argument. Problem appears to be the replacement of simple_strtoul() with kstrtoull(). It looks like simple_strtoul() parsed the address, but kstrtoull() doesn't. Reverting the kstrtoull() back to simple_strtoul() fixes the problem for me. The command line in question is found in arch/openrisc/boot/dts/or1ksim.dts. There are similar command lines in other dts files, which makes me suspect that this construct is also used by out-of-kernel command lines. Besides, the code looks for ',' after the address, which suggests that the above should be a valid argument. Is this change to kstrtoull really necessary ? If yes, can you make it work with existing command lines ? Thanks, Guenter > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com> > --- > drivers/tty/serial/8250/8250_core.c | 2 +- > drivers/tty/serial/earlycon.c | 7 +++---- > drivers/tty/serial/serial_core.c | 12 +++++++++--- > include/linux/serial_core.h | 2 +- > 4 files changed, 14 insertions(+), 9 deletions(-) > > diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c > index 13ad5c3..f64d6cd 100644 > --- a/drivers/tty/serial/8250/8250_core.c > +++ b/drivers/tty/serial/8250/8250_core.c > @@ -639,7 +639,7 @@ static int univ8250_console_match(struct console *co, char *name, int idx, > { > char match[] = "uart"; /* 8250-specific earlycon name */ > unsigned char iotype; > - unsigned long addr; > + resource_size_t addr; > int i; > > if (strncmp(name, match, 4) != 0) > diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c > index 067783f..3940280 100644 > --- a/drivers/tty/serial/earlycon.c > +++ b/drivers/tty/serial/earlycon.c > @@ -38,7 +38,7 @@ static struct earlycon_device early_console_dev = { > .con = &early_con, > }; > > -static void __iomem * __init earlycon_map(unsigned long paddr, size_t size) > +static void __iomem * __init earlycon_map(resource_size_t paddr, size_t size) > { > void __iomem *base; > #ifdef CONFIG_FIX_EARLYCON_MEM > @@ -49,8 +49,7 @@ static void __iomem * __init earlycon_map(unsigned long paddr, size_t size) > base = ioremap(paddr, size); > #endif > if (!base) > - pr_err("%s: Couldn't map 0x%llx\n", __func__, > - (unsigned long long)paddr); > + pr_err("%s: Couldn't map %pa\n", __func__, &paddr); > > return base; > } > @@ -92,7 +91,7 @@ static int __init parse_options(struct earlycon_device *device, char *options) > { > struct uart_port *port = &device->port; > int length; > - unsigned long addr; > + resource_size_t addr; > > if (uart_parse_earlycon(options, &port->iotype, &addr, &options)) > return -EINVAL; > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c > index 9fc1533..89f5d6a 100644 > --- a/drivers/tty/serial/serial_core.c > +++ b/drivers/tty/serial/serial_core.c > @@ -1938,11 +1938,14 @@ uart_get_console(struct uart_port *ports, int nr, struct console *co) > * console=<name>,0x<addr>,<options> > * is also accepted; the returned @iotype will be UPIO_MEM. > * > - * Returns 0 on success or -EINVAL on failure > + * Returns 0 on success, -EINVAL or -ERANGE on failure > */ > -int uart_parse_earlycon(char *p, unsigned char *iotype, unsigned long *addr, > +int uart_parse_earlycon(char *p, unsigned char *iotype, resource_size_t *addr, > char **options) > { > + int ret; > + unsigned long long tmp; > + > if (strncmp(p, "mmio,", 5) == 0) { > *iotype = UPIO_MEM; > p += 5; > @@ -1968,7 +1971,10 @@ int uart_parse_earlycon(char *p, unsigned char *iotype, unsigned long *addr, > return -EINVAL; > } > > - *addr = simple_strtoul(p, NULL, 0); > + ret = kstrtoull(p, 0, &tmp); > + if (ret) > + return ret; > + *addr = tmp; > p = strchr(p, ','); > if (p) > p++; > diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h > index 2f44e20..cdba6f1 100644 > --- a/include/linux/serial_core.h > +++ b/include/linux/serial_core.h > @@ -374,7 +374,7 @@ extern int of_setup_earlycon(const struct earlycon_id *match, > > struct uart_port *uart_get_console(struct uart_port *ports, int nr, > struct console *c); > -int uart_parse_earlycon(char *p, unsigned char *iotype, unsigned long *addr, > +int uart_parse_earlycon(char *p, unsigned char *iotype, resource_size_t *addr, > char **options); > void uart_parse_options(char *options, int *baud, int *parity, int *bits, > int *flow); ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] serial: core: Fix handling of options after MMIO address 2016-09-11 0:45 ` Guenter Roeck @ 2016-09-12 11:29 ` Alexander Sverdlin 2016-09-12 13:27 ` Guenter Roeck 0 siblings, 1 reply; 4+ messages in thread From: Alexander Sverdlin @ 2016-09-12 11:29 UTC (permalink / raw) To: Guenter Roeck, linux-kernel; +Cc: linux-serial, Jiri Slaby, Greg Kroah-Hartman Guenter Roeck reported a regression caused by commit "serial: earlycon: Extend earlycon command line option to support 64-bit addresses": console= and earlycon= options have the following format: ...,<addr>,<options> Historically used here simple_strtoul() had no problems with comma, but the new and recommended kstrtoull() requires null-terminated string and returns -EINVAL in case there are "options" at the end. There is no recommended to use function currently that will support it, so stick to obsolete simple_strtoull() variant. Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com> Reported-by: Guenter Roeck <linux@roeck-us.net> --- drivers/tty/serial/serial_core.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 89f5d6a..f2c8a8d 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -1938,14 +1938,11 @@ uart_get_console(struct uart_port *ports, int nr, struct console *co) * console=<name>,0x<addr>,<options> * is also accepted; the returned @iotype will be UPIO_MEM. * - * Returns 0 on success, -EINVAL or -ERANGE on failure + * Returns 0 on success or -EINVAL on failure */ int uart_parse_earlycon(char *p, unsigned char *iotype, resource_size_t *addr, char **options) { - int ret; - unsigned long long tmp; - if (strncmp(p, "mmio,", 5) == 0) { *iotype = UPIO_MEM; p += 5; @@ -1971,10 +1968,11 @@ int uart_parse_earlycon(char *p, unsigned char *iotype, resource_size_t *addr, return -EINVAL; } - ret = kstrtoull(p, 0, &tmp); - if (ret) - return ret; - *addr = tmp; + /* + * Before you replace it with kstrtoull(), think about options separator + * (',') it will not tolerate + */ + *addr = simple_strtoull(p, NULL, 0); p = strchr(p, ','); if (p) p++; ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] serial: core: Fix handling of options after MMIO address 2016-09-12 11:29 ` [PATCH] serial: core: Fix handling of options after MMIO address Alexander Sverdlin @ 2016-09-12 13:27 ` Guenter Roeck 0 siblings, 0 replies; 4+ messages in thread From: Guenter Roeck @ 2016-09-12 13:27 UTC (permalink / raw) To: Alexander Sverdlin, linux-kernel Cc: linux-serial, Jiri Slaby, Greg Kroah-Hartman On 09/12/2016 04:29 AM, Alexander Sverdlin wrote: > Guenter Roeck reported a regression caused by commit "serial: earlycon: > Extend earlycon command line option to support 64-bit addresses": > > console= and earlycon= options have the following format: > ...,<addr>,<options> > > Historically used here simple_strtoul() had no problems with comma, but the > new and recommended kstrtoull() requires null-terminated string and returns > -EINVAL in case there are "options" at the end. There is no recommended to > use function currently that will support it, so stick to obsolete > simple_strtoull() variant. > > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com> > Reported-by: Guenter Roeck <linux@roeck-us.net> Reviewed-by: Guenter Roeck <linux@roeck-us.net> Tested-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/tty/serial/serial_core.c | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c > index 89f5d6a..f2c8a8d 100644 > --- a/drivers/tty/serial/serial_core.c > +++ b/drivers/tty/serial/serial_core.c > @@ -1938,14 +1938,11 @@ uart_get_console(struct uart_port *ports, int nr, struct console *co) > * console=<name>,0x<addr>,<options> > * is also accepted; the returned @iotype will be UPIO_MEM. > * > - * Returns 0 on success, -EINVAL or -ERANGE on failure > + * Returns 0 on success or -EINVAL on failure > */ > int uart_parse_earlycon(char *p, unsigned char *iotype, resource_size_t *addr, > char **options) > { > - int ret; > - unsigned long long tmp; > - > if (strncmp(p, "mmio,", 5) == 0) { > *iotype = UPIO_MEM; > p += 5; > @@ -1971,10 +1968,11 @@ int uart_parse_earlycon(char *p, unsigned char *iotype, resource_size_t *addr, > return -EINVAL; > } > > - ret = kstrtoull(p, 0, &tmp); > - if (ret) > - return ret; > - *addr = tmp; > + /* > + * Before you replace it with kstrtoull(), think about options separator > + * (',') it will not tolerate > + */ > + *addr = simple_strtoull(p, NULL, 0); > p = strchr(p, ','); > if (p) > p++; > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-09-12 13:27 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-09-02 11:20 [PATCH] serial: earlycon: Extend earlycon command line option to support 64-bit addresses Alexander Sverdlin 2016-09-11 0:45 ` Guenter Roeck 2016-09-12 11:29 ` [PATCH] serial: core: Fix handling of options after MMIO address Alexander Sverdlin 2016-09-12 13:27 ` Guenter Roeck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).