On 19/06/14 12:29, Srinivas Kandagatla wrote:

Hi Dan,

On 19/06/14 11:38, Daniel Thompson wrote:

quoted

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.

You mean just drop the COMPILE_TEST?

In generally I like as much code as possible to compile on x86. Its
worthwhile protection against the excessive/accidental ARMisms which
could easily impact less common architectures (such as SH).

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.

I'm particularly keen on doing the right thing where readl_relaxed() is
concerned because this function has a compiler barrier on ARM but not on
x86.

Since having asc_in/asc_out made it easy to portably make these changes
I decided is was better to be redundantly exemplary than conceal secret
portability issues.

Don't feel that strongly though. Can easily change it if you're unconvinced.

--srini

quoted

Signed-off-by: Daniel Thompson <redacted>
Cc: Srinivas Kandagatla <redacted>
Cc: Maxime Coquelin <redacted>
Cc: Patrice Chotard <redacted>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <redacted>
Cc: kernel@stlinux.com
Cc: linux-serial@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
  }

  /*