[PATCH 00/14] serial: tegra: Tegra186 support and fixes

[PATCH 00/14] serial: tegra: Tegra186 support and fixes

[Krishna Yarlagadda]

Series of patches adding enhancements to exising UART driver and adding
support for new chip Tegra186 and Tegra194.

Tegra186 uses GPCDMA for dma transfers which is still not available in
mainstream. However, it can work in PIO/FIFO mode and support added for it.
Also Tegra186 has a hardware issue where it does not meet tolernace +/-4% and
to work around it, device tree entries provided to adjust baud rate for a
particular range.

Ahung Cheng (2):
  serial: tegra: avoid reg access when clk disabled
  serial: tegra: protect IER against LCR.DLAB

Andreas Abel (1):
  serial: tegra: add internal loopback functionality

Krishna Yarlagadda (7):
  serial: tegra: report error to upper tty layer
  serial: tegra: add compatible for new chips
  serial: tegra: check for FIFO mode enabled status
  serial: tegra: DT for Adjusted baud rates
  serial: tegra: add support to adjust baud rate
  serial: tegra: report clk rate errors
  serial: tegra: Add PIO mode support

Shardar Shariff Md (4):
  serial: tegra: add support to ignore read
  serial: tegra: flush the RX fifo on frame error
  serial: tegra: set maximum num of uart ports to 8
  serial: tegra: add support to use 8 bytes trigger

 .../bindings/serial/nvidia,tegra20-hsuart.txt      |  35 +-
 drivers/tty/serial/serial-tegra.c                  | 405 ++++++++++++++++++---
 2 files changed, 385 insertions(+), 55 deletions(-)

-- 
2.7.4

[PATCH 01/14] serial: tegra: add internal loopback functionality

[Krishna Yarlagadda]

From: Andreas Abel <aabel@nvidia.com>

Add the internal loopback functionality that can be enabled with
TIOCM_LOOP.

Signed-off-by: Andreas Abel <aabel@nvidia.com>
Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 drivers/tty/serial/serial-tegra.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
index d5269aa..19f4c24 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -4,7 +4,7 @@
  *
  * High-speed serial driver for NVIDIA Tegra SoCs
  *
- * Copyright (c) 2012-2013, NVIDIA CORPORATION.  All rights reserved.
+ * Copyright (c) 2012-2019, NVIDIA CORPORATION.  All rights reserved.
  *
  * Author: Laxman Dewangan <ldewangan@nvidia.com>
  */
@@ -192,16 +192,34 @@ static void set_dtr(struct tegra_uart_port *tup, bool active)
 	}
 }
 
+static void set_loopbk(struct tegra_uart_port *tup, bool active)
+{
+	unsigned long mcr = tup->mcr_shadow;
+
+	if (active)
+		mcr |= UART_MCR_LOOP;
+	else
+		mcr &= ~UART_MCR_LOOP;
+
+	if (mcr != tup->mcr_shadow) {
+		tegra_uart_write(tup, mcr, UART_MCR);
+		tup->mcr_shadow = mcr;
+	}
+}
+
 static void tegra_uart_set_mctrl(struct uart_port *u, unsigned int mctrl)
 {
 	struct tegra_uart_port *tup = to_tegra_uport(u);
-	int dtr_enable;
+	int enable;
 
 	tup->rts_active = !!(mctrl & TIOCM_RTS);
 	set_rts(tup, tup->rts_active);
 
-	dtr_enable = !!(mctrl & TIOCM_DTR);
-	set_dtr(tup, dtr_enable);
+	enable = !!(mctrl & TIOCM_DTR);
+	set_dtr(tup, enable);
+
+	enable = !!(mctrl & TIOCM_LOOP);
+	set_loopbk(tup, enable);
 }
 
 static void tegra_uart_break_ctl(struct uart_port *u, int break_ctl)
-- 
2.7.4

Re: [PATCH 01/14] serial: tegra: add internal loopback functionality

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 492 bytes --]

On Mon, Aug 12, 2019 at 04:58:10PM +0530, Krishna Yarlagadda wrote:
> From: Andreas Abel <aabel@nvidia.com>
> 
> Add the internal loopback functionality that can be enabled with
> TIOCM_LOOP.
> 
> Signed-off-by: Andreas Abel <aabel@nvidia.com>
> Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> ---
>  drivers/tty/serial/serial-tegra.c | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 02/14] serial: tegra: add support to ignore read

[Krishna Yarlagadda]

From: Shardar Shariff Md <smohammed@nvidia.com>

Add support to ignore read characters if CREAD flag is not set.

Signed-off-by: Shardar Shariff Md <smohammed@nvidia.com>
Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 drivers/tty/serial/serial-tegra.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
index 19f4c24..93d299e 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -542,6 +542,9 @@ static void tegra_uart_handle_rx_pio(struct tegra_uart_port *tup,
 		ch = (unsigned char) tegra_uart_read(tup, UART_RX);
 		tup->uport.icount.rx++;
 
+		if (tup->uport.ignore_status_mask & UART_LSR_DR)
+			continue;
+
 		if (!uart_handle_sysrq_char(&tup->uport, ch) && tty)
 			tty_insert_flip_char(tty, ch, flag);
 	} while (1);
@@ -562,6 +565,10 @@ static void tegra_uart_copy_rx_to_tty(struct tegra_uart_port *tup,
 		dev_err(tup->uport.dev, "No tty port\n");
 		return;
 	}
+
+	if (tup->uport.ignore_status_mask & UART_LSR_DR)
+		return;
+
 	dma_sync_single_for_cpu(tup->uport.dev, tup->rx_dma_buf_phys,
 				TEGRA_UART_RX_DMA_BUFFER_SIZE, DMA_FROM_DEVICE);
 	copied = tty_insert_flip_string(tty,
@@ -1190,6 +1197,11 @@ static void tegra_uart_set_termios(struct uart_port *u,
 	tegra_uart_write(tup, tup->ier_shadow, UART_IER);
 	tegra_uart_read(tup, UART_IER);
 
+	tup->uport.ignore_status_mask = 0;
+	/* Ignore all characters if CREAD is not set */
+	if ((termios->c_cflag & CREAD) == 0)
+		tup->uport.ignore_status_mask |= UART_LSR_DR;
+
 	spin_unlock_irqrestore(&u->lock, flags);
 }
 
-- 
2.7.4

Re: [PATCH 02/14] serial: tegra: add support to ignore read

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2175 bytes --]

On Mon, Aug 12, 2019 at 04:58:11PM +0530, Krishna Yarlagadda wrote:
> From: Shardar Shariff Md <smohammed@nvidia.com>
> 
> Add support to ignore read characters if CREAD flag is not set.
> 
> Signed-off-by: Shardar Shariff Md <smohammed@nvidia.com>
> Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> ---
>  drivers/tty/serial/serial-tegra.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
> index 19f4c24..93d299e 100644
> --- a/drivers/tty/serial/serial-tegra.c
> +++ b/drivers/tty/serial/serial-tegra.c
> @@ -542,6 +542,9 @@ static void tegra_uart_handle_rx_pio(struct tegra_uart_port *tup,
>  		ch = (unsigned char) tegra_uart_read(tup, UART_RX);
>  		tup->uport.icount.rx++;
>  
> +		if (tup->uport.ignore_status_mask & UART_LSR_DR)
> +			continue;
> +
>  		if (!uart_handle_sysrq_char(&tup->uport, ch) && tty)
>  			tty_insert_flip_char(tty, ch, flag);

Is it a good idea to ignore even sysrq characters if CREAD is not set?
According to termios, CREAD enables the receiver, so technically if it
isn't set you can't even receive sysrq characters. But I don't know if
there are any rules regarding this.

Is this the same way that other drivers work?

Thierry

>  	} while (1);
> @@ -562,6 +565,10 @@ static void tegra_uart_copy_rx_to_tty(struct tegra_uart_port *tup,
>  		dev_err(tup->uport.dev, "No tty port\n");
>  		return;
>  	}
> +
> +	if (tup->uport.ignore_status_mask & UART_LSR_DR)
> +		return;
> +
>  	dma_sync_single_for_cpu(tup->uport.dev, tup->rx_dma_buf_phys,
>  				TEGRA_UART_RX_DMA_BUFFER_SIZE, DMA_FROM_DEVICE);
>  	copied = tty_insert_flip_string(tty,
> @@ -1190,6 +1197,11 @@ static void tegra_uart_set_termios(struct uart_port *u,
>  	tegra_uart_write(tup, tup->ier_shadow, UART_IER);
>  	tegra_uart_read(tup, UART_IER);
>  
> +	tup->uport.ignore_status_mask = 0;
> +	/* Ignore all characters if CREAD is not set */
> +	if ((termios->c_cflag & CREAD) == 0)
> +		tup->uport.ignore_status_mask |= UART_LSR_DR;
> +
>  	spin_unlock_irqrestore(&u->lock, flags);
>  }
>  
> -- 
> 2.7.4
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH 02/14] serial: tegra: add support to ignore read

[Krishna Yarlagadda]

> -----Original Message-----
> From: linux-tegra-owner@vger.kernel.org <linux-tegra-
> owner@vger.kernel.org> On Behalf Of Thierry Reding
> Sent: Tuesday, August 13, 2019 3:12 PM
> To: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> Cc: gregkh@linuxfoundation.org; robh+dt@kernel.org;
> mark.rutland@arm.com; Jonathan Hunter <jonathanh@nvidia.com>; Laxman
> Dewangan <ldewangan@nvidia.com>; jslaby@suse.com; linux-
> serial@vger.kernel.org; devicetree@vger.kernel.org; linux-
> tegra@vger.kernel.org; linux-kernel@vger.kernel.org; Shardar Mohammed
> <smohammed@nvidia.com>
> Subject: Re: [PATCH 02/14] serial: tegra: add support to ignore read
> 
> On Mon, Aug 12, 2019 at 04:58:11PM +0530, Krishna Yarlagadda wrote:
> > From: Shardar Shariff Md <smohammed@nvidia.com>
> >
> > Add support to ignore read characters if CREAD flag is not set.
> >
> > Signed-off-by: Shardar Shariff Md <smohammed@nvidia.com>
> > Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> > ---
> >  drivers/tty/serial/serial-tegra.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/drivers/tty/serial/serial-tegra.c
> > b/drivers/tty/serial/serial-tegra.c
> > index 19f4c24..93d299e 100644
> > --- a/drivers/tty/serial/serial-tegra.c
> > +++ b/drivers/tty/serial/serial-tegra.c
> > @@ -542,6 +542,9 @@ static void tegra_uart_handle_rx_pio(struct
> tegra_uart_port *tup,
> >  		ch = (unsigned char) tegra_uart_read(tup, UART_RX);
> >  		tup->uport.icount.rx++;
> >
> > +		if (tup->uport.ignore_status_mask & UART_LSR_DR)
> > +			continue;
> > +
> >  		if (!uart_handle_sysrq_char(&tup->uport, ch) && tty)
> >  			tty_insert_flip_char(tty, ch, flag);
> 
> Is it a good idea to ignore even sysrq characters if CREAD is not set?
> According to termios, CREAD enables the receiver, so technically if it isn't set
> you can't even receive sysrq characters. But I don't know if there are any
> rules regarding this.
> 
> Is this the same way that other drivers work?
> 
> Thierry
> 
Looked into few drivers and sysrq characters are not ignored. This driver does
not support console driver. So this might not be needed.

KY
> >  	} while (1);
> > @@ -562,6 +565,10 @@ static void tegra_uart_copy_rx_to_tty(struct
> tegra_uart_port *tup,
> >  		dev_err(tup->uport.dev, "No tty port\n");
> >  		return;
> >  	}
> > +
> > +	if (tup->uport.ignore_status_mask & UART_LSR_DR)
> > +		return;
> > +
> >  	dma_sync_single_for_cpu(tup->uport.dev, tup->rx_dma_buf_phys,
> >  				TEGRA_UART_RX_DMA_BUFFER_SIZE,
> DMA_FROM_DEVICE);
> >  	copied = tty_insert_flip_string(tty, @@ -1190,6 +1197,11 @@ static
> > void tegra_uart_set_termios(struct uart_port *u,
> >  	tegra_uart_write(tup, tup->ier_shadow, UART_IER);
> >  	tegra_uart_read(tup, UART_IER);
> >
> > +	tup->uport.ignore_status_mask = 0;
> > +	/* Ignore all characters if CREAD is not set */
> > +	if ((termios->c_cflag & CREAD) == 0)
> > +		tup->uport.ignore_status_mask |= UART_LSR_DR;
> > +
> >  	spin_unlock_irqrestore(&u->lock, flags);  }
> >
> > --
> > 2.7.4
> >

[PATCH 03/14] serial: tegra: avoid reg access when clk disabled

[Krishna Yarlagadda]

From: Ahung Cheng <ahcheng@nvidia.com>

This avoids two race conditions from the UART shutdown sequence both
leading to 'Machine check error in AXI2APB' and kernel oops.

One was that the clock was disabled before the DMA was terminated making
it possible for the DMA callbacks to be called after the clock was
disabled. These callbacks could write to the UART registers causing
timeout.

The second was that the clock was disabled before the UART was
completely flagged as closed. This is done after the shutdown is called
and a new write could be started after the clock was disabled.
tegra_uart_start_pio_tx could be called causing timeout.

Given that the baud rate is reset at the end of shutdown sequence, this
fix is to examine the baud rate to avoid register access from both race
conditions.

Besides, terminate the DMA before disabling the clock.

Signed-off-by: Ahung Cheng <ahcheng@nvidia.com>
Signed-off-by: Shardar Mohammed <smohammed@nvidia.com>
Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 drivers/tty/serial/serial-tegra.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
index 93d299e..d908465 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -126,6 +126,8 @@ struct tegra_uart_port {
 
 static void tegra_uart_start_next_tx(struct tegra_uart_port *tup);
 static int tegra_uart_start_rx_dma(struct tegra_uart_port *tup);
+static void tegra_uart_dma_channel_free(struct tegra_uart_port *tup,
+					bool dma_to_memory);
 
 static inline unsigned long tegra_uart_read(struct tegra_uart_port *tup,
 		unsigned long reg)
@@ -458,6 +460,9 @@ static void tegra_uart_start_next_tx(struct tegra_uart_port *tup)
 	unsigned long count;
 	struct circ_buf *xmit = &tup->uport.state->xmit;
 
+	if (WARN_ON(!tup->current_baud))
+		return;
+
 	tail = (unsigned long)&xmit->buf[xmit->tail];
 	count = CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE);
 	if (!count)
@@ -829,6 +834,12 @@ static void tegra_uart_hw_deinit(struct tegra_uart_port *tup)
 	tup->current_baud = 0;
 	spin_unlock_irqrestore(&tup->uport.lock, flags);
 
+	tup->rx_in_progress = 0;
+	tup->tx_in_progress = 0;
+
+	tegra_uart_dma_channel_free(tup, true);
+	tegra_uart_dma_channel_free(tup, false);
+
 	clk_disable_unprepare(tup->uart_clk);
 }
 
@@ -1066,12 +1077,6 @@ static void tegra_uart_shutdown(struct uart_port *u)
 	struct tegra_uart_port *tup = to_tegra_uport(u);
 
 	tegra_uart_hw_deinit(tup);
-
-	tup->rx_in_progress = 0;
-	tup->tx_in_progress = 0;
-
-	tegra_uart_dma_channel_free(tup, true);
-	tegra_uart_dma_channel_free(tup, false);
 	free_irq(u->irq, tup);
 }
 
-- 
2.7.4

Re: [PATCH 03/14] serial: tegra: avoid reg access when clk disabled

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3295 bytes --]

On Mon, Aug 12, 2019 at 04:58:12PM +0530, Krishna Yarlagadda wrote:
> From: Ahung Cheng <ahcheng@nvidia.com>
> 
> This avoids two race conditions from the UART shutdown sequence both
> leading to 'Machine check error in AXI2APB' and kernel oops.
> 
> One was that the clock was disabled before the DMA was terminated making
> it possible for the DMA callbacks to be called after the clock was
> disabled. These callbacks could write to the UART registers causing
> timeout.
> 
> The second was that the clock was disabled before the UART was
> completely flagged as closed. This is done after the shutdown is called
> and a new write could be started after the clock was disabled.
> tegra_uart_start_pio_tx could be called causing timeout.
> 
> Given that the baud rate is reset at the end of shutdown sequence, this
> fix is to examine the baud rate to avoid register access from both race
> conditions.
> 
> Besides, terminate the DMA before disabling the clock.
> 
> Signed-off-by: Ahung Cheng <ahcheng@nvidia.com>
> Signed-off-by: Shardar Mohammed <smohammed@nvidia.com>
> Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> ---
>  drivers/tty/serial/serial-tegra.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
> index 93d299e..d908465 100644
> --- a/drivers/tty/serial/serial-tegra.c
> +++ b/drivers/tty/serial/serial-tegra.c
> @@ -126,6 +126,8 @@ struct tegra_uart_port {
>  
>  static void tegra_uart_start_next_tx(struct tegra_uart_port *tup);
>  static int tegra_uart_start_rx_dma(struct tegra_uart_port *tup);
> +static void tegra_uart_dma_channel_free(struct tegra_uart_port *tup,
> +					bool dma_to_memory);
>  
>  static inline unsigned long tegra_uart_read(struct tegra_uart_port *tup,
>  		unsigned long reg)
> @@ -458,6 +460,9 @@ static void tegra_uart_start_next_tx(struct tegra_uart_port *tup)
>  	unsigned long count;
>  	struct circ_buf *xmit = &tup->uport.state->xmit;
>  
> +	if (WARN_ON(!tup->current_baud))
> +		return;

Are the race conditions that you are describing something which can be
triggered by the user? If so, it's not a good idea to use a WARN_ON,
because that could lead to some userspace spamming the log with these,
potentially on purpose.

Thierry

> +
>  	tail = (unsigned long)&xmit->buf[xmit->tail];
>  	count = CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE);
>  	if (!count)
> @@ -829,6 +834,12 @@ static void tegra_uart_hw_deinit(struct tegra_uart_port *tup)
>  	tup->current_baud = 0;
>  	spin_unlock_irqrestore(&tup->uport.lock, flags);
>  
> +	tup->rx_in_progress = 0;
> +	tup->tx_in_progress = 0;
> +
> +	tegra_uart_dma_channel_free(tup, true);
> +	tegra_uart_dma_channel_free(tup, false);
> +
>  	clk_disable_unprepare(tup->uart_clk);
>  }
>  
> @@ -1066,12 +1077,6 @@ static void tegra_uart_shutdown(struct uart_port *u)
>  	struct tegra_uart_port *tup = to_tegra_uport(u);
>  
>  	tegra_uart_hw_deinit(tup);
> -
> -	tup->rx_in_progress = 0;
> -	tup->tx_in_progress = 0;
> -
> -	tegra_uart_dma_channel_free(tup, true);
> -	tegra_uart_dma_channel_free(tup, false);
>  	free_irq(u->irq, tup);
>  }
>  
> -- 
> 2.7.4
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH 03/14] serial: tegra: avoid reg access when clk disabled

[Krishna Yarlagadda]

> -----Original Message-----
> From: linux-tegra-owner@vger.kernel.org <linux-tegra- 
> owner@vger.kernel.org> On Behalf Of Thierry Reding
> Sent: Tuesday, August 13, 2019 3:16 PM
> To: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> Cc: gregkh@linuxfoundation.org; robh+dt@kernel.org; 
> mark.rutland@arm.com; Jonathan Hunter <jonathanh@nvidia.com>; Laxman 
> Dewangan <ldewangan@nvidia.com>; jslaby@suse.com; linux- 
> serial@vger.kernel.org; devicetree@vger.kernel.org; linux- 
> tegra@vger.kernel.org; linux-kernel@vger.kernel.org; Ahung Cheng 
> <ahcheng@nvidia.com>; Shardar Mohammed <smohammed@nvidia.com>
> Subject: Re: [PATCH 03/14] serial: tegra: avoid reg access when clk 
> disabled
> 
> On Mon, Aug 12, 2019 at 04:58:12PM +0530, Krishna Yarlagadda wrote:
> > From: Ahung Cheng <ahcheng@nvidia.com>
> >
> > This avoids two race conditions from the UART shutdown sequence both 
> > leading to 'Machine check error in AXI2APB' and kernel oops.
> >
> > One was that the clock was disabled before the DMA was terminated 
> > making it possible for the DMA callbacks to be called after the 
> > clock was disabled. These callbacks could write to the UART 
> > registers causing timeout.
> >
> > The second was that the clock was disabled before the UART was 
> > completely flagged as closed. This is done after the shutdown is 
> > called and a new write could be started after the clock was disabled.
> > tegra_uart_start_pio_tx could be called causing timeout.
> >
> > Given that the baud rate is reset at the end of shutdown sequence, 
> > this fix is to examine the baud rate to avoid register access from 
> > both race conditions.
> >
> > Besides, terminate the DMA before disabling the clock.
> >
> > Signed-off-by: Ahung Cheng <ahcheng@nvidia.com>
> > Signed-off-by: Shardar Mohammed <smohammed@nvidia.com>
> > Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> > ---
> >  drivers/tty/serial/serial-tegra.c | 17 +++++++++++------
> >  1 file changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/tty/serial/serial-tegra.c
> > b/drivers/tty/serial/serial-tegra.c
> > index 93d299e..d908465 100644
> > --- a/drivers/tty/serial/serial-tegra.c
> > +++ b/drivers/tty/serial/serial-tegra.c
> > @@ -126,6 +126,8 @@ struct tegra_uart_port {
> >
> >  static void tegra_uart_start_next_tx(struct tegra_uart_port *tup); 
> > static int tegra_uart_start_rx_dma(struct tegra_uart_port *tup);
> > +static void tegra_uart_dma_channel_free(struct tegra_uart_port *tup,
> > +					bool dma_to_memory);
> >
> >  static inline unsigned long tegra_uart_read(struct tegra_uart_port *tup,
> >  		unsigned long reg)
> > @@ -458,6 +460,9 @@ static void tegra_uart_start_next_tx(struct
> tegra_uart_port *tup)
> >  	unsigned long count;
> >  	struct circ_buf *xmit = &tup->uport.state->xmit;
> >
> > +	if (WARN_ON(!tup->current_baud))
> > +		return;
> 
> Are the race conditions that you are describing something which can be 
> triggered by the user? If so, it's not a good idea to use a WARN_ON, 
> because that could lead to some userspace spamming the log with these, 
> potentially on purpose.
> 
> Thierry
> 
These are triggered by user events. I will remove WARN_ON

 KY
> > +
> >  	tail = (unsigned long)&xmit->buf[xmit->tail];
> >  	count = CIRC_CNT_TO_END(xmit->head, xmit->tail,
> UART_XMIT_SIZE);
> >  	if (!count)
> > @@ -829,6 +834,12 @@ static void tegra_uart_hw_deinit(struct
> tegra_uart_port *tup)
> >  	tup->current_baud = 0;
> >  	spin_unlock_irqrestore(&tup->uport.lock, flags);
> >
> > +	tup->rx_in_progress = 0;
> > +	tup->tx_in_progress = 0;
> > +
> > +	tegra_uart_dma_channel_free(tup, true);
> > +	tegra_uart_dma_channel_free(tup, false);
> > +
> >  	clk_disable_unprepare(tup->uart_clk);
> >  }
> >
> > @@ -1066,12 +1077,6 @@ static void tegra_uart_shutdown(struct
> uart_port *u)
> >  	struct tegra_uart_port *tup = to_tegra_uport(u);
> >
> >  	tegra_uart_hw_deinit(tup);
> > -
> > -	tup->rx_in_progress = 0;
> > -	tup->tx_in_progress = 0;
> > -
> > -	tegra_uart_dma_channel_free(tup, true);
> > -	tegra_uart_dma_channel_free(tup, false);
> >  	free_irq(u->irq, tup);
> >  }
> >
> > --
> > 2.7.4
> >

[PATCH 04/14] serial: tegra: protect IER against LCR.DLAB

[Krishna Yarlagadda]

From: Ahung Cheng <ahcheng@nvidia.com>

The IER and DLH registers occupy the same address space, selected by
the LCR.DLAB bit. Hence, add port lock to protect IER when LCR.DLAB bit
is set.

Signed-off-by: Ahung Cheng <ahcheng@nvidia.com>
Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 drivers/tty/serial/serial-tegra.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
index d908465..ae7225c 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -296,6 +296,7 @@ static int tegra_set_baudrate(struct tegra_uart_port *tup, unsigned int baud)
 	unsigned long rate;
 	unsigned int divisor;
 	unsigned long lcr;
+	unsigned long flags;
 	int ret;
 
 	if (tup->current_baud == baud)
@@ -315,6 +316,7 @@ static int tegra_set_baudrate(struct tegra_uart_port *tup, unsigned int baud)
 		divisor = DIV_ROUND_CLOSEST(rate, baud * 16);
 	}
 
+	spin_lock_irqsave(&tup->uport.lock, flags);
 	lcr = tup->lcr_shadow;
 	lcr |= UART_LCR_DLAB;
 	tegra_uart_write(tup, lcr, UART_LCR);
@@ -327,6 +329,7 @@ static int tegra_set_baudrate(struct tegra_uart_port *tup, unsigned int baud)
 
 	/* Dummy read to ensure the write is posted */
 	tegra_uart_read(tup, UART_SCR);
+	spin_unlock_irqrestore(&tup->uport.lock, flags);
 
 	tup->current_baud = baud;
 
-- 
2.7.4

Re: [PATCH 04/14] serial: tegra: protect IER against LCR.DLAB

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 528 bytes --]

On Mon, Aug 12, 2019 at 04:58:13PM +0530, Krishna Yarlagadda wrote:
> From: Ahung Cheng <ahcheng@nvidia.com>
> 
> The IER and DLH registers occupy the same address space, selected by
> the LCR.DLAB bit. Hence, add port lock to protect IER when LCR.DLAB bit
> is set.
> 
> Signed-off-by: Ahung Cheng <ahcheng@nvidia.com>
> Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> ---
>  drivers/tty/serial/serial-tegra.c | 3 +++
>  1 file changed, 3 insertions(+)

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 05/14] serial: tegra: flush the RX fifo on frame error

[Krishna Yarlagadda]

From: Shardar Shariff Md <smohammed@nvidia.com>

FIFO reset/flush code implemented now does not follow programming
guidelines. RTS line has to be turned off while flushing fifos to
avoid new transfers. Also check LSR bits UART_LSR_TEMT and UART_LSR_DR
to confirm fifos are flushed.

Signed-off-by: Shardar Shariff Md <smohammed@nvidia.com>
Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 drivers/tty/serial/serial-tegra.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
index ae7225c..f6a3f4e 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -266,6 +266,10 @@ static void tegra_uart_wait_sym_time(struct tegra_uart_port *tup,
 static void tegra_uart_fifo_reset(struct tegra_uart_port *tup, u8 fcr_bits)
 {
 	unsigned long fcr = tup->fcr_shadow;
+	unsigned int lsr, tmout = 10000;
+
+	if (tup->rts_active)
+		set_rts(tup, false);
 
 	if (tup->cdata->allow_txfifo_reset_fifo_mode) {
 		fcr |= fcr_bits & (UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT);
@@ -289,6 +293,17 @@ static void tegra_uart_fifo_reset(struct tegra_uart_port *tup, u8 fcr_bits)
 	 * to propagate, otherwise data could be lost.
 	 */
 	tegra_uart_wait_cycle_time(tup, 32);
+
+	do {
+		lsr = tegra_uart_read(tup, UART_LSR);
+		if (lsr | UART_LSR_TEMT)
+			if (!(lsr & UART_LSR_DR))
+				break;
+		udelay(1);
+	} while (--tmout);
+
+	if (tup->rts_active)
+		set_rts(tup, true);
 }
 
 static int tegra_set_baudrate(struct tegra_uart_port *tup, unsigned int baud)
-- 
2.7.4

Re: [PATCH 05/14] serial: tegra: flush the RX fifo on frame error

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1903 bytes --]

On Mon, Aug 12, 2019 at 04:58:14PM +0530, Krishna Yarlagadda wrote:
> From: Shardar Shariff Md <smohammed@nvidia.com>
> 
> FIFO reset/flush code implemented now does not follow programming
> guidelines. RTS line has to be turned off while flushing fifos to
> avoid new transfers. Also check LSR bits UART_LSR_TEMT and UART_LSR_DR
> to confirm fifos are flushed.

You use inconsistent spelling for FIFO here.

> Signed-off-by: Shardar Shariff Md <smohammed@nvidia.com>
> Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> ---
>  drivers/tty/serial/serial-tegra.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
> index ae7225c..f6a3f4e 100644
> --- a/drivers/tty/serial/serial-tegra.c
> +++ b/drivers/tty/serial/serial-tegra.c
> @@ -266,6 +266,10 @@ static void tegra_uart_wait_sym_time(struct tegra_uart_port *tup,
>  static void tegra_uart_fifo_reset(struct tegra_uart_port *tup, u8 fcr_bits)
>  {
>  	unsigned long fcr = tup->fcr_shadow;
> +	unsigned int lsr, tmout = 10000;
> +
> +	if (tup->rts_active)
> +		set_rts(tup, false);
>  
>  	if (tup->cdata->allow_txfifo_reset_fifo_mode) {
>  		fcr |= fcr_bits & (UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT);
> @@ -289,6 +293,17 @@ static void tegra_uart_fifo_reset(struct tegra_uart_port *tup, u8 fcr_bits)
>  	 * to propagate, otherwise data could be lost.
>  	 */
>  	tegra_uart_wait_cycle_time(tup, 32);
> +
> +	do {
> +		lsr = tegra_uart_read(tup, UART_LSR);
> +		if (lsr | UART_LSR_TEMT)
> +			if (!(lsr & UART_LSR_DR))

Can't both of these go on the same line?

Thierry

> +				break;
> +		udelay(1);
> +	} while (--tmout);
> +
> +	if (tup->rts_active)
> +		set_rts(tup, true);
>  }
>  
>  static int tegra_set_baudrate(struct tegra_uart_port *tup, unsigned int baud)
> -- 
> 2.7.4
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH 05/14] serial: tegra: flush the RX fifo on frame error

[Krishna Yarlagadda]

> -----Original Message-----
> From: Thierry Reding <thierry.reding@gmail.com>
> Sent: Tuesday, August 13, 2019 3:19 PM
> To: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> Cc: gregkh@linuxfoundation.org; robh+dt@kernel.org;
> mark.rutland@arm.com; Jonathan Hunter <jonathanh@nvidia.com>; Laxman
> Dewangan <ldewangan@nvidia.com>; jslaby@suse.com; linux-
> serial@vger.kernel.org; devicetree@vger.kernel.org; linux-
> tegra@vger.kernel.org; linux-kernel@vger.kernel.org; Shardar Mohammed
> <smohammed@nvidia.com>
> Subject: Re: [PATCH 05/14] serial: tegra: flush the RX fifo on frame error
> 
> On Mon, Aug 12, 2019 at 04:58:14PM +0530, Krishna Yarlagadda wrote:
> > From: Shardar Shariff Md <smohammed@nvidia.com>
> >
> > FIFO reset/flush code implemented now does not follow programming
> > guidelines. RTS line has to be turned off while flushing fifos to
> > avoid new transfers. Also check LSR bits UART_LSR_TEMT and
> UART_LSR_DR
> > to confirm fifos are flushed.
> 
> You use inconsistent spelling for FIFO here.
Will fix in next version and take care of this in all patches

> 
> > Signed-off-by: Shardar Shariff Md <smohammed@nvidia.com>
> > Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> > ---
> >  drivers/tty/serial/serial-tegra.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/drivers/tty/serial/serial-tegra.c
> > b/drivers/tty/serial/serial-tegra.c
> > index ae7225c..f6a3f4e 100644
> > --- a/drivers/tty/serial/serial-tegra.c
> > +++ b/drivers/tty/serial/serial-tegra.c
> > @@ -266,6 +266,10 @@ static void tegra_uart_wait_sym_time(struct
> > tegra_uart_port *tup,  static void tegra_uart_fifo_reset(struct
> > tegra_uart_port *tup, u8 fcr_bits)  {
> >  	unsigned long fcr = tup->fcr_shadow;
> > +	unsigned int lsr, tmout = 10000;
> > +
> > +	if (tup->rts_active)
> > +		set_rts(tup, false);
> >
> >  	if (tup->cdata->allow_txfifo_reset_fifo_mode) {
> >  		fcr |= fcr_bits & (UART_FCR_CLEAR_RCVR |
> UART_FCR_CLEAR_XMIT); @@
> > -289,6 +293,17 @@ static void tegra_uart_fifo_reset(struct tegra_uart_port
> *tup, u8 fcr_bits)
> >  	 * to propagate, otherwise data could be lost.
> >  	 */
> >  	tegra_uart_wait_cycle_time(tup, 32);
> > +
> > +	do {
> > +		lsr = tegra_uart_read(tup, UART_LSR);
> > +		if (lsr | UART_LSR_TEMT)
> > +			if (!(lsr & UART_LSR_DR))
> 
> Can't both of these go on the same line?
> 
> Thierry
> 
They can be. Will fix in next version
KY

> > +				break;
> > +		udelay(1);
> > +	} while (--tmout);
> > +
> > +	if (tup->rts_active)
> > +		set_rts(tup, true);
> >  }
> >
> >  static int tegra_set_baudrate(struct tegra_uart_port *tup, unsigned
> > int baud)
> > --
> > 2.7.4
> >

[PATCH 06/14] serial: tegra: report error to upper tty layer

[Krishna Yarlagadda]

Report overrun/parity/frame/break errors to top tty layer. Add support
to ignore break character if IGNBRK is set.

Signed-off-by: Shardar Shariff Md <smohammed@nvidia.com>
Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 drivers/tty/serial/serial-tegra.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
index f6a3f4e..7ab81bb 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -374,13 +374,21 @@ static char tegra_uart_decode_rx_error(struct tegra_uart_port *tup,
 			tup->uport.icount.frame++;
 			dev_err(tup->uport.dev, "Got frame errors\n");
 		} else if (lsr & UART_LSR_BI) {
-			dev_err(tup->uport.dev, "Got Break\n");
-			tup->uport.icount.brk++;
-			/* If FIFO read error without any data, reset Rx FIFO */
+			/*
+			 * Break error
+			 * If FIFO read error without any data, reset Rx FIFO
+			 */
 			if (!(lsr & UART_LSR_DR) && (lsr & UART_LSR_FIFOE))
 				tegra_uart_fifo_reset(tup, UART_FCR_CLEAR_RCVR);
+			if (tup->uport.ignore_status_mask & UART_LSR_BI)
+				return TTY_BREAK;
+			flag = TTY_BREAK;
+			tup->uport.icount.brk++;
+			dev_err(tup->uport.dev, "Got Break\n");
 		}
+		uart_insert_char(&tup->uport, lsr, UART_LSR_OE, 0, flag);
 	}
+
 	return flag;
 }
 
@@ -562,6 +570,9 @@ static void tegra_uart_handle_rx_pio(struct tegra_uart_port *tup,
 			break;
 
 		flag = tegra_uart_decode_rx_error(tup, lsr);
+		if (flag != TTY_NORMAL)
+			continue;
+
 		ch = (unsigned char) tegra_uart_read(tup, UART_RX);
 		tup->uport.icount.rx++;
 
@@ -1224,6 +1235,8 @@ static void tegra_uart_set_termios(struct uart_port *u,
 	/* Ignore all characters if CREAD is not set */
 	if ((termios->c_cflag & CREAD) == 0)
 		tup->uport.ignore_status_mask |= UART_LSR_DR;
+	if (termios->c_iflag & IGNBRK)
+		tup->uport.ignore_status_mask |= UART_LSR_BI;
 
 	spin_unlock_irqrestore(&u->lock, flags);
 }
-- 
2.7.4

Re: [PATCH 06/14] serial: tegra: report error to upper tty layer

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2378 bytes --]

On Mon, Aug 12, 2019 at 04:58:15PM +0530, Krishna Yarlagadda wrote:
> Report overrun/parity/frame/break errors to top tty layer. Add support
> to ignore break character if IGNBRK is set.
> 
> Signed-off-by: Shardar Shariff Md <smohammed@nvidia.com>
> Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> ---
>  drivers/tty/serial/serial-tegra.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
> index f6a3f4e..7ab81bb 100644
> --- a/drivers/tty/serial/serial-tegra.c
> +++ b/drivers/tty/serial/serial-tegra.c
> @@ -374,13 +374,21 @@ static char tegra_uart_decode_rx_error(struct tegra_uart_port *tup,
>  			tup->uport.icount.frame++;
>  			dev_err(tup->uport.dev, "Got frame errors\n");
>  		} else if (lsr & UART_LSR_BI) {
> -			dev_err(tup->uport.dev, "Got Break\n");
> -			tup->uport.icount.brk++;
> -			/* If FIFO read error without any data, reset Rx FIFO */
> +			/*
> +			 * Break error
> +			 * If FIFO read error without any data, reset Rx FIFO
> +			 */
>  			if (!(lsr & UART_LSR_DR) && (lsr & UART_LSR_FIFOE))
>  				tegra_uart_fifo_reset(tup, UART_FCR_CLEAR_RCVR);
> +			if (tup->uport.ignore_status_mask & UART_LSR_BI)
> +				return TTY_BREAK;
> +			flag = TTY_BREAK;
> +			tup->uport.icount.brk++;
> +			dev_err(tup->uport.dev, "Got Break\n");

I know this is preexisting, but why do we want to output an error
message in these cases. Isn't it perfectly legal for this to happen?

Thierry

>  		}
> +		uart_insert_char(&tup->uport, lsr, UART_LSR_OE, 0, flag);
>  	}
> +
>  	return flag;
>  }
>  
> @@ -562,6 +570,9 @@ static void tegra_uart_handle_rx_pio(struct tegra_uart_port *tup,
>  			break;
>  
>  		flag = tegra_uart_decode_rx_error(tup, lsr);
> +		if (flag != TTY_NORMAL)
> +			continue;
> +
>  		ch = (unsigned char) tegra_uart_read(tup, UART_RX);
>  		tup->uport.icount.rx++;
>  
> @@ -1224,6 +1235,8 @@ static void tegra_uart_set_termios(struct uart_port *u,
>  	/* Ignore all characters if CREAD is not set */
>  	if ((termios->c_cflag & CREAD) == 0)
>  		tup->uport.ignore_status_mask |= UART_LSR_DR;
> +	if (termios->c_iflag & IGNBRK)
> +		tup->uport.ignore_status_mask |= UART_LSR_BI;
>  
>  	spin_unlock_irqrestore(&u->lock, flags);
>  }
> -- 
> 2.7.4
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH 06/14] serial: tegra: report error to upper tty layer

[Krishna Yarlagadda]

> -----Original Message-----
> From: Thierry Reding <thierry.reding@gmail.com>
> Sent: Tuesday, August 13, 2019 3:22 PM
> To: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> Cc: gregkh@linuxfoundation.org; robh+dt@kernel.org; 
> mark.rutland@arm.com; Jonathan Hunter <jonathanh@nvidia.com>; Laxman 
> Dewangan <ldewangan@nvidia.com>; jslaby@suse.com; linux- 
> serial@vger.kernel.org; devicetree@vger.kernel.org; linux- 
> tegra@vger.kernel.org; linux-kernel@vger.kernel.org; Shardar Mohammed 
> <smohammed@nvidia.com>
> Subject: Re: [PATCH 06/14] serial: tegra: report error to upper tty 
> layer
> 
> On Mon, Aug 12, 2019 at 04:58:15PM +0530, Krishna Yarlagadda wrote:
> > Report overrun/parity/frame/break errors to top tty layer. Add 
> > support to ignore break character if IGNBRK is set.
> >
> > Signed-off-by: Shardar Shariff Md <smohammed@nvidia.com>
> > Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> > ---
> >  drivers/tty/serial/serial-tegra.c | 19 ++++++++++++++++---
> >  1 file changed, 16 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/tty/serial/serial-tegra.c
> > b/drivers/tty/serial/serial-tegra.c
> > index f6a3f4e..7ab81bb 100644
> > --- a/drivers/tty/serial/serial-tegra.c
> > +++ b/drivers/tty/serial/serial-tegra.c
> > @@ -374,13 +374,21 @@ static char tegra_uart_decode_rx_error(struct
> tegra_uart_port *tup,
> >  			tup->uport.icount.frame++;
> >  			dev_err(tup->uport.dev, "Got frame errors\n");
> >  		} else if (lsr & UART_LSR_BI) {
> > -			dev_err(tup->uport.dev, "Got Break\n");
> > -			tup->uport.icount.brk++;
> > -			/* If FIFO read error without any data, reset Rx FIFO
> */
> > +			/*
> > +			 * Break error
> > +			 * If FIFO read error without any data, reset Rx FIFO
> > +			 */
> >  			if (!(lsr & UART_LSR_DR) && (lsr & UART_LSR_FIFOE))
> >  				tegra_uart_fifo_reset(tup,
> UART_FCR_CLEAR_RCVR);
> > +			if (tup->uport.ignore_status_mask & UART_LSR_BI)
> > +				return TTY_BREAK;
> > +			flag = TTY_BREAK;
> > +			tup->uport.icount.brk++;
> > +			dev_err(tup->uport.dev, "Got Break\n");
> 
> I know this is preexisting, but why do we want to output an error 
> message in these cases. Isn't it perfectly legal for this to happen?
> 
> Thierry
> 
It is valid to have breaks for sysrq requests. But they also indicate possible mismatch in baud rate. So warning user as this could be potential issue. I will change this to dev_dbg to avoid spamming user in valid cases.

KY
> >  		}
> > +		uart_insert_char(&tup->uport, lsr, UART_LSR_OE, 0, flag);
> >  	}
> > +
> >  	return flag;
> >  }
> >
> > @@ -562,6 +570,9 @@ static void tegra_uart_handle_rx_pio(struct
> tegra_uart_port *tup,
> >  			break;
> >
> >  		flag = tegra_uart_decode_rx_error(tup, lsr);
> > +		if (flag != TTY_NORMAL)
> > +			continue;
> > +
> >  		ch = (unsigned char) tegra_uart_read(tup, UART_RX);
> >  		tup->uport.icount.rx++;
> >
> > @@ -1224,6 +1235,8 @@ static void tegra_uart_set_termios(struct
> uart_port *u,
> >  	/* Ignore all characters if CREAD is not set */
> >  	if ((termios->c_cflag & CREAD) == 0)
> >  		tup->uport.ignore_status_mask |= UART_LSR_DR;
> > +	if (termios->c_iflag & IGNBRK)
> > +		tup->uport.ignore_status_mask |= UART_LSR_BI;
> >
> >  	spin_unlock_irqrestore(&u->lock, flags);  }
> > --
> > 2.7.4
> >

[PATCH 07/14] serial: tegra: add compatible for new chips

[Krishna Yarlagadda]

Add new compatible string for Tegra186. It differs from earlier chips
as it has fifo mode enable check and 8 byte dma buffer.
Add new compatible string for Tegra194. Tegra194 has different error
tolerance levels for baud rate compared to older chips.

Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt b/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt
index d7edf73..187ec78 100644
--- a/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt
+++ b/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt
@@ -1,7 +1,8 @@
 NVIDIA Tegra20/Tegra30 high speed (DMA based) UART controller driver.
 
 Required properties:
-- compatible : should be "nvidia,tegra30-hsuart", "nvidia,tegra20-hsuart".
+- compatible : should be "nvidia,tegra30-hsuart", "nvidia,tegra20-hsuart",
+  nvidia,tegra186-hsuart, nvidia,tegra194-hsuart.
 - reg: Should contain UART controller registers location and length.
 - interrupts: Should contain UART controller interrupts.
 - clocks: Must contain one entry, for the module clock.
-- 
2.7.4

Re: [PATCH 07/14] serial: tegra: add compatible for new chips

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1935 bytes --]

On Mon, Aug 12, 2019 at 04:58:16PM +0530, Krishna Yarlagadda wrote:
> Add new compatible string for Tegra186. It differs from earlier chips
> as it has fifo mode enable check and 8 byte dma buffer.
> Add new compatible string for Tegra194. Tegra194 has different error
> tolerance levels for baud rate compared to older chips.
> 
> Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> ---
>  Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

I think device tree binding patches are supposed to start with
"dt-binding: ...".

Also: "fifo" -> "FIFO" and "dma" -> "DMA" in the commit message.

> 
> diff --git a/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt b/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt
> index d7edf73..187ec78 100644
> --- a/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt
> +++ b/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt
> @@ -1,7 +1,8 @@
>  NVIDIA Tegra20/Tegra30 high speed (DMA based) UART controller driver.
>  
>  Required properties:
> -- compatible : should be "nvidia,tegra30-hsuart", "nvidia,tegra20-hsuart".
> +- compatible : should be "nvidia,tegra30-hsuart", "nvidia,tegra20-hsuart",
> +  nvidia,tegra186-hsuart, nvidia,tegra194-hsuart.

Please use quotes around the compatible strings like the existing ones.
Also, I think it might be better to list these explicitly rather than
just give a list of potential values. As it is right now, it's
impossible to tell whether this is meant to say "should be all of these"
or whether it is meant to say "should be one of these".

Thierry

>  - reg: Should contain UART controller registers location and length.
>  - interrupts: Should contain UART controller interrupts.
>  - clocks: Must contain one entry, for the module clock.
> -- 
> 2.7.4
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH 07/14] serial: tegra: add compatible for new chips

[Krishna Yarlagadda]

> -----Original Message-----
> From: Thierry Reding <thierry.reding@gmail.com>
> Sent: Tuesday, August 13, 2019 3:26 PM
> To: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> Cc: gregkh@linuxfoundation.org; robh+dt@kernel.org;
> mark.rutland@arm.com; Jonathan Hunter <jonathanh@nvidia.com>; Laxman
> Dewangan <ldewangan@nvidia.com>; jslaby@suse.com; linux-
> serial@vger.kernel.org; devicetree@vger.kernel.org; linux-
> tegra@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 07/14] serial: tegra: add compatible for new chips
> 
> On Mon, Aug 12, 2019 at 04:58:16PM +0530, Krishna Yarlagadda wrote:
> > Add new compatible string for Tegra186. It differs from earlier chips
> > as it has fifo mode enable check and 8 byte dma buffer.
> > Add new compatible string for Tegra194. Tegra194 has different error
> > tolerance levels for baud rate compared to older chips.
> >
> > Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> > ---
> >  Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt |
> > 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> I think device tree binding patches are supposed to start with
> "dt-binding: ...".
> 
> Also: "fifo" -> "FIFO" and "dma" -> "DMA" in the commit message.
> 
> >
> > diff --git
> > a/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt
> > b/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt
> > index d7edf73..187ec78 100644
> > ---
> > a/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt
> > +++ b/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.t
> > +++ xt
> > @@ -1,7 +1,8 @@
> >  NVIDIA Tegra20/Tegra30 high speed (DMA based) UART controller driver.
> >
> >  Required properties:
> > -- compatible : should be "nvidia,tegra30-hsuart", "nvidia,tegra20-hsuart".
> > +- compatible : should be "nvidia,tegra30-hsuart",
> > +"nvidia,tegra20-hsuart",
> > +  nvidia,tegra186-hsuart, nvidia,tegra194-hsuart.
> 
> Please use quotes around the compatible strings like the existing ones.
> Also, I think it might be better to list these explicitly rather than just give a list
> of potential values. As it is right now, it's impossible to tell whether this is
> meant to say "should be all of these"
> or whether it is meant to say "should be one of these".
> 
> Thierry
> 
Will update in next version.
KY
> >  - reg: Should contain UART controller registers location and length.
> >  - interrupts: Should contain UART controller interrupts.
> >  - clocks: Must contain one entry, for the module clock.
> > --
> > 2.7.4
> >

[PATCH 08/14] serial: tegra: check for FIFO mode enabled status

[Krishna Yarlagadda]

Chips prior to Tegra186 needed delay of 3 UART clock cycles to avoid
data loss. This issue is fixed in Tegra186 and a new flag is added to
check if fifo mode is enabled. chip data updated to check if this flag
is available for a chip. Tegra186 has new compatible to enable this
flag.

Signed-off-by: Shardar Shariff Md <smohammed@nvidia.com>
Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 drivers/tty/serial/serial-tegra.c | 52 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 46 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
index 7ab81bb..e0379d9 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -72,6 +72,8 @@
 #define TEGRA_TX_PIO				1
 #define TEGRA_TX_DMA				2
 
+#define TEGRA_UART_FCR_IIR_FIFO_EN		0x40
+
 /**
  * tegra_uart_chip_data: SOC specific data.
  *
@@ -84,6 +86,7 @@ struct tegra_uart_chip_data {
 	bool	tx_fifo_full_status;
 	bool	allow_txfifo_reset_fifo_mode;
 	bool	support_clk_src_div;
+	bool	fifo_mode_enable_status;
 };
 
 struct tegra_uart_port {
@@ -263,6 +266,22 @@ static void tegra_uart_wait_sym_time(struct tegra_uart_port *tup,
 			tup->current_baud));
 }
 
+static int tegra_uart_is_fifo_mode_enabled(struct tegra_uart_port *tup)
+{
+	unsigned long iir;
+	unsigned int tmout = 100;
+
+	do {
+		iir = tegra_uart_read(tup, UART_IIR);
+		if (iir & TEGRA_UART_FCR_IIR_FIFO_EN)
+			return 0;
+		udelay(1);
+	} while (--tmout);
+	dev_err(tup->uport.dev, "FIFO mode not enabled\n");
+
+	return -EIO;
+}
+
 static void tegra_uart_fifo_reset(struct tegra_uart_port *tup, u8 fcr_bits)
 {
 	unsigned long fcr = tup->fcr_shadow;
@@ -282,6 +301,8 @@ static void tegra_uart_fifo_reset(struct tegra_uart_port *tup, u8 fcr_bits)
 		tegra_uart_write(tup, fcr, UART_FCR);
 		fcr |= UART_FCR_ENABLE_FIFO;
 		tegra_uart_write(tup, fcr, UART_FCR);
+		if (tup->cdata->fifo_mode_enable_status)
+			tegra_uart_is_fifo_mode_enabled(tup);
 	}
 
 	/* Dummy read to ensure the write is posted */
@@ -918,12 +939,19 @@ static int tegra_uart_hw_init(struct tegra_uart_port *tup)
 	/* Dummy read to ensure the write is posted */
 	tegra_uart_read(tup, UART_SCR);
 
-	/*
-	 * For all tegra devices (up to t210), there is a hardware issue that
-	 * requires software to wait for 3 UART clock periods after enabling
-	 * the TX fifo, otherwise data could be lost.
-	 */
-	tegra_uart_wait_cycle_time(tup, 3);
+	if (tup->cdata->fifo_mode_enable_status) {
+		ret = tegra_uart_is_fifo_mode_enabled(tup);
+		if (ret < 0)
+			return ret;
+	} else {
+		/*
+		 * For all tegra devices (up to t210), there is a hardware
+		 * issue that requires software to wait for 3 UART clock
+		 * periods after enabling the TX fifo, otherwise data could
+		 * be lost.
+		 */
+		tegra_uart_wait_cycle_time(tup, 3);
+	}
 
 	/*
 	 * Initialize the UART with default configuration
@@ -1294,12 +1322,21 @@ static struct tegra_uart_chip_data tegra20_uart_chip_data = {
 	.tx_fifo_full_status		= false,
 	.allow_txfifo_reset_fifo_mode	= true,
 	.support_clk_src_div		= false,
+	.fifo_mode_enable_status	= false,
 };
 
 static struct tegra_uart_chip_data tegra30_uart_chip_data = {
 	.tx_fifo_full_status		= true,
 	.allow_txfifo_reset_fifo_mode	= false,
 	.support_clk_src_div		= true,
+	.fifo_mode_enable_status	= false,
+};
+
+static struct tegra_uart_chip_data tegra186_uart_chip_data = {
+	.tx_fifo_full_status		= true,
+	.allow_txfifo_reset_fifo_mode	= false,
+	.support_clk_src_div		= true,
+	.fifo_mode_enable_status	= true,
 };
 
 static const struct of_device_id tegra_uart_of_match[] = {
@@ -1310,6 +1347,9 @@ static const struct of_device_id tegra_uart_of_match[] = {
 		.compatible	= "nvidia,tegra20-hsuart",
 		.data		= &tegra20_uart_chip_data,
 	}, {
+		.compatible     = "nvidia,tegra186-hsuart",
+		.data		= &tegra186_uart_chip_data,
+	}, {
 	},
 };
 MODULE_DEVICE_TABLE(of, tegra_uart_of_match);
-- 
2.7.4

Re: [PATCH 08/14] serial: tegra: check for FIFO mode enabled status

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 4831 bytes --]

On Mon, Aug 12, 2019 at 04:58:17PM +0530, Krishna Yarlagadda wrote:
> Chips prior to Tegra186 needed delay of 3 UART clock cycles to avoid
> data loss. This issue is fixed in Tegra186 and a new flag is added to
> check if fifo mode is enabled. chip data updated to check if this flag
> is available for a chip. Tegra186 has new compatible to enable this
> flag.
> 
> Signed-off-by: Shardar Shariff Md <smohammed@nvidia.com>
> Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> ---
>  drivers/tty/serial/serial-tegra.c | 52 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 46 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
> index 7ab81bb..e0379d9 100644
> --- a/drivers/tty/serial/serial-tegra.c
> +++ b/drivers/tty/serial/serial-tegra.c
> @@ -72,6 +72,8 @@
>  #define TEGRA_TX_PIO				1
>  #define TEGRA_TX_DMA				2
>  
> +#define TEGRA_UART_FCR_IIR_FIFO_EN		0x40
> +
>  /**
>   * tegra_uart_chip_data: SOC specific data.
>   *
> @@ -84,6 +86,7 @@ struct tegra_uart_chip_data {
>  	bool	tx_fifo_full_status;
>  	bool	allow_txfifo_reset_fifo_mode;
>  	bool	support_clk_src_div;
> +	bool	fifo_mode_enable_status;
>  };
>  
>  struct tegra_uart_port {
> @@ -263,6 +266,22 @@ static void tegra_uart_wait_sym_time(struct tegra_uart_port *tup,
>  			tup->current_baud));
>  }
>  
> +static int tegra_uart_is_fifo_mode_enabled(struct tegra_uart_port *tup)

I think this is a bad name. "is" makes it sound like this will return a
boolean value. Also, this doesn't really check whether FIFO mode is
enabled, but rather it waits for the FIFO mode to become enabled.
Perhaps, then, a better name would be

	tegra_uart_wait_fifo_mode_enabled()

?

> +{
> +	unsigned long iir;
> +	unsigned int tmout = 100;
> +
> +	do {
> +		iir = tegra_uart_read(tup, UART_IIR);
> +		if (iir & TEGRA_UART_FCR_IIR_FIFO_EN)
> +			return 0;
> +		udelay(1);
> +	} while (--tmout);
> +	dev_err(tup->uport.dev, "FIFO mode not enabled\n");

I'd push this out to callers. That way this function becomes useful in
situations where you don't want to output an error.

> +
> +	return -EIO;

-ETIMEDOUT?

Thierry

> +}
> +
>  static void tegra_uart_fifo_reset(struct tegra_uart_port *tup, u8 fcr_bits)
>  {
>  	unsigned long fcr = tup->fcr_shadow;
> @@ -282,6 +301,8 @@ static void tegra_uart_fifo_reset(struct tegra_uart_port *tup, u8 fcr_bits)
>  		tegra_uart_write(tup, fcr, UART_FCR);
>  		fcr |= UART_FCR_ENABLE_FIFO;
>  		tegra_uart_write(tup, fcr, UART_FCR);
> +		if (tup->cdata->fifo_mode_enable_status)
> +			tegra_uart_is_fifo_mode_enabled(tup);
>  	}
>  
>  	/* Dummy read to ensure the write is posted */
> @@ -918,12 +939,19 @@ static int tegra_uart_hw_init(struct tegra_uart_port *tup)
>  	/* Dummy read to ensure the write is posted */
>  	tegra_uart_read(tup, UART_SCR);
>  
> -	/*
> -	 * For all tegra devices (up to t210), there is a hardware issue that
> -	 * requires software to wait for 3 UART clock periods after enabling
> -	 * the TX fifo, otherwise data could be lost.
> -	 */
> -	tegra_uart_wait_cycle_time(tup, 3);
> +	if (tup->cdata->fifo_mode_enable_status) {
> +		ret = tegra_uart_is_fifo_mode_enabled(tup);
> +		if (ret < 0)
> +			return ret;
> +	} else {
> +		/*
> +		 * For all tegra devices (up to t210), there is a hardware
> +		 * issue that requires software to wait for 3 UART clock
> +		 * periods after enabling the TX fifo, otherwise data could
> +		 * be lost.
> +		 */
> +		tegra_uart_wait_cycle_time(tup, 3);
> +	}
>  
>  	/*
>  	 * Initialize the UART with default configuration
> @@ -1294,12 +1322,21 @@ static struct tegra_uart_chip_data tegra20_uart_chip_data = {
>  	.tx_fifo_full_status		= false,
>  	.allow_txfifo_reset_fifo_mode	= true,
>  	.support_clk_src_div		= false,
> +	.fifo_mode_enable_status	= false,
>  };
>  
>  static struct tegra_uart_chip_data tegra30_uart_chip_data = {
>  	.tx_fifo_full_status		= true,
>  	.allow_txfifo_reset_fifo_mode	= false,
>  	.support_clk_src_div		= true,
> +	.fifo_mode_enable_status	= false,
> +};
> +
> +static struct tegra_uart_chip_data tegra186_uart_chip_data = {
> +	.tx_fifo_full_status		= true,
> +	.allow_txfifo_reset_fifo_mode	= false,
> +	.support_clk_src_div		= true,
> +	.fifo_mode_enable_status	= true,
>  };
>  
>  static const struct of_device_id tegra_uart_of_match[] = {
> @@ -1310,6 +1347,9 @@ static const struct of_device_id tegra_uart_of_match[] = {
>  		.compatible	= "nvidia,tegra20-hsuart",
>  		.data		= &tegra20_uart_chip_data,
>  	}, {
> +		.compatible     = "nvidia,tegra186-hsuart",
> +		.data		= &tegra186_uart_chip_data,
> +	}, {
>  	},
>  };
>  MODULE_DEVICE_TABLE(of, tegra_uart_of_match);
> -- 
> 2.7.4
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH 08/14] serial: tegra: check for FIFO mode enabled status

[Krishna Yarlagadda]

> -----Original Message-----
> From: linux-tegra-owner@vger.kernel.org <linux-tegra-
> owner@vger.kernel.org> On Behalf Of Thierry Reding
> Sent: Tuesday, August 13, 2019 3:34 PM
> To: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> Cc: gregkh@linuxfoundation.org; robh+dt@kernel.org;
> mark.rutland@arm.com; Jonathan Hunter <jonathanh@nvidia.com>; Laxman
> Dewangan <ldewangan@nvidia.com>; jslaby@suse.com; linux-
> serial@vger.kernel.org; devicetree@vger.kernel.org; linux-
> tegra@vger.kernel.org; linux-kernel@vger.kernel.org; Shardar Mohammed
> <smohammed@nvidia.com>
> Subject: Re: [PATCH 08/14] serial: tegra: check for FIFO mode enabled status
> 
> On Mon, Aug 12, 2019 at 04:58:17PM +0530, Krishna Yarlagadda wrote:
> > Chips prior to Tegra186 needed delay of 3 UART clock cycles to avoid
> > data loss. This issue is fixed in Tegra186 and a new flag is added to
> > check if fifo mode is enabled. chip data updated to check if this flag
> > is available for a chip. Tegra186 has new compatible to enable this
> > flag.
> >
> > Signed-off-by: Shardar Shariff Md <smohammed@nvidia.com>
> > Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> > ---
> >  drivers/tty/serial/serial-tegra.c | 52
> > ++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 46 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/tty/serial/serial-tegra.c
> > b/drivers/tty/serial/serial-tegra.c
> > index 7ab81bb..e0379d9 100644
> > --- a/drivers/tty/serial/serial-tegra.c
> > +++ b/drivers/tty/serial/serial-tegra.c
> > @@ -72,6 +72,8 @@
> >  #define TEGRA_TX_PIO				1
> >  #define TEGRA_TX_DMA				2
> >
> > +#define TEGRA_UART_FCR_IIR_FIFO_EN		0x40
> > +
> >  /**
> >   * tegra_uart_chip_data: SOC specific data.
> >   *
> > @@ -84,6 +86,7 @@ struct tegra_uart_chip_data {
> >  	bool	tx_fifo_full_status;
> >  	bool	allow_txfifo_reset_fifo_mode;
> >  	bool	support_clk_src_div;
> > +	bool	fifo_mode_enable_status;
> >  };
> >
> >  struct tegra_uart_port {
> > @@ -263,6 +266,22 @@ static void tegra_uart_wait_sym_time(struct
> tegra_uart_port *tup,
> >  			tup->current_baud));
> >  }
> >
> > +static int tegra_uart_is_fifo_mode_enabled(struct tegra_uart_port
> > +*tup)
> 
> I think this is a bad name. "is" makes it sound like this will return a boolean
> value. Also, this doesn't really check whether FIFO mode is enabled, but
> rather it waits for the FIFO mode to become enabled.
> Perhaps, then, a better name would be
> 
> 	tegra_uart_wait_fifo_mode_enabled()
> 
> ?
> 
Sounds good. Will make changes

> > +{
> > +	unsigned long iir;
> > +	unsigned int tmout = 100;
> > +
> > +	do {
> > +		iir = tegra_uart_read(tup, UART_IIR);
> > +		if (iir & TEGRA_UART_FCR_IIR_FIFO_EN)
> > +			return 0;
> > +		udelay(1);
> > +	} while (--tmout);
> > +	dev_err(tup->uport.dev, "FIFO mode not enabled\n");
> 
> I'd push this out to callers. That way this function becomes useful in
> situations where you don't want to output an error.
> 
> > +
> > +	return -EIO;
> 
> -ETIMEDOUT?
> 
> Thierry
> 
Will push this to caller and change the error code
KY
> > +}
> > +
> >  static void tegra_uart_fifo_reset(struct tegra_uart_port *tup, u8
> > fcr_bits)  {
> >  	unsigned long fcr = tup->fcr_shadow; @@ -282,6 +301,8 @@ static
> void
> > tegra_uart_fifo_reset(struct tegra_uart_port *tup, u8 fcr_bits)
> >  		tegra_uart_write(tup, fcr, UART_FCR);
> >  		fcr |= UART_FCR_ENABLE_FIFO;
> >  		tegra_uart_write(tup, fcr, UART_FCR);
> > +		if (tup->cdata->fifo_mode_enable_status)
> > +			tegra_uart_is_fifo_mode_enabled(tup);
> >  	}
> >
> >  	/* Dummy read to ensure the write is posted */ @@ -918,12 +939,19
> @@
> > static int tegra_uart_hw_init(struct tegra_uart_port *tup)
> >  	/* Dummy read to ensure the write is posted */
> >  	tegra_uart_read(tup, UART_SCR);
> >
> > -	/*
> > -	 * For all tegra devices (up to t210), there is a hardware issue that
> > -	 * requires software to wait for 3 UART clock periods after enabling
> > -	 * the TX fifo, otherwise data could be lost.
> > -	 */
> > -	tegra_uart_wait_cycle_time(tup, 3);
> > +	if (tup->cdata->fifo_mode_enable_status) {
> > +		ret = tegra_uart_is_fifo_mode_enabled(tup);
> > +		if (ret < 0)
> > +			return ret;
> > +	} else {
> > +		/*
> > +		 * For all tegra devices (up to t210), there is a hardware
> > +		 * issue that requires software to wait for 3 UART clock
> > +		 * periods after enabling the TX fifo, otherwise data could
> > +		 * be lost.
> > +		 */
> > +		tegra_uart_wait_cycle_time(tup, 3);
> > +	}
> >
> >  	/*
> >  	 * Initialize the UART with default configuration @@ -1294,12
> > +1322,21 @@ static struct tegra_uart_chip_data tegra20_uart_chip_data =
> {
> >  	.tx_fifo_full_status		= false,
> >  	.allow_txfifo_reset_fifo_mode	= true,
> >  	.support_clk_src_div		= false,
> > +	.fifo_mode_enable_status	= false,
> >  };
> >
> >  static struct tegra_uart_chip_data tegra30_uart_chip_data = {
> >  	.tx_fifo_full_status		= true,
> >  	.allow_txfifo_reset_fifo_mode	= false,
> >  	.support_clk_src_div		= true,
> > +	.fifo_mode_enable_status	= false,
> > +};
> > +
> > +static struct tegra_uart_chip_data tegra186_uart_chip_data = {
> > +	.tx_fifo_full_status		= true,
> > +	.allow_txfifo_reset_fifo_mode	= false,
> > +	.support_clk_src_div		= true,
> > +	.fifo_mode_enable_status	= true,
> >  };
> >
> >  static const struct of_device_id tegra_uart_of_match[] = { @@ -1310,6
> > +1347,9 @@ static const struct of_device_id tegra_uart_of_match[] = {
> >  		.compatible	= "nvidia,tegra20-hsuart",
> >  		.data		= &tegra20_uart_chip_data,
> >  	}, {
> > +		.compatible     = "nvidia,tegra186-hsuart",
> > +		.data		= &tegra186_uart_chip_data,
> > +	}, {
> >  	},
> >  };
> >  MODULE_DEVICE_TABLE(of, tegra_uart_of_match);
> > --
> > 2.7.4
> >

[PATCH 09/14] serial: tegra: set maximum num of uart ports to 8

[Krishna Yarlagadda]

From: Shardar Shariff Md <smohammed@nvidia.com>

Set maximum number of UART ports to 8 as older chips have 7 ports and
Tergra194 and later chips will have 8 ports. Add this info to chip data
and register uart driver in platform driver probe.

Signed-off-by: Shardar Shariff Md <smohammed@nvidia.com>
Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 drivers/tty/serial/serial-tegra.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
index e0379d9..329923c 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -62,7 +62,7 @@
 #define TEGRA_UART_TX_TRIG_4B			0x20
 #define TEGRA_UART_TX_TRIG_1B			0x30
 
-#define TEGRA_UART_MAXIMUM			5
+#define TEGRA_UART_MAXIMUM			8
 
 /* Default UART setting when started: 115200 no parity, stop, 8 data bits */
 #define TEGRA_UART_DEFAULT_BAUD			115200
@@ -87,6 +87,7 @@ struct tegra_uart_chip_data {
 	bool	allow_txfifo_reset_fifo_mode;
 	bool	support_clk_src_div;
 	bool	fifo_mode_enable_status;
+	int	uart_max_port;
 };
 
 struct tegra_uart_port {
@@ -1323,6 +1324,7 @@ static struct tegra_uart_chip_data tegra20_uart_chip_data = {
 	.allow_txfifo_reset_fifo_mode	= true,
 	.support_clk_src_div		= false,
 	.fifo_mode_enable_status	= false,
+	.uart_max_port			= 5,
 };
 
 static struct tegra_uart_chip_data tegra30_uart_chip_data = {
@@ -1330,6 +1332,7 @@ static struct tegra_uart_chip_data tegra30_uart_chip_data = {
 	.allow_txfifo_reset_fifo_mode	= false,
 	.support_clk_src_div		= true,
 	.fifo_mode_enable_status	= false,
+	.uart_max_port			= 5,
 };
 
 static struct tegra_uart_chip_data tegra186_uart_chip_data = {
@@ -1337,6 +1340,7 @@ static struct tegra_uart_chip_data tegra186_uart_chip_data = {
 	.allow_txfifo_reset_fifo_mode	= false,
 	.support_clk_src_div		= true,
 	.fifo_mode_enable_status	= true,
+	.uart_max_port			= 5,
 };
 
 static const struct of_device_id tegra_uart_of_match[] = {
@@ -1386,6 +1390,7 @@ static int tegra_uart_probe(struct platform_device *pdev)
 	u->type = PORT_TEGRA;
 	u->fifosize = 32;
 	tup->cdata = cdata;
+	tegra_uart_driver.nr = cdata->uart_max_port;
 
 	platform_set_drvdata(pdev, tup);
 	resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1411,6 +1416,13 @@ static int tegra_uart_probe(struct platform_device *pdev)
 		return PTR_ERR(tup->rst);
 	}
 
+	ret = uart_register_driver(&tegra_uart_driver);
+	if (ret < 0) {
+		pr_err("Could not register %s driver\n",
+		       tegra_uart_driver.driver_name);
+		return ret;
+	}
+
 	u->iotype = UPIO_MEM32;
 	ret = platform_get_irq(pdev, 0);
 	if (ret < 0) {
@@ -1472,13 +1484,6 @@ static int __init tegra_uart_init(void)
 {
 	int ret;
 
-	ret = uart_register_driver(&tegra_uart_driver);
-	if (ret < 0) {
-		pr_err("Could not register %s driver\n",
-			tegra_uart_driver.driver_name);
-		return ret;
-	}
-
 	ret = platform_driver_register(&tegra_uart_platform_driver);
 	if (ret < 0) {
 		pr_err("Uart platform driver register failed, e = %d\n", ret);
-- 
2.7.4

Re: [PATCH 09/14] serial: tegra: set maximum num of uart ports to 8

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 4760 bytes --]

On Mon, Aug 12, 2019 at 04:58:18PM +0530, Krishna Yarlagadda wrote:
> From: Shardar Shariff Md <smohammed@nvidia.com>
> 
> Set maximum number of UART ports to 8 as older chips have 7 ports and
> Tergra194 and later chips will have 8 ports. Add this info to chip data
> and register uart driver in platform driver probe.
> 
> Signed-off-by: Shardar Shariff Md <smohammed@nvidia.com>
> Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> ---
>  drivers/tty/serial/serial-tegra.c | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
> index e0379d9..329923c 100644
> --- a/drivers/tty/serial/serial-tegra.c
> +++ b/drivers/tty/serial/serial-tegra.c
> @@ -62,7 +62,7 @@
>  #define TEGRA_UART_TX_TRIG_4B			0x20
>  #define TEGRA_UART_TX_TRIG_1B			0x30
>  
> -#define TEGRA_UART_MAXIMUM			5
> +#define TEGRA_UART_MAXIMUM			8
>  
>  /* Default UART setting when started: 115200 no parity, stop, 8 data bits */
>  #define TEGRA_UART_DEFAULT_BAUD			115200
> @@ -87,6 +87,7 @@ struct tegra_uart_chip_data {
>  	bool	allow_txfifo_reset_fifo_mode;
>  	bool	support_clk_src_div;
>  	bool	fifo_mode_enable_status;
> +	int	uart_max_port;
>  };
>  
>  struct tegra_uart_port {
> @@ -1323,6 +1324,7 @@ static struct tegra_uart_chip_data tegra20_uart_chip_data = {
>  	.allow_txfifo_reset_fifo_mode	= true,
>  	.support_clk_src_div		= false,
>  	.fifo_mode_enable_status	= false,
> +	.uart_max_port			= 5,
>  };
>  
>  static struct tegra_uart_chip_data tegra30_uart_chip_data = {
> @@ -1330,6 +1332,7 @@ static struct tegra_uart_chip_data tegra30_uart_chip_data = {
>  	.allow_txfifo_reset_fifo_mode	= false,
>  	.support_clk_src_div		= true,
>  	.fifo_mode_enable_status	= false,
> +	.uart_max_port			= 5,
>  };
>  
>  static struct tegra_uart_chip_data tegra186_uart_chip_data = {
> @@ -1337,6 +1340,7 @@ static struct tegra_uart_chip_data tegra186_uart_chip_data = {
>  	.allow_txfifo_reset_fifo_mode	= false,
>  	.support_clk_src_div		= true,
>  	.fifo_mode_enable_status	= true,
> +	.uart_max_port			= 5,

You say in the commit message that the older chips have 7 ports, but
here you say they have 5. Which one is it?

>  };
>  
>  static const struct of_device_id tegra_uart_of_match[] = {
> @@ -1386,6 +1390,7 @@ static int tegra_uart_probe(struct platform_device *pdev)
>  	u->type = PORT_TEGRA;
>  	u->fifosize = 32;
>  	tup->cdata = cdata;
> +	tegra_uart_driver.nr = cdata->uart_max_port;
>  
>  	platform_set_drvdata(pdev, tup);
>  	resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> @@ -1411,6 +1416,13 @@ static int tegra_uart_probe(struct platform_device *pdev)
>  		return PTR_ERR(tup->rst);
>  	}
>  
> +	ret = uart_register_driver(&tegra_uart_driver);
> +	if (ret < 0) {
> +		pr_err("Could not register %s driver\n",
> +		       tegra_uart_driver.driver_name);
> +		return ret;
> +	}

I don't think this is the right place for this. You're going to try to
register the driver once for each instance of the Tegra UART that will
be probed.

I'm surprised that this works at all because there's a BUG_ON() early
in uart_register_driver() that checks for the existence of drv->state,
which means that the second instance of tegra_uart_probe() should
trigger that and cause the kernel to crash.

I think it's better to either create an additional of_device_id table
that is used to match on the top-level node's compatible string and
which only contains the maximum number of ports for the given SoC, or
you could add code to tegra_uart_init() that counts the number of ports
that do match and initialize tegra_uart_driver.nr using that number. It
would something like this:

	unsigned int count = 0;

	for_each_matching_node(np, &tegra_uart_of_match)
		count++;

	tegra_uart_driver.nr = count;

You could also add additional checks in the loop, perhaps something
like:

	for_each_matching_node(np, &tegra_uart_of_match)
		if (of_device_is_available(np))
			count++

Though that would prevent any UARTs from getting added via dynamic
device tree manipulation.

Thierry

> +
>  	u->iotype = UPIO_MEM32;
>  	ret = platform_get_irq(pdev, 0);
>  	if (ret < 0) {
> @@ -1472,13 +1484,6 @@ static int __init tegra_uart_init(void)
>  {
>  	int ret;
>  
> -	ret = uart_register_driver(&tegra_uart_driver);
> -	if (ret < 0) {
> -		pr_err("Could not register %s driver\n",
> -			tegra_uart_driver.driver_name);
> -		return ret;
> -	}
> -
>  	ret = platform_driver_register(&tegra_uart_platform_driver);
>  	if (ret < 0) {
>  		pr_err("Uart platform driver register failed, e = %d\n", ret);
> -- 
> 2.7.4
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH 09/14] serial: tegra: set maximum num of uart ports to 8

[Krishna Yarlagadda]

> -----Original Message-----
> From: Thierry Reding <thierry.reding@gmail.com>
> Sent: Tuesday, August 13, 2019 3:50 PM
> To: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> Cc: gregkh@linuxfoundation.org; robh+dt@kernel.org; 
> mark.rutland@arm.com; Jonathan Hunter <jonathanh@nvidia.com>; Laxman 
> Dewangan <ldewangan@nvidia.com>; jslaby@suse.com; linux- 
> serial@vger.kernel.org; devicetree@vger.kernel.org; linux- 
> tegra@vger.kernel.org; linux-kernel@vger.kernel.org; Shardar Mohammed 
> <smohammed@nvidia.com>
> Subject: Re: [PATCH 09/14] serial: tegra: set maximum num of uart 
> ports to 8
> 
> On Mon, Aug 12, 2019 at 04:58:18PM +0530, Krishna Yarlagadda wrote:
> > From: Shardar Shariff Md <smohammed@nvidia.com>
> >
> > Set maximum number of UART ports to 8 as older chips have 7 ports 
> > and
> > Tergra194 and later chips will have 8 ports. Add this info to chip 
> > data and register uart driver in platform driver probe.
> >
> > Signed-off-by: Shardar Shariff Md <smohammed@nvidia.com>
> > Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> > ---
> >  drivers/tty/serial/serial-tegra.c | 21 +++++++++++++--------
> >  1 file changed, 13 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/tty/serial/serial-tegra.c
> > b/drivers/tty/serial/serial-tegra.c
> > index e0379d9..329923c 100644
> > --- a/drivers/tty/serial/serial-tegra.c
> > +++ b/drivers/tty/serial/serial-tegra.c
> > @@ -62,7 +62,7 @@
> >  #define TEGRA_UART_TX_TRIG_4B			0x20
> >  #define TEGRA_UART_TX_TRIG_1B			0x30
> >
> > -#define TEGRA_UART_MAXIMUM			5
> > +#define TEGRA_UART_MAXIMUM			8
> >
> >  /* Default UART setting when started: 115200 no parity, stop, 8 data bits */
> >  #define TEGRA_UART_DEFAULT_BAUD			115200
> > @@ -87,6 +87,7 @@ struct tegra_uart_chip_data {
> >  	bool	allow_txfifo_reset_fifo_mode;
> >  	bool	support_clk_src_div;
> >  	bool	fifo_mode_enable_status;
> > +	int	uart_max_port;
> >  };
> >
> >  struct tegra_uart_port {
> > @@ -1323,6 +1324,7 @@ static struct tegra_uart_chip_data
> tegra20_uart_chip_data = {
> >  	.allow_txfifo_reset_fifo_mode	= true,
> >  	.support_clk_src_div		= false,
> >  	.fifo_mode_enable_status	= false,
> > +	.uart_max_port			= 5,
> >  };
> >
> >  static struct tegra_uart_chip_data tegra30_uart_chip_data = { @@
> > -1330,6 +1332,7 @@ static struct tegra_uart_chip_data
> tegra30_uart_chip_data = {
> >  	.allow_txfifo_reset_fifo_mode	= false,
> >  	.support_clk_src_div		= true,
> >  	.fifo_mode_enable_status	= false,
> > +	.uart_max_port			= 5,
> >  };
> >
> >  static struct tegra_uart_chip_data tegra186_uart_chip_data = { @@
> > -1337,6 +1340,7 @@ static struct tegra_uart_chip_data
> tegra186_uart_chip_data = {
> >  	.allow_txfifo_reset_fifo_mode	= false,
> >  	.support_clk_src_div		= true,
> >  	.fifo_mode_enable_status	= true,
> > +	.uart_max_port			= 5,
> 
> You say in the commit message that the older chips have 7 ports, but 
> here you say they have 5. Which one is it?
> 
> >  };
> >
> >  static const struct of_device_id tegra_uart_of_match[] = { @@ 
> > -1386,6
> > +1390,7 @@ static int tegra_uart_probe(struct platform_device *pdev)
> >  	u->type = PORT_TEGRA;
> >  	u->fifosize = 32;
> >  	tup->cdata = cdata;
> > +	tegra_uart_driver.nr = cdata->uart_max_port;
> >
> >  	platform_set_drvdata(pdev, tup);
> >  	resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> @@
> > -1411,6 +1416,13 @@ static int tegra_uart_probe(struct 
> > platform_device
> *pdev)
> >  		return PTR_ERR(tup->rst);
> >  	}
> >
> > +	ret = uart_register_driver(&tegra_uart_driver);
> > +	if (ret < 0) {
> > +		pr_err("Could not register %s driver\n",
> > +		       tegra_uart_driver.driver_name);
> > +		return ret;
> > +	}
> 
> I don't think this is the right place for this. You're going to try to 
> register the driver once for each instance of the Tegra UART that will be probed.
> 
> I'm surprised that this works at all because there's a BUG_ON() early 
> in
> uart_register_driver() that checks for the existence of drv->state, 
> which means that the second instance of tegra_uart_probe() should 
> trigger that and cause the kernel to crash.
> 
> I think it's better to either create an additional of_device_id table 
> that is used to match on the top-level node's compatible string and 
> which only contains the maximum number of ports for the given SoC, or 
> you could add code to
> tegra_uart_init() that counts the number of ports that do match and 
> initialize tegra_uart_driver.nr using that number. It would something like this:
> 
> 	unsigned int count = 0;
> 
> 	for_each_matching_node(np, &tegra_uart_of_match)
> 		count++;
> 
> 	tegra_uart_driver.nr = count;
> 
> You could also add additional checks in the loop, perhaps something
> like:
> 
> 	for_each_matching_node(np, &tegra_uart_of_match)
> 		if (of_device_is_available(np))
> 			count++
> 
> Though that would prevent any UARTs from getting added via dynamic 
> device tree manipulation.
> 
> Thierry
> 
Multiple port entries does result in failures which I missed. I will fix this
as suggested.
KY
> > +
> >  	u->iotype = UPIO_MEM32;
> >  	ret = platform_get_irq(pdev, 0);
> >  	if (ret < 0) {
> > @@ -1472,13 +1484,6 @@ static int __init tegra_uart_init(void)  {
> >  	int ret;
> >
> > -	ret = uart_register_driver(&tegra_uart_driver);
> > -	if (ret < 0) {
> > -		pr_err("Could not register %s driver\n",
> > -			tegra_uart_driver.driver_name);
> > -		return ret;
> > -	}
> > -
> >  	ret = platform_driver_register(&tegra_uart_platform_driver);
> >  	if (ret < 0) {
> >  		pr_err("Uart platform driver register failed, e = %d\n", ret);
> > --
> > 2.7.4
> >

[PATCH 10/14] serial: tegra: add support to use 8 bytes trigger

[Krishna Yarlagadda]

From: Shardar Shariff Md <smohammed@nvidia.com>

Add support to use 8 bytes trigger for Tegra186 SOC.

Signed-off-by: Shardar Shariff Md <smohammed@nvidia.com>
Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 drivers/tty/serial/serial-tegra.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
index 329923c..03d1d20 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -88,6 +88,7 @@ struct tegra_uart_chip_data {
 	bool	support_clk_src_div;
 	bool	fifo_mode_enable_status;
 	int	uart_max_port;
+	int	dma_burst_bytes;
 };
 
 struct tegra_uart_port {
@@ -933,7 +934,12 @@ static int tegra_uart_hw_init(struct tegra_uart_port *tup)
 	 * programmed in the DMA registers.
 	 */
 	tup->fcr_shadow = UART_FCR_ENABLE_FIFO;
-	tup->fcr_shadow |= UART_FCR_R_TRIG_01;
+
+	if (tup->cdata->dma_burst_bytes == 8)
+		tup->fcr_shadow |= UART_FCR_R_TRIG_10;
+	else
+		tup->fcr_shadow |= UART_FCR_R_TRIG_01;
+
 	tup->fcr_shadow |= TEGRA_UART_TX_TRIG_16B;
 	tegra_uart_write(tup, tup->fcr_shadow, UART_FCR);
 
@@ -1046,7 +1052,7 @@ static int tegra_uart_dma_channel_allocate(struct tegra_uart_port *tup,
 		}
 		dma_sconfig.src_addr = tup->uport.mapbase;
 		dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
-		dma_sconfig.src_maxburst = 4;
+		dma_sconfig.src_maxburst = tup->cdata->dma_burst_bytes;
 		tup->rx_dma_chan = dma_chan;
 		tup->rx_dma_buf_virt = dma_buf;
 		tup->rx_dma_buf_phys = dma_phys;
@@ -1325,6 +1331,7 @@ static struct tegra_uart_chip_data tegra20_uart_chip_data = {
 	.support_clk_src_div		= false,
 	.fifo_mode_enable_status	= false,
 	.uart_max_port			= 5,
+	.dma_burst_bytes		= 4,
 };
 
 static struct tegra_uart_chip_data tegra30_uart_chip_data = {
@@ -1333,6 +1340,7 @@ static struct tegra_uart_chip_data tegra30_uart_chip_data = {
 	.support_clk_src_div		= true,
 	.fifo_mode_enable_status	= false,
 	.uart_max_port			= 5,
+	.dma_burst_bytes		= 4,
 };
 
 static struct tegra_uart_chip_data tegra186_uart_chip_data = {
@@ -1341,6 +1349,7 @@ static struct tegra_uart_chip_data tegra186_uart_chip_data = {
 	.support_clk_src_div		= true,
 	.fifo_mode_enable_status	= true,
 	.uart_max_port			= 5,
+	.dma_burst_bytes		= 8,
 };
 
 static const struct of_device_id tegra_uart_of_match[] = {
-- 
2.7.4

Re: [PATCH 10/14] serial: tegra: add support to use 8 bytes trigger

[Jon Hunter]

On 12/08/2019 12:28, Krishna Yarlagadda wrote:
> From: Shardar Shariff Md <smohammed@nvidia.com>
> 
> Add support to use 8 bytes trigger for Tegra186 SOC.
> 
> Signed-off-by: Shardar Shariff Md <smohammed@nvidia.com>
> Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> ---
>  drivers/tty/serial/serial-tegra.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
> index 329923c..03d1d20 100644
> --- a/drivers/tty/serial/serial-tegra.c
> +++ b/drivers/tty/serial/serial-tegra.c
> @@ -88,6 +88,7 @@ struct tegra_uart_chip_data {
>  	bool	support_clk_src_div;
>  	bool	fifo_mode_enable_status;
>  	int	uart_max_port;
> +	int	dma_burst_bytes;

I assume that this is a maximum, so why not say max_dma_burst_bytes?

>  };
>  
>  struct tegra_uart_port {
> @@ -933,7 +934,12 @@ static int tegra_uart_hw_init(struct tegra_uart_port *tup)
>  	 * programmed in the DMA registers.
>  	 */
>  	tup->fcr_shadow = UART_FCR_ENABLE_FIFO;
> -	tup->fcr_shadow |= UART_FCR_R_TRIG_01;
> +
> +	if (tup->cdata->dma_burst_bytes == 8)
> +		tup->fcr_shadow |= UART_FCR_R_TRIG_10;
> +	else
> +		tup->fcr_shadow |= UART_FCR_R_TRIG_01;
> +
>  	tup->fcr_shadow |= TEGRA_UART_TX_TRIG_16B;
>  	tegra_uart_write(tup, tup->fcr_shadow, UART_FCR);
>  
> @@ -1046,7 +1052,7 @@ static int tegra_uart_dma_channel_allocate(struct tegra_uart_port *tup,
>  		}
>  		dma_sconfig.src_addr = tup->uport.mapbase;
>  		dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
> -		dma_sconfig.src_maxburst = 4;
> +		dma_sconfig.src_maxburst = tup->cdata->dma_burst_bytes;
>  		tup->rx_dma_chan = dma_chan;
>  		tup->rx_dma_buf_virt = dma_buf;
>  		tup->rx_dma_buf_phys = dma_phys;
> @@ -1325,6 +1331,7 @@ static struct tegra_uart_chip_data tegra20_uart_chip_data = {
>  	.support_clk_src_div		= false,
>  	.fifo_mode_enable_status	= false,
>  	.uart_max_port			= 5,
> +	.dma_burst_bytes		= 4,

Isn't it simpler to store the TRIG value here?

Jon

-- 
nvpublic

RE: [PATCH 10/14] serial: tegra: add support to use 8 bytes trigger

[Krishna Yarlagadda]

> -----Original Message-----
> From: Jonathan Hunter <jonathanh@nvidia.com>
> Sent: Tuesday, August 20, 2019 1:59 AM
> To: Krishna Yarlagadda <kyarlagadda@nvidia.com>;
> gregkh@linuxfoundation.org; robh+dt@kernel.org; mark.rutland@arm.com;
> thierry.reding@gmail.com; Laxman Dewangan <ldewangan@nvidia.com>;
> jslaby@suse.com
> Cc: linux-serial@vger.kernel.org; devicetree@vger.kernel.org; linux-
> tegra@vger.kernel.org; linux-kernel@vger.kernel.org; Shardar Mohammed
> <smohammed@nvidia.com>
> Subject: Re: [PATCH 10/14] serial: tegra: add support to use 8 bytes trigger
> 
> 
> On 12/08/2019 12:28, Krishna Yarlagadda wrote:
> > From: Shardar Shariff Md <smohammed@nvidia.com>
> >
> > Add support to use 8 bytes trigger for Tegra186 SOC.
> >
> > Signed-off-by: Shardar Shariff Md <smohammed@nvidia.com>
> > Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> > ---
> >  drivers/tty/serial/serial-tegra.c | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
> > index 329923c..03d1d20 100644
> > --- a/drivers/tty/serial/serial-tegra.c
> > +++ b/drivers/tty/serial/serial-tegra.c
> > @@ -88,6 +88,7 @@ struct tegra_uart_chip_data {
> >  	bool	support_clk_src_div;
> >  	bool	fifo_mode_enable_status;
> >  	int	uart_max_port;
> > +	int	dma_burst_bytes;
> 
> I assume that this is a maximum, so why not say max_dma_burst_bytes?
Yes. This is maximum. Will update.

> 
> >  };
> >
> >  struct tegra_uart_port {
> > @@ -933,7 +934,12 @@ static int tegra_uart_hw_init(struct
> tegra_uart_port *tup)
> >  	 * programmed in the DMA registers.
> >  	 */
> >  	tup->fcr_shadow = UART_FCR_ENABLE_FIFO;
> > -	tup->fcr_shadow |= UART_FCR_R_TRIG_01;
> > +
> > +	if (tup->cdata->dma_burst_bytes == 8)
> > +		tup->fcr_shadow |= UART_FCR_R_TRIG_10;
> > +	else
> > +		tup->fcr_shadow |= UART_FCR_R_TRIG_01;
> > +
> >  	tup->fcr_shadow |= TEGRA_UART_TX_TRIG_16B;
> >  	tegra_uart_write(tup, tup->fcr_shadow, UART_FCR);
> >
> > @@ -1046,7 +1052,7 @@ static int
> tegra_uart_dma_channel_allocate(struct tegra_uart_port *tup,
> >  		}
> >  		dma_sconfig.src_addr = tup->uport.mapbase;
> >  		dma_sconfig.src_addr_width =
> DMA_SLAVE_BUSWIDTH_1_BYTE;
> > -		dma_sconfig.src_maxburst = 4;
> > +		dma_sconfig.src_maxburst = tup->cdata->dma_burst_bytes;
> >  		tup->rx_dma_chan = dma_chan;
> >  		tup->rx_dma_buf_virt = dma_buf;
> >  		tup->rx_dma_buf_phys = dma_phys;
> > @@ -1325,6 +1331,7 @@ static struct tegra_uart_chip_data
> tegra20_uart_chip_data = {
> >  	.support_clk_src_div		= false,
> >  	.fifo_mode_enable_status	= false,
> >  	.uart_max_port			= 5,
> > +	.dma_burst_bytes		= 4,
> 
> Isn't it simpler to store the TRIG value here?
> 
> Jon
TRIG value will have to be converted to max burst and one of it needs to be
derived from other.
KY
> 
> --
> nvpublic

[PATCH 11/14] serial: tegra: DT for Adjusted baud rates

[Krishna Yarlagadda]

Tegra186 chip has a hardware issue resulting in frame errors when
tolerance level for baud rate is negative. Provided entries to adjust
baud rate to be within acceptable range and work with devices that
can send negative baud rate. Also report error when baud rate set is
out of tolerance range of controller updated in device tree.

Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 .../bindings/serial/nvidia,tegra20-hsuart.txt      | 32 ++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt b/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt
index 187ec78..1ce3fd4 100644
--- a/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt
+++ b/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt
@@ -20,6 +20,37 @@ Required properties:
 Optional properties:
 - nvidia,enable-modem-interrupt: Enable modem interrupts. Should be enable
 		only if all 8 lines of UART controller are pinmuxed.
+- nvidia,adjust-baud-rates: List of entries providing percentage of baud rate
+  adjustment within a range.
+  Each entry contains sets of 3 values. Range low/high and adjusted rate.
+  <range_low range_high adjusted_rate>
+  When baud rate set on controller falls within the range mentioned in this
+  field, baud rate will be adjusted by percentage mentioned here.
+  Ex: <9600 115200 200>
+  Increase baud rate by 2% when set baud rate falls within range 9600 to 115200
+
+Baud Rate tolerance:
+  Standard UART devices are expected to have tolerance for baud rate error by
+  -4 to +4 %. All Tegra devices till Tegra210 had this support. However,
+  Tegra186 chip has a known hardware issue. UART Rx baud rate tolerance level
+  is 0% to +4% in 1-stop config. Otherwise, the received data will have
+  corruption/invalid framing errors. Parker errata suggests adjusting baud
+  rate to be higher than the deviations observed in Tx.
+
+  Tx deviation of connected device can be captured over scope (or noted from
+  its spec) for valid range and Tegra baud rate has to be set above actual
+  Tx baud rate observed. To do this we use nvidia,adjust-baud-rates
+
+  As an example, consider there is deviation observed in Tx for baud rates as
+  listed below.
+  0 to 9600 has 1% deviation
+  9600 to 115200 2% deviation
+  This slight deviation is expcted and Tegra UART is expected to handle it. Due
+  to the issue stated above, baud rate on Tegra UART should be set equal to or
+  above deviation observed for avoiding frame errors.
+  Property should be set like this
+  nvidia,adjust-baud-rates = <0 9600 100>,
+  			     <9600 115200 200>;
 
 Example:
 
@@ -34,4 +65,5 @@ serial@70006000 {
 	reset-names = "serial";
 	dmas = <&apbdma 8>, <&apbdma 8>;
 	dma-names = "rx", "tx";
+	nvidia,adjust-baud-rates = <1000000 4000000 136>; /* 1.36% shift */
 };
-- 
2.7.4

Re: [PATCH 11/14] serial: tegra: DT for Adjusted baud rates

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3408 bytes --]

On Mon, Aug 12, 2019 at 04:58:20PM +0530, Krishna Yarlagadda wrote:
> Tegra186 chip has a hardware issue resulting in frame errors when
> tolerance level for baud rate is negative. Provided entries to adjust
> baud rate to be within acceptable range and work with devices that
> can send negative baud rate. Also report error when baud rate set is
> out of tolerance range of controller updated in device tree.
> 
> Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> ---
>  .../bindings/serial/nvidia,tegra20-hsuart.txt      | 32 ++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt b/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt
> index 187ec78..1ce3fd4 100644
> --- a/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt
> +++ b/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt
> @@ -20,6 +20,37 @@ Required properties:
>  Optional properties:
>  - nvidia,enable-modem-interrupt: Enable modem interrupts. Should be enable
>  		only if all 8 lines of UART controller are pinmuxed.
> +- nvidia,adjust-baud-rates: List of entries providing percentage of baud rate
> +  adjustment within a range.
> +  Each entry contains sets of 3 values. Range low/high and adjusted rate.
> +  <range_low range_high adjusted_rate>
> +  When baud rate set on controller falls within the range mentioned in this
> +  field, baud rate will be adjusted by percentage mentioned here.
> +  Ex: <9600 115200 200>
> +  Increase baud rate by 2% when set baud rate falls within range 9600 to 115200
> +
> +Baud Rate tolerance:
> +  Standard UART devices are expected to have tolerance for baud rate error by
> +  -4 to +4 %. All Tegra devices till Tegra210 had this support. However,
> +  Tegra186 chip has a known hardware issue. UART Rx baud rate tolerance level
> +  is 0% to +4% in 1-stop config. Otherwise, the received data will have
> +  corruption/invalid framing errors. Parker errata suggests adjusting baud
> +  rate to be higher than the deviations observed in Tx.

The above sounds like the tolerance deviation is a characteristic of the
Tegra186 chip. If the board design does not influence the deviation, why
can't we encode this in the driver? Why do we need a description of this
in device tree?

Thierry

> +
> +  Tx deviation of connected device can be captured over scope (or noted from
> +  its spec) for valid range and Tegra baud rate has to be set above actual
> +  Tx baud rate observed. To do this we use nvidia,adjust-baud-rates
> +
> +  As an example, consider there is deviation observed in Tx for baud rates as
> +  listed below.
> +  0 to 9600 has 1% deviation
> +  9600 to 115200 2% deviation
> +  This slight deviation is expcted and Tegra UART is expected to handle it. Due
> +  to the issue stated above, baud rate on Tegra UART should be set equal to or
> +  above deviation observed for avoiding frame errors.
> +  Property should be set like this
> +  nvidia,adjust-baud-rates = <0 9600 100>,
> +  			     <9600 115200 200>;
>  
>  Example:
>  
> @@ -34,4 +65,5 @@ serial@70006000 {
>  	reset-names = "serial";
>  	dmas = <&apbdma 8>, <&apbdma 8>;
>  	dma-names = "rx", "tx";
> +	nvidia,adjust-baud-rates = <1000000 4000000 136>; /* 1.36% shift */
>  };
> -- 
> 2.7.4
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH 11/14] serial: tegra: DT for Adjusted baud rates

[Krishna Yarlagadda]

> -----Original Message-----
> From: Thierry Reding <thierry.reding@gmail.com>
> Sent: Tuesday, August 13, 2019 3:55 PM
> To: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> Cc: gregkh@linuxfoundation.org; robh+dt@kernel.org;
> mark.rutland@arm.com; Jonathan Hunter <jonathanh@nvidia.com>; Laxman
> Dewangan <ldewangan@nvidia.com>; jslaby@suse.com; linux-
> serial@vger.kernel.org; devicetree@vger.kernel.org; linux-
> tegra@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 11/14] serial: tegra: DT for Adjusted baud rates
> 
> On Mon, Aug 12, 2019 at 04:58:20PM +0530, Krishna Yarlagadda wrote:
> > Tegra186 chip has a hardware issue resulting in frame errors when
> > tolerance level for baud rate is negative. Provided entries to adjust
> > baud rate to be within acceptable range and work with devices that can
> > send negative baud rate. Also report error when baud rate set is out
> > of tolerance range of controller updated in device tree.
> >
> > Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
> > ---
> >  .../bindings/serial/nvidia,tegra20-hsuart.txt      | 32
> ++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt
> > b/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt
> > index 187ec78..1ce3fd4 100644
> > ---
> > a/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt
> > +++ b/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.t
> > +++ xt
> > @@ -20,6 +20,37 @@ Required properties:
> >  Optional properties:
> >  - nvidia,enable-modem-interrupt: Enable modem interrupts. Should be
> enable
> >  		only if all 8 lines of UART controller are pinmuxed.
> > +- nvidia,adjust-baud-rates: List of entries providing percentage of
> > +baud rate
> > +  adjustment within a range.
> > +  Each entry contains sets of 3 values. Range low/high and adjusted rate.
> > +  <range_low range_high adjusted_rate>
> > +  When baud rate set on controller falls within the range mentioned
> > +in this
> > +  field, baud rate will be adjusted by percentage mentioned here.
> > +  Ex: <9600 115200 200>
> > +  Increase baud rate by 2% when set baud rate falls within range 9600
> > +to 115200
> > +
> > +Baud Rate tolerance:
> > +  Standard UART devices are expected to have tolerance for baud rate
> > +error by
> > +  -4 to +4 %. All Tegra devices till Tegra210 had this support.
> > +However,
> > +  Tegra186 chip has a known hardware issue. UART Rx baud rate
> > +tolerance level
> > +  is 0% to +4% in 1-stop config. Otherwise, the received data will
> > +have
> > +  corruption/invalid framing errors. Parker errata suggests adjusting
> > +baud
> > +  rate to be higher than the deviations observed in Tx.
> 
> The above sounds like the tolerance deviation is a characteristic of the
> Tegra186 chip. If the board design does not influence the deviation, why
> can't we encode this in the driver? Why do we need a description of this in
> device tree?
> 
> Thierry
> 
Tolerance is chip specific and info regarding this is part of chip data.
Explained it here to set context for adjusting baud rates based on slave device
as below. Slave device clock might not be accurate and if the actual clock of
slave device would end up out of tolernace range then we will see frame errors.

KY
> > +
> > +  Tx deviation of connected device can be captured over scope (or
> > + noted from  its spec) for valid range and Tegra baud rate has to be
> > + set above actual  Tx baud rate observed. To do this we use
> > + nvidia,adjust-baud-rates
> > +
> > +  As an example, consider there is deviation observed in Tx for baud
> > + rates as  listed below.
> > +  0 to 9600 has 1% deviation
> > +  9600 to 115200 2% deviation
> > +  This slight deviation is expcted and Tegra UART is expected to
> > + handle it. Due  to the issue stated above, baud rate on Tegra UART
> > + should be set equal to or  above deviation observed for avoiding frame
> errors.
> > +  Property should be set like this
> > +  nvidia,adjust-baud-rates = <0 9600 100>,
> > +  			     <9600 115200 200>;
> >
> >  Example:
> >
> > @@ -34,4 +65,5 @@ serial@70006000 {
> >  	reset-names = "serial";
> >  	dmas = <&apbdma 8>, <&apbdma 8>;
> >  	dma-names = "rx", "tx";
> > +	nvidia,adjust-baud-rates = <1000000 4000000 136>; /* 1.36% shift */
> >  };
> > --
> > 2.7.4
> >

[PATCH 12/14] serial: tegra: add support to adjust baud rate

[Krishna Yarlagadda]

Add support to adjust baud rates to fall under supported tolerance
range through DT.

Tegra186 chip has a hardware issue resulting in frame errors when
tolerance level for baud rate is negative. Provided entries to adjust
baud rate to be within acceptable range and work with devices that
can send negative baud rate. Also report error when baud rate set is
out of tolerance range of controller updated in device tree.

Signed-off-by: Shardar Shariff Md <smohammed@nvidia.com>
Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 drivers/tty/serial/serial-tegra.c | 68 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
index 03d1d20..3c9e5c5 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -91,6 +91,12 @@ struct tegra_uart_chip_data {
 	int	dma_burst_bytes;
 };
 
+struct tegra_baud_tolerance {
+	u32 lower_range_baud;
+	u32 upper_range_baud;
+	s32 tolerance;
+};
+
 struct tegra_uart_port {
 	struct uart_port			uport;
 	const struct tegra_uart_chip_data	*cdata;
@@ -127,6 +133,8 @@ struct tegra_uart_port {
 	dma_cookie_t				rx_cookie;
 	unsigned int				tx_bytes_requested;
 	unsigned int				rx_bytes_requested;
+	struct tegra_baud_tolerance		*baud_tolerance;
+	int					n_adjustable_baud_rates;
 };
 
 static void tegra_uart_start_next_tx(struct tegra_uart_port *tup);
@@ -329,6 +337,21 @@ static void tegra_uart_fifo_reset(struct tegra_uart_port *tup, u8 fcr_bits)
 		set_rts(tup, true);
 }
 
+static long tegra_get_tolerance_rate(struct tegra_uart_port *tup,
+				     unsigned int baud, long rate)
+{
+	int i;
+
+	for (i = 0; i < tup->n_adjustable_baud_rates; ++i) {
+		if (baud >= tup->baud_tolerance[i].lower_range_baud &&
+		    baud <= tup->baud_tolerance[i].upper_range_baud)
+			return (rate + (rate *
+				tup->baud_tolerance[i].tolerance) / 10000);
+	}
+
+	return rate;
+}
+
 static int tegra_set_baudrate(struct tegra_uart_port *tup, unsigned int baud)
 {
 	unsigned long rate;
@@ -342,6 +365,9 @@ static int tegra_set_baudrate(struct tegra_uart_port *tup, unsigned int baud)
 
 	if (tup->cdata->support_clk_src_div) {
 		rate = baud * 16;
+		if (tup->n_adjustable_baud_rates)
+			rate = tegra_get_tolerance_rate(tup, baud, rate);
+
 		ret = clk_set_rate(tup->uart_clk, rate);
 		if (ret < 0) {
 			dev_err(tup->uport.dev,
@@ -1312,6 +1338,12 @@ static int tegra_uart_parse_dt(struct platform_device *pdev,
 {
 	struct device_node *np = pdev->dev.of_node;
 	int port;
+	int ret;
+	int index;
+	u32 pval;
+	int count;
+	int n_entries;
+
 
 	port = of_alias_get_id(np, "serial");
 	if (port < 0) {
@@ -1322,6 +1354,42 @@ static int tegra_uart_parse_dt(struct platform_device *pdev,
 
 	tup->enable_modem_interrupt = of_property_read_bool(np,
 					"nvidia,enable-modem-interrupt");
+	n_entries = of_property_count_u32_elems(np, "nvidia,adjust-baud-rates");
+	if (n_entries > 0) {
+		tup->n_adjustable_baud_rates = n_entries / 3;
+		tup->baud_tolerance =
+		devm_kzalloc(&pdev->dev, (tup->n_adjustable_baud_rates) *
+			     sizeof(*tup->baud_tolerance), GFP_KERNEL);
+		if (!tup->baud_tolerance)
+			return -ENOMEM;
+		for (count = 0, index = 0; count < n_entries; count += 3,
+		     index++) {
+			ret =
+			of_property_read_u32_index(np,
+						   "nvidia,adjust-baud-rates",
+						   count, &pval);
+			if (!ret)
+				tup->baud_tolerance[index].lower_range_baud =
+				pval;
+			ret =
+			of_property_read_u32_index(np,
+						   "nvidia,adjust-baud-rates",
+						   count + 1, &pval);
+			if (!ret)
+				tup->baud_tolerance[index].upper_range_baud =
+				pval;
+			ret =
+			of_property_read_u32_index(np,
+						   "nvidia,adjust-baud-rates",
+						   count + 2, &pval);
+			if (!ret)
+				tup->baud_tolerance[index].tolerance =
+				(s32)pval;
+		}
+	} else {
+		tup->n_adjustable_baud_rates = 0;
+	}
+
 	return 0;
 }
 
-- 
2.7.4

[PATCH 13/14] serial: tegra: report clk rate errors

[Krishna Yarlagadda]

Standard UART controllers support +/-4% baud rate error tolerance.
Tegra186 only supports 0% to +4% error tolerance whereas other Tegra
chips support standard +/-4% rate. Add chip data for knowing error
tolerance level for each soc. Creating new compatible for Tegra194 chip
as it supports baud rate error tolerance of -2 to +2 %, different from
older chips.

Signed-off-by: Shardar Shariff Md <smohammed@nvidia.com>
Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 drivers/tty/serial/serial-tegra.c | 58 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 56 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
index 3c9e5c5..3e02f27 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -89,6 +89,8 @@ struct tegra_uart_chip_data {
 	bool	fifo_mode_enable_status;
 	int	uart_max_port;
 	int	dma_burst_bytes;
+	int	error_tolerance_low_range;
+	int	error_tolerance_high_range;
 };
 
 struct tegra_baud_tolerance {
@@ -135,6 +137,8 @@ struct tegra_uart_port {
 	unsigned int				rx_bytes_requested;
 	struct tegra_baud_tolerance		*baud_tolerance;
 	int					n_adjustable_baud_rates;
+	int					required_rate;
+	int					configured_rate;
 };
 
 static void tegra_uart_start_next_tx(struct tegra_uart_port *tup);
@@ -352,6 +356,22 @@ static long tegra_get_tolerance_rate(struct tegra_uart_port *tup,
 	return rate;
 }
 
+static int tegra_check_rate_in_range(struct tegra_uart_port *tup)
+{
+	long diff;
+
+	diff = ((long)(tup->configured_rate - tup->required_rate) * 10000)
+		/ tup->required_rate;
+	if (diff < (tup->cdata->error_tolerance_low_range * 100) ||
+	    diff > (tup->cdata->error_tolerance_high_range * 100)) {
+		dev_err(tup->uport.dev,
+			"configured baud rate is out of range by %d", diff);
+		return -EIO;
+	}
+
+	return 0;
+}
+
 static int tegra_set_baudrate(struct tegra_uart_port *tup, unsigned int baud)
 {
 	unsigned long rate;
@@ -365,6 +385,8 @@ static int tegra_set_baudrate(struct tegra_uart_port *tup, unsigned int baud)
 
 	if (tup->cdata->support_clk_src_div) {
 		rate = baud * 16;
+		tup->required_rate = rate;
+
 		if (tup->n_adjustable_baud_rates)
 			rate = tegra_get_tolerance_rate(tup, baud, rate);
 
@@ -374,7 +396,11 @@ static int tegra_set_baudrate(struct tegra_uart_port *tup, unsigned int baud)
 				"clk_set_rate() failed for rate %lu\n", rate);
 			return ret;
 		}
+		tup->configured_rate = clk_get_rate(tup->uart_clk);
 		divisor = 1;
+		ret = tegra_check_rate_in_range(tup);
+		if (ret < 0)
+			return ret;
 	} else {
 		rate = clk_get_rate(tup->uart_clk);
 		divisor = DIV_ROUND_CLOSEST(rate, baud * 16);
@@ -992,7 +1018,11 @@ static int tegra_uart_hw_init(struct tegra_uart_port *tup)
 	 * enqueued
 	 */
 	tup->lcr_shadow = TEGRA_UART_DEFAULT_LSR;
-	tegra_set_baudrate(tup, TEGRA_UART_DEFAULT_BAUD);
+	ret = tegra_set_baudrate(tup, TEGRA_UART_DEFAULT_BAUD);
+	if (ret < 0) {
+		dev_err(tup->uport.dev, "Failed to set baud rate\n");
+		return ret;
+	}
 	tup->fcr_shadow |= UART_FCR_DMA_SELECT;
 	tegra_uart_write(tup, tup->fcr_shadow, UART_FCR);
 
@@ -1191,6 +1221,7 @@ static void tegra_uart_set_termios(struct uart_port *u,
 	struct clk *parent_clk = clk_get_parent(tup->uart_clk);
 	unsigned long parent_clk_rate = clk_get_rate(parent_clk);
 	int max_divider = (tup->cdata->support_clk_src_div) ? 0x7FFF : 0xFFFF;
+	int ret;
 
 	max_divider *= 16;
 	spin_lock_irqsave(&u->lock, flags);
@@ -1263,7 +1294,11 @@ static void tegra_uart_set_termios(struct uart_port *u,
 			parent_clk_rate/max_divider,
 			parent_clk_rate/16);
 	spin_unlock_irqrestore(&u->lock, flags);
-	tegra_set_baudrate(tup, baud);
+	ret = tegra_set_baudrate(tup, baud);
+	if (ret < 0) {
+		dev_err(tup->uport.dev, "Failed to set baud rate\n");
+		return;
+	}
 	if (tty_termios_baud_rate(termios))
 		tty_termios_encode_baud_rate(termios, baud, baud);
 	spin_lock_irqsave(&u->lock, flags);
@@ -1400,6 +1435,8 @@ static struct tegra_uart_chip_data tegra20_uart_chip_data = {
 	.fifo_mode_enable_status	= false,
 	.uart_max_port			= 5,
 	.dma_burst_bytes		= 4,
+	.error_tolerance_low_range	= 0,
+	.error_tolerance_high_range	= 4,
 };
 
 static struct tegra_uart_chip_data tegra30_uart_chip_data = {
@@ -1409,6 +1446,8 @@ static struct tegra_uart_chip_data tegra30_uart_chip_data = {
 	.fifo_mode_enable_status	= false,
 	.uart_max_port			= 5,
 	.dma_burst_bytes		= 4,
+	.error_tolerance_low_range	= 0,
+	.error_tolerance_high_range	= 4,
 };
 
 static struct tegra_uart_chip_data tegra186_uart_chip_data = {
@@ -1418,6 +1457,18 @@ static struct tegra_uart_chip_data tegra186_uart_chip_data = {
 	.fifo_mode_enable_status	= true,
 	.uart_max_port			= 5,
 	.dma_burst_bytes		= 8,
+	.error_tolerance_low_range	= 0,
+	.error_tolerance_high_range	= 4,
+};
+
+static struct tegra_uart_chip_data tegra194_uart_chip_data = {
+	.tx_fifo_full_status		= true,
+	.allow_txfifo_reset_fifo_mode	= false,
+	.support_clk_src_div		= true,
+	.fifo_mode_enable_status	= true,
+	.dma_burst_bytes		= 8,
+	.error_tolerance_low_range	= -2,
+	.error_tolerance_high_range	= 2,
 };
 
 static const struct of_device_id tegra_uart_of_match[] = {
@@ -1431,6 +1482,9 @@ static const struct of_device_id tegra_uart_of_match[] = {
 		.compatible     = "nvidia,tegra186-hsuart",
 		.data		= &tegra186_uart_chip_data,
 	}, {
+		.compatible     = "nvidia,tegra194-hsuart",
+		.data		= &tegra194_uart_chip_data,
+	}, {
 	},
 };
 MODULE_DEVICE_TABLE(of, tegra_uart_of_match);
-- 
2.7.4

[PATCH 14/14] serial: tegra: Add PIO mode support

[Krishna Yarlagadda]

Add PIO mode support in receive and transmit path with RX interrupt
trigger of 16 bytes for Tegra194 and older chips.

Signed-off-by: Shardar Shariff Md <smohammed@nvidia.com>
Signed-off-by: Krishna Yarlagadda <kyarlagadda@nvidia.com>
---
 drivers/tty/serial/serial-tegra.c | 117 ++++++++++++++++++++++++++++----------
 1 file changed, 86 insertions(+), 31 deletions(-)

diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
index 3e02f27..6e5b418 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -139,6 +139,8 @@ struct tegra_uart_port {
 	int					n_adjustable_baud_rates;
 	int					required_rate;
 	int					configured_rate;
+	bool					use_rx_pio;
+	bool					use_tx_pio;
 };
 
 static void tegra_uart_start_next_tx(struct tegra_uart_port *tup);
@@ -569,7 +571,7 @@ static void tegra_uart_start_next_tx(struct tegra_uart_port *tup)
 	if (!count)
 		return;
 
-	if (count < TEGRA_UART_MIN_DMA)
+	if (tup->use_tx_pio || count < TEGRA_UART_MIN_DMA)
 		tegra_uart_start_pio_tx(tup, count);
 	else if (BYTES_TO_ALIGN(tail) > 0)
 		tegra_uart_start_pio_tx(tup, BYTES_TO_ALIGN(tail));
@@ -802,6 +804,18 @@ static void tegra_uart_handle_modem_signal_change(struct uart_port *u)
 		uart_handle_cts_change(&tup->uport, msr & UART_MSR_CTS);
 }
 
+static void do_handle_rx_pio(struct tegra_uart_port *tup)
+{
+	struct tty_struct *tty = tty_port_tty_get(&tup->uport.state->port);
+	struct tty_port *port = &tup->uport.state->port;
+
+	tegra_uart_handle_rx_pio(tup, port);
+	if (tty) {
+		tty_flip_buffer_push(port);
+		tty_kref_put(tty);
+	}
+}
+
 static irqreturn_t tegra_uart_isr(int irq, void *data)
 {
 	struct tegra_uart_port *tup = data;
@@ -815,7 +829,7 @@ static irqreturn_t tegra_uart_isr(int irq, void *data)
 	while (1) {
 		iir = tegra_uart_read(tup, UART_IIR);
 		if (iir & UART_IIR_NO_INT) {
-			if (is_rx_int) {
+			if (!tup->use_rx_pio && is_rx_int) {
 				tegra_uart_handle_rx_dma(tup);
 				if (tup->rx_in_progress) {
 					ier = tup->ier_shadow;
@@ -843,7 +857,7 @@ static irqreturn_t tegra_uart_isr(int irq, void *data)
 		case 4: /* End of data */
 		case 6: /* Rx timeout */
 		case 2: /* Receive */
-			if (!is_rx_int) {
+			if (!tup->use_rx_pio && !is_rx_int) {
 				is_rx_int = true;
 				/* Disable Rx interrupts */
 				ier = tup->ier_shadow;
@@ -853,6 +867,8 @@ static irqreturn_t tegra_uart_isr(int irq, void *data)
 					UART_IER_RTOIE | TEGRA_UART_IER_EORD);
 				tup->ier_shadow = ier;
 				tegra_uart_write(tup, ier, UART_IER);
+			} else {
+				do_handle_rx_pio(tup);
 			}
 			break;
 
@@ -871,6 +887,7 @@ static irqreturn_t tegra_uart_isr(int irq, void *data)
 static void tegra_uart_stop_rx(struct uart_port *u)
 {
 	struct tegra_uart_port *tup = to_tegra_uport(u);
+	struct tty_port *port = &tup->uport.state->port;
 	struct dma_tx_state state;
 	unsigned long ier;
 
@@ -888,9 +905,13 @@ static void tegra_uart_stop_rx(struct uart_port *u)
 	tup->ier_shadow = ier;
 	tegra_uart_write(tup, ier, UART_IER);
 	tup->rx_in_progress = 0;
-	dmaengine_terminate_all(tup->rx_dma_chan);
-	dmaengine_tx_status(tup->rx_dma_chan, tup->rx_cookie, &state);
-	tegra_uart_rx_buffer_push(tup, state.residue);
+	if (tup->rx_dma_chan && !tup->use_rx_pio) {
+		dmaengine_terminate_all(tup->rx_dma_chan);
+		dmaengine_tx_status(tup->rx_dma_chan, tup->rx_cookie, &state);
+		tegra_uart_rx_buffer_push(tup, state.residue);
+	} else {
+		tegra_uart_handle_rx_pio(tup, port);
+	}
 }
 
 static void tegra_uart_hw_deinit(struct tegra_uart_port *tup)
@@ -941,8 +962,10 @@ static void tegra_uart_hw_deinit(struct tegra_uart_port *tup)
 	tup->rx_in_progress = 0;
 	tup->tx_in_progress = 0;
 
-	tegra_uart_dma_channel_free(tup, true);
-	tegra_uart_dma_channel_free(tup, false);
+	if (!tup->use_rx_pio)
+		tegra_uart_dma_channel_free(tup, true);
+	if (!tup->use_tx_pio)
+		tegra_uart_dma_channel_free(tup, false);
 
 	clk_disable_unprepare(tup->uart_clk);
 }
@@ -987,10 +1010,14 @@ static int tegra_uart_hw_init(struct tegra_uart_port *tup)
 	 */
 	tup->fcr_shadow = UART_FCR_ENABLE_FIFO;
 
-	if (tup->cdata->dma_burst_bytes == 8)
-		tup->fcr_shadow |= UART_FCR_R_TRIG_10;
-	else
-		tup->fcr_shadow |= UART_FCR_R_TRIG_01;
+	if (tup->use_rx_pio) {
+		tup->fcr_shadow |= UART_FCR_R_TRIG_11;
+	} else {
+		if (tup->cdata->dma_burst_bytes == 8)
+			tup->fcr_shadow |= UART_FCR_R_TRIG_10;
+		else
+			tup->fcr_shadow |= UART_FCR_R_TRIG_01;
+	}
 
 	tup->fcr_shadow |= TEGRA_UART_TX_TRIG_16B;
 	tegra_uart_write(tup, tup->fcr_shadow, UART_FCR);
@@ -1017,19 +1044,23 @@ static int tegra_uart_hw_init(struct tegra_uart_port *tup)
 	 * (115200, N, 8, 1) so that the receive DMA buffer may be
 	 * enqueued
 	 */
-	tup->lcr_shadow = TEGRA_UART_DEFAULT_LSR;
 	ret = tegra_set_baudrate(tup, TEGRA_UART_DEFAULT_BAUD);
 	if (ret < 0) {
 		dev_err(tup->uport.dev, "Failed to set baud rate\n");
 		return ret;
 	}
-	tup->fcr_shadow |= UART_FCR_DMA_SELECT;
-	tegra_uart_write(tup, tup->fcr_shadow, UART_FCR);
+	if (!tup->use_rx_pio) {
+		tup->lcr_shadow = TEGRA_UART_DEFAULT_LSR;
+		tup->fcr_shadow |= UART_FCR_DMA_SELECT;
+		tegra_uart_write(tup, tup->fcr_shadow, UART_FCR);
 
-	ret = tegra_uart_start_rx_dma(tup);
-	if (ret < 0) {
-		dev_err(tup->uport.dev, "Not able to start Rx DMA\n");
-		return ret;
+		ret = tegra_uart_start_rx_dma(tup);
+		if (ret < 0) {
+			dev_err(tup->uport.dev, "Not able to start Rx DMA\n");
+			return ret;
+		}
+	} else {
+		tegra_uart_write(tup, tup->fcr_shadow, UART_FCR);
 	}
 	tup->rx_in_progress = 1;
 
@@ -1051,7 +1082,12 @@ static int tegra_uart_hw_init(struct tegra_uart_port *tup)
 	 * both the EORD as well as RX_TIMEOUT - SW sees RX_TIMEOUT first
 	 * then the EORD.
 	 */
-	tup->ier_shadow = UART_IER_RLSI | UART_IER_RTOIE | TEGRA_UART_IER_EORD;
+	if (!tup->use_rx_pio)
+		tup->ier_shadow = UART_IER_RLSI | UART_IER_RTOIE |
+			TEGRA_UART_IER_EORD;
+	else
+		tup->ier_shadow = UART_IER_RLSI | UART_IER_RTOIE | UART_IER_RDI;
+
 	tegra_uart_write(tup, tup->ier_shadow, UART_IER);
 	return 0;
 }
@@ -1146,16 +1182,22 @@ static int tegra_uart_startup(struct uart_port *u)
 	struct tegra_uart_port *tup = to_tegra_uport(u);
 	int ret;
 
-	ret = tegra_uart_dma_channel_allocate(tup, false);
-	if (ret < 0) {
-		dev_err(u->dev, "Tx Dma allocation failed, err = %d\n", ret);
-		return ret;
+	if (!tup->use_tx_pio) {
+		ret = tegra_uart_dma_channel_allocate(tup, false);
+		if (ret < 0) {
+			dev_err(u->dev, "Tx Dma allocation failed, err = %d\n",
+				ret);
+			return ret;
+		}
 	}
 
-	ret = tegra_uart_dma_channel_allocate(tup, true);
-	if (ret < 0) {
-		dev_err(u->dev, "Rx Dma allocation failed, err = %d\n", ret);
-		goto fail_rx_dma;
+	if (!tup->use_rx_pio) {
+		ret = tegra_uart_dma_channel_allocate(tup, true);
+		if (ret < 0) {
+			dev_err(u->dev, "Rx Dma allocation failed, err = %d\n",
+				ret);
+			goto fail_rx_dma;
+		}
 	}
 
 	ret = tegra_uart_hw_init(tup);
@@ -1173,9 +1215,11 @@ static int tegra_uart_startup(struct uart_port *u)
 	return 0;
 
 fail_hw_init:
-	tegra_uart_dma_channel_free(tup, true);
+	if (!tup->use_rx_pio)
+		tegra_uart_dma_channel_free(tup, true);
 fail_rx_dma:
-	tegra_uart_dma_channel_free(tup, false);
+	if (!tup->use_tx_pio)
+		tegra_uart_dma_channel_free(tup, false);
 	return ret;
 }
 
@@ -1379,7 +1423,6 @@ static int tegra_uart_parse_dt(struct platform_device *pdev,
 	int count;
 	int n_entries;
 
-
 	port = of_alias_get_id(np, "serial");
 	if (port < 0) {
 		dev_err(&pdev->dev, "failed to get alias id, errno %d\n", port);
@@ -1389,6 +1432,18 @@ static int tegra_uart_parse_dt(struct platform_device *pdev,
 
 	tup->enable_modem_interrupt = of_property_read_bool(np,
 					"nvidia,enable-modem-interrupt");
+
+	index = of_property_match_string(np, "dma-names", "rx");
+	if (index < 0) {
+		tup->use_rx_pio = true;
+		dev_info(&pdev->dev, "RX in PIO mode\n");
+	}
+	index = of_property_match_string(np, "dma-names", "tx");
+	if (index < 0) {
+		tup->use_tx_pio = true;
+		dev_info(&pdev->dev, "TX in PIO mode\n");
+	}
+
 	n_entries = of_property_count_u32_elems(np, "nvidia,adjust-baud-rates");
 	if (n_entries > 0) {
 		tup->n_adjustable_baud_rates = n_entries / 3;
-- 
2.7.4