Arnd Bergmann a ?crit :

On Friday 31 August 2012, Stephen Boyd wrote:

quoted

quoted

+static int hvc_dcc_put_chars_v6(uint32_t vt, const char *buf, int count)
+{
+     int i;
+
+     for (i = 0; i < count; i++) {
+             while (__dcc_getstatus_v6() & DCC_STATUS_TX_V6)
+                     cpu_relax();
+
+             __dcc_putchar_v6(buf[i]);
+     }
+
+     return count;
+}

It's unfortunate that the main logic is duplicated. I wonder if we could
push the runtime decision slightly lower into the accessor functions
instead and make some new functions dcc_tx_busy() and dcc_rx_busy() or
something. Then these loops stay the same.

The code is so small (30 asm + 30 C code) that I wonder if worth adding
complexity in the code.
Also calling cpu_architecture isn't free and if the want to put the runtime
decision into the hot path, this means we need to cache the result.

Agreed. Ideally, you should be able to get the code to be compiled into
the same binary as before for ARMv6+. If only the inline assembly differs,
you can do something like

static inline char __dcc_getchar(void)
{
        char __c;

        if (__LINUX_ARM_ARCH >= 6)
		asm volatile("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg"
	                : "=r" (__c));
	else
		asm volatile ("mrc p14, 0, %0, c1, c0  @ read comms data reg"
			: "=r" (ret));
        isb();

        return __c;
}

	Arnd

Yes doing that will be great!

But Alan wanted "all be runtime handled".

May be we can do something like:


static int cpu_arch;

static inline char __dcc_getchar(void)
{
         char __c;

         if (cpu_arch >= 6)
 		asm volatile("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg"
 	                : "=r" (__c));
 	else
 		asm volatile ("mrc p14, 0, %0, c1, c0  @ read comms data reg"
 			: "=r" (ret));
         isb();

         return __c;
}

static int __init hvc_dcc_console_init(void)
{
	cpu_arch = cpu_architecture();
...

}
Matthieu