@@ -59,6 +59,69 @@ static inline void __dcc_putchar(char c) isb(); }+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.
+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.
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
+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
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;
}
It's not possible to build a kernel that runs on ARMv5 (or below) and also on
ARMv6 (or above), so the effect would be exactly the same. While we are putting
a lot of effort into making all sorts of ARMv6 and ARMv7 based systems work
with the same kernel binary, it's highly unlikely we would ever need the
above to be runtime selected, because there are a lot of other differences
at assembly level.
Arnd
Le Tue, 25 Sep 2012 15:44:57 +0000,
Arnd Bergmann [off-list ref] a ?crit :
It's not possible to build a kernel that runs on ARMv5 (or below) and
also on ARMv6 (or above), so the effect would be exactly the same.
While we are putting a lot of effort into making all sorts of ARMv6
and ARMv7 based systems work with the same kernel binary, it's highly
unlikely we would ever need the above to be runtime selected, because
there are a lot of other differences at assembly level.
I know but Alan was not happy with the static version with ifdef [1]
[2]. That why a dynamic version was done.
Matthieu
[1]
quoted
There are no plans to ever make it possible; there are too many
significant differences between ARMv4, v5 architectures and
ARMv6,v7 architectures to warrant making this runtime selectable.
Then bury this crap in platform files please not in the drivers/tty
layer code. Make it a platform driver provided callback or something.
Alan
[2]
quoted
I posted a new v2 patch that make the selection dynamic.
Thanks - fine with that one - or with burying it in headers etc in
the arm subdirs.
Please consider adding some sort of commit text. Does this add some new
feature I may want on some downstream distro kernel?
ok
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.
Do you see any multiple character inputs? I think you may need an isb
here similar to the v6/7 code and in the putchar as well.
I don't see multiple character.
On armv5 isb is only a memory barrier (__asm__ __volatile__ ("" : : : "memory"))
and it may be not need for dcc operation.
Matthieu