* Re: [PATCH 4/6] tty: serial: lpuart: add imx7ulp support
From: Dong Aisheng @ 2017-05-10 6:14 UTC (permalink / raw)
To: Stefan Agner
Cc: Dong Aisheng, linux-serial, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, gregkh, jslaby, Fugang Duan,
Mingkai.Hu, yangbo.lu
In-Reply-To: <697fae59cb4e5ff921bb83614325b3f1@agner.ch>
Hi Stefan,
On Wed, May 10, 2017 at 12:10 PM, Stefan Agner <stefan@agner.ch> wrote:
> On 2017-05-09 00:50, Dong Aisheng wrote:
>> The lpuart of imx7ulp is basically the same as ls1021a. It's also
>> 32 bit width register, but unlike ls1021a, it's little endian.
>> Besides that, imx7ulp lpuart has a minor different register layout
>> from ls1021a that it has four extra registers (verid, param, global,
>> pincfg) located at the beginning of register map, which are currently
>> not used by the driver and less to be used later.
>>
>> To ease the register difference handling, we add a reg_off member
>> in lpuart_soc_data structure to represent if the normal
>> lpuart32_{read|write} requires plus a offset to hide the issue.
>>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Jiri Slaby <jslaby@suse.com>
>> Cc: Fugang Duan <fugang.duan@nxp.com>
>> Cc: Stefan Agner <stefan@agner.ch>
>> Cc: Mingkai Hu <Mingkai.Hu@nxp.com>
>> Cc: Yangbo Lu <yangbo.lu@nxp.com>
>> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
>> ---
>> drivers/tty/serial/fsl_lpuart.c | 21 ++++++++++++++++++---
>> 1 file changed, 18 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
>> index bddd041..1cdb3f9 100644
>> --- a/drivers/tty/serial/fsl_lpuart.c
>> +++ b/drivers/tty/serial/fsl_lpuart.c
>> @@ -231,7 +231,11 @@
>> #define DEV_NAME "ttyLP"
>> #define UART_NR 6
>>
>> +/* IMX lpuart has four extra unused regs located at the beginning */
>> +#define IMX_REG_OFF 0x10
>> +
>> static bool lpuart_is_be;
>> +static u8 lpuart_reg_off;
>
> Global variables? That hardly works once you have two UARTs...
>
lpuart_reg_off is SoC specific and there's no two UART
with two different reg offset.
> Instead of adding a fixed offset to any write you could just add the
> offset to sport->port.membase...
>
That's intended as i don't want the changes to be too intrusive.
If adding offset in sport->port.xxx, then we have to change the basic
lpuart32_read/write API which is called through the whole driver.
Regards
Dong Aisheng
> --
> Stefan
>
>>
>> struct lpuart_port {
>> struct uart_port port;
>> @@ -263,6 +267,7 @@ struct lpuart_port {
>> struct lpuart_soc_data {
>> bool is_32;
>> bool is_be;
>> + u8 reg_off;
>> };
>>
>> static struct lpuart_soc_data vf_data = {
>> @@ -272,11 +277,19 @@ static struct lpuart_soc_data vf_data = {
>> static struct lpuart_soc_data ls_data = {
>> .is_32 = true,
>> .is_be = true,
>> + .reg_off = 0x0,
>> +};
>> +
>> +static struct lpuart_soc_data imx_data = {
>> + .is_32 = true,
>> + .is_be = false,
>> + .reg_off = IMX_REG_OFF,
>> };
>>
>> static const struct of_device_id lpuart_dt_ids[] = {
>> { .compatible = "fsl,vf610-lpuart", .data = &vf_data, },
>> { .compatible = "fsl,ls1021a-lpuart", .data = &ls_data, },
>> + { .compatible = "fsl,imx7ulp-lpuart", .data = &imx_data, },
>> { /* sentinel */ }
>> };
>> MODULE_DEVICE_TABLE(of, lpuart_dt_ids);
>> @@ -286,15 +299,16 @@ static void lpuart_dma_tx_complete(void *arg);
>>
>> static u32 lpuart32_read(void __iomem *addr)
>> {
>> - return lpuart_is_be ? ioread32be(addr) : readl(addr);
>> + return lpuart_is_be ? ioread32be(addr + lpuart_reg_off) :
>> + readl(addr + lpuart_reg_off);
>> }
>>
>> static void lpuart32_write(u32 val, void __iomem *addr)
>> {
>> if (lpuart_is_be)
>> - iowrite32be(val, addr);
>> + iowrite32be(val, addr + lpuart_reg_off);
>> else
>> - writel(val, addr);
>> + writel(val, addr + lpuart_reg_off);
>> }
>>
>> static void lpuart_stop_tx(struct uart_port *port)
>> @@ -2008,6 +2022,7 @@ static int lpuart_probe(struct platform_device *pdev)
>> sport->port.line = ret;
>> sport->lpuart32 = sdata->is_32;
>> lpuart_is_be = sdata->is_be;
>> + lpuart_reg_off = sdata->reg_off;
>>
>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> sport->port.membase = devm_ioremap_resource(&pdev->dev, res);
^ permalink raw reply
* Re: [PATCH 1/6] tty: serial: lpuart: introduce lpuart_soc_data to represent SoC property
From: Dong Aisheng @ 2017-05-10 6:06 UTC (permalink / raw)
To: Stefan Agner
Cc: Dong Aisheng, linux-serial, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, gregkh, jslaby, Fugang Duan,
Mingkai.Hu, yangbo.lu
In-Reply-To: <83e341e654f5e8e788ee684bf3de51e6@agner.ch>
Hi Stefan,
On Wed, May 10, 2017 at 11:50 AM, Stefan Agner <stefan@agner.ch> wrote:
> On 2017-05-09 00:50, Dong Aisheng wrote:
>> This is used to dynamically check the SoC specific lpuart properies.
>> Currently only the checking of 32 bit register width is added which
>> functions the same as before. More will be added later for supporting
>> new chips.
>>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Jiri Slaby <jslaby@suse.com>
>> Cc: Fugang Duan <fugang.duan@nxp.com>
>> Cc: Stefan Agner <stefan@agner.ch>
>> Cc: Mingkai Hu <Mingkai.Hu@nxp.com>
>> Cc: Yangbo Lu <yangbo.lu@nxp.com>
>> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
>> ---
>> drivers/tty/serial/fsl_lpuart.c | 25 ++++++++++++++++++-------
>> 1 file changed, 18 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
>> index 15df1ba7..cd4e905 100644
>> --- a/drivers/tty/serial/fsl_lpuart.c
>> +++ b/drivers/tty/serial/fsl_lpuart.c
>> @@ -258,13 +258,21 @@ struct lpuart_port {
>> wait_queue_head_t dma_wait;
>> };
>>
>> +struct lpuart_soc_data {
>> + bool is_32;
>> +};
>> +
>> +static struct lpuart_soc_data vf_data = {
>> + .is_32 = false,
>> +};
>> +
>> +static struct lpuart_soc_data ls_data = {
>> + .is_32 = true,
>> +};
>
> This could be const I guess?
>
Yes, of course.
Thanks for the pointing out.
Regards
Dong Aisheng
> --
> Stefan
>
>> +
>> static const struct of_device_id lpuart_dt_ids[] = {
>> - {
>> - .compatible = "fsl,vf610-lpuart",
>> - },
>> - {
>> - .compatible = "fsl,ls1021a-lpuart",
>> - },
>> + { .compatible = "fsl,vf610-lpuart", .data = &vf_data, },
>> + { .compatible = "fsl,ls1021a-lpuart", .data = &ls_data, },
>> { /* sentinel */ }
>> };
>> MODULE_DEVICE_TABLE(of, lpuart_dt_ids);
>> @@ -1971,6 +1979,9 @@ static struct uart_driver lpuart_reg = {
>>
>> static int lpuart_probe(struct platform_device *pdev)
>> {
>> + const struct of_device_id *of_id = of_match_device(lpuart_dt_ids,
>> + &pdev->dev);
>> + const struct lpuart_soc_data *sdata = of_id->data;
>> struct device_node *np = pdev->dev.of_node;
>> struct lpuart_port *sport;
>> struct resource *res;
>> @@ -1988,7 +1999,7 @@ static int lpuart_probe(struct platform_device *pdev)
>> return ret;
>> }
>> sport->port.line = ret;
>> - sport->lpuart32 = of_device_is_compatible(np, "fsl,ls1021a-lpuart");
>> + sport->lpuart32 = sdata->is_32;
>>
>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> sport->port.membase = devm_ioremap_resource(&pdev->dev, res);
^ permalink raw reply
* Re: [PATCH 4/6] tty: serial: lpuart: add imx7ulp support
From: Stefan Agner @ 2017-05-10 4:10 UTC (permalink / raw)
To: Dong Aisheng
Cc: linux-serial, linux-kernel, linux-arm-kernel, gregkh, jslaby,
fugang.duan, Mingkai.Hu, yangbo.lu
In-Reply-To: <1494316248-24052-5-git-send-email-aisheng.dong@nxp.com>
On 2017-05-09 00:50, Dong Aisheng wrote:
> The lpuart of imx7ulp is basically the same as ls1021a. It's also
> 32 bit width register, but unlike ls1021a, it's little endian.
> Besides that, imx7ulp lpuart has a minor different register layout
> from ls1021a that it has four extra registers (verid, param, global,
> pincfg) located at the beginning of register map, which are currently
> not used by the driver and less to be used later.
>
> To ease the register difference handling, we add a reg_off member
> in lpuart_soc_data structure to represent if the normal
> lpuart32_{read|write} requires plus a offset to hide the issue.
>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jiri Slaby <jslaby@suse.com>
> Cc: Fugang Duan <fugang.duan@nxp.com>
> Cc: Stefan Agner <stefan@agner.ch>
> Cc: Mingkai Hu <Mingkai.Hu@nxp.com>
> Cc: Yangbo Lu <yangbo.lu@nxp.com>
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---
> drivers/tty/serial/fsl_lpuart.c | 21 ++++++++++++++++++---
> 1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> index bddd041..1cdb3f9 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -231,7 +231,11 @@
> #define DEV_NAME "ttyLP"
> #define UART_NR 6
>
> +/* IMX lpuart has four extra unused regs located at the beginning */
> +#define IMX_REG_OFF 0x10
> +
> static bool lpuart_is_be;
> +static u8 lpuart_reg_off;
Global variables? That hardly works once you have two UARTs...
Instead of adding a fixed offset to any write you could just add the
offset to sport->port.membase...
--
Stefan
>
> struct lpuart_port {
> struct uart_port port;
> @@ -263,6 +267,7 @@ struct lpuart_port {
> struct lpuart_soc_data {
> bool is_32;
> bool is_be;
> + u8 reg_off;
> };
>
> static struct lpuart_soc_data vf_data = {
> @@ -272,11 +277,19 @@ static struct lpuart_soc_data vf_data = {
> static struct lpuart_soc_data ls_data = {
> .is_32 = true,
> .is_be = true,
> + .reg_off = 0x0,
> +};
> +
> +static struct lpuart_soc_data imx_data = {
> + .is_32 = true,
> + .is_be = false,
> + .reg_off = IMX_REG_OFF,
> };
>
> static const struct of_device_id lpuart_dt_ids[] = {
> { .compatible = "fsl,vf610-lpuart", .data = &vf_data, },
> { .compatible = "fsl,ls1021a-lpuart", .data = &ls_data, },
> + { .compatible = "fsl,imx7ulp-lpuart", .data = &imx_data, },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, lpuart_dt_ids);
> @@ -286,15 +299,16 @@ static void lpuart_dma_tx_complete(void *arg);
>
> static u32 lpuart32_read(void __iomem *addr)
> {
> - return lpuart_is_be ? ioread32be(addr) : readl(addr);
> + return lpuart_is_be ? ioread32be(addr + lpuart_reg_off) :
> + readl(addr + lpuart_reg_off);
> }
>
> static void lpuart32_write(u32 val, void __iomem *addr)
> {
> if (lpuart_is_be)
> - iowrite32be(val, addr);
> + iowrite32be(val, addr + lpuart_reg_off);
> else
> - writel(val, addr);
> + writel(val, addr + lpuart_reg_off);
> }
>
> static void lpuart_stop_tx(struct uart_port *port)
> @@ -2008,6 +2022,7 @@ static int lpuart_probe(struct platform_device *pdev)
> sport->port.line = ret;
> sport->lpuart32 = sdata->is_32;
> lpuart_is_be = sdata->is_be;
> + lpuart_reg_off = sdata->reg_off;
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> sport->port.membase = devm_ioremap_resource(&pdev->dev, res);
^ permalink raw reply
* Re: [PATCH 2/6] tty: serial: lpuart: add little endian 32 bit register support
From: Stefan Agner @ 2017-05-10 3:58 UTC (permalink / raw)
To: Dong Aisheng
Cc: linux-serial, linux-kernel, linux-arm-kernel, gregkh, jslaby,
fugang.duan, Mingkai.Hu, yangbo.lu
In-Reply-To: <1494316248-24052-3-git-send-email-aisheng.dong@nxp.com>
On 2017-05-09 00:50, Dong Aisheng wrote:
> It's based on the exist lpuart32 read/write implementation.
>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jiri Slaby <jslaby@suse.com> (supporter:TTY LAYER)
> Cc: Fugang Duan <fugang.duan@nxp.com>
> Cc: Stefan Agner <stefan@agner.ch>
> Cc: Mingkai Hu <Mingkai.Hu@nxp.com>
> Cc: Yangbo Lu <yangbo.lu@nxp.com>
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---
> drivers/tty/serial/fsl_lpuart.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> index cd4e905..bddd041 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -231,6 +231,8 @@
> #define DEV_NAME "ttyLP"
> #define UART_NR 6
>
> +static bool lpuart_is_be;
> +
Other LS1021a IP's such as SPI use the big-endian device tree property
along with regmap.
See e.g.
drivers/spi/spi-fsl-dspi.c
(Used in vf610 in little endian mode and ls1021a in big endian)
Not sure if we want to switch to regmap, but you can also get the
property using of_get_property.
The ls1021a lpuart node do not specify big-endian at the moment (would
probably good to add it), so I would leave big-endian the driver default
and check for little-endian for the new device and check whether that is
specified:
of_get_property(dn, "little-endian", NULL)
--
Stefan
> struct lpuart_port {
> struct uart_port port;
> struct clk *clk;
> @@ -260,6 +262,7 @@ struct lpuart_port {
>
> struct lpuart_soc_data {
> bool is_32;
> + bool is_be;
> };
>
> static struct lpuart_soc_data vf_data = {
> @@ -268,6 +271,7 @@ static struct lpuart_soc_data vf_data = {
>
> static struct lpuart_soc_data ls_data = {
> .is_32 = true,
> + .is_be = true,
> };
>
> static const struct of_device_id lpuart_dt_ids[] = {
> @@ -282,12 +286,15 @@ static void lpuart_dma_tx_complete(void *arg);
>
> static u32 lpuart32_read(void __iomem *addr)
> {
> - return ioread32be(addr);
> + return lpuart_is_be ? ioread32be(addr) : readl(addr);
> }
>
> static void lpuart32_write(u32 val, void __iomem *addr)
> {
> - iowrite32be(val, addr);
> + if (lpuart_is_be)
> + iowrite32be(val, addr);
> + else
> + writel(val, addr);
> }
>
> static void lpuart_stop_tx(struct uart_port *port)
> @@ -2000,6 +2007,7 @@ static int lpuart_probe(struct platform_device *pdev)
> }
> sport->port.line = ret;
> sport->lpuart32 = sdata->is_32;
> + lpuart_is_be = sdata->is_be;
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> sport->port.membase = devm_ioremap_resource(&pdev->dev, res);
^ permalink raw reply
* Re: [PATCH 1/6] tty: serial: lpuart: introduce lpuart_soc_data to represent SoC property
From: Stefan Agner @ 2017-05-10 3:50 UTC (permalink / raw)
To: Dong Aisheng
Cc: linux-serial, linux-kernel, linux-arm-kernel, gregkh, jslaby,
fugang.duan, Mingkai.Hu, yangbo.lu
In-Reply-To: <1494316248-24052-2-git-send-email-aisheng.dong@nxp.com>
On 2017-05-09 00:50, Dong Aisheng wrote:
> This is used to dynamically check the SoC specific lpuart properies.
> Currently only the checking of 32 bit register width is added which
> functions the same as before. More will be added later for supporting
> new chips.
>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jiri Slaby <jslaby@suse.com>
> Cc: Fugang Duan <fugang.duan@nxp.com>
> Cc: Stefan Agner <stefan@agner.ch>
> Cc: Mingkai Hu <Mingkai.Hu@nxp.com>
> Cc: Yangbo Lu <yangbo.lu@nxp.com>
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---
> drivers/tty/serial/fsl_lpuart.c | 25 ++++++++++++++++++-------
> 1 file changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> index 15df1ba7..cd4e905 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -258,13 +258,21 @@ struct lpuart_port {
> wait_queue_head_t dma_wait;
> };
>
> +struct lpuart_soc_data {
> + bool is_32;
> +};
> +
> +static struct lpuart_soc_data vf_data = {
> + .is_32 = false,
> +};
> +
> +static struct lpuart_soc_data ls_data = {
> + .is_32 = true,
> +};
This could be const I guess?
--
Stefan
> +
> static const struct of_device_id lpuart_dt_ids[] = {
> - {
> - .compatible = "fsl,vf610-lpuart",
> - },
> - {
> - .compatible = "fsl,ls1021a-lpuart",
> - },
> + { .compatible = "fsl,vf610-lpuart", .data = &vf_data, },
> + { .compatible = "fsl,ls1021a-lpuart", .data = &ls_data, },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, lpuart_dt_ids);
> @@ -1971,6 +1979,9 @@ static struct uart_driver lpuart_reg = {
>
> static int lpuart_probe(struct platform_device *pdev)
> {
> + const struct of_device_id *of_id = of_match_device(lpuart_dt_ids,
> + &pdev->dev);
> + const struct lpuart_soc_data *sdata = of_id->data;
> struct device_node *np = pdev->dev.of_node;
> struct lpuart_port *sport;
> struct resource *res;
> @@ -1988,7 +1999,7 @@ static int lpuart_probe(struct platform_device *pdev)
> return ret;
> }
> sport->port.line = ret;
> - sport->lpuart32 = of_device_is_compatible(np, "fsl,ls1021a-lpuart");
> + sport->lpuart32 = sdata->is_32;
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> sport->port.membase = devm_ioremap_resource(&pdev->dev, res);
^ permalink raw reply
* Re: [regression, bisected] 4.11+ imx uarts broken
From: Uwe Kleine-König @ 2017-05-09 14:31 UTC (permalink / raw)
To: Mika Penttilä; +Cc: kernel, linux-serial, linux-kernel
In-Reply-To: <a1fa7693-c9e9-fea8-fbb7-a20ebf1dd287@nextfour.com>
On Tue, May 09, 2017 at 10:25:40AM +0300, Mika Penttilä wrote:
> On 05/09/2017 10:14 AM, Uwe Kleine-König wrote:
> > Hello Mika,
> >
> > On Tue, May 09, 2017 at 07:18:09AM +0300, Mika Penttilä wrote:
> >> The following commit e61c38d85b7 makes the uarts on i.MX6 nonfunctional (no data transmitted or received).
> >> With e61c38d85b7 reverted the uarts work ok.
> >>
> >> -------------------
> >> commit e61c38d85b7392e033ee03bca46f1d6006156175
> >> Author: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> >> Date: Tue Apr 4 11:18:51 2017 +0200
> >>
> >> serial: imx: setup DCEDTE early and ensure DCD and RI irqs to be off
> >>
> >> --------------------
> >
> > are you operating the UART in DTE or DCE mode? Does this affect all
> > UARTs or only those that are not used in the bootloader?
>
> I am operating in DCE mode. The debug/console uart works ok, but two others don't.
>
> >
> > Looking at the patch I wonder if setting IMX21_UCR3_RXDMUXSEL |
> > UCR3_ADNIMP is missing for you.
> >
>
> Probably yes, but I can verify this later and get back to you.
can you please test this patch:
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 33509b4beaec..2182548ff0e1 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -2193,9 +2193,14 @@ static int serial_imx_probe(struct platform_device *pdev)
*/
writel(IMX21_UCR3_RXDMUXSEL | UCR3_ADNIMP | UCR3_DSR,
sport->port.membase + UCR3);
-
} else {
+ unsigned long ucr3 = UCR3_DSR;
+
+ if (!is_imx1_uart(sport))
+ ucr3 |= IMX21_UCR3_RXDMUXSEL | UCR3_ADNIMP;
+
writel(0, sport->port.membase + UFCR);
+ writel(ucr3, sport->port.membase + UCR3);
}
clk_disable_unprepare(sport->clk_ipg);
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply related
* RE: [PATCH 0/6] tty: serial: lpuart: add imx7ulp support
From: Andy Duan @ 2017-05-09 11:13 UTC (permalink / raw)
To: linux-serial@vger.kernel.org
Cc: linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, gregkh@linuxfoundation.org,
jslaby@suse.com, stefan@agner.ch, Mingkai Hu, Y.B. Lu, A.S. Dong
In-Reply-To: <1494316248-24052-1-git-send-email-aisheng.dong@nxp.com>
From: Dong Aisheng <aisheng.dong@nxp.com> Sent: Tuesday, May 09, 2017 3:51 PM
>To: linux-serial@vger.kernel.org
>Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
>gregkh@linuxfoundation.org; jslaby@suse.com; Andy Duan
><fugang.duan@nxp.com>; stefan@agner.ch; Mingkai Hu
><mingkai.hu@nxp.com>; Y.B. Lu <yangbo.lu@nxp.com>; A.S. Dong
><aisheng.dong@nxp.com>
>Subject: [PATCH 0/6] tty: serial: lpuart: add imx7ulp support
>
>This patch series mainly intends to add imx7ulp support which is also using FSL
>lpuart.
>
>The lpuart in imx7ulp is basically the same as ls1021a. It's also
>32 bit width register, but unlike ls1021a, it's little endian.
>Besides that, imx7ulp lpuart has a minor different register layout from ls1021a
>that it has four extra registers (verid, param, global,
>pincfg) located at the beginning of register map, which are currently not used
>by the driver and less to be used later.
>
>Furthermore, this patch serial also add a new more accurate baud rate
>calculation method as MX7ULP can't divide a suitable baud rate with the
>default setting.
>
>Currently the new baud rate calculation is only enabled on MX7ULP.
>However, i guess the Layerscape may also be able to use it as there seems to
>be no difference in baud rate setting register after checking the Layerscape
>Reference Manual.
>
>As i don't have Layerscape boards, i can't test it, so i only enable it for MX7ULP
>by default to avoid a potential break.
>
>I copied LayerScape guys in this series and hope they can help test later.
>If it works on Layerscape as well, then they can switch to the new setting too
>and totally remove the old stuff.
>
>Dong Aisheng (6):
> tty: serial: lpuart: introduce lpuart_soc_data to represent SoC
> property
> tty: serial: lpuart: add little endian 32 bit register support
> dt-bindings: serial: fsl-lpuart: add i.MX7ULP support
> tty: serial: lpuart: add imx7ulp support
> tty: serial: lpuart: add earlycon support for imx7ulp
> tty: serial: lpuart: add a more accurate baud rate calculation method
>
> .../devicetree/bindings/serial/fsl-lpuart.txt | 2 +
> drivers/tty/serial/fsl_lpuart.c | 149 ++++++++++++++++++---
> 2 files changed, 136 insertions(+), 15 deletions(-)
>
>--
>2.7.4
The series looks fine.
Acked-by: Fugang Duan <fugang.duan@nxp.com>
^ permalink raw reply
* Re: [PATCH v9 3/3] printk: fix double printing with earlycon
From: Sabrina Dubroca @ 2017-05-09 8:29 UTC (permalink / raw)
To: Aleksey Makarov
Cc: linux-serial, linux-kernel, Sudeep Holla, Greg Kroah-Hartman,
Peter Hurley, Jiri Slaby, Robin Murphy, Steven Rostedt,
Nair, Jayachandran, Sergey Senozhatsky, Petr Mladek
In-Reply-To: <20170405202006.18234-1-aleksey.makarov@linaro.org>
Hi Aleksey,
2017-04-05, 23:20:00 +0300, Aleksey Makarov wrote:
> If a console was specified by ACPI SPCR table _and_ command line
> parameters like "console=ttyAMA0" _and_ "earlycon" were specified,
> then log messages appear twice.
>
> The root cause is that the code traverses the list of specified
> consoles (the `console_cmdline` array) and stops at the first match.
> But it may happen that the same console is referred by the elements
> of this array twice:
>
> pl011,mmio,0x87e024000000,115200 -- from SPCR
> ttyAMA0 -- from command line
>
> but in this case `preferred_console` points to the second entry and
> the flag CON_CONSDEV is not set, so bootconsole is not deregistered.
>
> To fix that, introduce an invariant "The last non-braille console
> is always the preferred one" on the entries of the console_cmdline
> array. Then traverse it in reverse order to be sure that if
> the console is preferred then it will be the first matching entry.
> Introduce variable console_cmdline_cnt that keeps the number
> of elements of the console_cmdline array (Petr Mladek). It helps
> to get rid of the loop that searches for the end of this array.
That's caused a change of behavior in my qemu setup, with this cmdline
root=/dev/sda1 console=ttyS1 console=ttyS0
Before, the kernel logs appeared on ttyS1, and I logged in with ttyS0
(with my setup, ttyS1 is a file and ttyS0 is unix socket). Now, the
kernel logs go to ttyS0. I need to swap the two console= parameters to
restore behavior.
There might be some other problem (in qemu?) though, because adding
console=tty0 anywhere on that cmdline makes the logs appear on both
tty0 and one ttyS* (but only one of them, and the ordering of the
ttyS* matters).
Thanks,
--
Sabrina
^ permalink raw reply
* [PATCH 6/6] tty: serial: lpuart: add a more accurate baud rate calculation method
From: Dong Aisheng @ 2017-05-09 7:50 UTC (permalink / raw)
To: linux-serial
Cc: linux-kernel, linux-arm-kernel, gregkh, jslaby, fugang.duan,
stefan, Mingkai.Hu, yangbo.lu, Dong Aisheng
In-Reply-To: <1494316248-24052-1-git-send-email-aisheng.dong@nxp.com>
On new LPUART versions, the oversampling ratio for the receiver can be
changed from 4x (00011) to 32x (11111) which could help us get a more
accurate baud rate divider.
The idea is to use the best OSR (over-sampling rate) possible.
Note, OSR is typically hard-set to 16 in other LPUART instantiations.
Loop to find the best OSR value possible, one that generates minimum
baud diff iterate through the rest of the supported values of OSR.
Currently only i.MX7ULP is using it.
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Fugang Duan <fugang.duan@nxp.com>
Cc: Stefan Agner <stefan@agner.ch>
Cc: Mingkai Hu <Mingkai.Hu@nxp.com>
Cc: Yangbo Lu <yangbo.lu@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
drivers/tty/serial/fsl_lpuart.c | 85 ++++++++++++++++++++++++++++++++++++++---
1 file changed, 79 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 5b485e8..6b2abb7 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -140,6 +140,8 @@
#define UARTBAUD_SBNS 0x00002000
#define UARTBAUD_SBR 0x00000000
#define UARTBAUD_SBR_MASK 0x1fff
+#define UARTBAUD_OSR_MASK 0x1f
+#define UARTBAUD_OSR_SHIFT 24
#define UARTSTAT_LBKDIF 0x80000000
#define UARTSTAT_RXEDGIF 0x40000000
@@ -1508,6 +1510,72 @@ lpuart_set_termios(struct uart_port *port, struct ktermios *termios,
}
static void
+lpuart32_serial_setbrg(struct lpuart_port *sport, unsigned int baudrate)
+{
+ u32 sbr, osr, baud_diff, tmp_osr, tmp_sbr, tmp_diff, tmp;
+ u32 clk = sport->port.uartclk;
+
+ /*
+ * The idea is to use the best OSR (over-sampling rate) possible.
+ * Note, OSR is typically hard-set to 16 in other LPUART instantiations.
+ * Loop to find the best OSR value possible, one that generates minimum
+ * baud_diff iterate through the rest of the supported values of OSR.
+ *
+ * Calculation Formula:
+ * Baud Rate = baud clock / ((OSR+1) × SBR)
+ */
+ baud_diff = baudrate;
+ osr = 0;
+ sbr = 0;
+
+ for (tmp_osr = 4; tmp_osr <= 32; tmp_osr++) {
+ /* calculate the temporary sbr value */
+ tmp_sbr = (clk / (baudrate * tmp_osr));
+ if (tmp_sbr == 0)
+ tmp_sbr = 1;
+
+ /*
+ * calculate the baud rate difference based on the temporary
+ * osr and sbr values
+ */
+ tmp_diff = clk / (tmp_osr * tmp_sbr) - baudrate;
+
+ /* select best values between sbr and sbr+1 */
+ tmp = clk / (tmp_osr * (tmp_sbr + 1));
+ if (tmp_diff > (baudrate - tmp)) {
+ tmp_diff = baudrate - tmp;
+ tmp_sbr++;
+ }
+
+ if (tmp_diff <= baud_diff) {
+ baud_diff = tmp_diff;
+ osr = tmp_osr;
+ sbr = tmp_sbr;
+ }
+ }
+
+ /* handle buadrate outside acceptable rate */
+ if (baud_diff > ((baudrate / 100) * 3))
+ dev_warn(sport->port.dev,
+ "unacceptable baud rate difference of more than 3%%\n");
+
+ tmp = lpuart32_read(sport->port.membase + UARTBAUD);
+
+ if ((osr > 3) && (osr < 8))
+ tmp |= UARTBAUD_BOTHEDGE;
+
+ tmp &= ~(UARTBAUD_OSR_MASK << UARTBAUD_OSR_SHIFT);
+ tmp |= (((osr-1) & UARTBAUD_OSR_MASK) << UARTBAUD_OSR_SHIFT);
+
+ tmp &= ~UARTBAUD_SBR_MASK;
+ tmp |= sbr & UARTBAUD_SBR_MASK;
+
+ tmp &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
+
+ lpuart32_write(tmp, sport->port.membase + UARTBAUD);
+}
+
+static void
lpuart32_set_termios(struct uart_port *port, struct ktermios *termios,
struct ktermios *old)
{
@@ -1613,12 +1681,17 @@ lpuart32_set_termios(struct uart_port *port, struct ktermios *termios,
lpuart32_write(old_ctrl & ~(UARTCTRL_TE | UARTCTRL_RE),
sport->port.membase + UARTCTRL);
- sbr = sport->port.uartclk / (16 * baud);
- bd &= ~UARTBAUD_SBR_MASK;
- bd |= sbr & UARTBAUD_SBR_MASK;
- bd |= UARTBAUD_BOTHEDGE;
- bd &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
- lpuart32_write(bd, sport->port.membase + UARTBAUD);
+ if (of_device_is_compatible(port->dev->of_node, "fsl,imx7ulp-lpuart")) {
+ lpuart32_serial_setbrg(sport, baud);
+ } else {
+ sbr = sport->port.uartclk / (16 * baud);
+ bd &= ~UARTBAUD_SBR_MASK;
+ bd |= sbr & UARTBAUD_SBR_MASK;
+ bd |= UARTBAUD_BOTHEDGE;
+ bd &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
+ lpuart32_write(bd, sport->port.membase + UARTBAUD);
+ }
+
lpuart32_write(modem, sport->port.membase + UARTMODIR);
lpuart32_write(ctrl, sport->port.membase + UARTCTRL);
/* restore control register */
--
2.7.4
^ permalink raw reply related
* [PATCH 5/6] tty: serial: lpuart: add earlycon support for imx7ulp
From: Dong Aisheng @ 2017-05-09 7:50 UTC (permalink / raw)
To: linux-serial
Cc: linux-kernel, linux-arm-kernel, gregkh, jslaby, fugang.duan,
stefan, Mingkai.Hu, yangbo.lu, Dong Aisheng
In-Reply-To: <1494316248-24052-1-git-send-email-aisheng.dong@nxp.com>
earlycon is executed quite early before the device tree probe,
so we need configure the correct reg_off for imx7ulp during
early console setup.
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Fugang Duan <fugang.duan@nxp.com>
Cc: Stefan Agner <stefan@agner.ch>
Cc: Mingkai Hu <Mingkai.Hu@nxp.com>
Cc: Yangbo Lu <yangbo.lu@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
drivers/tty/serial/fsl_lpuart.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 1cdb3f9..5b485e8 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -1978,8 +1978,20 @@ static int __init lpuart32_early_console_setup(struct earlycon_device *device,
return 0;
}
+static int __init lpuart32_imx_early_console_setup(struct earlycon_device *device,
+ const char *opt)
+{
+ if (!device->port.membase)
+ return -ENODEV;
+
+ lpuart_reg_off = IMX_REG_OFF;
+ device->con->write = lpuart32_early_write;
+
+ return 0;
+}
OF_EARLYCON_DECLARE(lpuart, "fsl,vf610-lpuart", lpuart_early_console_setup);
OF_EARLYCON_DECLARE(lpuart32, "fsl,ls1021a-lpuart", lpuart32_early_console_setup);
+OF_EARLYCON_DECLARE(lpuart32, "fsl,imx7ulp-lpuart", lpuart32_imx_early_console_setup);
EARLYCON_DECLARE(lpuart, lpuart_early_console_setup);
EARLYCON_DECLARE(lpuart32, lpuart32_early_console_setup);
--
2.7.4
^ permalink raw reply related
* [PATCH 4/6] tty: serial: lpuart: add imx7ulp support
From: Dong Aisheng @ 2017-05-09 7:50 UTC (permalink / raw)
To: linux-serial
Cc: linux-kernel, linux-arm-kernel, gregkh, jslaby, fugang.duan,
stefan, Mingkai.Hu, yangbo.lu, Dong Aisheng
In-Reply-To: <1494316248-24052-1-git-send-email-aisheng.dong@nxp.com>
The lpuart of imx7ulp is basically the same as ls1021a. It's also
32 bit width register, but unlike ls1021a, it's little endian.
Besides that, imx7ulp lpuart has a minor different register layout
from ls1021a that it has four extra registers (verid, param, global,
pincfg) located at the beginning of register map, which are currently
not used by the driver and less to be used later.
To ease the register difference handling, we add a reg_off member
in lpuart_soc_data structure to represent if the normal
lpuart32_{read|write} requires plus a offset to hide the issue.
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Fugang Duan <fugang.duan@nxp.com>
Cc: Stefan Agner <stefan@agner.ch>
Cc: Mingkai Hu <Mingkai.Hu@nxp.com>
Cc: Yangbo Lu <yangbo.lu@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
drivers/tty/serial/fsl_lpuart.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index bddd041..1cdb3f9 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -231,7 +231,11 @@
#define DEV_NAME "ttyLP"
#define UART_NR 6
+/* IMX lpuart has four extra unused regs located at the beginning */
+#define IMX_REG_OFF 0x10
+
static bool lpuart_is_be;
+static u8 lpuart_reg_off;
struct lpuart_port {
struct uart_port port;
@@ -263,6 +267,7 @@ struct lpuart_port {
struct lpuart_soc_data {
bool is_32;
bool is_be;
+ u8 reg_off;
};
static struct lpuart_soc_data vf_data = {
@@ -272,11 +277,19 @@ static struct lpuart_soc_data vf_data = {
static struct lpuart_soc_data ls_data = {
.is_32 = true,
.is_be = true,
+ .reg_off = 0x0,
+};
+
+static struct lpuart_soc_data imx_data = {
+ .is_32 = true,
+ .is_be = false,
+ .reg_off = IMX_REG_OFF,
};
static const struct of_device_id lpuart_dt_ids[] = {
{ .compatible = "fsl,vf610-lpuart", .data = &vf_data, },
{ .compatible = "fsl,ls1021a-lpuart", .data = &ls_data, },
+ { .compatible = "fsl,imx7ulp-lpuart", .data = &imx_data, },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, lpuart_dt_ids);
@@ -286,15 +299,16 @@ static void lpuart_dma_tx_complete(void *arg);
static u32 lpuart32_read(void __iomem *addr)
{
- return lpuart_is_be ? ioread32be(addr) : readl(addr);
+ return lpuart_is_be ? ioread32be(addr + lpuart_reg_off) :
+ readl(addr + lpuart_reg_off);
}
static void lpuart32_write(u32 val, void __iomem *addr)
{
if (lpuart_is_be)
- iowrite32be(val, addr);
+ iowrite32be(val, addr + lpuart_reg_off);
else
- writel(val, addr);
+ writel(val, addr + lpuart_reg_off);
}
static void lpuart_stop_tx(struct uart_port *port)
@@ -2008,6 +2022,7 @@ static int lpuart_probe(struct platform_device *pdev)
sport->port.line = ret;
sport->lpuart32 = sdata->is_32;
lpuart_is_be = sdata->is_be;
+ lpuart_reg_off = sdata->reg_off;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
sport->port.membase = devm_ioremap_resource(&pdev->dev, res);
--
2.7.4
^ permalink raw reply related
* [PATCH 3/6] dt-bindings: serial: fsl-lpuart: add i.MX7ULP support
From: Dong Aisheng @ 2017-05-09 7:50 UTC (permalink / raw)
To: linux-serial
Cc: linux-kernel, linux-arm-kernel, gregkh, jslaby, fugang.duan,
stefan, Mingkai.Hu, yangbo.lu, Dong Aisheng, devicetree,
Rob Herring
In-Reply-To: <1494316248-24052-1-git-send-email-aisheng.dong@nxp.com>
The lpuart of imx7ulp is basically the same as ls1021a. It's also
32 bit width register, but unlike ls1021a, it's little endian.
Besides that, imx7ulp lpuart has a minor different register layout
from ls1021a.
Cc: devicetree@vger.kernel.org
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Fugang Duan <fugang.duan@nxp.com>
Cc: Stefan Agner <stefan@agner.ch>
Cc: Mingkai Hu <Mingkai.Hu@nxp.com>
Cc: Yangbo Lu <yangbo.lu@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
Documentation/devicetree/bindings/serial/fsl-lpuart.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/serial/fsl-lpuart.txt b/Documentation/devicetree/bindings/serial/fsl-lpuart.txt
index c95005e..a1252a0 100644
--- a/Documentation/devicetree/bindings/serial/fsl-lpuart.txt
+++ b/Documentation/devicetree/bindings/serial/fsl-lpuart.txt
@@ -6,6 +6,8 @@ Required properties:
on Vybrid vf610 SoC with 8-bit register organization
- "fsl,ls1021a-lpuart" for lpuart compatible with the one integrated
on LS1021A SoC with 32-bit big-endian register organization
+ - "fsl,imx7ulp-lpuart" for lpuart compatible with the one integrated
+ on i.MX7ULP SoC with 32-bit little-endian register organization
- reg : Address and length of the register set for the device
- interrupts : Should contain uart interrupt
- clocks : phandle + clock specifier pairs, one for each entry in clock-names
--
2.7.4
^ permalink raw reply related
* [PATCH 2/6] tty: serial: lpuart: add little endian 32 bit register support
From: Dong Aisheng @ 2017-05-09 7:50 UTC (permalink / raw)
To: linux-serial
Cc: linux-kernel, linux-arm-kernel, gregkh, jslaby, fugang.duan,
stefan, Mingkai.Hu, yangbo.lu, Dong Aisheng
In-Reply-To: <1494316248-24052-1-git-send-email-aisheng.dong@nxp.com>
It's based on the exist lpuart32 read/write implementation.
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com> (supporter:TTY LAYER)
Cc: Fugang Duan <fugang.duan@nxp.com>
Cc: Stefan Agner <stefan@agner.ch>
Cc: Mingkai Hu <Mingkai.Hu@nxp.com>
Cc: Yangbo Lu <yangbo.lu@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
drivers/tty/serial/fsl_lpuart.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index cd4e905..bddd041 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -231,6 +231,8 @@
#define DEV_NAME "ttyLP"
#define UART_NR 6
+static bool lpuart_is_be;
+
struct lpuart_port {
struct uart_port port;
struct clk *clk;
@@ -260,6 +262,7 @@ struct lpuart_port {
struct lpuart_soc_data {
bool is_32;
+ bool is_be;
};
static struct lpuart_soc_data vf_data = {
@@ -268,6 +271,7 @@ static struct lpuart_soc_data vf_data = {
static struct lpuart_soc_data ls_data = {
.is_32 = true,
+ .is_be = true,
};
static const struct of_device_id lpuart_dt_ids[] = {
@@ -282,12 +286,15 @@ static void lpuart_dma_tx_complete(void *arg);
static u32 lpuart32_read(void __iomem *addr)
{
- return ioread32be(addr);
+ return lpuart_is_be ? ioread32be(addr) : readl(addr);
}
static void lpuart32_write(u32 val, void __iomem *addr)
{
- iowrite32be(val, addr);
+ if (lpuart_is_be)
+ iowrite32be(val, addr);
+ else
+ writel(val, addr);
}
static void lpuart_stop_tx(struct uart_port *port)
@@ -2000,6 +2007,7 @@ static int lpuart_probe(struct platform_device *pdev)
}
sport->port.line = ret;
sport->lpuart32 = sdata->is_32;
+ lpuart_is_be = sdata->is_be;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
sport->port.membase = devm_ioremap_resource(&pdev->dev, res);
--
2.7.4
^ permalink raw reply related
* [PATCH 1/6] tty: serial: lpuart: introduce lpuart_soc_data to represent SoC property
From: Dong Aisheng @ 2017-05-09 7:50 UTC (permalink / raw)
To: linux-serial
Cc: Dong Aisheng, fugang.duan, gregkh, yangbo.lu, linux-kernel,
stefan, Mingkai.Hu, jslaby, linux-arm-kernel
In-Reply-To: <1494316248-24052-1-git-send-email-aisheng.dong@nxp.com>
This is used to dynamically check the SoC specific lpuart properies.
Currently only the checking of 32 bit register width is added which
functions the same as before. More will be added later for supporting
new chips.
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Fugang Duan <fugang.duan@nxp.com>
Cc: Stefan Agner <stefan@agner.ch>
Cc: Mingkai Hu <Mingkai.Hu@nxp.com>
Cc: Yangbo Lu <yangbo.lu@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
drivers/tty/serial/fsl_lpuart.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 15df1ba7..cd4e905 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -258,13 +258,21 @@ struct lpuart_port {
wait_queue_head_t dma_wait;
};
+struct lpuart_soc_data {
+ bool is_32;
+};
+
+static struct lpuart_soc_data vf_data = {
+ .is_32 = false,
+};
+
+static struct lpuart_soc_data ls_data = {
+ .is_32 = true,
+};
+
static const struct of_device_id lpuart_dt_ids[] = {
- {
- .compatible = "fsl,vf610-lpuart",
- },
- {
- .compatible = "fsl,ls1021a-lpuart",
- },
+ { .compatible = "fsl,vf610-lpuart", .data = &vf_data, },
+ { .compatible = "fsl,ls1021a-lpuart", .data = &ls_data, },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, lpuart_dt_ids);
@@ -1971,6 +1979,9 @@ static struct uart_driver lpuart_reg = {
static int lpuart_probe(struct platform_device *pdev)
{
+ const struct of_device_id *of_id = of_match_device(lpuart_dt_ids,
+ &pdev->dev);
+ const struct lpuart_soc_data *sdata = of_id->data;
struct device_node *np = pdev->dev.of_node;
struct lpuart_port *sport;
struct resource *res;
@@ -1988,7 +1999,7 @@ static int lpuart_probe(struct platform_device *pdev)
return ret;
}
sport->port.line = ret;
- sport->lpuart32 = of_device_is_compatible(np, "fsl,ls1021a-lpuart");
+ sport->lpuart32 = sdata->is_32;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
sport->port.membase = devm_ioremap_resource(&pdev->dev, res);
--
2.7.4
^ permalink raw reply related
* [PATCH 0/6] tty: serial: lpuart: add imx7ulp support
From: Dong Aisheng @ 2017-05-09 7:50 UTC (permalink / raw)
To: linux-serial
Cc: linux-kernel, linux-arm-kernel, gregkh, jslaby, fugang.duan,
stefan, Mingkai.Hu, yangbo.lu, Dong Aisheng
This patch series mainly intends to add imx7ulp support which is also
using FSL lpuart.
The lpuart in imx7ulp is basically the same as ls1021a. It's also
32 bit width register, but unlike ls1021a, it's little endian.
Besides that, imx7ulp lpuart has a minor different register layout
from ls1021a that it has four extra registers (verid, param, global,
pincfg) located at the beginning of register map, which are currently
not used by the driver and less to be used later.
Furthermore, this patch serial also add a new more accurate baud rate
calculation method as MX7ULP can't divide a suitable baud rate
with the default setting.
Currently the new baud rate calculation is only enabled on MX7ULP.
However, i guess the Layerscape may also be able to use it as there
seems to be no difference in baud rate setting register after checking
the Layerscape Reference Manual.
As i don't have Layerscape boards, i can't test it, so i only enable it
for MX7ULP by default to avoid a potential break.
I copied LayerScape guys in this series and hope they can help test later.
If it works on Layerscape as well, then they can switch to the new setting
too and totally remove the old stuff.
Dong Aisheng (6):
tty: serial: lpuart: introduce lpuart_soc_data to represent SoC
property
tty: serial: lpuart: add little endian 32 bit register support
dt-bindings: serial: fsl-lpuart: add i.MX7ULP support
tty: serial: lpuart: add imx7ulp support
tty: serial: lpuart: add earlycon support for imx7ulp
tty: serial: lpuart: add a more accurate baud rate calculation method
.../devicetree/bindings/serial/fsl-lpuart.txt | 2 +
drivers/tty/serial/fsl_lpuart.c | 149 ++++++++++++++++++---
2 files changed, 136 insertions(+), 15 deletions(-)
--
2.7.4
^ permalink raw reply
* Re: [regression, bisected] 4.11+ imx uarts broken
From: Mika Penttilä @ 2017-05-09 7:25 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: linux-serial, kernel, linux-kernel
In-Reply-To: <20170509071435.kxt6pjlglzinf2w5@pengutronix.de>
On 05/09/2017 10:14 AM, Uwe Kleine-König wrote:
> Hello Mika,
>
> On Tue, May 09, 2017 at 07:18:09AM +0300, Mika Penttilä wrote:
>> The following commit e61c38d85b7 makes the uarts on i.MX6 nonfunctional (no data transmitted or received).
>> With e61c38d85b7 reverted the uarts work ok.
>>
>> -------------------
>> commit e61c38d85b7392e033ee03bca46f1d6006156175
>> Author: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>> Date: Tue Apr 4 11:18:51 2017 +0200
>>
>> serial: imx: setup DCEDTE early and ensure DCD and RI irqs to be off
>>
>> --------------------
>
> are you operating the UART in DTE or DCE mode? Does this affect all
> UARTs or only those that are not used in the bootloader?
I am operating in DCE mode. The debug/console uart works ok, but two others don't.
>
> Looking at the patch I wonder if setting IMX21_UCR3_RXDMUXSEL |
> UCR3_ADNIMP is missing for you.
>
Probably yes, but I can verify this later and get back to you.
> Can you please check which hunk of e61c38d85b73 is giving you problems?
>
> Best regards
> Uwe
>
--Mika
^ permalink raw reply
* Re: [regression, bisected] 4.11+ imx uarts broken
From: Uwe Kleine-König @ 2017-05-09 7:14 UTC (permalink / raw)
To: Mika Penttilä; +Cc: linux-serial, kernel, linux-kernel
In-Reply-To: <f6fded83-f062-94da-ec1b-b2ab6e6c5101@nextfour.com>
Hello Mika,
On Tue, May 09, 2017 at 07:18:09AM +0300, Mika Penttilä wrote:
> The following commit e61c38d85b7 makes the uarts on i.MX6 nonfunctional (no data transmitted or received).
> With e61c38d85b7 reverted the uarts work ok.
>
> -------------------
> commit e61c38d85b7392e033ee03bca46f1d6006156175
> Author: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Date: Tue Apr 4 11:18:51 2017 +0200
>
> serial: imx: setup DCEDTE early and ensure DCD and RI irqs to be off
>
> --------------------
are you operating the UART in DTE or DCE mode? Does this affect all
UARTs or only those that are not used in the bootloader?
Looking at the patch I wonder if setting IMX21_UCR3_RXDMUXSEL |
UCR3_ADNIMP is missing for you.
Can you please check which hunk of e61c38d85b73 is giving you problems?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH] serdev: Restore serdev_device_write_buf for atomic context
From: Johan Hovold @ 2017-05-08 15:18 UTC (permalink / raw)
To: Rob Herring
Cc: Stefan Wahren, Johan Hovold, Greg Kroah-Hartman, Jiri Slaby,
Sebastian Reichel, Guenter Roeck, Andy Shevchenko, Andrey Smirnov,
linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CAL_JsqLCiyU8kU2NaLQRz9aS-k7TNr3Tf45Wt3H3DuSyMAuvDg@mail.gmail.com>
On Thu, May 04, 2017 at 03:32:53PM -0500, Rob Herring wrote:
> On Thu, May 4, 2017 at 11:22 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> > Am 02.05.2017 um 15:18 schrieb Johan Hovold:
> >> On Tue, May 02, 2017 at 07:41:34AM -0500, Rob Herring wrote:
> >>> On Tue, May 2, 2017 at 4:06 AM, Johan Hovold <johan@kernel.org> wrote:
> >>>> On Fri, Apr 28, 2017 at 01:47:21PM +0200, Stefan Wahren wrote:
> >>>>> Starting with commit 6fe729c4bdae ("serdev: Add serdev_device_write
> >>>>> subroutine") the function serdev_device_write_buf cannot be used in
> >>>>> atomic context anymore (mutex_lock is sleeping). So restore the old
> >>>>> behavior.
> >>>> Yeah, preventing use in atomic context seems unnecessary, although any
> >>>> clients writing must now deal with serialisation themselves (as before,
> >>>> and as they should).
> >>> We could just remove the mutex for serdev_device_write and always make
> >>> the client responsible for serialization.
> >> That sounds reasonable.
> >
> > So it's unwanted to have 2 write functions (non-atomic, atomic)?
>
> No, it's unwanted to have more than we need.
>
> Looking closer, we'd also have to ensure the wait for completion is
> not called also. So probably better to just leave it as you have done
> it.
Indeed. Sorry if my reply above was unclear on that point (i.e. that
Stefan's patch is still needed regardless of whether we keep the mutex
or not).
Thanks,
Johan
^ permalink raw reply
* Re: [GIT PULL] TTY patches for 4.12-rc1
From: Greg KH @ 2017-05-08 9:48 UTC (permalink / raw)
To: Linus Torvalds, Jiri Slaby
Cc: Stephen Rothwell, Andrew Morton, linux-kernel, linux-serial
In-Reply-To: <20170508094624.GA7452@kroah.com>
On Mon, May 08, 2017 at 11:46:24AM +0200, Greg KH wrote:
> The following changes since commit 4f7d029b9bf009fbee76bb10c0c4351a1870d2f3:
>
> Linux 4.11-rc7 (2017-04-16 13:00:18 -0700)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/ tags/tty-4.12-rc1
>
> for you to fetch changes up to 8e1c21f486944bf92f2a981f23ee811a45f5eaff:
>
> serial: small Makefile reordering (2017-04-18 18:01:52 +0200)
>
> ----------------------------------------------------------------
> TTY/Serial patches for 4.12-rc1
>
> Here is the "big" TTY/Serial patch updates for 4.12-rc1
>
> Not a lot of new things here, the normal number of serial driver updates
> and additions, tiny bugs fixed, and some core files split up to make
> future changes a bit easier for Nicolas's "tiny-tty" work.
>
> All of these have been in linux-next for a while. There will be a merge
> conflict with include/linux/serdev.h coming from the bluetooth tree
> merge, which we knew about, as we wanted some of the serdev changes to
> go in through that tree. I'll send the expected merge result as a
> follow-on message.
And below is the ideal merge fixup as created by Stephen, and reported a
few weeks ago. I've tested it and it works here for me.
thanks,
greg k-h
------------
Date: Thu, 13 Apr 2017 13:36:31 +1000
From: Stephen Rothwell <sfr@canb.auug.org.au>
To: Greg KH <greg@kroah.com>, Gustavo Padovan <gustavo@padovan.org>
Cc: Linux-Next Mailing List <linux-next@vger.kernel.org>, Linux Kernel Mailing
List <linux-kernel@vger.kernel.org>, Sebastian Reichel
<sre@kernel.org>, Marcel Holtmann <marcel@holtmann.org>, Andrey Smirnov
<andrew.smirnov@gmail.com>
Subject: linux-next: manual merge of the tty tree with the bluetooth tree
Hi Greg,
Today's linux-next merge of the tty tree got a conflict in:
include/linux/serdev.h
between commits:
b3f80c8f75ef ("serdev: add serdev_device_wait_until_sent")
5659dab26f09 ("serdev: implement get/set tiocm")
from the bluetooth tree and commit:
6fe729c4bdae ("serdev: Add serdev_device_write subroutine")
from the tty tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc include/linux/serdev.h
index 37395b8eb8f1,0beaff886992..000000000000
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@@ -191,10 -190,8 +195,11 @@@ int serdev_device_open(struct serdev_de
void serdev_device_close(struct serdev_device *);
unsigned int serdev_device_set_baudrate(struct serdev_device *, unsigned int);
void serdev_device_set_flow_control(struct serdev_device *, bool);
+void serdev_device_wait_until_sent(struct serdev_device *, long);
+int serdev_device_get_tiocm(struct serdev_device *);
+int serdev_device_set_tiocm(struct serdev_device *, int, int);
- int serdev_device_write_buf(struct serdev_device *, const unsigned char *, size_t);
+ void serdev_device_write_wakeup(struct serdev_device *);
+ int serdev_device_write(struct serdev_device *, const unsigned char *, size_t, unsigned long);
void serdev_device_write_flush(struct serdev_device *);
int serdev_device_write_room(struct serdev_device *);
@@@ -231,16 -228,8 +236,17 @@@ static inline unsigned int serdev_devic
return 0;
}
static inline void serdev_device_set_flow_control(struct serdev_device *sdev, bool enable) {}
+static inline void serdev_device_wait_until_sent(struct serdev_device *sdev, long timeout) {}
+static inline int serdev_device_get_tiocm(struct serdev_device *serdev)
+{
+ return -ENOTSUPP;
+}
+static inline int serdev_device_set_tiocm(struct serdev_device *serdev, int set, int clear)
+{
+ return -ENOTSUPP;
+}
- static inline int serdev_device_write_buf(struct serdev_device *sdev, const unsigned char *buf, size_t count)
+ static inline int serdev_device_write(struct serdev_device *sdev, const unsigned char *buf,
+ size_t count, unsigned long timeout)
{
return -ENODEV;
}
^ permalink raw reply
* [GIT PULL] TTY patches for 4.12-rc1
From: Greg KH @ 2017-05-08 9:46 UTC (permalink / raw)
To: Linus Torvalds, Jiri Slaby
Cc: Stephen Rothwell, Andrew Morton, linux-kernel, linux-serial
The following changes since commit 4f7d029b9bf009fbee76bb10c0c4351a1870d2f3:
Linux 4.11-rc7 (2017-04-16 13:00:18 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/ tags/tty-4.12-rc1
for you to fetch changes up to 8e1c21f486944bf92f2a981f23ee811a45f5eaff:
serial: small Makefile reordering (2017-04-18 18:01:52 +0200)
----------------------------------------------------------------
TTY/Serial patches for 4.12-rc1
Here is the "big" TTY/Serial patch updates for 4.12-rc1
Not a lot of new things here, the normal number of serial driver updates
and additions, tiny bugs fixed, and some core files split up to make
future changes a bit easier for Nicolas's "tiny-tty" work.
All of these have been in linux-next for a while. There will be a merge
conflict with include/linux/serdev.h coming from the bluetooth tree
merge, which we knew about, as we wanted some of the serdev changes to
go in through that tree. I'll send the expected merge result as a
follow-on message.
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
----------------------------------------------------------------
Adam Borowski (2):
vt: set mouse selection word-chars to gpm's default
vt: make mouse selection of non-ASCII consistent
Aleksey Makarov (1):
Revert "tty: serial: pl011: add ttyAMA for matching pl011 console"
Alexandre Belloni (2):
tty/serial: atmel: increase ATMEL_MAX_UART
tty/serial: atmel: remove cache when unnecessary
Andrey Smirnov (2):
linux/serdev.h: Replace 'ctrl->serdev' with 'serdev'
serdev: Add serdev_device_write subroutine
Andy Shevchenko (3):
serial: 8250_exar: Fix spelling of "driver"
serial: core: constify struct uart_port {name} field
serial: core: Re-use struct uart_port {name} field
Chunyan Zhang (2):
dt-bindings: arm: Add bindings for SP9860G
dt-bindings: serial: add a new compatible string for SC9860
Denis Kirjanov (1):
tty/hvc_console: fix console lock ordering with spinlock
Elena Reshetova (1):
drivers: convert sbd_duart.map_guard from atomic_t to refcount_t
Geert Uytterhoeven (3):
serial: sh-sci: Fix hang in sci_reset()
serial: sh-sci: Fix late enablement of AUTORTS
serial: sh-sci: Fix (AUTO)RTS in sci_init_pins()
Greg Kroah-Hartman (4):
Revert "tty: Fix ldisc crash on reopened tty"
Merge 4.11-rc4 into tty-next
Merge 4.11-rc5 into tty-next
Merge 4.11-rc7 into tty-next
Jan Kiszka (1):
serial: 8250_lpss: Unconditionally set PCI master for Quark
Jayachandran C (1):
tty: amba-pl011: Fix spurious TX interrupts
Jiri Slaby (1):
tty: serial_core, remove state checks in uart_poll*
Joe Perches (1):
drivers/tty: Convert remaining uses of pr_warning to pr_warn
Johan Hovold (5):
tty: close race between device register and open
tty: drop obsolete termios_locked comments
tty: reset termios state on device registration
serial: omap: fix runtime-pm handling on unbind
serial: omap: suspend device on probe errors
Lee Jones (1):
serial: st-asc: Change default baudrate from 9600 to 115200
Lionel Debieve (1):
tty: serial: st-asc: Make the locking RT aware
Lukas Redlinger (1):
serial: 8250_fintek: Enable high speed mode on Fintek F81866
Marc Gonzalez (1):
serial: 8250_early: Add earlycon support for Palmchip UART
Marek Szyprowski (3):
serial: samsung: Use right device for DMA-mapping calls
serial: samsung: Add missing checks for dma_map_single failure
serial: samsung: Remove useless spinlock
Nicolas Pitre (4):
console: move console_init() out of tty_io.c
tty: move baudrate handling code to a file of its own
tty: split job control support into a file of its own
serial: small Makefile reordering
Peter Hurley (1):
tty: Fix ldisc crash on reopened tty
Peter Senna Tschudin (1):
imx-serial: Reduce RX DMA startup latency when opening for reading
Philipp Zabel (1):
serial: 8250_dw: simplify optional reset handling
Richard Genoud (1):
tty/serial: atmel: move atmel_serial header into driver directory
Sam Povilus (1):
uartlite: Adding a kernel parameter for the number of uartlites
Samuel Thibault (1):
braille-console: Fix value returned by _braille_console_setup
Sebastian Reichel (1):
tty: serial: omap: add UPF_BOOT_AUTOCONF flag for DT init
Shubhrajyoti Datta (3):
serial: xilinx_uartps: Add pm runtime support
serial: xuartps: Cleanup the clock enable
serial: xuartps: Enable clocks in the pm disable case also
Stefan Agner (1):
tty: serial: fsl_lpuart: lock port on console write
Takatoshi Akiyama (1):
serial: sh-sci: Fix panic when serial console and DMA are enabled
Thadeu Lima de Souza Cascardo (1):
tty: fix comment typo s/repsonsible/responsible/
Tim Gardner (1):
tty: Disable default console blanking interval
Timur Tabi (1):
tty: pl011: use "qdf2400_e44" as the earlycon name for QDF2400 E44
Tobias Klauser (3):
tty: n_gsm: Use net_device_stats from struct net_device
serial: altera_jtaguart: add earlycon support
serial: altera_uart: add earlycon support
Uwe Kleine-König (1):
serial: imx: setup DCEDTE early and ensure DCD and RI irqs to be off
Vignesh R (3):
serial: 8250: 8250_core: Use dev_name() during request_irq()
tty: serial_core: Add name field to uart_port struct
serial: 8250: 8250_core: Fix irq name for 8250 serial IRQ
Wang YanQing (1):
tty: pty: Fix ldisc flush after userspace become aware of the data already
Wei Qiao (1):
serial: sprd: adjust TIMEOUT to a big value
Documentation/devicetree/bindings/arm/sprd.txt | 13 +-
.../devicetree/bindings/serial/sprd-uart.txt | 14 +-
MAINTAINERS | 2 +-
drivers/tty/Makefile | 3 +-
drivers/tty/hvc/hvc_console.c | 4 +-
drivers/tty/hvc/hvcs.c | 2 +-
drivers/tty/n_gsm.c | 21 +-
drivers/tty/pty.c | 7 +-
drivers/tty/serdev/core.c | 36 +-
drivers/tty/serial/8250/8250_core.c | 2 +-
drivers/tty/serial/8250/8250_dw.c | 13 +-
drivers/tty/serial/8250/8250_early.c | 24 +
drivers/tty/serial/8250/8250_exar.c | 2 +-
drivers/tty/serial/8250/8250_fintek.c | 43 +-
drivers/tty/serial/8250/8250_lpss.c | 3 +-
drivers/tty/serial/8250/8250_port.c | 4 +-
drivers/tty/serial/Kconfig | 11 +
drivers/tty/serial/Makefile | 3 +-
drivers/tty/serial/altera_jtaguart.c | 20 +
drivers/tty/serial/altera_uart.c | 32 ++
drivers/tty/serial/amba-pl011.c | 54 +-
drivers/tty/serial/atmel_serial.c | 7 +-
.../linux => drivers/tty/serial}/atmel_serial.h | 0
drivers/tty/serial/fsl_lpuart.c | 20 +
drivers/tty/serial/imx.c | 99 +++-
drivers/tty/serial/omap-serial.c | 12 +-
drivers/tty/serial/samsung.c | 44 +-
drivers/tty/serial/sb1250-duart.c | 18 +-
drivers/tty/serial/serial_core.c | 34 +-
drivers/tty/serial/sh-sci.c | 43 +-
drivers/tty/serial/sprd_serial.c | 2 +-
drivers/tty/serial/st-asc.c | 10 +-
drivers/tty/serial/uartlite.c | 2 +-
drivers/tty/serial/xilinx_uartps.c | 71 ++-
drivers/tty/tty_baudrate.c | 232 ++++++++
drivers/tty/tty_io.c | 635 ++-------------------
drivers/tty/tty_ioctl.c | 222 -------
drivers/tty/tty_jobctrl.c | 554 ++++++++++++++++++
drivers/tty/vt/selection.c | 18 +-
drivers/tty/vt/vt.c | 2 +-
include/linux/console.h | 2 +
include/linux/serdev.h | 21 +-
include/linux/serial_core.h | 1 +
include/linux/tty.h | 13 +-
init/main.c | 2 +-
kernel/printk/braille.c | 15 +-
kernel/printk/braille.h | 13 +-
kernel/printk/printk.c | 24 +
48 files changed, 1397 insertions(+), 1032 deletions(-)
rename {include/linux => drivers/tty/serial}/atmel_serial.h (100%)
create mode 100644 drivers/tty/tty_baudrate.c
create mode 100644 drivers/tty/tty_jobctrl.c
^ permalink raw reply
* Re: [PATCH] serdev: Restore serdev_device_write_buf for atomic context
From: Rob Herring @ 2017-05-04 20:32 UTC (permalink / raw)
To: Stefan Wahren
Cc: Johan Hovold, Greg Kroah-Hartman, Jiri Slaby, Sebastian Reichel,
Guenter Roeck, Andy Shevchenko, Andrey Smirnov,
linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <b62674e7-5c50-585b-eade-7cef0a400d02@i2se.com>
On Thu, May 4, 2017 at 11:22 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> Am 02.05.2017 um 15:18 schrieb Johan Hovold:
>> On Tue, May 02, 2017 at 07:41:34AM -0500, Rob Herring wrote:
>>> On Tue, May 2, 2017 at 4:06 AM, Johan Hovold <johan@kernel.org> wrote:
>>>> On Fri, Apr 28, 2017 at 01:47:21PM +0200, Stefan Wahren wrote:
>>>>> Starting with commit 6fe729c4bdae ("serdev: Add serdev_device_write
>>>>> subroutine") the function serdev_device_write_buf cannot be used in
>>>>> atomic context anymore (mutex_lock is sleeping). So restore the old
>>>>> behavior.
>>>> Yeah, preventing use in atomic context seems unnecessary, although any
>>>> clients writing must now deal with serialisation themselves (as before,
>>>> and as they should).
>>> We could just remove the mutex for serdev_device_write and always make
>>> the client responsible for serialization.
>> That sounds reasonable.
>
> So it's unwanted to have 2 write functions (non-atomic, atomic)?
No, it's unwanted to have more than we need.
Looking closer, we'd also have to ensure the wait for completion is
not called also. So probably better to just leave it as you have done
it.
Acked-by: Rob Herring <robh@kernel.org>
Rob
^ permalink raw reply
* Re: [PATCH] serdev: Restore serdev_device_write_buf for atomic context
From: Stefan Wahren @ 2017-05-04 16:22 UTC (permalink / raw)
To: Johan Hovold, Rob Herring
Cc: Greg Kroah-Hartman, Jiri Slaby, Sebastian Reichel, Guenter Roeck,
Andy Shevchenko, Andrey Smirnov, linux-serial@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20170502131854.GA17710@localhost>
Am 02.05.2017 um 15:18 schrieb Johan Hovold:
> On Tue, May 02, 2017 at 07:41:34AM -0500, Rob Herring wrote:
>> On Tue, May 2, 2017 at 4:06 AM, Johan Hovold <johan@kernel.org> wrote:
>>> On Fri, Apr 28, 2017 at 01:47:21PM +0200, Stefan Wahren wrote:
>>>> Starting with commit 6fe729c4bdae ("serdev: Add serdev_device_write
>>>> subroutine") the function serdev_device_write_buf cannot be used in
>>>> atomic context anymore (mutex_lock is sleeping). So restore the old
>>>> behavior.
>>> Yeah, preventing use in atomic context seems unnecessary, although any
>>> clients writing must now deal with serialisation themselves (as before,
>>> and as they should).
>> We could just remove the mutex for serdev_device_write and always make
>> the client responsible for serialization.
> That sounds reasonable.
So it's unwanted to have 2 write functions (non-atomic, atomic)?
Stefan
^ permalink raw reply
* Re: [PATCH] tty: serdev: fix serdev_device_write return value
From: Greg Kroah-Hartman @ 2017-05-03 18:06 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Johan Hovold, Rob Herring, linux-serial@vger.kernel.org,
linux-kernel@vger.kernel.org, Andrey Smirnov
In-Reply-To: <CAHp75Vc+pK5cMkO=1dHR-Xx8zhe4RR0Zv5KDvChZE6MUfVQwcQ@mail.gmail.com>
On Wed, May 03, 2017 at 08:44:07PM +0300, Andy Shevchenko wrote:
> On Tue, May 2, 2017 at 12:25 PM, Johan Hovold <johan@kernel.org> wrote:
> > On Mon, May 01, 2017 at 07:17:14PM -0500, Rob Herring wrote:
>
> >> - return ret < 0 ? ret : (count ? -ETIMEDOUT : 0);
> >> + return ret < 0 ? ret : (count ? -ETIMEDOUT : wr_cnt);
> >
> > That's some nasty use of the ternary operator. Ditching it completely
> > would be more readable.
> >
> > if (ret < 0)
> > return ret;
> >
> > if (count)
> > return -ETIMEDOUT;
> >
> > return wr_count;
>
>
> While I agree on the first part, I would go still with one ternary at the end:
>
> return count ? -ETIMEDOUT : wr_count;
Ick, no, make it easy to read, we write code for developers first, the
compiler second. Ditching it completly is a good idea.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] tty: serdev: fix serdev_device_write return value
From: Andy Shevchenko @ 2017-05-03 17:44 UTC (permalink / raw)
To: Johan Hovold
Cc: Rob Herring, Greg Kroah-Hartman, linux-serial@vger.kernel.org,
linux-kernel@vger.kernel.org, Andrey Smirnov
In-Reply-To: <20170502092527.GC2973@localhost>
On Tue, May 2, 2017 at 12:25 PM, Johan Hovold <johan@kernel.org> wrote:
> On Mon, May 01, 2017 at 07:17:14PM -0500, Rob Herring wrote:
>> - return ret < 0 ? ret : (count ? -ETIMEDOUT : 0);
>> + return ret < 0 ? ret : (count ? -ETIMEDOUT : wr_cnt);
>
> That's some nasty use of the ternary operator. Ditching it completely
> would be more readable.
>
> if (ret < 0)
> return ret;
>
> if (count)
> return -ETIMEDOUT;
>
> return wr_count;
While I agree on the first part, I would go still with one ternary at the end:
return count ? -ETIMEDOUT : wr_count;
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [GIT PULL] TTY/Serial driver fixes for 4.11-rc4
From: Greg KH @ 2017-05-03 12:01 UTC (permalink / raw)
To: Vegard Nossum
Cc: Dmitry Vyukov, Linus Torvalds, Jiri Slaby, Andrew Morton, LKML,
linux-serial
In-Reply-To: <CAOMGZ=GJah0GNXzscGGkYgScbZaeSM_TpxmrLZuxhrqWmBX9BQ@mail.gmail.com>
On Tue, May 02, 2017 at 11:52:40PM +0200, Vegard Nossum wrote:
> On 2 May 2017 at 18:35, Dmitry Vyukov <dvyukov@google.com> wrote:
> > On Fri, Apr 14, 2017 at 2:30 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> >> On Fri, Apr 14, 2017 at 11:41:26AM +0200, Vegard Nossum wrote:
> >>> On 13 April 2017 at 20:34, Greg KH <gregkh@linuxfoundation.org> wrote:
> >>> > On Thu, Apr 13, 2017 at 09:07:40AM -0700, Linus Torvalds wrote:
> >>> >> On Thu, Apr 13, 2017 at 3:50 AM, Vegard Nossum <vegard.nossum@gmail.com> wrote:
> >>> So the original problem is that the vmalloc() in n_tty_open() can
> >>> fail, and that will panic in tty_set_ldisc()/tty_ldisc_restore()
> >>> because of its unwillingness to proceed if the tty doesn't have an
> >>> ldisc.
> >>>
> >>> Dmitry fixed this by allowing tty->ldisc == NULL in the case of memory
> >>> allocation failure as we can see from the comment in tty_set_ldisc().
> >>>
> >>> Unfortunately, it would appear that some other bits of code do not
> >>> like tty->ldisc == NULL (other than the crash in this thread, I saw
> >>> 2-3 similar crashes in other functions, e.g. poll()). I see two
> >>> possibilities:
> >>>
> >>> 1) make other code handle tty->ldisc == NULL.
> >>>
> >>> 2) don't close/free the old ldisc until the new one has been
> >>> successfully created/initialised/opened/attached to the tty, and
> >>> return an error to userspace if changing it failed.
> >>>
> >>> I'm leaning towards #2 as the more obviously correct fix, it makes
> >>> tty_set_ldisc() transactional, the fix seems limited in scope to
> >>> tty_set_ldisc() itself, and we don't need to make every other bit of
> >>> code that uses tty->ldisc handle the NULL case.
> >>
> >> That sounds reasonable to me, care to work on a patch for this?
> >
> > Vegard, do you know how to do this?
> > That was first thing that I tried, but I did not manage to make it
> > work. disc is tied to tty, so it's not that one can create a fully
> > initialized disc on the side and then simply swap pointers. Looking at
> > the code now, there is at least TTY_LDISC_OPEN bit in tty. But as far
> > as I remember there were more fundamental problems. Or maybe I just
> > did not try too hard.
>
> I had a look at it but like you said, the tty/ldisc relationship is
> complicated :-/
>
> Maybe we can split up ldisc initialisation into two methods so that
> the first one (e.g. ->alloc) does all the allocation and is allowed to
> fail and the second one (e.g. ->open) is not allowed to fail. Then you
> can allocate a new ldisc without freeing the old one and only swap
> them over if the allocation succeeded.
>
> That would require fixing up ->open for all the ldisc drivers though,
> I'm not sure how easy/feasible it is.
We don't have that many ldisc drivers, so it shouldn't be that hard to
change to use this. It makes a lot more sense to fix this the correct
way like this.
> I'll think about possible solutions, but I have no prior experience
> with the tty code. In the meantime syzkaller also hit a couple of
> other fun tty/pty bugs including a write/ioctl race that results in
> buffer overflow :-/
Ugh, let me know what they are and we'll work to fix them.
Thanks for all of the work you all are doing in finding these issues, I
might grumble about having to fix this and what a pain it is, but it's
just me being grumpy about the tty code, not your effort. Your effort
is much appreciated.
thanks,
greg k-h
^ 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