* Re: [PATCH V2] tty: serial: imx: allow breaks to be received when using dma
From: Martin Hicks @ 2018-02-20 18:01 UTC (permalink / raw)
To: Troy Kisky
Cc: gregkh, mort, linux-serial, u.kleine-koenig, fabio.estevam,
linux-arm-kernel, l.stach
In-Reply-To: <20180209212240.14095-1-troy.kisky@boundarydevices.com>
On Fri, Feb 09, 2018 at 01:22:40PM -0800, Troy Kisky wrote:
> This allows me to login after sending a break when service
> serial-getty@ttymxc0.service is running
>
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
Tested-by: Martin Hicks <mort@bork.org>
Works for me.
mh
>
> ---
> v2: rebase only
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 1d7ca382bc12..2eb8c4a20d68 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -927,7 +927,6 @@ static void dma_rx_callback(void *data)
> status = dmaengine_tx_status(chan, (dma_cookie_t)0, &state);
>
> if (status == DMA_ERROR) {
> - dev_err(sport->port.dev, "DMA transaction error.\n");
> clear_rx_errors(sport);
> return;
> }
> @@ -1028,6 +1027,7 @@ static int start_rx_dma(struct imx_port *sport)
>
> static void clear_rx_errors(struct imx_port *sport)
> {
> + struct tty_port *port = &sport->port.state->port;
> unsigned int status_usr1, status_usr2;
>
> status_usr1 = readl(sport->port.membase + USR1);
> @@ -1036,12 +1036,18 @@ static void clear_rx_errors(struct imx_port *sport)
> if (status_usr2 & USR2_BRCD) {
> sport->port.icount.brk++;
> writel(USR2_BRCD, sport->port.membase + USR2);
> - } else if (status_usr1 & USR1_FRAMERR) {
> - sport->port.icount.frame++;
> - writel(USR1_FRAMERR, sport->port.membase + USR1);
> - } else if (status_usr1 & USR1_PARITYERR) {
> - sport->port.icount.parity++;
> - writel(USR1_PARITYERR, sport->port.membase + USR1);
> + if (tty_insert_flip_char(port, 0, TTY_BREAK) == 0)
> + sport->port.icount.buf_overrun++;
> + tty_flip_buffer_push(port);
> + } else {
> + dev_err(sport->port.dev, "DMA transaction error.\n");
> + if (status_usr1 & USR1_FRAMERR) {
> + sport->port.icount.frame++;
> + writel(USR1_FRAMERR, sport->port.membase + USR1);
> + } else if (status_usr1 & USR1_PARITYERR) {
> + sport->port.icount.parity++;
> + writel(USR1_PARITYERR, sport->port.membase + USR1);
> + }
> }
>
> if (status_usr2 & USR2_ORE) {
> --
> 2.14.1
>
--
Martin Hicks P.Eng. | mort@bork.org
Bork Consulting Inc. | +1 (613) 266-2296
^ permalink raw reply
* [PATCH v3 1/1] tty: serial: imx: allow breaks to be received when using dma
From: Troy Kisky @ 2018-02-20 18:20 UTC (permalink / raw)
To: gregkh
Cc: mort, Troy Kisky, linux-serial, u.kleine-koenig, fabio.estevam,
linux-arm-kernel, l.stach
This allows me to login after sending a break when service
serial-getty@ttymxc0.service is running
The "tty_insert_flip_char(port, 0, TTY_BREAK)" in clear_rx_errors
fixes this by allowing the higher layers to see a break.
Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
Tested-by: Martin Hicks <mort@bork.org>
---
v2: rebase only
v3: change commit message as requested by Fabio,
add tested-by
---
drivers/tty/serial/imx.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 1d7ca382bc12..2eb8c4a20d68 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -927,7 +927,6 @@ static void dma_rx_callback(void *data)
status = dmaengine_tx_status(chan, (dma_cookie_t)0, &state);
if (status == DMA_ERROR) {
- dev_err(sport->port.dev, "DMA transaction error.\n");
clear_rx_errors(sport);
return;
}
@@ -1028,6 +1027,7 @@ static int start_rx_dma(struct imx_port *sport)
static void clear_rx_errors(struct imx_port *sport)
{
+ struct tty_port *port = &sport->port.state->port;
unsigned int status_usr1, status_usr2;
status_usr1 = readl(sport->port.membase + USR1);
@@ -1036,12 +1036,18 @@ static void clear_rx_errors(struct imx_port *sport)
if (status_usr2 & USR2_BRCD) {
sport->port.icount.brk++;
writel(USR2_BRCD, sport->port.membase + USR2);
- } else if (status_usr1 & USR1_FRAMERR) {
- sport->port.icount.frame++;
- writel(USR1_FRAMERR, sport->port.membase + USR1);
- } else if (status_usr1 & USR1_PARITYERR) {
- sport->port.icount.parity++;
- writel(USR1_PARITYERR, sport->port.membase + USR1);
+ if (tty_insert_flip_char(port, 0, TTY_BREAK) == 0)
+ sport->port.icount.buf_overrun++;
+ tty_flip_buffer_push(port);
+ } else {
+ dev_err(sport->port.dev, "DMA transaction error.\n");
+ if (status_usr1 & USR1_FRAMERR) {
+ sport->port.icount.frame++;
+ writel(USR1_FRAMERR, sport->port.membase + USR1);
+ } else if (status_usr1 & USR1_PARITYERR) {
+ sport->port.icount.parity++;
+ writel(USR1_PARITYERR, sport->port.membase + USR1);
+ }
}
if (status_usr2 & USR2_ORE) {
--
2.14.1
^ permalink raw reply related
* [PATCH v3 1/1] tty: serial: imx: allow breaks to be received when using dma
From: Troy Kisky @ 2018-02-20 18:20 UTC (permalink / raw)
To: gregkh
Cc: mort, Troy Kisky, linux-serial, u.kleine-koenig, fabio.estevam,
linux-arm-kernel, l.stach
In-Reply-To: <20180220182038.4325-1-troy.kisky@boundarydevices.com>
This allows me to login after sending a break when service
serial-getty@ttymxc0.service is running
The "tty_insert_flip_char(port, 0, TTY_BREAK)" in clear_rx_errors
fixes this by allowing the higher layers to see a break.
Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
Tested-by: Martin Hicks <mort@bork.org>
---
v2: rebase only
v3: change commit message as requested by Fabio,
add tested-by
---
drivers/tty/serial/imx.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 1d7ca382bc12..2eb8c4a20d68 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -927,7 +927,6 @@ static void dma_rx_callback(void *data)
status = dmaengine_tx_status(chan, (dma_cookie_t)0, &state);
if (status == DMA_ERROR) {
- dev_err(sport->port.dev, "DMA transaction error.\n");
clear_rx_errors(sport);
return;
}
@@ -1028,6 +1027,7 @@ static int start_rx_dma(struct imx_port *sport)
static void clear_rx_errors(struct imx_port *sport)
{
+ struct tty_port *port = &sport->port.state->port;
unsigned int status_usr1, status_usr2;
status_usr1 = readl(sport->port.membase + USR1);
@@ -1036,12 +1036,18 @@ static void clear_rx_errors(struct imx_port *sport)
if (status_usr2 & USR2_BRCD) {
sport->port.icount.brk++;
writel(USR2_BRCD, sport->port.membase + USR2);
- } else if (status_usr1 & USR1_FRAMERR) {
- sport->port.icount.frame++;
- writel(USR1_FRAMERR, sport->port.membase + USR1);
- } else if (status_usr1 & USR1_PARITYERR) {
- sport->port.icount.parity++;
- writel(USR1_PARITYERR, sport->port.membase + USR1);
+ if (tty_insert_flip_char(port, 0, TTY_BREAK) == 0)
+ sport->port.icount.buf_overrun++;
+ tty_flip_buffer_push(port);
+ } else {
+ dev_err(sport->port.dev, "DMA transaction error.\n");
+ if (status_usr1 & USR1_FRAMERR) {
+ sport->port.icount.frame++;
+ writel(USR1_FRAMERR, sport->port.membase + USR1);
+ } else if (status_usr1 & USR1_PARITYERR) {
+ sport->port.icount.parity++;
+ writel(USR1_PARITYERR, sport->port.membase + USR1);
+ }
}
if (status_usr2 & USR2_ORE) {
--
2.14.1
^ permalink raw reply related
* Re: [PATCH v3 1/1] tty: serial: imx: allow breaks to be received when using dma
From: Fabio Estevam @ 2018-02-20 18:25 UTC (permalink / raw)
To: Troy Kisky
Cc: Greg Kroah-Hartman, mort, linux-serial, Uwe Kleine-König,
Fabio Estevam,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Lucas Stach
In-Reply-To: <20180220182038.4325-1-troy.kisky@boundarydevices.com>
On Tue, Feb 20, 2018 at 3:20 PM, Troy Kisky
<troy.kisky@boundarydevices.com> wrote:
> This allows me to login after sending a break when service
> serial-getty@ttymxc0.service is running
>
> The "tty_insert_flip_char(port, 0, TTY_BREAK)" in clear_rx_errors
> fixes this by allowing the higher layers to see a break.
>
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> Tested-by: Martin Hicks <mort@bork.org>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
^ permalink raw reply
* Re: [PATCH v3 1/1] tty: serial: imx: allow breaks to be received when using dma
From: Uwe Kleine-König @ 2018-02-20 19:59 UTC (permalink / raw)
To: Troy Kisky
Cc: gregkh, mort, linux-serial, fabio.estevam, linux-arm-kernel,
l.stach
In-Reply-To: <20180220182038.4325-1-troy.kisky@boundarydevices.com>
On Tue, Feb 20, 2018 at 10:20:37AM -0800, Troy Kisky wrote:
> This allows me to login after sending a break when service
> serial-getty@ttymxc0.service is running
>
> The "tty_insert_flip_char(port, 0, TTY_BREAK)" in clear_rx_errors
> fixes this by allowing the higher layers to see a break.
>
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> Tested-by: Martin Hicks <mort@bork.org>
>
> ---
> v2: rebase only
> v3: change commit message as requested by Fabio,
> add tested-by
> ---
> drivers/tty/serial/imx.c | 20 +++++++++++++-------
> 1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 1d7ca382bc12..2eb8c4a20d68 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -927,7 +927,6 @@ static void dma_rx_callback(void *data)
> status = dmaengine_tx_status(chan, (dma_cookie_t)0, &state);
>
> if (status == DMA_ERROR) {
> - dev_err(sport->port.dev, "DMA transaction error.\n");
> clear_rx_errors(sport);
> return;
> }
> @@ -1028,6 +1027,7 @@ static int start_rx_dma(struct imx_port *sport)
>
> static void clear_rx_errors(struct imx_port *sport)
> {
> + struct tty_port *port = &sport->port.state->port;
> unsigned int status_usr1, status_usr2;
>
> status_usr1 = readl(sport->port.membase + USR1);
> @@ -1036,12 +1036,18 @@ static void clear_rx_errors(struct imx_port *sport)
> if (status_usr2 & USR2_BRCD) {
> sport->port.icount.brk++;
> writel(USR2_BRCD, sport->port.membase + USR2);
> - } else if (status_usr1 & USR1_FRAMERR) {
> - sport->port.icount.frame++;
> - writel(USR1_FRAMERR, sport->port.membase + USR1);
> - } else if (status_usr1 & USR1_PARITYERR) {
> - sport->port.icount.parity++;
> - writel(USR1_PARITYERR, sport->port.membase + USR1);
> + if (tty_insert_flip_char(port, 0, TTY_BREAK) == 0)
> + sport->port.icount.buf_overrun++;
> + tty_flip_buffer_push(port);
I think this needs to call uart_handle_break() as imx_rxint() does. Not sure
how to properly handle SYSRQ in the dma case though.
> + } else {
> + dev_err(sport->port.dev, "DMA transaction error.\n");
> + if (status_usr1 & USR1_FRAMERR) {
> + sport->port.icount.frame++;
> + writel(USR1_FRAMERR, sport->port.membase + USR1);
> + } else if (status_usr1 & USR1_PARITYERR) {
> + sport->port.icount.parity++;
> + writel(USR1_PARITYERR, sport->port.membase + USR1);
> + }
> }
>
> if (status_usr2 & USR2_ORE) {
> --
> 2.14.1
>
>
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH v3 1/1] tty: serial: imx: allow breaks to be received when using dma
From: Troy Kisky @ 2018-02-20 20:25 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: gregkh, mort, linux-serial, fabio.estevam, linux-arm-kernel,
l.stach
In-Reply-To: <20180220195944.kgl4olfc7eitzpxg@pengutronix.de>
On 2/20/2018 11:59 AM, Uwe Kleine-König wrote:
> On Tue, Feb 20, 2018 at 10:20:37AM -0800, Troy Kisky wrote:
>> This allows me to login after sending a break when service
>> serial-getty@ttymxc0.service is running
>>
>> The "tty_insert_flip_char(port, 0, TTY_BREAK)" in clear_rx_errors
>> fixes this by allowing the higher layers to see a break.
>>
>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>> Tested-by: Martin Hicks <mort@bork.org>
>>
>> ---
>> v2: rebase only
>> v3: change commit message as requested by Fabio,
>> add tested-by
>> ---
>> drivers/tty/serial/imx.c | 20 +++++++++++++-------
>> 1 file changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
>> index 1d7ca382bc12..2eb8c4a20d68 100644
>> --- a/drivers/tty/serial/imx.c
>> +++ b/drivers/tty/serial/imx.c
>> @@ -927,7 +927,6 @@ static void dma_rx_callback(void *data)
>> status = dmaengine_tx_status(chan, (dma_cookie_t)0, &state);
>>
>> if (status == DMA_ERROR) {
>> - dev_err(sport->port.dev, "DMA transaction error.\n");
>> clear_rx_errors(sport);
>> return;
>> }
>> @@ -1028,6 +1027,7 @@ static int start_rx_dma(struct imx_port *sport)
>>
>> static void clear_rx_errors(struct imx_port *sport)
>> {
>> + struct tty_port *port = &sport->port.state->port;
>> unsigned int status_usr1, status_usr2;
>>
>> status_usr1 = readl(sport->port.membase + USR1);
>> @@ -1036,12 +1036,18 @@ static void clear_rx_errors(struct imx_port *sport)
>> if (status_usr2 & USR2_BRCD) {
>> sport->port.icount.brk++;
>> writel(USR2_BRCD, sport->port.membase + USR2);
>> - } else if (status_usr1 & USR1_FRAMERR) {
>> - sport->port.icount.frame++;
>> - writel(USR1_FRAMERR, sport->port.membase + USR1);
>> - } else if (status_usr1 & USR1_PARITYERR) {
>> - sport->port.icount.parity++;
>> - writel(USR1_PARITYERR, sport->port.membase + USR1);
>> + if (tty_insert_flip_char(port, 0, TTY_BREAK) == 0)
>> + sport->port.icount.buf_overrun++;
>> + tty_flip_buffer_push(port);
>
> I think this needs to call uart_handle_break() as imx_rxint() does. Not sure
> how to properly handle SYSRQ in the dma case though.
>
SYSRQ is only for the console port, and console port is never dma. But UPF_SAK may be
an issue. I don't know enough about "Secure Attention Key" to say.
Should I add uart_handle_break anyway ?
>> + } else {
>> + dev_err(sport->port.dev, "DMA transaction error.\n");
>> + if (status_usr1 & USR1_FRAMERR) {
>> + sport->port.icount.frame++;
>> + writel(USR1_FRAMERR, sport->port.membase + USR1);
>> + } else if (status_usr1 & USR1_PARITYERR) {
>> + sport->port.icount.parity++;
>> + writel(USR1_PARITYERR, sport->port.membase + USR1);
>> + }
>> }
>>
>> if (status_usr2 & USR2_ORE) {
>> --
>> 2.14.1
>>
>>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 1/1] tty: serial: imx: allow breaks to be received when using dma
From: Uwe Kleine-König @ 2018-02-21 8:17 UTC (permalink / raw)
To: Troy Kisky
Cc: gregkh, mort, linux-serial, fabio.estevam, linux-arm-kernel,
l.stach
In-Reply-To: <6a93d743-05ef-0b23-9419-913b9d2c78ee@boundarydevices.com>
Hello Troy,
On Tue, Feb 20, 2018 at 12:25:16PM -0800, Troy Kisky wrote:
> >> @@ -1028,6 +1027,7 @@ static int start_rx_dma(struct imx_port *sport)
> >>
> >> static void clear_rx_errors(struct imx_port *sport)
> >> {
> >> + struct tty_port *port = &sport->port.state->port;
> >> unsigned int status_usr1, status_usr2;
> >>
> >> status_usr1 = readl(sport->port.membase + USR1);
> >> @@ -1036,12 +1036,18 @@ static void clear_rx_errors(struct imx_port *sport)
> >> if (status_usr2 & USR2_BRCD) {
> >> sport->port.icount.brk++;
> >> writel(USR2_BRCD, sport->port.membase + USR2);
> >> - } else if (status_usr1 & USR1_FRAMERR) {
> >> - sport->port.icount.frame++;
> >> - writel(USR1_FRAMERR, sport->port.membase + USR1);
> >> - } else if (status_usr1 & USR1_PARITYERR) {
> >> - sport->port.icount.parity++;
> >> - writel(USR1_PARITYERR, sport->port.membase + USR1);
> >> + if (tty_insert_flip_char(port, 0, TTY_BREAK) == 0)
> >> + sport->port.icount.buf_overrun++;
> >> + tty_flip_buffer_push(port);
> >
> > I think this needs to call uart_handle_break() as imx_rxint() does. Not sure
> > how to properly handle SYSRQ in the dma case though.
> >
>
>
> SYSRQ is only for the console port, and console port is never dma. But UPF_SAK may be
> an issue. I don't know enough about "Secure Attention Key" to say.
>
>
> Should I add uart_handle_break anyway ?
You're right about both SYSRQ and SAK. I think uart_handle_break is the
right one for the latter.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH v2 0/9] serial: Fix out-of-bounds accesses through DT aliases
From: Geert Uytterhoeven @ 2018-02-23 13:38 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devicetree, Barry Song, Geert Uytterhoeven, Vineet Gupta,
Michal Simek, linux-kernel, linux-renesas-soc, linux-serial,
Jiri Slaby, linux-snps-arc, linux-arm-kernel
Hi all,
Serial drivers used on DT platforms use the "serialN" alias in DT to
obtain the serial port index for a specific port. Drivers typically use
a fixed-size array for keeping track of all available serial ports.
However, several drivers do not perform any validation on the index
obtained from DT, which may lead to out-of-bounds accesses of these
fixed-size arrays.
While the DTB passed to the kernel might be considered trusted, some of
these out-of-bounds accesses can be triggered by a legitimate DTB:
- In some drivers the size of the array is defined by a Kconfig
symbol, so a user who doesn't need all serial ports may lower this
value rightfully,
- Tomorrow's new SoC may have more serial ports than the fixed-size
array in today's driver can accommodate, which the user may forget
to enlarge.
Hence this series fixes that by adding checks for out-of-range aliases,
logging an error message when triggered.
Changes compared to v1:
- Fix Fixes references,
- Use ARRAY_SIZE(),
- Fix off-by-one error in patch [5/9],
- Document where the non-DT case is also fixed by a patch.
Tested on r8a7791/koelsch (sh-sci), all other drivers were
compile-tested only.
Thanks for your comments!
Geert Uytterhoeven (9):
serial: arc_uart: Fix out-of-bounds access through DT alias
serial: fsl_lpuart: Fix out-of-bounds access through DT alias
serial: imx: Fix out-of-bounds access through serial port index
serial: mxs-auart: Fix out-of-bounds access through serial port index
serial: pxa: Fix out-of-bounds access through serial port index
serial: samsung: Fix out-of-bounds access through serial port index
serial: sh-sci: Fix out-of-bounds access through DT alias
serial: sirf: Fix out-of-bounds access through DT alias
serial: xuartps: Fix out-of-bounds access through DT alias
drivers/tty/serial/arc_uart.c | 5 +++++
drivers/tty/serial/fsl_lpuart.c | 4 ++++
drivers/tty/serial/imx.c | 6 ++++++
drivers/tty/serial/mxs-auart.c | 4 ++++
drivers/tty/serial/pxa.c | 4 ++++
drivers/tty/serial/samsung.c | 4 ++++
drivers/tty/serial/sh-sci.c | 4 ++++
drivers/tty/serial/sirfsoc_uart.c | 5 +++++
drivers/tty/serial/xilinx_uartps.c | 2 +-
9 files changed, 37 insertions(+), 1 deletion(-)
--
2.7.4
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH v2 1/9] serial: arc_uart: Fix out-of-bounds access through DT alias
From: Geert Uytterhoeven @ 2018-02-23 13:38 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devicetree, Barry Song, Geert Uytterhoeven, Vineet Gupta,
Michal Simek, linux-kernel, linux-renesas-soc, linux-serial,
Jiri Slaby, linux-snps-arc, linux-arm-kernel
In-Reply-To: <1519393117-31998-1-git-send-email-geert+renesas@glider.be>
The arc_uart_ports[] array is indexed using a value derived from the
"serialN" alias in DT, which may lead to an out-of-bounds access.
Fix this by adding a range check.
Note that the array size is defined by a Kconfig symbol
(CONFIG_SERIAL_ARC_NR_PORTS), so this can even be triggered using a
legitimate DTB.
Fixes: ea28fd56fcde69af ("serial/arc-uart: switch to devicetree based probing")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
- Fix Fixes reference,
- Use ARRAY_SIZE().
---
drivers/tty/serial/arc_uart.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/tty/serial/arc_uart.c b/drivers/tty/serial/arc_uart.c
index 2599f9ecccfe7769..d904a3a345e74785 100644
--- a/drivers/tty/serial/arc_uart.c
+++ b/drivers/tty/serial/arc_uart.c
@@ -593,6 +593,11 @@ static int arc_serial_probe(struct platform_device *pdev)
if (dev_id < 0)
dev_id = 0;
+ if (dev_id >= ARRAY_SIZE(arc_uart_ports)) {
+ dev_err(&pdev->dev, "serial%d out of range\n", dev_id);
+ return -EINVAL;
+ }
+
uart = &arc_uart_ports[dev_id];
port = &uart->port;
--
2.7.4
^ permalink raw reply related
* [PATCH v2 3/9] serial: imx: Fix out-of-bounds access through serial port index
From: Geert Uytterhoeven @ 2018-02-23 13:38 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devicetree, Barry Song, Geert Uytterhoeven, Vineet Gupta,
Michal Simek, linux-kernel, linux-renesas-soc, linux-serial,
Jiri Slaby, linux-snps-arc, linux-arm-kernel
In-Reply-To: <1519393117-31998-1-git-send-email-geert+renesas@glider.be>
The imx_ports[] array is indexed using a value derived from the
"serialN" alias in DT, or from platform data, which may lead to an
out-of-bounds access.
Fix this by adding a range check.
Fixes: ff05967a07225ab6 ("serial/imx: add of_alias_get_id() reference back")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
- Fix Fixes reference,
- Add blank line,
- Use ARRAY_SIZE(),
- Update patch description for platform data.
---
drivers/tty/serial/imx.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 1d7ca382bc12b238..7c9bdc8e34ac9109 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -2042,6 +2042,12 @@ static int serial_imx_probe(struct platform_device *pdev)
else if (ret < 0)
return ret;
+ if (sport->port.line >= ARRAY_SIZE(imx_ports)) {
+ dev_err(&pdev->dev, "serial%d out of range\n",
+ sport->port.line);
+ return -EINVAL;
+ }
+
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(base))
--
2.7.4
^ permalink raw reply related
* [PATCH v2 4/9] serial: mxs-auart: Fix out-of-bounds access through serial port index
From: Geert Uytterhoeven @ 2018-02-23 13:38 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devicetree, Barry Song, Geert Uytterhoeven, Vineet Gupta,
Michal Simek, linux-kernel, linux-renesas-soc, linux-serial,
Jiri Slaby, linux-snps-arc, linux-arm-kernel
In-Reply-To: <1519393117-31998-1-git-send-email-geert+renesas@glider.be>
The auart_port[] array is indexed using a value derived from the
"serialN" alias in DT, or from platform data, which may lead to an
out-of-bounds access.
Fix this by adding a range check.
Fixes: 1ea6607d4cdc9179 ("serial: mxs-auart: Allow device tree probing")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
- Fix Fixes reference,
- Use ARRAY_SIZE(),
- Update patch description for platform data.
---
drivers/tty/serial/mxs-auart.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index 079dc47aa142d8e1..caa8a41b6e71df9e 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -1663,6 +1663,10 @@ static int mxs_auart_probe(struct platform_device *pdev)
s->port.line = pdev->id < 0 ? 0 : pdev->id;
else if (ret < 0)
return ret;
+ if (s->port.line >= ARRAY_SIZE(auart_port)) {
+ dev_err(&pdev->dev, "serial%d out of range\n", s->port.line);
+ return -EINVAL;
+ }
if (of_id) {
pdev->id_entry = of_id->data;
--
2.7.4
^ permalink raw reply related
* [PATCH v2 6/9] serial: samsung: Fix out-of-bounds access through serial port index
From: Geert Uytterhoeven @ 2018-02-23 13:38 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devicetree, Barry Song, Geert Uytterhoeven, Vineet Gupta,
Michal Simek, linux-kernel, linux-renesas-soc, linux-serial,
Jiri Slaby, linux-snps-arc, linux-arm-kernel
In-Reply-To: <1519393117-31998-1-git-send-email-geert+renesas@glider.be>
The s3c24xx_serial_ports[] array is indexed using a value derived from
the "serialN" alias in DT, or from an incrementing probe index, which
may lead to an out-of-bounds access.
Fix this by adding a range check.
Note that the array size is defined by a Kconfig symbol
(CONFIG_SERIAL_SAMSUNG_UARTS), so this can even be triggered using
a legitimate DTB or legitimate board code.
Fixes: 13a9f6c64fdc55eb ("serial: samsung: Consider DT alias when probing ports")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
- Fix Fixes reference,
- Use ARRAY_SIZE(),
- Update patch description for non-DT case.
---
drivers/tty/serial/samsung.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index f9fecc5ed0cee826..3f2f8c118ce09d56 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -1818,6 +1818,10 @@ static int s3c24xx_serial_probe(struct platform_device *pdev)
dbg("s3c24xx_serial_probe(%p) %d\n", pdev, index);
+ if (index >= ARRAY_SIZE(s3c24xx_serial_ports)) {
+ dev_err(&pdev->dev, "serial%d out of range\n", index);
+ return -EINVAL;
+ }
ourport = &s3c24xx_serial_ports[index];
ourport->drv_data = s3c24xx_get_driver_data(pdev);
--
2.7.4
^ permalink raw reply related
* [PATCH v2 7/9] serial: sh-sci: Fix out-of-bounds access through DT alias
From: Geert Uytterhoeven @ 2018-02-23 13:38 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devicetree, Barry Song, Geert Uytterhoeven, Vineet Gupta,
Michal Simek, linux-kernel, linux-renesas-soc, linux-serial,
Jiri Slaby, linux-snps-arc, linux-arm-kernel
In-Reply-To: <1519393117-31998-1-git-send-email-geert+renesas@glider.be>
The sci_ports[] array is indexed using a value derived from the
"serialN" alias in DT, which may lead to an out-of-bounds access.
Fix this by adding a range check.
Note that the array size is defined by a Kconfig symbol
(CONFIG_SERIAL_SH_SCI_NR_UARTS), so this can even be triggered using a
legitimate DTB.
Fixes: 97ed9790c514066b ("serial: sh-sci: Remove unused platform data capabilities field")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
- Fix Fixes reference,
- Use ARRAY_SIZE().
---
drivers/tty/serial/sh-sci.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 4d14f321cbec95e0..f6a6610d434efc33 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -3096,6 +3096,10 @@ static struct plat_sci_port *sci_parse_dt(struct platform_device *pdev,
dev_err(&pdev->dev, "failed to get alias id (%d)\n", id);
return NULL;
}
+ if (id >= ARRAY_SIZE(sci_ports)) {
+ dev_err(&pdev->dev, "serial%d out of range\n", id);
+ return NULL;
+ }
sp = &sci_ports[id];
*dev_id = id;
--
2.7.4
^ permalink raw reply related
* [PATCH v2 9/9] serial: xuartps: Fix out-of-bounds access through DT alias
From: Geert Uytterhoeven @ 2018-02-23 13:38 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devicetree, Barry Song, Geert Uytterhoeven, Vineet Gupta,
Michal Simek, linux-kernel, linux-renesas-soc, linux-serial,
Jiri Slaby, linux-snps-arc, linux-arm-kernel
In-Reply-To: <1519393117-31998-1-git-send-email-geert+renesas@glider.be>
The cdns_uart_port[] array is indexed using a value derived from the
"serialN" alias in DT, which may lead to an out-of-bounds access.
Fix this by adding a range check.
Fixes: 928e9263492069ee ("tty: xuartps: Initialize ports according to aliases")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
- Fix Fixes reference.
---
drivers/tty/serial/xilinx_uartps.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index b9b2bc76bcac606c..abcb4d09a2d866d0 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1110,7 +1110,7 @@ static struct uart_port *cdns_uart_get_port(int id)
struct uart_port *port;
/* Try the given port id if failed use default method */
- if (cdns_uart_port[id].mapbase != 0) {
+ if (id < CDNS_UART_NR_PORTS && cdns_uart_port[id].mapbase != 0) {
/* Find the next unused port */
for (id = 0; id < CDNS_UART_NR_PORTS; id++)
if (cdns_uart_port[id].mapbase == 0)
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2 9/9] serial: xuartps: Fix out-of-bounds access through DT alias
From: Michal Simek @ 2018-02-23 13:41 UTC (permalink / raw)
To: Geert Uytterhoeven, Greg Kroah-Hartman
Cc: devicetree, Barry Song, Vineet Gupta, Michal Simek, linux-kernel,
linux-renesas-soc, linux-serial, Jiri Slaby, linux-snps-arc,
linux-arm-kernel
In-Reply-To: <1519393117-31998-10-git-send-email-geert+renesas@glider.be>
On 23.2.2018 14:38, Geert Uytterhoeven wrote:
> The cdns_uart_port[] array is indexed using a value derived from the
> "serialN" alias in DT, which may lead to an out-of-bounds access.
>
> Fix this by adding a range check.
>
> Fixes: 928e9263492069ee ("tty: xuartps: Initialize ports according to aliases")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v2:
> - Fix Fixes reference.
> ---
> drivers/tty/serial/xilinx_uartps.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
> index b9b2bc76bcac606c..abcb4d09a2d866d0 100644
> --- a/drivers/tty/serial/xilinx_uartps.c
> +++ b/drivers/tty/serial/xilinx_uartps.c
> @@ -1110,7 +1110,7 @@ static struct uart_port *cdns_uart_get_port(int id)
> struct uart_port *port;
>
> /* Try the given port id if failed use default method */
> - if (cdns_uart_port[id].mapbase != 0) {
> + if (id < CDNS_UART_NR_PORTS && cdns_uart_port[id].mapbase != 0) {
> /* Find the next unused port */
> for (id = 0; id < CDNS_UART_NR_PORTS; id++)
> if (cdns_uart_port[id].mapbase == 0)
>
Reviewed-by: Michal Simek <michal.simek@xilinx.com>
Thanks,
Michal
^ permalink raw reply
* Re: [PATCH v2 3/9] serial: imx: Fix out-of-bounds access through serial port index
From: Uwe Kleine-König @ 2018-02-23 13:51 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: devicetree, Barry Song, Greg Kroah-Hartman, Michal Simek,
linux-kernel, linux-renesas-soc, Vineet Gupta, linux-serial,
Jiri Slaby, linux-snps-arc, linux-arm-kernel
In-Reply-To: <1519393117-31998-4-git-send-email-geert+renesas@glider.be>
On Fri, Feb 23, 2018 at 02:38:31PM +0100, Geert Uytterhoeven wrote:
> The imx_ports[] array is indexed using a value derived from the
> "serialN" alias in DT, or from platform data, which may lead to an
> out-of-bounds access.
>
> Fix this by adding a range check.
>
> Fixes: ff05967a07225ab6 ("serial/imx: add of_alias_get_id() reference back")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Thanks for your time
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH v4 1/1] tty: serial: imx: allow breaks to be received when using dma
From: Troy Kisky @ 2018-02-24 2:27 UTC (permalink / raw)
To: gregkh
Cc: mort, Troy Kisky, linux-serial, u.kleine-koenig, fabio.estevam,
linux-arm-kernel, l.stach
This allows me to login after sending a break when service
serial-getty@ttymxc0.service is running
The "tty_insert_flip_char(port, 0, TTY_BREAK)" in clear_rx_errors
fixes this by allowing the higher layers to see a break.
Also, call uart_handle_break to handle possible
"secure attention key."
FYI: Martin said the ROM sdma firmware works with this patch,
but external sdma firmware still does not send breaks on a
i.mx6UL
Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
Tested-by: Martin Hicks <mort@bork.org>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
v2: rebase only
v3: change commit message as requested by Fabio,
add tested-by
v4: add uart_handle_break as Uwe requested
add Reviewed-by
added to commit message
---
drivers/tty/serial/imx.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 1d7ca382bc12..ace96283bdb8 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -927,7 +927,6 @@ static void dma_rx_callback(void *data)
status = dmaengine_tx_status(chan, (dma_cookie_t)0, &state);
if (status == DMA_ERROR) {
- dev_err(sport->port.dev, "DMA transaction error.\n");
clear_rx_errors(sport);
return;
}
@@ -1028,6 +1027,7 @@ static int start_rx_dma(struct imx_port *sport)
static void clear_rx_errors(struct imx_port *sport)
{
+ struct tty_port *port = &sport->port.state->port;
unsigned int status_usr1, status_usr2;
status_usr1 = readl(sport->port.membase + USR1);
@@ -1036,12 +1036,19 @@ static void clear_rx_errors(struct imx_port *sport)
if (status_usr2 & USR2_BRCD) {
sport->port.icount.brk++;
writel(USR2_BRCD, sport->port.membase + USR2);
- } else if (status_usr1 & USR1_FRAMERR) {
- sport->port.icount.frame++;
- writel(USR1_FRAMERR, sport->port.membase + USR1);
- } else if (status_usr1 & USR1_PARITYERR) {
- sport->port.icount.parity++;
- writel(USR1_PARITYERR, sport->port.membase + USR1);
+ uart_handle_break(&sport->port);
+ if (tty_insert_flip_char(port, 0, TTY_BREAK) == 0)
+ sport->port.icount.buf_overrun++;
+ tty_flip_buffer_push(port);
+ } else {
+ dev_err(sport->port.dev, "DMA transaction error.\n");
+ if (status_usr1 & USR1_FRAMERR) {
+ sport->port.icount.frame++;
+ writel(USR1_FRAMERR, sport->port.membase + USR1);
+ } else if (status_usr1 & USR1_PARITYERR) {
+ sport->port.icount.parity++;
+ writel(USR1_PARITYERR, sport->port.membase + USR1);
+ }
}
if (status_usr2 & USR2_ORE) {
--
2.14.1
^ permalink raw reply related
* Re: [PATCH v4 1/1] tty: serial: imx: allow breaks to be received when using dma
From: Uwe Kleine-König @ 2018-02-24 9:13 UTC (permalink / raw)
To: Troy Kisky
Cc: gregkh, mort, linux-serial, fabio.estevam, linux-arm-kernel,
l.stach
In-Reply-To: <20180224022750.20942-1-troy.kisky@boundarydevices.com>
On Fri, Feb 23, 2018 at 06:27:50PM -0800, Troy Kisky wrote:
> This allows me to login after sending a break when service
> serial-getty@ttymxc0.service is running
>
> The "tty_insert_flip_char(port, 0, TTY_BREAK)" in clear_rx_errors
> fixes this by allowing the higher layers to see a break.
>
> Also, call uart_handle_break to handle possible
> "secure attention key."
>
> FYI: Martin said the ROM sdma firmware works with this patch,
> but external sdma firmware still does not send breaks on a
> i.mx6UL
>
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> Tested-by: Martin Hicks <mort@bork.org>
> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [RFC PATCH 0/2] serial: 8250_dw: IO space + polling mode support
From: John Garry @ 2018-02-26 9:33 UTC (permalink / raw)
To: Andy Shevchenko, gregkh, jslaby, p.zabel, heiko, ed.blake, jhogan
Cc: linux-serial, linux-kernel, linuxarm
In-Reply-To: <1519407117.10722.124.camel@linux.intel.com>
On 23/02/2018 17:31, Andy Shevchenko wrote:
> On Fri, 2018-02-23 at 11:02 +0000, John Garry wrote:
>> On 23/02/2018 10:30, Andy Shevchenko wrote:
>>> On Fri, 2018-02-23 at 02:42 +0800, John Garry wrote:
>>>> There is a requirement
>>
>>> Where?
>>
Hi Andy,
>> We require it for a development board for our hip06 platform.
>
> Okay, and this particular platform uses Synopsys IP?
As I see this uart is really a virtual 8250, so HW details like apb
clocks and the like are hidden, so may not be relevant.
However I will check with the BMC team to know the specific details.
>
>>>> for supporting an 8250-compatible UART with
>>>> the following profile/features:
>>>> - platform device
>>>> - polling mode (i.e. no interrupt support)
>>>> - ACPI FW
>>>
>>> Elaborate this one, please.
>>
>> So we need to define our own HID here, and cannot use PNP compatible
>> CID
>> (like PNP0501) as we cannot use the 8250 PNP driver.
>
> Why not? What are the impediments?
>
To support the host controller for this device, we will create an MFD,
i.e. platform device, per slave device.
>> This is related to the Hisi LPC ACPI support, where we would create
>> an
>> MFD (i.e. platform device) for the UART.
>
> Why you can't do properly in ACPI?
>
>>>> - IO port iotype
>>>> - 16550-compatible
>>>>
>>>> For OF, we have 8250_of.c, and for PNP device we have 8250_pnp.c
>>>> drivers. However there does not seem to any driver satisfying
>>>> the above requirements. So this RFC is to find opinion on
>>>> modifying the Synopsys DW 8250_dw.c driver to support these
>>>> generic features.
>>>
>>> Synopsys 8250 is a particular case of platform drivers. It doesn't
>>> satisfy "8250-compatible UART" requirement.
>
>> Right, but I wanted to try to use the generic parts of the driver to
>> support this UART to save writing yet another driver.
>
> It's still odd. Why this one, why not 8250_foo_bar to touch instead?
>
Agreed, it's odd. I choose as 8250_dw.c as it has ACPI support already.
And I recognise the hw from popularity. No stronger reasons than that.
Thanks,
John
^ permalink raw reply
* Re: [RFC PATCH 0/2] serial: 8250_dw: IO space + polling mode support
From: Andy Shevchenko @ 2018-02-26 9:53 UTC (permalink / raw)
To: John Garry, gregkh, jslaby, p.zabel, heiko, ed.blake, jhogan
Cc: linux-serial, linux-kernel, linuxarm
In-Reply-To: <88214a3e-82cc-c931-804c-7dc90fb8721f@huawei.com>
On Mon, 2018-02-26 at 09:33 +0000, John Garry wrote:
> On 23/02/2018 17:31, Andy Shevchenko wrote:
> > On Fri, 2018-02-23 at 11:02 +0000, John Garry wrote:
> > > On 23/02/2018 10:30, Andy Shevchenko wrote:
> > > > On Fri, 2018-02-23 at 02:42 +0800, John Garry wrote:
> > > We require it for a development board for our hip06 platform.
> >
> > Okay, and this particular platform uses Synopsys IP?
>
> As I see this uart is really a virtual 8250, so HW details like apb
> clocks and the like are hidden, so may not be relevant.
Good to know.
> However I will check with the BMC team to know the specific details.
> > > > > for supporting an 8250-compatible UART with
> > > > > the following profile/features:
> > > > > - platform device
> > > > > - polling mode (i.e. no interrupt support)
> > > > > - ACPI FW
> > > >
> > > > Elaborate this one, please.
> > >
> > > So we need to define our own HID here, and cannot use PNP
> > > compatible
> > > CID
> > > (like PNP0501) as we cannot use the 8250 PNP driver.
> >
> > Why not? What are the impediments?
> >
>
> To support the host controller for this device, we will create an
> MFD,
> i.e. platform device, per slave device.
There is no answer here...
> > > This is related to the Hisi LPC ACPI support, where we would
> > > create
> > > an
> > > MFD (i.e. platform device) for the UART.
> >
> > Why you can't do properly in ACPI?
No answer here either.
Sorry, but with this level of communication it's no go for the series.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [RFC PATCH 0/2] serial: 8250_dw: IO space + polling mode support
From: John Garry @ 2018-02-26 10:45 UTC (permalink / raw)
To: Andy Shevchenko, gregkh, jslaby, p.zabel, heiko, ed.blake, jhogan
Cc: linux-serial, linux-kernel, linuxarm
In-Reply-To: <1519638828.10722.153.camel@linux.intel.com>
Hi Andy,
>>>> We require it for a development board for our hip06 platform.
>>>
>>> Okay, and this particular platform uses Synopsys IP?
>>
>> As I see this uart is really a virtual 8250, so HW details like apb
>> clocks and the like are hidden, so may not be relevant.
>
> Good to know.
>
>> However I will check with the BMC team to know the specific details.
>
>>>>>> for supporting an 8250-compatible UART with
>>>>>> the following profile/features:
>>>>>> - platform device
>>>>>> - polling mode (i.e. no interrupt support)
>>>>>> - ACPI FW
>>>>>
>>>>> Elaborate this one, please.
>>>>
>>>> So we need to define our own HID here, and cannot use PNP
>>>> compatible
>>>> CID
>>>> (like PNP0501) as we cannot use the 8250 PNP driver.
>>>
>>> Why not? What are the impediments?
>>>
>>
>> To support the host controller for this device, we will create an
>> MFD,
>> i.e. platform device, per slave device.
>
> There is no answer here...
>
>>>> This is related to the Hisi LPC ACPI support, where we would
>>>> create
>>>> an
>>>> MFD (i.e. platform device) for the UART.
>>>
>>> Why you can't do properly in ACPI?
>
> No answer here either.
>
> Sorry, but with this level of communication it's no go for the series.
>
Sorry if my answers did not tell you want you want to know.
My point was that the 8250_pnp driver would be used for a pnp_device,
but we are creating a platform device for this UART slave so would
require a platform device driver, that which 8250_dw.c is. But I will
check on pnp device support.
Much appreciated,
John
^ permalink raw reply
* Re: [RFC PATCH 2/2] serial: 8250_dw: support polling mode
From: John Garry @ 2018-02-26 10:54 UTC (permalink / raw)
To: Andy Shevchenko, gregkh, slaby, p.zabel, heiko, ed.blake, jhogan
Cc: linux-serial, linux-kernel, linuxarm
In-Reply-To: <1519407313.10722.127.camel@linux.intel.com>
On 23/02/2018 17:35, Andy Shevchenko wrote:
> On Fri, 2018-02-23 at 02:42 +0800, John Garry wrote:
>> It would be useful to make this driver support some
>> 8250-compatible devices which have no interrupt line.
>>
>> For these, we allow for no interrupt, and will fallback on
>> polling mode.
>>
>> Note: the 8250 dt bindings state that "interrupts"
>> is a required property:
>> "interrupts : should contain uart interrupt."
>>
>> But the 8250_of.c driver can live without it. So
>> this patch is going this way also.
>
> It should be documented in the binding.
Agreed. I find the wording a bit ambigious in bindings/serial/8250.txt
and snps-dw-apb-uart.txt:
Required properties:
[...]
- interrupts : should contain uart interrupt.
It uses the word "should", and not "must". Maybe "should" means "must"...
>
>> if (irq < 0) {
>> - if (irq != -EPROBE_DEFER)
>> - dev_err(dev, "cannot get irq\n");
>> - return irq;
>> + if (irq == -EPROBE_DEFER)
>> + return irq;
>> + dev_warn(dev, "cannot get irq, using polling
>> mode\n");
>> + irq = 0;
>
> NO_IRQ _is_ 0. You need to do something like
>
> if (irq < 0) }
> ... leave existing code ...
> }
>
> if (!irq)
> dev_warn(, "Use polling mode\n");
OK
>
Thanks,
John
^ permalink raw reply
* Re: [RFC PATCH 0/2] serial: 8250_dw: IO space + polling mode support
From: Andy Shevchenko @ 2018-02-26 11:28 UTC (permalink / raw)
To: John Garry, gregkh, jslaby, p.zabel, heiko, ed.blake, jhogan
Cc: linux-serial, linux-kernel, linuxarm
In-Reply-To: <7221877b-a84a-63a1-ef86-1991f358df72@huawei.com>
On Mon, 2018-02-26 at 10:45 +0000, John Garry wrote:
> > > > > > > for supporting an 8250-compatible UART with
> > > > > > > the following profile/features:
> > > > > > > - platform device
> > > > > > > - polling mode (i.e. no interrupt support)
> > > > > > > - ACPI FW
> > > > > >
> > > > > > Elaborate this one, please.
> > > > >
> > > > > So we need to define our own HID here, and cannot use PNP
> > > > > compatible
> > > > > CID
> > > > > (like PNP0501) as we cannot use the 8250 PNP driver.
> > > >
> > > > Why not? What are the impediments?
> > > >
> > >
> > > To support the host controller for this device, we will create an
> > > MFD,
> > > i.e. platform device, per slave device.
> >
> > There is no answer here...
> >
> > > > > This is related to the Hisi LPC ACPI support, where we would
> > > > > create
> > > > > an
> > > > > MFD (i.e. platform device) for the UART.
> > > >
> > > > Why you can't do properly in ACPI?
> >
> > No answer here either.
> >
> > Sorry, but with this level of communication it's no go for the
> > series.
> >
>
> Sorry if my answers did not tell you want you want to know.
>
> My point was that the 8250_pnp driver would be used for a pnp_device,
> but we are creating a platform device for this UART slave so would
> require a platform device driver, that which 8250_dw.c is. But I will
> check on pnp device support.
Perhaps it's not visible, though below is a description of the drivers
we have:
8250_dw - OF/ACPI driver for Synopsys DW (+ DW DMA)
8250_lpss - PCI driver for Synopsys DW (+ DW DMA)
8250_of - generic 8250 compatible driver for OF
8250_pci - generic 8250 compatible driver for PCI
8250_pnp - generic 8250 compatible driver for ACPI
8250_* (except core parts) - custom glue drivers per some IPs
By description you gave your driver fits 8250_pnp if ACPI tables crafted
properly.
Share the ACPI excerpt and we can discuss further how to improve them.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [RFC PATCH 0/2] serial: 8250_dw: IO space + polling mode support
From: John Garry @ 2018-02-26 11:56 UTC (permalink / raw)
To: Andy Shevchenko, gregkh, jslaby, p.zabel, heiko, ed.blake, jhogan
Cc: linux-serial, linux-kernel, linuxarm
In-Reply-To: <1519644517.10722.179.camel@linux.intel.com>
>>>>> Why you can't do properly in ACPI?
>>>
>>> No answer here either.
>>>
>>> Sorry, but with this level of communication it's no go for the
>>> series.
>>>
>>
>> Sorry if my answers did not tell you want you want to know.
>>
>> My point was that the 8250_pnp driver would be used for a pnp_device,
>> but we are creating a platform device for this UART slave so would
>> require a platform device driver, that which 8250_dw.c is. But I will
>> check on pnp device support.
>
Hi Andy,
> Perhaps it's not visible, though below is a description of the drivers
> we have:
>
> 8250_dw - OF/ACPI driver for Synopsys DW (+ DW DMA)
> 8250_lpss - PCI driver for Synopsys DW (+ DW DMA)
> 8250_of - generic 8250 compatible driver for OF
> 8250_pci - generic 8250 compatible driver for PCI
> 8250_pnp - generic 8250 compatible driver for ACPI
>
> 8250_* (except core parts) - custom glue drivers per some IPs
>
> By description you gave your driver fits 8250_pnp if ACPI tables crafted
> properly.
>
> Share the ACPI excerpt and we can discuss further how to improve them.
>
For a bit of background, MFD support was discussed here initially:
https://lkml.org/lkml/2017/6/13/796
Here is the ACPI table:
Scope(_SB) {
Device (LPC0) {
Name (_HID, "HISI0191") // HiSi LPC
Name (_CRS, ResourceTemplate () {
Memory32Fixed (ReadWrite, 0xa01b0000, 0x1000)
})
}
Device (LPC0.CON0) {
Name (_HID, "HISI1031")
// Name (_CID, "PNP0501") // cannot support PNP
Name (LORS, ResourceTemplate() {
QWordIO (
ResourceConsumer,
MinNotFixed, // _MIF
MaxNotFixed, // _MAF
PosDecode,
EntireRange,
0x0, // _GRA
0x2F8, // _MIN
0x3fff, // _MAX
0x0, // _TRA
0x08, // _LEN
, ,
IO02
The latest framework changes and host driver patchset are here:
https://lkml.org/lkml/2018/2/19/465
Here is how we probe the children:
static int hisi_lpc_acpi_set_io_res(struct device *child,
struct device *hostdev,
const struct resource **res,
int *num_res)
{
[ ... In this part we just get the child resources ]
/* translate the I/O resources */
for (i = 0; i < count; i++) {
int ret;
if (!(resources[i].flags & IORESOURCE_IO))
continue;
ret = hisi_lpc_acpi_xlat_io_res(adev, host, &resources[i]);
if (ret) {
dev_err(child, "translate IO range failed(%d)\n", ret);
return ret;
}
}
*res = resources;
*num_res = count;
return 0;
}
static int hisi_lpc_acpi_probe(struct device *hostdev)
{
struct acpi_device *adev = ACPI_COMPANION(hostdev);
struct hisi_lpc_mfd_cell *hisi_lpc_mfd_cells;
struct mfd_cell *mfd_cells;
struct acpi_device *child;
int size, ret, count = 0, cell_num = 0;
list_for_each_entry(child, &adev->children, node)
cell_num++;
/* allocate the mfd cell and companion acpi info, one per child */
size = sizeof(*mfd_cells) + sizeof(*hisi_lpc_mfd_cells);
mfd_cells = devm_kcalloc(hostdev, cell_num, size, GFP_KERNEL);
if (!mfd_cells)
return -ENOMEM;
hisi_lpc_mfd_cells = (struct hisi_lpc_mfd_cell *)
&mfd_cells[cell_num];
/* Only consider the children of the host */
list_for_each_entry(child, &adev->children, node) {
struct mfd_cell *mfd_cell = &mfd_cells[count];
struct hisi_lpc_mfd_cell *hisi_lpc_mfd_cell =
&hisi_lpc_mfd_cells[count];
struct mfd_cell_acpi_match *acpi_match =
&hisi_lpc_mfd_cell->acpi_match;
char *name = hisi_lpc_mfd_cell[count].name;
char *pnpid = hisi_lpc_mfd_cell[count].pnpid;
struct mfd_cell_acpi_match match = {
.pnpid = pnpid,
};
snprintf(name, MFD_CHILD_NAME_LEN, MFD_CHILD_NAME_PREFIX"%s",
acpi_device_hid(child));
snprintf(pnpid, ACPI_ID_LEN, "%s", acpi_device_hid(child));
memcpy(acpi_match, &match, sizeof(*acpi_match));
mfd_cell->name = name;
mfd_cell->acpi_match = acpi_match;
ret = hisi_lpc_acpi_set_io_res(&child->dev, &adev->dev,
&mfd_cell->resources,
&mfd_cell->num_resources);
if (ret) {
dev_warn(&child->dev, "set resource fail(%d)\n", ret);
return ret;
}
count++;
}
ret = mfd_add_devices(hostdev, PLATFORM_DEVID_NONE,
mfd_cells, cell_num, NULL, 0, NULL);
if (ret) {
dev_err(hostdev, "failed to add mfd cells (%d)\n", ret);
return ret;
}
return 0;
}
As you know, this is not accepted upstream yet, but I really hope I'm
close. Hence the RFC tag for the UART patchset.
Please let me know if you require more details.
Thanks,
John
^ permalink raw reply
* Re: [RFC PATCH 0/2] serial: 8250_dw: IO space + polling mode support
From: Andy Shevchenko @ 2018-02-26 12:21 UTC (permalink / raw)
To: John Garry, gregkh, jslaby, p.zabel, heiko, ed.blake, jhogan
Cc: linux-serial, linux-kernel, linuxarm
In-Reply-To: <b5904a5a-66d8-4385-cc5b-a52529e9d28f@huawei.com>
On Mon, 2018-02-26 at 11:56 +0000, John Garry wrote:
> > > > > > Why you can't do properly in ACPI?
> > > >
> > > > No answer here either.
> > > >
> > > > Sorry, but with this level of communication it's no go for the
> > > > series.
> > > >
> > >
> > > Sorry if my answers did not tell you want you want to know.
> > >
> > > My point was that the 8250_pnp driver would be used for a
> > > pnp_device,
> > > but we are creating a platform device for this UART slave so would
> > > require a platform device driver, that which 8250_dw.c is. But I
> > > will
> > > check on pnp device support.
>
> Hi Andy,
>
> > Perhaps it's not visible, though below is a description of the
> > drivers
> > we have:
> >
> > 8250_dw - OF/ACPI driver for Synopsys DW (+ DW DMA)
> > 8250_lpss - PCI driver for Synopsys DW (+ DW DMA)
> > 8250_of - generic 8250 compatible driver for OF
> > 8250_pci - generic 8250 compatible driver for PCI
> > 8250_pnp - generic 8250 compatible driver for ACPI
> >
> > 8250_* (except core parts) - custom glue drivers per some IPs
> >
> > By description you gave your driver fits 8250_pnp if ACPI tables
> > crafted
> > properly.
> >
> > Share the ACPI excerpt and we can discuss further how to improve
> > them.
> >
>
> For a bit of background, MFD support was discussed here initially:
> https://lkml.org/lkml/2017/6/13/796
>
> Here is the ACPI table:
> Scope(_SB) {
> Device (LPC0) {
> Name (_HID, "HISI0191") // HiSi LPC
> Name (_CRS, ResourceTemplate () {
> Memory32Fixed (ReadWrite, 0xa01b0000, 0x1000)
> })
> }
>
> Device (LPC0.CON0) {
> Name (_HID, "HISI1031")
> // Name (_CID, "PNP0501") // cannot support PNP
> Name (LORS, ResourceTemplate() {
> QWordIO (
> ResourceConsumer,
> MinNotFixed, // _MIF
> MaxNotFixed, // _MAF
> PosDecode,
> EntireRange,
> 0x0, // _GRA
> 0x2F8, // _MIN
> 0x3fff, // _MAX
Shouldn't be 0x2ff ?
> 0x0, // _TRA
> 0x08, // _LEN
> , ,
> IO02
>
>
> The latest framework changes and host driver patchset are here:
> https://lkml.org/lkml/2018/2/19/465
>
It still doesn't explain impediments you have.
Just few approaches comes to my mind:
- move UART outside of parent device
- register PNP driver manually for that cell instead of MFD
- use serial8250 platform driver (I totally forgot that we have a
generic platform driver, so, it might be what you need to use at the
end)
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox