[PATCH 0/7] tty: 8250: omap: workaround for IP errata and a bug fix

[PATCH 0/7] tty: 8250: omap: workaround for IP errata and a bug fix

[Sekhar Nori]

This series works around "Advisory 21" as documented in
AM437x SoC errata[1]. AM335x and DRA7x also suffer from
the same errata and chip design team is in the process
of updating the errata documents of those devices as well.

Patch 1/7 fixes a related bug but can be applied independently.

Series applies to v4.2-rc1

[1] http://www.ti.com/lit/er/sprz408b/sprz408b.pdf

Sekhar Nori (7):
  tty: serial: 8250: omap: fix kernel crash in suspend-to-ram
  Documentation: DT: omap_serial: document missing compatible
  tty: 8250: omap: introduce function to update mdr1
  tty: 8250: omap eliminate use of of_machine_is_compatible()
  tty: 8250: workaround errata on disabling UART after using DMA
  tty: 8250: omap: workaround module disable errata on dra7x SoCs
  ARM: dts: dra7: workaround UART module disable errata

 .../devicetree/bindings/serial/omap_serial.txt     |   3 +
 arch/arm/boot/dts/am33xx.dtsi                      |  12 +--
 arch/arm/boot/dts/dra7.dtsi                        |  20 ++--
 drivers/tty/serial/8250/8250_omap.c                | 110 ++++++++++++++++-----
 4 files changed, 103 insertions(+), 42 deletions(-)

-- 
2.4.4.408.g16da57c

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RESEND 1/7] tty: serial: 8250: omap: fix kernel crash in suspend-to-ram

[Sekhar Nori]

omap_device infrastructure has a suspend_noirq hook which
runtime suspends all devices late in the suspend cycle (see
_od_suspend_noirq() in arch/arm/mach-omap2/omap_device.c)

This leads to a NULL pointer exception in 8250_omap driver
since by the time omap8250_runtime_suspend() is called, 8250_dma
driver has already set rxchan to NULL via serial8250_release_dma().

Make an explicit check to see if rxchan is NULL in
runtime_{suspend|resume} hooks to fix this.

Signed-off-by: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
---
Previous version: http://www.spinics.net/lists/linux-omap/msg119459.html
No change in this version except rebased to v4.2-rc1

 drivers/tty/serial/8250/8250_omap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index d75a66c72750..20c5b9c4c288 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -1285,7 +1285,7 @@ static int omap8250_runtime_suspend(struct device *dev)
 			return -EBUSY;
 	}
 
-	if (up->dma)
+	if (up->dma && up->dma->rxchan)
 		omap_8250_rx_dma(up, UART_IIR_RX_TIMEOUT);
 
 	priv->latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE;
@@ -1310,7 +1310,7 @@ static int omap8250_runtime_resume(struct device *dev)
 	if (loss_cntx)
 		omap8250_restore_regs(up);
 
-	if (up->dma)
+	if (up->dma && up->dma->rxchan)
 		omap_8250_rx_dma(up, 0);
 
 	priv->latency = priv->calc_latency;
-- 
2.4.4.408.g16da57c

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RESEND 1/7] tty: serial: 8250: omap: fix kernel crash in suspend-to-ram

[Peter Hurley]

Hi Sekhar,

On 07/06/2015 05:47 AM, Sekhar Nori wrote:
> omap_device infrastructure has a suspend_noirq hook which
> runtime suspends all devices late in the suspend cycle (see
> _od_suspend_noirq() in arch/arm/mach-omap2/omap_device.c)

Why is omap2 the only arch/SoC that does this; ie., call the runtime
callbacks after the system pm callbacks?

Whatever positive it brings, it's a mess at the driver level.
For example, this driver has to hook prepare()/complete() so it can
set local state so that it can detect when the runtime suspend
is being called during system suspend.

Regards,
Peter Hurley


> This leads to a NULL pointer exception in 8250_omap driver
> since by the time omap8250_runtime_suspend() is called, 8250_dma
> driver has already set rxchan to NULL via serial8250_release_dma().
> 
> Make an explicit check to see if rxchan is NULL in
> runtime_{suspend|resume} hooks to fix this.
> 
> Signed-off-by: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
> ---
> Previous version: http://www.spinics.net/lists/linux-omap/msg119459.html
> No change in this version except rebased to v4.2-rc1
> 
>  drivers/tty/serial/8250/8250_omap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> index d75a66c72750..20c5b9c4c288 100644
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -1285,7 +1285,7 @@ static int omap8250_runtime_suspend(struct device *dev)
>  			return -EBUSY;
>  	}
>  
> -	if (up->dma)
> +	if (up->dma && up->dma->rxchan)
>  		omap_8250_rx_dma(up, UART_IIR_RX_TIMEOUT);
>  
>  	priv->latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE;
> @@ -1310,7 +1310,7 @@ static int omap8250_runtime_resume(struct device *dev)
>  	if (loss_cntx)
>  		omap8250_restore_regs(up);
>  
> -	if (up->dma)
> +	if (up->dma && up->dma->rxchan)
>  		omap_8250_rx_dma(up, 0);
>  
>  	priv->latency = priv->calc_latency;
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RESEND 1/7] tty: serial: 8250: omap: fix kernel crash in suspend-to-ram

[Sekhar Nori]

Hi Peter,

On Wednesday 08 July 2015 07:34 PM, Peter Hurley wrote:
> Hi Sekhar,
> 
> On 07/06/2015 05:47 AM, Sekhar Nori wrote:
>> omap_device infrastructure has a suspend_noirq hook which
>> runtime suspends all devices late in the suspend cycle (see
>> _od_suspend_noirq() in arch/arm/mach-omap2/omap_device.c)
> 
> Why is omap2 the only arch/SoC that does this; ie., call the runtime
> callbacks after the system pm callbacks?

Even I am not sure why this needs to be done. I _think_ it was done to
make sure all IPs are idled even if driver did not implement system
sleep ops, but I could be wrong.

Cc: Kevin Hilman since he added this support.

In any case, I think this patch is okay, as the additional NULL pointer
check is still valid.

Thanks,
Sekhar

> 
> Whatever positive it brings, it's a mess at the driver level.
> For example, this driver has to hook prepare()/complete() so it can
> set local state so that it can detect when the runtime suspend
> is being called during system suspend.
> 
> Regards,
> Peter Hurley
> 
> 
>> This leads to a NULL pointer exception in 8250_omap driver
>> since by the time omap8250_runtime_suspend() is called, 8250_dma
>> driver has already set rxchan to NULL via serial8250_release_dma().
>>
>> Make an explicit check to see if rxchan is NULL in
>> runtime_{suspend|resume} hooks to fix this.
>>
>> Signed-off-by: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
>> ---
>> Previous version: http://www.spinics.net/lists/linux-omap/msg119459.html
>> No change in this version except rebased to v4.2-rc1
>>
>>  drivers/tty/serial/8250/8250_omap.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
>> index d75a66c72750..20c5b9c4c288 100644
>> --- a/drivers/tty/serial/8250/8250_omap.c
>> +++ b/drivers/tty/serial/8250/8250_omap.c
>> @@ -1285,7 +1285,7 @@ static int omap8250_runtime_suspend(struct device *dev)
>>  			return -EBUSY;
>>  	}
>>  
>> -	if (up->dma)
>> +	if (up->dma && up->dma->rxchan)
>>  		omap_8250_rx_dma(up, UART_IIR_RX_TIMEOUT);
>>  
>>  	priv->latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE;
>> @@ -1310,7 +1310,7 @@ static int omap8250_runtime_resume(struct device *dev)
>>  	if (loss_cntx)
>>  		omap8250_restore_regs(up);
>>  
>> -	if (up->dma)
>> +	if (up->dma && up->dma->rxchan)
>>  		omap_8250_rx_dma(up, 0);
>>  
>>  	priv->latency = priv->calc_latency;
>>
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/7] Documentation: DT: omap_serial: document missing compatible

[Sekhar Nori]

The compatible "ti,am4372-uart" is used in arch/arm/boot/dts/am4372.dtsi
but not documented. Add necessary documentation.

Signed-off-by: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
---
 Documentation/devicetree/bindings/serial/omap_serial.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/serial/omap_serial.txt b/Documentation/devicetree/bindings/serial/omap_serial.txt
index 54c2a155c783..d3bd2b1ec401 100644
--- a/Documentation/devicetree/bindings/serial/omap_serial.txt
+++ b/Documentation/devicetree/bindings/serial/omap_serial.txt
@@ -4,6 +4,7 @@ Required properties:
 - compatible : should be "ti,omap2-uart" for OMAP2 controllers
 - compatible : should be "ti,omap3-uart" for OMAP3 controllers
 - compatible : should be "ti,omap4-uart" for OMAP4 controllers
+- compatible : should be "ti,am4372-uart" for AM437x controllers
 - reg : address and length of the register space
 - interrupts or interrupts-extended : Should contain the uart interrupt
                                       specifier or both the interrupt
-- 
2.4.4.408.g16da57c

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/7] tty: 8250: omap: introduce function to update mdr1

[Sekhar Nori]

updating mdr1 register on OMAP needs to take care of
errata i202. Introduce a function to update mdr1.

This will be useful later on when mdr1 needs to be
written to from other places. No functional change.

Signed-off-by: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
---
 drivers/tty/serial/8250/8250_omap.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 20c5b9c4c288..d9c96b993a84 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -232,6 +232,15 @@ static void omap8250_update_scr(struct uart_8250_port *up,
 	serial_out(up, UART_OMAP_SCR, priv->scr);
 }
 
+static void omap8250_update_mdr1(struct uart_8250_port *up,
+				 struct omap8250_priv *priv)
+{
+	if (priv->habit & UART_ERRATA_i202_MDR1_ACCESS)
+		omap_8250_mdr1_errataset(up, priv);
+	else
+		serial_out(up, UART_OMAP_MDR1, priv->mdr1);
+}
+
 static void omap8250_restore_regs(struct uart_8250_port *up)
 {
 	struct omap8250_priv *priv = up->port.private_data;
@@ -282,11 +291,9 @@ static void omap8250_restore_regs(struct uart_8250_port *up)
 	serial_out(up, UART_XOFF1, priv->xoff);
 
 	serial_out(up, UART_LCR, up->lcr);
-	/* need mode A for FCR */
-	if (priv->habit & UART_ERRATA_i202_MDR1_ACCESS)
-		omap_8250_mdr1_errataset(up, priv);
-	else
-		serial_out(up, UART_OMAP_MDR1, priv->mdr1);
+
+	omap8250_update_mdr1(up, priv);
+
 	up->port.ops->set_mctrl(&up->port, up->port.mctrl);
 }
 
-- 
2.4.4.408.g16da57c

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/7] tty: 8250: omap: introduce function to update mdr1

[Peter Hurley]

Hi Sekhar,

On 07/06/2015 05:47 AM, Sekhar Nori wrote:
> updating mdr1 register on OMAP needs to take care of
> errata i202. Introduce a function to update mdr1.
> 
> This will be useful later on when mdr1 needs to be
> written to from other places. No functional change.

This changelog is not clear. May I suggest:

serial: 8250_omap: Refactor MDR1 update

The errata [1] workaround implemented in follow-on patch,
"serial: 8250_omap: workaround errata on disabling UART after using DMA",
requires MDR1 register programming.

Extract MDR1 register update into helper function, omap8250_update_mdr1().

[1] Advisory 21 in http://www.ti.com/lit/er/sprz408b/sprz408b.pdf


Regards,
Peter Hurley

> Signed-off-by: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
> ---
>  drivers/tty/serial/8250/8250_omap.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> index 20c5b9c4c288..d9c96b993a84 100644
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -232,6 +232,15 @@ static void omap8250_update_scr(struct uart_8250_port *up,
>  	serial_out(up, UART_OMAP_SCR, priv->scr);
>  }
>  
> +static void omap8250_update_mdr1(struct uart_8250_port *up,
> +				 struct omap8250_priv *priv)
> +{
> +	if (priv->habit & UART_ERRATA_i202_MDR1_ACCESS)
> +		omap_8250_mdr1_errataset(up, priv);
> +	else
> +		serial_out(up, UART_OMAP_MDR1, priv->mdr1);
> +}
> +
>  static void omap8250_restore_regs(struct uart_8250_port *up)
>  {
>  	struct omap8250_priv *priv = up->port.private_data;
> @@ -282,11 +291,9 @@ static void omap8250_restore_regs(struct uart_8250_port *up)
>  	serial_out(up, UART_XOFF1, priv->xoff);
>  
>  	serial_out(up, UART_LCR, up->lcr);
> -	/* need mode A for FCR */
> -	if (priv->habit & UART_ERRATA_i202_MDR1_ACCESS)
> -		omap_8250_mdr1_errataset(up, priv);
> -	else
> -		serial_out(up, UART_OMAP_MDR1, priv->mdr1);
> +
> +	omap8250_update_mdr1(up, priv);
> +
>  	up->port.ops->set_mctrl(&up->port, up->port.mctrl);
>  }
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/7] tty: 8250: omap: introduce function to update mdr1

[Sekhar Nori]

On Thursday 09 July 2015 05:59 AM, Peter Hurley wrote:
> Hi Sekhar,
> 
> On 07/06/2015 05:47 AM, Sekhar Nori wrote:
>> updating mdr1 register on OMAP needs to take care of
>> errata i202. Introduce a function to update mdr1.
>>
>> This will be useful later on when mdr1 needs to be
>> written to from other places. No functional change.
> 
> This changelog is not clear. May I suggest:
> 
> serial: 8250_omap: Refactor MDR1 update
> 
> The errata [1] workaround implemented in follow-on patch,
> "serial: 8250_omap: workaround errata on disabling UART after using DMA",
> requires MDR1 register programming.
> 
> Extract MDR1 register update into helper function, omap8250_update_mdr1().
> 
> [1] Advisory 21 in http://www.ti.com/lit/er/sprz408b/sprz408b.pdf

Alright, will use this description instead.

Thanks,
Sekhar

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/7] tty: 8250: omap eliminate use of of_machine_is_compatible()

[Sekhar Nori]

Use of of_machine_is_compatible() for AM335x specific DMA
quirk in 8250_omap driver makes it ugly to extend the
quirk for other platforms. Instead use a new compatible.

The new compatible will also make it easier to care of other
quirks specific to AM335x and like SoCs.

This patch does break backward DTB compatibility for users of
8250_omap driver on AM335x boards. However, the 8250_omap driver
is new and omap_serial is still the default choice driver for UART
and so choosing to break compatibility over keeping the code
around forever.

Signed-off-by: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
---
 .../devicetree/bindings/serial/omap_serial.txt     |  1 +
 arch/arm/boot/dts/am33xx.dtsi                      | 12 ++++----
 drivers/tty/serial/8250/8250_omap.c                | 35 +++++++++++++---------
 3 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/omap_serial.txt b/Documentation/devicetree/bindings/serial/omap_serial.txt
index d3bd2b1ec401..0ee88209b341 100644
--- a/Documentation/devicetree/bindings/serial/omap_serial.txt
+++ b/Documentation/devicetree/bindings/serial/omap_serial.txt
@@ -5,6 +5,7 @@ Required properties:
 - compatible : should be "ti,omap3-uart" for OMAP3 controllers
 - compatible : should be "ti,omap4-uart" for OMAP4 controllers
 - compatible : should be "ti,am4372-uart" for AM437x controllers
+- compatible : should be "ti,am3352-uart" for AM335x controllers
 - reg : address and length of the register space
 - interrupts or interrupts-extended : Should contain the uart interrupt
                                       specifier or both the interrupt
diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 21fcc440fc1a..b76f9a2ce05d 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -210,7 +210,7 @@
 		};
 
 		uart0: serial@44e09000 {
-			compatible = "ti,omap3-uart";
+			compatible = "ti,am3352-uart", "ti,omap3-uart";
 			ti,hwmods = "uart1";
 			clock-frequency = <48000000>;
 			reg = <0x44e09000 0x2000>;
@@ -221,7 +221,7 @@
 		};
 
 		uart1: serial@48022000 {
-			compatible = "ti,omap3-uart";
+			compatible = "ti,am3352-uart", "ti,omap3-uart";
 			ti,hwmods = "uart2";
 			clock-frequency = <48000000>;
 			reg = <0x48022000 0x2000>;
@@ -232,7 +232,7 @@
 		};
 
 		uart2: serial@48024000 {
-			compatible = "ti,omap3-uart";
+			compatible = "ti,am3352-uart", "ti,omap3-uart";
 			ti,hwmods = "uart3";
 			clock-frequency = <48000000>;
 			reg = <0x48024000 0x2000>;
@@ -243,7 +243,7 @@
 		};
 
 		uart3: serial@481a6000 {
-			compatible = "ti,omap3-uart";
+			compatible = "ti,am3352-uart", "ti,omap3-uart";
 			ti,hwmods = "uart4";
 			clock-frequency = <48000000>;
 			reg = <0x481a6000 0x2000>;
@@ -252,7 +252,7 @@
 		};
 
 		uart4: serial@481a8000 {
-			compatible = "ti,omap3-uart";
+			compatible = "ti,am3352-uart", "ti,omap3-uart";
 			ti,hwmods = "uart5";
 			clock-frequency = <48000000>;
 			reg = <0x481a8000 0x2000>;
@@ -261,7 +261,7 @@
 		};
 
 		uart5: serial@481aa000 {
-			compatible = "ti,omap3-uart";
+			compatible = "ti,am3352-uart", "ti,omap3-uart";
 			ti,hwmods = "uart6";
 			clock-frequency = <48000000>;
 			reg = <0x481aa000 0x2000>;
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index d9c96b993a84..52566387ec37 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -16,6 +16,7 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/of_gpio.h>
 #include <linux/of_irq.h>
 #include <linux/delay.h>
@@ -537,14 +538,14 @@ static void omap_serial_fill_features_erratas(struct uart_8250_port *up,
 
 	switch (revision) {
 	case OMAP_UART_REV_46:
-		priv->habit = UART_ERRATA_i202_MDR1_ACCESS;
+		priv->habit |= UART_ERRATA_i202_MDR1_ACCESS;
 		break;
 	case OMAP_UART_REV_52:
-		priv->habit = UART_ERRATA_i202_MDR1_ACCESS |
+		priv->habit |= UART_ERRATA_i202_MDR1_ACCESS |
 				OMAP_UART_WER_HAS_TX_WAKEUP;
 		break;
 	case OMAP_UART_REV_63:
-		priv->habit = UART_ERRATA_i202_MDR1_ACCESS |
+		priv->habit |= UART_ERRATA_i202_MDR1_ACCESS |
 			OMAP_UART_WER_HAS_TX_WAKEUP;
 		break;
 	default:
@@ -1061,6 +1062,17 @@ static int omap8250_no_handle_irq(struct uart_port *port)
 	return 0;
 }
 
+static const u8 am3352_habit = OMAP_DMA_TX_KICK;
+
+static const struct of_device_id omap8250_dt_ids[] = {
+	{ .compatible = "ti,omap2-uart" },
+	{ .compatible = "ti,omap3-uart" },
+	{ .compatible = "ti,omap4-uart" },
+	{ .compatible = "ti,am3352-uart", .data = &am3352_habit, },
+	{},
+};
+MODULE_DEVICE_TABLE(of, omap8250_dt_ids);
+
 static int omap8250_probe(struct platform_device *pdev)
 {
 	struct resource *regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1125,11 +1137,17 @@ static int omap8250_probe(struct platform_device *pdev)
 	up.port.unthrottle = omap_8250_unthrottle;
 
 	if (pdev->dev.of_node) {
+		const struct of_device_id *id;
+
 		ret = of_alias_get_id(pdev->dev.of_node, "serial");
 
 		of_property_read_u32(pdev->dev.of_node, "clock-frequency",
 				     &up.port.uartclk);
 		priv->wakeirq = irq_of_parse_and_map(pdev->dev.of_node, 1);
+
+		id = of_match_device(of_match_ptr(omap8250_dt_ids), &pdev->dev);
+		if (id && id->data)
+			priv->habit |= *(u8 *)id->data;
 	} else {
 		ret = pdev->id;
 	}
@@ -1184,9 +1202,6 @@ static int omap8250_probe(struct platform_device *pdev)
 			priv->omap8250_dma.rx_size = RX_TRIGGER;
 			priv->omap8250_dma.rxconf.src_maxburst = RX_TRIGGER;
 			priv->omap8250_dma.txconf.dst_maxburst = TX_TRIGGER;
-
-			if (of_machine_is_compatible("ti,am33xx"))
-				priv->habit |= OMAP_DMA_TX_KICK;
 		}
 	}
 #endif
@@ -1374,14 +1389,6 @@ static const struct dev_pm_ops omap8250_dev_pm_ops = {
 	.complete       = omap8250_complete,
 };
 
-static const struct of_device_id omap8250_dt_ids[] = {
-	{ .compatible = "ti,omap2-uart" },
-	{ .compatible = "ti,omap3-uart" },
-	{ .compatible = "ti,omap4-uart" },
-	{},
-};
-MODULE_DEVICE_TABLE(of, omap8250_dt_ids);
-
 static struct platform_driver omap8250_platform_driver = {
 	.driver = {
 		.name		= "omap8250",
-- 
2.4.4.408.g16da57c

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] tty: 8250: omap eliminate use of of_machine_is_compatible()

[Peter Hurley]

Hi Sekhar,

On 07/06/2015 05:47 AM, Sekhar Nori wrote:
> Use of of_machine_is_compatible() for AM335x specific DMA
> quirk in 8250_omap driver makes it ugly to extend the
> quirk for other platforms. Instead use a new compatible.
> 
> The new compatible will also make it easier to care of other
> quirks specific to AM335x and like SoCs.
> 
> This patch does break backward DTB compatibility for users of
> 8250_omap driver on AM335x boards.

Not ok.

8250_omap was released with 3.19 and has been the default driver for
BeagleBone since 4.0.

Regards,
Peter Hurley

> However, the 8250_omap driver
> is new and omap_serial is still the default choice driver for UART
> and so choosing to break compatibility over keeping the code
> around forever.
> 
> Signed-off-by: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
> ---
>  .../devicetree/bindings/serial/omap_serial.txt     |  1 +
>  arch/arm/boot/dts/am33xx.dtsi                      | 12 ++++----
>  drivers/tty/serial/8250/8250_omap.c                | 35 +++++++++++++---------
>  3 files changed, 28 insertions(+), 20 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/serial/omap_serial.txt b/Documentation/devicetree/bindings/serial/omap_serial.txt
> index d3bd2b1ec401..0ee88209b341 100644
> --- a/Documentation/devicetree/bindings/serial/omap_serial.txt
> +++ b/Documentation/devicetree/bindings/serial/omap_serial.txt
> @@ -5,6 +5,7 @@ Required properties:
>  - compatible : should be "ti,omap3-uart" for OMAP3 controllers
>  - compatible : should be "ti,omap4-uart" for OMAP4 controllers
>  - compatible : should be "ti,am4372-uart" for AM437x controllers
> +- compatible : should be "ti,am3352-uart" for AM335x controllers
>  - reg : address and length of the register space
>  - interrupts or interrupts-extended : Should contain the uart interrupt
>                                        specifier or both the interrupt
> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> index 21fcc440fc1a..b76f9a2ce05d 100644
> --- a/arch/arm/boot/dts/am33xx.dtsi
> +++ b/arch/arm/boot/dts/am33xx.dtsi
> @@ -210,7 +210,7 @@
>  		};
>  
>  		uart0: serial@44e09000 {
> -			compatible = "ti,omap3-uart";
> +			compatible = "ti,am3352-uart", "ti,omap3-uart";
>  			ti,hwmods = "uart1";
>  			clock-frequency = <48000000>;
>  			reg = <0x44e09000 0x2000>;
> @@ -221,7 +221,7 @@
>  		};
>  
>  		uart1: serial@48022000 {
> -			compatible = "ti,omap3-uart";
> +			compatible = "ti,am3352-uart", "ti,omap3-uart";
>  			ti,hwmods = "uart2";
>  			clock-frequency = <48000000>;
>  			reg = <0x48022000 0x2000>;
> @@ -232,7 +232,7 @@
>  		};
>  
>  		uart2: serial@48024000 {
> -			compatible = "ti,omap3-uart";
> +			compatible = "ti,am3352-uart", "ti,omap3-uart";
>  			ti,hwmods = "uart3";
>  			clock-frequency = <48000000>;
>  			reg = <0x48024000 0x2000>;
> @@ -243,7 +243,7 @@
>  		};
>  
>  		uart3: serial@481a6000 {
> -			compatible = "ti,omap3-uart";
> +			compatible = "ti,am3352-uart", "ti,omap3-uart";
>  			ti,hwmods = "uart4";
>  			clock-frequency = <48000000>;
>  			reg = <0x481a6000 0x2000>;
> @@ -252,7 +252,7 @@
>  		};
>  
>  		uart4: serial@481a8000 {
> -			compatible = "ti,omap3-uart";
> +			compatible = "ti,am3352-uart", "ti,omap3-uart";
>  			ti,hwmods = "uart5";
>  			clock-frequency = <48000000>;
>  			reg = <0x481a8000 0x2000>;
> @@ -261,7 +261,7 @@
>  		};
>  
>  		uart5: serial@481aa000 {
> -			compatible = "ti,omap3-uart";
> +			compatible = "ti,am3352-uart", "ti,omap3-uart";
>  			ti,hwmods = "uart6";
>  			clock-frequency = <48000000>;
>  			reg = <0x481aa000 0x2000>;
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> index d9c96b993a84..52566387ec37 100644
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -16,6 +16,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/of_gpio.h>
>  #include <linux/of_irq.h>
>  #include <linux/delay.h>
> @@ -537,14 +538,14 @@ static void omap_serial_fill_features_erratas(struct uart_8250_port *up,
>  
>  	switch (revision) {
>  	case OMAP_UART_REV_46:
> -		priv->habit = UART_ERRATA_i202_MDR1_ACCESS;
> +		priv->habit |= UART_ERRATA_i202_MDR1_ACCESS;
>  		break;
>  	case OMAP_UART_REV_52:
> -		priv->habit = UART_ERRATA_i202_MDR1_ACCESS |
> +		priv->habit |= UART_ERRATA_i202_MDR1_ACCESS |
>  				OMAP_UART_WER_HAS_TX_WAKEUP;
>  		break;
>  	case OMAP_UART_REV_63:
> -		priv->habit = UART_ERRATA_i202_MDR1_ACCESS |
> +		priv->habit |= UART_ERRATA_i202_MDR1_ACCESS |
>  			OMAP_UART_WER_HAS_TX_WAKEUP;
>  		break;
>  	default:
> @@ -1061,6 +1062,17 @@ static int omap8250_no_handle_irq(struct uart_port *port)
>  	return 0;
>  }
>  
> +static const u8 am3352_habit = OMAP_DMA_TX_KICK;
> +
> +static const struct of_device_id omap8250_dt_ids[] = {
> +	{ .compatible = "ti,omap2-uart" },
> +	{ .compatible = "ti,omap3-uart" },
> +	{ .compatible = "ti,omap4-uart" },
> +	{ .compatible = "ti,am3352-uart", .data = &am3352_habit, },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, omap8250_dt_ids);
> +
>  static int omap8250_probe(struct platform_device *pdev)
>  {
>  	struct resource *regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> @@ -1125,11 +1137,17 @@ static int omap8250_probe(struct platform_device *pdev)
>  	up.port.unthrottle = omap_8250_unthrottle;
>  
>  	if (pdev->dev.of_node) {
> +		const struct of_device_id *id;
> +
>  		ret = of_alias_get_id(pdev->dev.of_node, "serial");
>  
>  		of_property_read_u32(pdev->dev.of_node, "clock-frequency",
>  				     &up.port.uartclk);
>  		priv->wakeirq = irq_of_parse_and_map(pdev->dev.of_node, 1);
> +
> +		id = of_match_device(of_match_ptr(omap8250_dt_ids), &pdev->dev);
> +		if (id && id->data)
> +			priv->habit |= *(u8 *)id->data;
>  	} else {
>  		ret = pdev->id;
>  	}
> @@ -1184,9 +1202,6 @@ static int omap8250_probe(struct platform_device *pdev)
>  			priv->omap8250_dma.rx_size = RX_TRIGGER;
>  			priv->omap8250_dma.rxconf.src_maxburst = RX_TRIGGER;
>  			priv->omap8250_dma.txconf.dst_maxburst = TX_TRIGGER;
> -
> -			if (of_machine_is_compatible("ti,am33xx"))
> -				priv->habit |= OMAP_DMA_TX_KICK;
>  		}
>  	}
>  #endif
> @@ -1374,14 +1389,6 @@ static const struct dev_pm_ops omap8250_dev_pm_ops = {
>  	.complete       = omap8250_complete,
>  };
>  
> -static const struct of_device_id omap8250_dt_ids[] = {
> -	{ .compatible = "ti,omap2-uart" },
> -	{ .compatible = "ti,omap3-uart" },
> -	{ .compatible = "ti,omap4-uart" },
> -	{},
> -};
> -MODULE_DEVICE_TABLE(of, omap8250_dt_ids);
> -
>  static struct platform_driver omap8250_platform_driver = {
>  	.driver = {
>  		.name		= "omap8250",
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] tty: 8250: omap eliminate use of of_machine_is_compatible()

[Sekhar Nori]

Hi Peter,

On Thursday 09 July 2015 05:30 AM, Peter Hurley wrote:
> Hi Sekhar,
> 
> On 07/06/2015 05:47 AM, Sekhar Nori wrote:
>> Use of of_machine_is_compatible() for AM335x specific DMA
>> quirk in 8250_omap driver makes it ugly to extend the
>> quirk for other platforms. Instead use a new compatible.
>>
>> The new compatible will also make it easier to care of other
>> quirks specific to AM335x and like SoCs.
>>
>> This patch does break backward DTB compatibility for users of
>> 8250_omap driver on AM335x boards.
> 
> Not ok.
> 
> 8250_omap was released with 3.19 and has been the default driver for
> BeagleBone since 4.0.

Alright, I guess I was really stretching my luck here. Will keep the
of_machine_is_compatible() around so there should be no backward
compatibility issue.

Thanks,
Sekhar

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/7] tty: 8250: workaround errata on disabling UART after using DMA

[Sekhar Nori]

AM335x, AM437x and DRA7x SoCs have an errata due to which UART
cannot be disabled after it has been used with DMA.

OMAP3 has a similar sounding errata which has been worked around
in a2fc36613ac1af2e9 ("ARM: OMAP3: Use manual idle for UARTs
because of DMA errata"). But the workaround used there does not
apply to AM335x, AM437x SoCs. These SoCs need a softreset of UART
before disabling them.

This patch implements that errata workaround. It is expected that
UART will be used with DMA so no explicit check for DMA usage
has been added for errata applicability.

Signed-off-by: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
---
 drivers/tty/serial/8250/8250_omap.c | 55 +++++++++++++++++++++++++++++++++----
 1 file changed, 49 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 52566387ec37..af25869d145f 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -33,6 +33,11 @@
 #define UART_ERRATA_i202_MDR1_ACCESS	(1 << 0)
 #define OMAP_UART_WER_HAS_TX_WAKEUP	(1 << 1)
 #define OMAP_DMA_TX_KICK		(1 << 2)
+/*
+ * See Advisory 21 in AM437x errata SPRZ408B, updated April 2015.
+ * The same errata is applicable to AM335x and DRA7x processors too.
+ */
+#define UART_ERRATA_CLOCK_DISABLE	(1 << 3)
 
 #define OMAP_UART_FCR_RX_TRIG		6
 #define OMAP_UART_FCR_TX_TRIG		4
@@ -54,6 +59,12 @@
 #define OMAP_UART_MVR_MAJ_SHIFT		8
 #define OMAP_UART_MVR_MIN_MASK		0x3f
 
+/* SYSC register bitmasks */
+#define OMAP_UART_SYSC_SOFTRESET	(1 << 1)
+
+/* SYSS register bitmasks */
+#define OMAP_UART_SYSS_RESETDONE	(1 << 0)
+
 #define UART_TI752_TLR_TX	0
 #define UART_TI752_TLR_RX	4
 
@@ -1062,13 +1073,15 @@ static int omap8250_no_handle_irq(struct uart_port *port)
 	return 0;
 }
 
-static const u8 am3352_habit = OMAP_DMA_TX_KICK;
+static const u8 am3352_habit = OMAP_DMA_TX_KICK | UART_ERRATA_CLOCK_DISABLE;
+static const u8 am4372_habit = UART_ERRATA_CLOCK_DISABLE;
 
 static const struct of_device_id omap8250_dt_ids[] = {
 	{ .compatible = "ti,omap2-uart" },
 	{ .compatible = "ti,omap3-uart" },
 	{ .compatible = "ti,omap4-uart" },
 	{ .compatible = "ti,am3352-uart", .data = &am3352_habit, },
+	{ .compatible = "ti,am4372-uart", .data = &am4372_habit, },
 	{},
 };
 MODULE_DEVICE_TABLE(of, omap8250_dt_ids);
@@ -1279,13 +1292,13 @@ static int omap8250_lost_context(struct uart_8250_port *up)
 {
 	u32 val;
 
-	val = serial_in(up, UART_OMAP_MDR1);
+	val = serial_in(up, UART_OMAP_SCR);
 	/*
-	 * If we lose context, then MDR1 is set to its reset value which is
-	 * UART_OMAP_MDR1_DISABLE. After set_termios() we set it either to 13x
-	 * or 16x but never to disable again.
+	 * If we lose context, then SCR is set to its reset value of zero.
+	 * After set_termios() we set bit 3 of SCR (TX_EMPTY_CTL_IT) to 1,
+	 * among other bits, to never set the register back to zero again.
 	 */
-	if (val == UART_OMAP_MDR1_DISABLE)
+	if (!val)
 		return 1;
 	return 0;
 }
@@ -1307,6 +1320,36 @@ static int omap8250_runtime_suspend(struct device *dev)
 			return -EBUSY;
 	}
 
+	if (priv->habit & UART_ERRATA_CLOCK_DISABLE) {
+		int sysc;
+		int syss;
+		int timeout = 100;
+
+		sysc = serial_in(up, UART_OMAP_SYSC);
+
+		/* softreset the UART */
+		sysc |= OMAP_UART_SYSC_SOFTRESET;
+		serial_out(up, UART_OMAP_SYSC, sysc);
+
+		/* By experiments, 1us enough for reset complete on AM335x */
+		do {
+			udelay(1);
+			syss = serial_in(up, UART_OMAP_SYSS);
+		} while (--timeout && !(syss & OMAP_UART_SYSS_RESETDONE));
+
+		if (!timeout) {
+			dev_err(dev, "timed out waiting for reset done\n");
+			return -ETIMEDOUT;
+		}
+
+		/*
+		 * For UART module wake-up to work, MDR1.MODESELECT should
+		 * not be set to "Disable", so update it.
+		 */
+		if (device_may_wakeup(dev))
+			omap8250_update_mdr1(up, priv);
+	}
+
 	if (up->dma && up->dma->rxchan)
 		omap_8250_rx_dma(up, UART_IIR_RX_TIMEOUT);
 
-- 
2.4.4.408.g16da57c

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/7] tty: 8250: workaround errata on disabling UART after using DMA

[Peter Hurley]

Hi Sekhar,

Patch title should start "serial: 8250_omap: .." since the patch
is specific to the 8250_omap serial driver.

On 07/06/2015 05:47 AM, Sekhar Nori wrote:
> AM335x, AM437x and DRA7x SoCs have an errata due to which UART
> cannot be disabled after it has been used with DMA.
             ^^^^^^
             idled

> OMAP3 has a similar sounding errata which has been worked around
> in a2fc36613ac1af2e9 ("ARM: OMAP3: Use manual idle for UARTs
> because of DMA errata"). But the workaround used there does not
> apply to AM335x, AM437x SoCs.

> These SoCs need a softreset of UART before disabling them.

"The UART module on these SoCs must be soft reset to go idle."
 
> This patch implements that errata workaround. It is expected that
> UART will be used with DMA so no explicit check for DMA usage
> has been added for errata applicability.

This changelog should also:

1. Reference the errata document.
2. Explain why SCR register has to be the context loss canary
   rather than MDR1.

> Signed-off-by: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
> ---
>  drivers/tty/serial/8250/8250_omap.c | 55 +++++++++++++++++++++++++++++++++----
>  1 file changed, 49 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> index 52566387ec37..af25869d145f 100644
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -33,6 +33,11 @@
>  #define UART_ERRATA_i202_MDR1_ACCESS	(1 << 0)
>  #define OMAP_UART_WER_HAS_TX_WAKEUP	(1 << 1)
>  #define OMAP_DMA_TX_KICK		(1 << 2)
> +/*
> + * See Advisory 21 in AM437x errata SPRZ408B, updated April 2015.
> + * The same errata is applicable to AM335x and DRA7x processors too.
> + */
> +#define UART_ERRATA_CLOCK_DISABLE	(1 << 3)
>  
>  #define OMAP_UART_FCR_RX_TRIG		6
>  #define OMAP_UART_FCR_TX_TRIG		4
> @@ -54,6 +59,12 @@
>  #define OMAP_UART_MVR_MAJ_SHIFT		8
>  #define OMAP_UART_MVR_MIN_MASK		0x3f
>  
> +/* SYSC register bitmasks */
> +#define OMAP_UART_SYSC_SOFTRESET	(1 << 1)
> +
> +/* SYSS register bitmasks */
> +#define OMAP_UART_SYSS_RESETDONE	(1 << 0)
> +
>  #define UART_TI752_TLR_TX	0
>  #define UART_TI752_TLR_RX	4
>  
> @@ -1062,13 +1073,15 @@ static int omap8250_no_handle_irq(struct uart_port *port)
>  	return 0;
>  }
>  
> -static const u8 am3352_habit = OMAP_DMA_TX_KICK;
> +static const u8 am3352_habit = OMAP_DMA_TX_KICK | UART_ERRATA_CLOCK_DISABLE;
> +static const u8 am4372_habit = UART_ERRATA_CLOCK_DISABLE;
>  
>  static const struct of_device_id omap8250_dt_ids[] = {
>  	{ .compatible = "ti,omap2-uart" },
>  	{ .compatible = "ti,omap3-uart" },
>  	{ .compatible = "ti,omap4-uart" },
>  	{ .compatible = "ti,am3352-uart", .data = &am3352_habit, },
> +	{ .compatible = "ti,am4372-uart", .data = &am4372_habit, },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, omap8250_dt_ids);
> @@ -1279,13 +1292,13 @@ static int omap8250_lost_context(struct uart_8250_port *up)
>  {
>  	u32 val;
>  
> -	val = serial_in(up, UART_OMAP_MDR1);
> +	val = serial_in(up, UART_OMAP_SCR);
>  	/*
> -	 * If we lose context, then MDR1 is set to its reset value which is
> -	 * UART_OMAP_MDR1_DISABLE. After set_termios() we set it either to 13x
> -	 * or 16x but never to disable again.
> +	 * If we lose context, then SCR is set to its reset value of zero.
> +	 * After set_termios() we set bit 3 of SCR (TX_EMPTY_CTL_IT) to 1,
> +	 * among other bits, to never set the register back to zero again.
>  	 */
> -	if (val == UART_OMAP_MDR1_DISABLE)
> +	if (!val)
>  		return 1;
>  	return 0;
>  }
> @@ -1307,6 +1320,36 @@ static int omap8250_runtime_suspend(struct device *dev)
>  			return -EBUSY;
>  	}
>  
> +	if (priv->habit & UART_ERRATA_CLOCK_DISABLE) {
> +		int sysc;
> +		int syss;
> +		int timeout = 100;
> +
> +		sysc = serial_in(up, UART_OMAP_SYSC);
> +
> +		/* softreset the UART */
> +		sysc |= OMAP_UART_SYSC_SOFTRESET;
> +		serial_out(up, UART_OMAP_SYSC, sysc);
> +
> +		/* By experiments, 1us enough for reset complete on AM335x */
> +		do {
> +			udelay(1);
> +			syss = serial_in(up, UART_OMAP_SYSS);
> +		} while (--timeout && !(syss & OMAP_UART_SYSS_RESETDONE));


If there is not a omap helper function to reset modules, there should be.
Open-coding the module reset is not appropriate.

> +		if (!timeout) {
> +			dev_err(dev, "timed out waiting for reset done\n");
> +			return -ETIMEDOUT;
> +		}
> +
> +		/*
> +		 * For UART module wake-up to work, MDR1.MODESELECT should
> +		 * not be set to "Disable", so update it.
> +		 */
> +		if (device_may_wakeup(dev))
> +			omap8250_update_mdr1(up, priv);

Should this be unconditional?

Regards,
Peter Hurley

> +	}
> +
>  	if (up->dma && up->dma->rxchan)
>  		omap_8250_rx_dma(up, UART_IIR_RX_TIMEOUT);
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/7] tty: 8250: workaround errata on disabling UART after using DMA

[Sekhar Nori]

Hi Peter,

On Thursday 09 July 2015 07:03 AM, Peter Hurley wrote:
> Hi Sekhar,
> 
> Patch title should start "serial: 8250_omap: .." since the patch
> is specific to the 8250_omap serial driver.

Yes, missed that.

> 
> On 07/06/2015 05:47 AM, Sekhar Nori wrote:
>> AM335x, AM437x and DRA7x SoCs have an errata due to which UART
>> cannot be disabled after it has been used with DMA.
>              ^^^^^^
>              idled

alright.

> 
>> OMAP3 has a similar sounding errata which has been worked around
>> in a2fc36613ac1af2e9 ("ARM: OMAP3: Use manual idle for UARTs
>> because of DMA errata"). But the workaround used there does not
>> apply to AM335x, AM437x SoCs.
> 
>> These SoCs need a softreset of UART before disabling them.
> 
> "The UART module on these SoCs must be soft reset to go idle."
>  
>> This patch implements that errata workaround. It is expected that
>> UART will be used with DMA so no explicit check for DMA usage
>> has been added for errata applicability.
> 
> This changelog should also:
> 
> 1. Reference the errata document.

Sure. I added that as comments in code. I can do that in the patch
description too.

> 2. Explain why SCR register has to be the context loss canary
>    rather than MDR1.

Okay, will explain that too.

>> @@ -1307,6 +1320,36 @@ static int omap8250_runtime_suspend(struct device *dev)
>>  			return -EBUSY;
>>  	}
>>  
>> +	if (priv->habit & UART_ERRATA_CLOCK_DISABLE) {
>> +		int sysc;
>> +		int syss;
>> +		int timeout = 100;
>> +
>> +		sysc = serial_in(up, UART_OMAP_SYSC);
>> +
>> +		/* softreset the UART */
>> +		sysc |= OMAP_UART_SYSC_SOFTRESET;
>> +		serial_out(up, UART_OMAP_SYSC, sysc);
>> +
>> +		/* By experiments, 1us enough for reset complete on AM335x */
>> +		do {
>> +			udelay(1);
>> +			syss = serial_in(up, UART_OMAP_SYSS);
>> +		} while (--timeout && !(syss & OMAP_UART_SYSS_RESETDONE));
> 
> 
> If there is not a omap helper function to reset modules, there should be.
> Open-coding the module reset is not appropriate.

There is work planned to have something implemented for OMAP as part of
drivers/reset/ API. AFAIK, this depends on some PRCM code clean-up
affecting multiple OMAP platforms and is couple of merge windows away
from taking shape.

Meanwhile, I think this is the best we can do. Other drivers like
i2c-omap have done something similar (see omap_i2c_reset()).

> 
>> +		if (!timeout) {
>> +			dev_err(dev, "timed out waiting for reset done\n");
>> +			return -ETIMEDOUT;
>> +		}
>> +
>> +		/*
>> +		 * For UART module wake-up to work, MDR1.MODESELECT should
>> +		 * not be set to "Disable", so update it.
>> +		 */
>> +		if (device_may_wakeup(dev))
>> +			omap8250_update_mdr1(up, priv);
> 
> Should this be unconditional?

I do not see it doing any harm if done unconditionally. But it will be
unnecessary. Unless the UART is being used for wakeup, we don't need
MDR1 restored at this stage. And omap8250_restore_regs() will eventually
restore the register anyway.

Thanks,
Sekhar

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/7] tty: 8250: workaround errata on disabling UART after using DMA

[Peter Hurley]

On 07/09/2015 10:15 AM, Sekhar Nori wrote:
> On Thursday 09 July 2015 07:03 AM, Peter Hurley wrote:

[...]

>>> @@ -1307,6 +1320,36 @@ static int omap8250_runtime_suspend(struct device *dev)
>>>  			return -EBUSY;
>>>  	}
>>>  
>>> +	if (priv->habit & UART_ERRATA_CLOCK_DISABLE) {
>>> +		int sysc;
>>> +		int syss;
>>> +		int timeout = 100;
>>> +
>>> +		sysc = serial_in(up, UART_OMAP_SYSC);
>>> +
>>> +		/* softreset the UART */
>>> +		sysc |= OMAP_UART_SYSC_SOFTRESET;
>>> +		serial_out(up, UART_OMAP_SYSC, sysc);
>>> +
>>> +		/* By experiments, 1us enough for reset complete on AM335x */
>>> +		do {
>>> +			udelay(1);
>>> +			syss = serial_in(up, UART_OMAP_SYSS);
>>> +		} while (--timeout && !(syss & OMAP_UART_SYSS_RESETDONE));
>>
>>
>> If there is not a omap helper function to reset modules, there should be.
>> Open-coding the module reset is not appropriate.
> 
> There is work planned to have something implemented for OMAP as part of
> drivers/reset/ API. AFAIK, this depends on some PRCM code clean-up
> affecting multiple OMAP platforms and is couple of merge windows away
> from taking shape.
> 
> Meanwhile, I think this is the best we can do. Other drivers like
> i2c-omap have done something similar (see omap_i2c_reset()).

Ok, then please make the reset a separate local function
(returning success/failure?). omap_8250_reset()?

A TODO or FIXME above it explaining
the temporary nature of the function would be appreciated :)

>>
>>> +		if (!timeout) {
>>> +			dev_err(dev, "timed out waiting for reset done\n");
>>> +			return -ETIMEDOUT;
>>> +		}
>>> +
>>> +		/*
>>> +		 * For UART module wake-up to work, MDR1.MODESELECT should
>>> +		 * not be set to "Disable", so update it.
>>> +		 */
>>> +		if (device_may_wakeup(dev))
>>> +			omap8250_update_mdr1(up, priv);
>>
>> Should this be unconditional?
> 
> I do not see it doing any harm if done unconditionally. But it will be
> unnecessary. Unless the UART is being used for wakeup, we don't need
> MDR1 restored at this stage. And omap8250_restore_regs() will eventually
> restore the register anyway.

Yeah, I understand that, but special-casing it isn't adding any value;
it's not as if you actually want the module to not be in UART mode.

And the comment could be single-line:

		/* Restore to UART mode after reset (for wakeup) */

Regards,
Peter Hurley

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/7] tty: 8250: workaround errata on disabling UART after using DMA

[Sekhar Nori]

Hi Peter,

On Saturday 11 July 2015 03:31 AM, Peter Hurley wrote:
> On 07/09/2015 10:15 AM, Sekhar Nori wrote:
>> On Thursday 09 July 2015 07:03 AM, Peter Hurley wrote:
> 
> [...]
> 
>>>> @@ -1307,6 +1320,36 @@ static int omap8250_runtime_suspend(struct device *dev)
>>>>  			return -EBUSY;
>>>>  	}
>>>>  
>>>> +	if (priv->habit & UART_ERRATA_CLOCK_DISABLE) {
>>>> +		int sysc;
>>>> +		int syss;
>>>> +		int timeout = 100;
>>>> +
>>>> +		sysc = serial_in(up, UART_OMAP_SYSC);
>>>> +
>>>> +		/* softreset the UART */
>>>> +		sysc |= OMAP_UART_SYSC_SOFTRESET;
>>>> +		serial_out(up, UART_OMAP_SYSC, sysc);
>>>> +
>>>> +		/* By experiments, 1us enough for reset complete on AM335x */
>>>> +		do {
>>>> +			udelay(1);
>>>> +			syss = serial_in(up, UART_OMAP_SYSS);
>>>> +		} while (--timeout && !(syss & OMAP_UART_SYSS_RESETDONE));
>>>
>>>
>>> If there is not a omap helper function to reset modules, there should be.
>>> Open-coding the module reset is not appropriate.
>>
>> There is work planned to have something implemented for OMAP as part of
>> drivers/reset/ API. AFAIK, this depends on some PRCM code clean-up
>> affecting multiple OMAP platforms and is couple of merge windows away
>> from taking shape.
>>
>> Meanwhile, I think this is the best we can do. Other drivers like
>> i2c-omap have done something similar (see omap_i2c_reset()).
> 
> Ok, then please make the reset a separate local function
> (returning success/failure?). omap_8250_reset()?
> 
> A TODO or FIXME above it explaining
> the temporary nature of the function would be appreciated :)

Okay, will do.

> 
>>>
>>>> +		if (!timeout) {
>>>> +			dev_err(dev, "timed out waiting for reset done\n");
>>>> +			return -ETIMEDOUT;
>>>> +		}
>>>> +
>>>> +		/*
>>>> +		 * For UART module wake-up to work, MDR1.MODESELECT should
>>>> +		 * not be set to "Disable", so update it.
>>>> +		 */
>>>> +		if (device_may_wakeup(dev))
>>>> +			omap8250_update_mdr1(up, priv);
>>>
>>> Should this be unconditional?
>>
>> I do not see it doing any harm if done unconditionally. But it will be
>> unnecessary. Unless the UART is being used for wakeup, we don't need
>> MDR1 restored at this stage. And omap8250_restore_regs() will eventually
>> restore the register anyway.
> 
> Yeah, I understand that, but special-casing it isn't adding any value;
> it's not as if you actually want the module to not be in UART mode.
> 
> And the comment could be single-line:
> 
> 		/* Restore to UART mode after reset (for wakeup) */

Alright, will change this as well.

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 6/7] tty: 8250: omap: workaround module disable errata on dra7x SoCs

[Sekhar Nori]

Due to Advisory 21 as documented in AM437x errata document,
UART module cannot be disabled once DMA is used. The only
workaround is to softreset the module before disabling it.

DRA7x UARTs are compatible to AM437x UARTs in terms of
this errata and prescribed workaround.

Enable usage of workaround for this errata on DRA7x SoCs.

Signed-off-by: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
---
 Documentation/devicetree/bindings/serial/omap_serial.txt | 1 +
 drivers/tty/serial/8250/8250_omap.c                      | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/omap_serial.txt b/Documentation/devicetree/bindings/serial/omap_serial.txt
index 0ee88209b341..7a71b5de77d6 100644
--- a/Documentation/devicetree/bindings/serial/omap_serial.txt
+++ b/Documentation/devicetree/bindings/serial/omap_serial.txt
@@ -6,6 +6,7 @@ Required properties:
 - compatible : should be "ti,omap4-uart" for OMAP4 controllers
 - compatible : should be "ti,am4372-uart" for AM437x controllers
 - compatible : should be "ti,am3352-uart" for AM335x controllers
+- compatible : should be "ti,dra742-uart" for DRA7x controllers
 - reg : address and length of the register space
 - interrupts or interrupts-extended : Should contain the uart interrupt
                                       specifier or both the interrupt
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index af25869d145f..9811f0bcba25 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -1082,6 +1082,7 @@ static const struct of_device_id omap8250_dt_ids[] = {
 	{ .compatible = "ti,omap4-uart" },
 	{ .compatible = "ti,am3352-uart", .data = &am3352_habit, },
 	{ .compatible = "ti,am4372-uart", .data = &am4372_habit, },
+	{ .compatible = "ti,dra742-uart", .data = &am4372_habit, },
 	{},
 };
 MODULE_DEVICE_TABLE(of, omap8250_dt_ids);
-- 
2.4.4.408.g16da57c

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 7/7] ARM: dts: dra7: workaround UART module disable errata

[Sekhar Nori]

Add "ti,dra742-uart" to the compatible list so the driver
workaround for UART module disable errata is enabled.

This does not break backward compatibility as existing DTBs
should continue to work with newer kernels albeit without the
capability of disabling the module when DMA is used.

Signed-off-by: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
---
 arch/arm/boot/dts/dra7.dtsi | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index 8f1e25bcecbd..49940e516f0f 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -397,7 +397,7 @@
 		};
 
 		uart1: serial@4806a000 {
-			compatible = "ti,omap4-uart";
+			compatible = "ti,dra742-uart", "ti,omap4-uart";
 			reg = <0x4806a000 0x100>;
 			interrupts-extended = <&crossbar_mpu GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "uart1";
@@ -408,7 +408,7 @@
 		};
 
 		uart2: serial@4806c000 {
-			compatible = "ti,omap4-uart";
+			compatible = "ti,dra742-uart", "ti,omap4-uart";
 			reg = <0x4806c000 0x100>;
 			interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "uart2";
@@ -419,7 +419,7 @@
 		};
 
 		uart3: serial@48020000 {
-			compatible = "ti,omap4-uart";
+			compatible = "ti,dra742-uart", "ti,omap4-uart";
 			reg = <0x48020000 0x100>;
 			interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "uart3";
@@ -430,7 +430,7 @@
 		};
 
 		uart4: serial@4806e000 {
-			compatible = "ti,omap4-uart";
+			compatible = "ti,dra742-uart", "ti,omap4-uart";
 			reg = <0x4806e000 0x100>;
 			interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "uart4";
@@ -441,7 +441,7 @@
 		};
 
 		uart5: serial@48066000 {
-			compatible = "ti,omap4-uart";
+			compatible = "ti,dra742-uart", "ti,omap4-uart";
 			reg = <0x48066000 0x100>;
 			interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "uart5";
@@ -452,7 +452,7 @@
 		};
 
 		uart6: serial@48068000 {
-			compatible = "ti,omap4-uart";
+			compatible = "ti,dra742-uart", "ti,omap4-uart";
 			reg = <0x48068000 0x100>;
 			interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "uart6";
@@ -463,7 +463,7 @@
 		};
 
 		uart7: serial@48420000 {
-			compatible = "ti,omap4-uart";
+			compatible = "ti,dra742-uart", "ti,omap4-uart";
 			reg = <0x48420000 0x100>;
 			interrupts = <GIC_SPI 218 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "uart7";
@@ -472,7 +472,7 @@
 		};
 
 		uart8: serial@48422000 {
-			compatible = "ti,omap4-uart";
+			compatible = "ti,dra742-uart", "ti,omap4-uart";
 			reg = <0x48422000 0x100>;
 			interrupts = <GIC_SPI 219 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "uart8";
@@ -481,7 +481,7 @@
 		};
 
 		uart9: serial@48424000 {
-			compatible = "ti,omap4-uart";
+			compatible = "ti,dra742-uart", "ti,omap4-uart";
 			reg = <0x48424000 0x100>;
 			interrupts = <GIC_SPI 220 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "uart9";
@@ -490,7 +490,7 @@
 		};
 
 		uart10: serial@4ae2b000 {
-			compatible = "ti,omap4-uart";
+			compatible = "ti,dra742-uart", "ti,omap4-uart";
 			reg = <0x4ae2b000 0x100>;
 			interrupts = <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "uart10";
-- 
2.4.4.408.g16da57c

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html