Subject: [PATCH v4 11/13] serial: asc: Adopt readl_/writel_relaxed()

From: [email-masked] (Srinivas Kandagatla)
Date: Thu, 19 Jun 2014 12:29:39 +0100

Hi Dan,

On 19/06/14 11:38, Daniel Thompson wrote:
> The architectures supported by this driver have expensive
> implementations of writel(), reliant on spin locks and explicit L2 cache
> management. These architectures provide a cheaper writel_relaxed() which
> is much better suited to peripherals that do not perform DMA. The
> situation with readl()/readl_relaxed()is similar although less acute.
>
> This driver does not use DMA and will be more power efficient and more
> robust (due to absense of spin locks during console I/O) if it uses the
> relaxed variants.
>
> This driver is cross compilable for testing purposes and remains
> compilable on all architectures by falling back to writel() when
> writel_relaxed() does not exist. We also include explicit compiler
> barriers. There are redundant on ARM and SH but important on
> x86 because it defines "relaxed" differently.
>
Why are we concern about x86 for this driver?
As per my understanding this IP is only seen on ARM and SH based CPUs so 
why cant we just use relaxed versions, why ifdefs?
I think, this would involve fixing the kconfig and make it depend on SH 
and ARM based platforms only.

On the other hand, This patch looks more generic and applicable to most 
of the drivers. Am not sure which way is the right one.

--srini

> Signed-off-by: Daniel Thompson <[email-masked]>
> Cc: Srinivas Kandagatla <[email-masked]>
> Cc: Maxime Coquelin <[email-masked]>
> Cc: Patrice Chotard <[email-masked]>
> Cc: Greg Kroah-Hartman <[email-masked]>
> Cc: Jiri Slaby <[email-masked]>
> Cc: kernel at stlinux.com
> Cc: linux-serial at vger.kernel.org
> ---
>   drivers/tty/serial/st-asc.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c
> index 4f376d8..58aa1c6 100644
> --- a/drivers/tty/serial/st-asc.c
> +++ b/drivers/tty/serial/st-asc.c
> @@ -152,12 +152,21 @@ static inline struct asc_port *to_asc_port(struct uart_port *port)
>
>   static inline u32 asc_in(struct uart_port *port, u32 offset)
>   {
> -	return readl(port->membase + offset);
> +	u32 r;
> +
> +	r = readl_relaxed(port->membase + offset);
> +	barrier();
> +	return r;
>   }
>
>   static inline void asc_out(struct uart_port *port, u32 offset, u32 value)
>   {
> +#ifdef writel_relaxed
> +	writel_relaxed(value, port->membase + offset);
> +	barrier();
> +#else
>   	writel(value, port->membase + offset);
> +#endif
>   }
>
>   /*
>