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@xxxxxx>
> ---
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
