Subject: [PATCH 1/3] OMAP2+: UART: Remove cpu checks for populating errata flags

From: [email-masked] (Jon Hunter)
Date: Tue, 27 Mar 2012 16:03:28 -0500

Hi Govindraj,

On 3/21/2012 5:24, Govindraj.R wrote:
> From: "Govindraj.R"<[email-masked]>
>
> Currently the errata is populated based on cpu checks this can
> be removed and replaced with module version check of uart ip block.
> MVR reg is provided within the uart reg map use the same
> to populate the errata and thus now errata population and handling
> can be managed within the driver itself.
>
> Cc: Paul Walmsley<[email-masked]>
> Cc: Kevin Hilman<[email-masked]>
> Signed-off-by: Felipe Balbi<[email-masked]>
> Signed-off-by: Govindraj.R<[email-masked]>
> ---
>   arch/arm/mach-omap2/serial.c                  |    8 ---
>   arch/arm/plat-omap/include/plat/omap-serial.h |    1 -
>   drivers/tty/serial/omap-serial.c              |   62 ++++++++++++++++++++++++-
>   3 files changed, 61 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
> index f590afc..330ee04 100644
> --- a/arch/arm/mach-omap2/serial.c
> +++ b/arch/arm/mach-omap2/serial.c
> @@ -357,14 +357,6 @@ void __init omap_serial_init_port(struct omap_board_data *bdata,
>   	omap_up.dma_rx_poll_rate = info->dma_rx_poll_rate;
>   	omap_up.autosuspend_timeout = info->autosuspend_timeout;
>
> -	/* Enable the MDR1 Errata i202 for OMAP2430/3xxx/44xx */
> -	if (!cpu_is_omap2420()&&  !cpu_is_ti816x())
> -		omap_up.errata |= UART_ERRATA_i202_MDR1_ACCESS;
> -
> -	/* Enable DMA Mode Force Idle Errata i291 for omap34xx/3630 */
> -	if (cpu_is_omap34xx() || cpu_is_omap3630())
> -		omap_up.errata |= UART_ERRATA_i291_DMA_FORCEIDLE;
> -
>   	pdata =&omap_up;
>   	pdata_size = sizeof(struct omap_uart_port_info);
>
> diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
> index 9ff4444..1a52725 100644
> --- a/arch/arm/plat-omap/include/plat/omap-serial.h
> +++ b/arch/arm/plat-omap/include/plat/omap-serial.h
> @@ -65,7 +65,6 @@ struct omap_uart_port_info {
>   	bool			dma_enabled;	/* To specify DMA Mode */
>   	unsigned int		uartclk;	/* UART clock rate */
>   	upf_t			flags;		/* UPF_* flags */
> -	u32			errata;
>   	unsigned int		dma_rx_buf_size;
>   	unsigned int		dma_rx_timeout;
>   	unsigned int		autosuspend_timeout;
> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> index f809041..c7666d6 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -44,6 +44,13 @@
>   #include<plat/dmtimer.h>
>   #include<plat/omap-serial.h>
>
> +#define UART_BUILD_REVISION(x, y)	(((x)<<  8) | (y))
> +
> +#define OMAP_UART_REV_42 0x0402
> +#define OMAP_UART_REV_46 0x0406
> +#define OMAP_UART_REV_52 0x0502
> +#define OMAP_UART_REV_63 0x0603
> +
>   #define DEFAULT_CLK_SPEED 48000000 /* 48Mhz*/
>
>   /* SCR register bitmasks */
> @@ -1346,6 +1353,58 @@ static void uart_tx_dma_callback(int lch, u16 ch_status, void *data)
>   	return;
>   }
>
> +static void omap_serial_fill_features_erratas(struct uart_omap_port *up)
> +{
> +	u32 mvr, scheme;
> +	u16 revision, major, minor;
> +
> +	mvr = serial_in(up, UART_OMAP_MVER);
> +
> +	/* Check revision register scheme */
> +	scheme = mvr&  (0x03<<  30);
> +	scheme>>= 30;

Minor nit-pick, why not just ...

	scheme = mvr >> 30;

> +	switch (scheme) {
> +	case 0: /* Legacy Scheme: OMAP2/3 */
> +		/* MINOR_REV[0:4], MAJOR_REV[4:7] */

Same scheme applies to OMAP1 devices too. So maybe we should include in 
the comment.

> +		major = (mvr&  0xf0)>>  4;
> +		minor = (mvr&  0x0f);
> +		break;
> +	case 1:
> +		/* New Scheme: OMAP4+ */
> +		/* MINOR_REV[0:5], MAJOR_REV[8:10] */
> +		major = (mvr&  0x7<<  8)>>  8;

Nit-pick ...

	major = (mvr >> 8) & 0x7;

> +		minor = (mvr&  0x3f);
> +		break;
> +	default:
> +		dev_warn(&up->pdev->dev,
> +			"Unknown %s revision, defaulting to highest\n",
> +			up->name);
> +		/* highest possible revision */
> +		major = 0xff;
> +		minor = 0xff;
> +	}
> +
> +	/* normalize revision for the driver */
> +	revision = UART_BUILD_REVISION(major, minor);
> +
> +	switch (revision) {
> +	case OMAP_UART_REV_46:
> +		up->errata |= (UART_ERRATA_i202_MDR1_ACCESS |
> +				UART_ERRATA_i291_DMA_FORCEIDLE);
> +		break;
> +	case OMAP_UART_REV_52:
> +		up->errata |= (UART_ERRATA_i202_MDR1_ACCESS |
> +				UART_ERRATA_i291_DMA_FORCEIDLE);
> +		break;
> +	case OMAP_UART_REV_63:
> +		up->errata |= UART_ERRATA_i202_MDR1_ACCESS;
> +		break;
> +	default:
> +		break;
> +	}

Should we also warn on an unknown revision?

> +}
> +
>   static struct omap_uart_port_info *of_get_uart_port_info(struct device *dev)
>   {
>   	struct omap_uart_port_info *omap_up_info;
> @@ -1443,7 +1502,6 @@ static int serial_omap_probe(struct platform_device *pdev)
>   						"%d\n", DEFAULT_CLK_SPEED);
>   	}
>   	up->uart_dma.uart_base = mem->start;
> -	up->errata = omap_up_info->errata;
>
>   	if (omap_up_info->dma_enabled) {
>   		up->uart_dma.uart_dma_tx = dma_tx->start;
> @@ -1473,6 +1531,8 @@ static int serial_omap_probe(struct platform_device *pdev)
>   	pm_runtime_enable(&pdev->dev);
>   	pm_runtime_get_sync(&pdev->dev);
>
> +	omap_serial_fill_features_erratas(up);
> +
>   	ui[up->port.line] = up;
>   	serial_omap_add_console_port(up);
>