* [PATCH] drivers: char: hvc: add arm JTAG DCC console support @ 2010-11-30 19:25 ` Daniel Walker 0 siblings, 0 replies; 79+ messages in thread From: Daniel Walker @ 2010-11-30 19:25 UTC (permalink / raw) To: linux-kernel Cc: linux-arm-msm, Daniel Walker, Tony Lindgren, Arnd Bergmann, Nicolas Pitre, Greg Kroah-Hartman, Mike Frysinger, Andrew Morton, Alan Cox, Randy Dunlap, FUJITA Tomonori, linuxppc-dev This driver adds a basic console that uses the arm JTAG DCC to transfer data back and forth. It has support for ARMv6 and ARMv7. This console is created under the HVC driver, and should be named /dev/hvcX (or /dev/hvc0 for example). Cc: Tony Lindgren <tony@atomide.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Nicolas Pitre <nico@fluxnic.net> Cc: Greg Kroah-Hartman <gregkh@suse.de> Cc: Mike Frysinger <vapier@gentoo.org> Signed-off-by: Daniel Walker <dwalker@codeaurora.org> --- drivers/char/Kconfig | 9 +++ drivers/char/Makefile | 1 + drivers/char/hvc_dcc.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 143 insertions(+), 0 deletions(-) create mode 100644 drivers/char/hvc_dcc.c diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig index 43d3395..d4a7776 100644 --- a/drivers/char/Kconfig +++ b/drivers/char/Kconfig @@ -682,6 +682,15 @@ config HVC_UDBG select HVC_DRIVER default n +config HVC_DCC + bool "ARM JTAG DCC console" + depends on ARM + select HVC_DRIVER + help + This console uses the JTAG DCC on ARM to create a console under the HVC + driver. This console is used through a JTAG only on ARM. If you don't have + a JTAG then you probably don't want this option. + config VIRTIO_CONSOLE tristate "Virtio console" depends on VIRTIO diff --git a/drivers/char/Makefile b/drivers/char/Makefile index ba53ec9..fa0b824 100644 --- a/drivers/char/Makefile +++ b/drivers/char/Makefile @@ -34,6 +34,7 @@ obj-$(CONFIG_HVC_CONSOLE) += hvc_vio.o hvsi.o obj-$(CONFIG_HVC_ISERIES) += hvc_iseries.o obj-$(CONFIG_HVC_RTAS) += hvc_rtas.o obj-$(CONFIG_HVC_TILE) += hvc_tile.o +obj-$(CONFIG_HVC_DCC) += hvc_dcc.o obj-$(CONFIG_HVC_BEAT) += hvc_beat.o obj-$(CONFIG_HVC_DRIVER) += hvc_console.o obj-$(CONFIG_HVC_IRQ) += hvc_irq.o diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c new file mode 100644 index 0000000..6470f63 --- /dev/null +++ b/drivers/char/hvc_dcc.c @@ -0,0 +1,133 @@ +/* Copyright (c) 2010, Code Aurora Forum. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +#include <linux/console.h> +#include <linux/delay.h> +#include <linux/err.h> +#include <linux/init.h> +#include <linux/moduleparam.h> +#include <linux/types.h> + +#include <asm/processor.h> + +#include "hvc_console.h" + +/* DCC Status Bits */ +#define DCC_STATUS_RX (1 << 30) +#define DCC_STATUS_TX (1 << 29) + +static inline u32 __dcc_getstatus(void) +{ + u32 __ret; + + asm("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" + : "=r" (__ret) : : "cc"); + + return __ret; +} + + +#if defined(CONFIG_CPU_V7) +static inline char __dcc_getchar(void) +{ + char __c; + + asm("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ + bne get_wait \n\ + mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" + : "=r" (__c) : : "cc"); + + return __c; +} +#else +static inline char __dcc_getchar(void) +{ + char __c; + + asm("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" + : "=r" (__c)); + + return __c; +} +#endif + +#if defined(CONFIG_CPU_V7) +static inline void __dcc_putchar(char c) +{ + asm("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ + bcs put_wait \n\ + mcr p14, 0, %0, c0, c5, 0 " + : : "r" (c) : "cc"); +} +#else +static inline void __dcc_putchar(char c) +{ + asm("mcr p14, 0, %0, c0, c5, 0 @ write a char" + : /* no output register */ + : "r" (c)); +} +#endif + +static int hvc_dcc_put_chars(uint32_t vt, const char *buf, int count) +{ + int i; + + for (i = 0; i < count; i++) { + while (__dcc_getstatus() & DCC_STATUS_TX) + cpu_relax(); + + __dcc_putchar((char)(buf[i] & 0xFF)); + } + + return count; +} + +static int hvc_dcc_get_chars(uint32_t vt, char *buf, int count) +{ + int i; + + for (i = 0; i < count; ++i) { + int c = -1; + + if (__dcc_getstatus() & DCC_STATUS_RX) + c = __dcc_getchar(); + if (c < 0) + break; + buf[i] = c; + } + + return i; +} + +static const struct hv_ops hvc_dcc_get_put_ops = { + .get_chars = hvc_dcc_get_chars, + .put_chars = hvc_dcc_put_chars, +}; + +static int __init hvc_dcc_console_init(void) +{ + hvc_instantiate(0, 0, &hvc_dcc_get_put_ops); + return 0; +} +console_initcall(hvc_dcc_console_init); + +static int __init hvc_dcc_init(void) +{ + hvc_alloc(0, 0, &hvc_dcc_get_put_ops, 128); + return 0; +} +device_initcall(hvc_dcc_init); -- 1.7.1 -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply related [flat|nested] 79+ messages in thread
* [PATCH] drivers: char: hvc: add arm JTAG DCC console support @ 2010-11-30 19:25 ` Daniel Walker 0 siblings, 0 replies; 79+ messages in thread From: Daniel Walker @ 2010-11-30 19:25 UTC (permalink / raw) To: linux-kernel Cc: Randy Dunlap, Daniel Walker, Mike Frysinger, Arnd Bergmann, Nicolas Pitre, Tony Lindgren, linux-arm-msm, Greg Kroah-Hartman, FUJITA Tomonori, Andrew Morton, linuxppc-dev, Alan Cox This driver adds a basic console that uses the arm JTAG DCC to transfer data back and forth. It has support for ARMv6 and ARMv7. This console is created under the HVC driver, and should be named /dev/hvcX (or /dev/hvc0 for example). Cc: Tony Lindgren <tony@atomide.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Nicolas Pitre <nico@fluxnic.net> Cc: Greg Kroah-Hartman <gregkh@suse.de> Cc: Mike Frysinger <vapier@gentoo.org> Signed-off-by: Daniel Walker <dwalker@codeaurora.org> --- drivers/char/Kconfig | 9 +++ drivers/char/Makefile | 1 + drivers/char/hvc_dcc.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 143 insertions(+), 0 deletions(-) create mode 100644 drivers/char/hvc_dcc.c diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig index 43d3395..d4a7776 100644 --- a/drivers/char/Kconfig +++ b/drivers/char/Kconfig @@ -682,6 +682,15 @@ config HVC_UDBG select HVC_DRIVER default n +config HVC_DCC + bool "ARM JTAG DCC console" + depends on ARM + select HVC_DRIVER + help + This console uses the JTAG DCC on ARM to create a console under the HVC + driver. This console is used through a JTAG only on ARM. If you don't have + a JTAG then you probably don't want this option. + config VIRTIO_CONSOLE tristate "Virtio console" depends on VIRTIO diff --git a/drivers/char/Makefile b/drivers/char/Makefile index ba53ec9..fa0b824 100644 --- a/drivers/char/Makefile +++ b/drivers/char/Makefile @@ -34,6 +34,7 @@ obj-$(CONFIG_HVC_CONSOLE) += hvc_vio.o hvsi.o obj-$(CONFIG_HVC_ISERIES) += hvc_iseries.o obj-$(CONFIG_HVC_RTAS) += hvc_rtas.o obj-$(CONFIG_HVC_TILE) += hvc_tile.o +obj-$(CONFIG_HVC_DCC) += hvc_dcc.o obj-$(CONFIG_HVC_BEAT) += hvc_beat.o obj-$(CONFIG_HVC_DRIVER) += hvc_console.o obj-$(CONFIG_HVC_IRQ) += hvc_irq.o diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c new file mode 100644 index 0000000..6470f63 --- /dev/null +++ b/drivers/char/hvc_dcc.c @@ -0,0 +1,133 @@ +/* Copyright (c) 2010, Code Aurora Forum. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +#include <linux/console.h> +#include <linux/delay.h> +#include <linux/err.h> +#include <linux/init.h> +#include <linux/moduleparam.h> +#include <linux/types.h> + +#include <asm/processor.h> + +#include "hvc_console.h" + +/* DCC Status Bits */ +#define DCC_STATUS_RX (1 << 30) +#define DCC_STATUS_TX (1 << 29) + +static inline u32 __dcc_getstatus(void) +{ + u32 __ret; + + asm("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" + : "=r" (__ret) : : "cc"); + + return __ret; +} + + +#if defined(CONFIG_CPU_V7) +static inline char __dcc_getchar(void) +{ + char __c; + + asm("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ + bne get_wait \n\ + mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" + : "=r" (__c) : : "cc"); + + return __c; +} +#else +static inline char __dcc_getchar(void) +{ + char __c; + + asm("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" + : "=r" (__c)); + + return __c; +} +#endif + +#if defined(CONFIG_CPU_V7) +static inline void __dcc_putchar(char c) +{ + asm("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ + bcs put_wait \n\ + mcr p14, 0, %0, c0, c5, 0 " + : : "r" (c) : "cc"); +} +#else +static inline void __dcc_putchar(char c) +{ + asm("mcr p14, 0, %0, c0, c5, 0 @ write a char" + : /* no output register */ + : "r" (c)); +} +#endif + +static int hvc_dcc_put_chars(uint32_t vt, const char *buf, int count) +{ + int i; + + for (i = 0; i < count; i++) { + while (__dcc_getstatus() & DCC_STATUS_TX) + cpu_relax(); + + __dcc_putchar((char)(buf[i] & 0xFF)); + } + + return count; +} + +static int hvc_dcc_get_chars(uint32_t vt, char *buf, int count) +{ + int i; + + for (i = 0; i < count; ++i) { + int c = -1; + + if (__dcc_getstatus() & DCC_STATUS_RX) + c = __dcc_getchar(); + if (c < 0) + break; + buf[i] = c; + } + + return i; +} + +static const struct hv_ops hvc_dcc_get_put_ops = { + .get_chars = hvc_dcc_get_chars, + .put_chars = hvc_dcc_put_chars, +}; + +static int __init hvc_dcc_console_init(void) +{ + hvc_instantiate(0, 0, &hvc_dcc_get_put_ops); + return 0; +} +console_initcall(hvc_dcc_console_init); + +static int __init hvc_dcc_init(void) +{ + hvc_alloc(0, 0, &hvc_dcc_get_put_ops, 128); + return 0; +} +device_initcall(hvc_dcc_init); -- 1.7.1 -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply related [flat|nested] 79+ messages in thread
* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support 2010-11-30 19:25 ` Daniel Walker @ 2010-11-30 19:57 ` Nicolas Pitre -1 siblings, 0 replies; 79+ messages in thread From: Nicolas Pitre @ 2010-11-30 19:57 UTC (permalink / raw) To: Daniel Walker Cc: linux-kernel, linux-arm-msm, Tony Lindgren, Arnd Bergmann, Greg Kroah-Hartman, Mike Frysinger, Andrew Morton, Alan Cox, Randy Dunlap, FUJITA Tomonori, linuxppc-dev On Tue, 30 Nov 2010, Daniel Walker wrote: > This driver adds a basic console that uses the arm JTAG > DCC to transfer data back and forth. It has support for > ARMv6 and ARMv7. > > This console is created under the HVC driver, and should be named > /dev/hvcX (or /dev/hvc0 for example). > > Cc: Tony Lindgren <tony@atomide.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Nicolas Pitre <nico@fluxnic.net> > Cc: Greg Kroah-Hartman <gregkh@suse.de> > Cc: Mike Frysinger <vapier@gentoo.org> > Signed-off-by: Daniel Walker <dwalker@codeaurora.org> Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org> This doesn't support both ARMv6 and ARMv7 at run time, but this can trivially be added later when needed. Nicolas ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support @ 2010-11-30 19:57 ` Nicolas Pitre 0 siblings, 0 replies; 79+ messages in thread From: Nicolas Pitre @ 2010-11-30 19:57 UTC (permalink / raw) To: Daniel Walker Cc: Randy Dunlap, Mike Frysinger, Arnd Bergmann, Tony Lindgren, linux-arm-msm, Greg Kroah-Hartman, linux-kernel, FUJITA Tomonori, Andrew Morton, linuxppc-dev, Alan Cox On Tue, 30 Nov 2010, Daniel Walker wrote: > This driver adds a basic console that uses the arm JTAG > DCC to transfer data back and forth. It has support for > ARMv6 and ARMv7. > > This console is created under the HVC driver, and should be named > /dev/hvcX (or /dev/hvc0 for example). > > Cc: Tony Lindgren <tony@atomide.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Nicolas Pitre <nico@fluxnic.net> > Cc: Greg Kroah-Hartman <gregkh@suse.de> > Cc: Mike Frysinger <vapier@gentoo.org> > Signed-off-by: Daniel Walker <dwalker@codeaurora.org> Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org> This doesn't support both ARMv6 and ARMv7 at run time, but this can trivially be added later when needed. Nicolas ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support 2010-11-30 19:57 ` Nicolas Pitre @ 2010-11-30 21:17 ` Arnd Bergmann -1 siblings, 0 replies; 79+ messages in thread From: Arnd Bergmann @ 2010-11-30 21:17 UTC (permalink / raw) To: Nicolas Pitre Cc: Daniel Walker, linux-kernel, linux-arm-msm, Tony Lindgren, Greg Kroah-Hartman, Mike Frysinger, Andrew Morton, Alan Cox, Randy Dunlap, FUJITA Tomonori, linuxppc-dev On Tuesday 30 November 2010, Nicolas Pitre wrote: > > Cc: Tony Lindgren <tony@atomide.com> > > Cc: Arnd Bergmann <arnd@arndb.de> > > Cc: Nicolas Pitre <nico@fluxnic.net> > > Cc: Greg Kroah-Hartman <gregkh@suse.de> > > Cc: Mike Frysinger <vapier@gentoo.org> > > Signed-off-by: Daniel Walker <dwalker@codeaurora.org> > > Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org> Acked-by: Arnd Bergmann <arnd@arndb.de> > This doesn't support both ARMv6 and ARMv7 at run time, but this can > trivially be added later when needed. I was about to make a similar comment when I saw yours ;-) ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support @ 2010-11-30 21:17 ` Arnd Bergmann 0 siblings, 0 replies; 79+ messages in thread From: Arnd Bergmann @ 2010-11-30 21:17 UTC (permalink / raw) To: Nicolas Pitre Cc: Randy Dunlap, Daniel Walker, Mike Frysinger, Tony Lindgren, linux-arm-msm, Greg Kroah-Hartman, linux-kernel, FUJITA Tomonori, Andrew Morton, linuxppc-dev, Alan Cox On Tuesday 30 November 2010, Nicolas Pitre wrote: > > Cc: Tony Lindgren <tony@atomide.com> > > Cc: Arnd Bergmann <arnd@arndb.de> > > Cc: Nicolas Pitre <nico@fluxnic.net> > > Cc: Greg Kroah-Hartman <gregkh@suse.de> > > Cc: Mike Frysinger <vapier@gentoo.org> > > Signed-off-by: Daniel Walker <dwalker@codeaurora.org> > > Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org> Acked-by: Arnd Bergmann <arnd@arndb.de> > This doesn't support both ARMv6 and ARMv7 at run time, but this can > trivially be added later when needed. I was about to make a similar comment when I saw yours ;-) ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support 2010-11-30 19:25 ` Daniel Walker @ 2010-12-01 5:30 ` Stephen Boyd -1 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2010-12-01 5:30 UTC (permalink / raw) To: Daniel Walker Cc: linux-kernel, linux-arm-msm, Tony Lindgren, Arnd Bergmann, Nicolas Pitre, Greg Kroah-Hartman, Mike Frysinger, Andrew Morton, Alan Cox, Randy Dunlap, FUJITA Tomonori, linuxppc-dev On 11/30/2010 11:25 AM, Daniel Walker wrote: > @@ -682,6 +682,15 @@ config HVC_UDBG > select HVC_DRIVER > default n > > +config HVC_DCC > + bool "ARM JTAG DCC console" > + depends on ARM > + select HVC_DRIVER > + help > + This console uses the JTAG DCC on ARM to create a console under the HVC Looks like you added one too many spaces for indent here. > diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c > new file mode 100644 > index 0000000..6470f63 > --- /dev/null > +++ b/drivers/char/hvc_dcc.c > +static inline u32 __dcc_getstatus(void) > +{ > + u32 __ret; > + > + asm("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" > + : "=r" (__ret) : : "cc"); > + > + return __ret; > +} Without marking this asm volatile my compiler decides it can cache the value of __ret in a register and then check the value of it continually in hvc_dcc_put_chars() (I had to replace get_wait/put_wait with 1 and fixup the branch otherwise my disassembler barfed on __dcc_(get|put)char). 00000000 <hvc_dcc_put_chars>: 0: ee103e11 mrc 14, 0, r3, cr0, cr1, {0} 4: e3a0c000 mov ip, #0 ; 0x0 8: e2033202 and r3, r3, #536870912 ; 0x20000000 c: ea000006 b 2c <hvc_dcc_put_chars+0x2c> 10: e3530000 cmp r3, #0 ; 0x0 14: 1afffffd bne 10 <hvc_dcc_put_chars+0x10> 18: e7d1000c ldrb r0, [r1, ip] 1c: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} 20: 2afffffd bcs 1c <hvc_dcc_put_chars+0x1c> 24: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} 28: e28cc001 add ip, ip, #1 ; 0x1 2c: e15c0002 cmp ip, r2 30: bafffff6 blt 10 <hvc_dcc_put_chars+0x10> 34: e1a00002 mov r0, r2 38: e12fff1e bx lr As you can see, the value of the mrc is checked against DCC_STATUS_TX (bit 29) and then stored in r3 for later use. Marking this volatile produces the following: 00000000 <hvc_dcc_put_chars>: 0: e3a03000 mov r3, #0 ; 0x0 4: ea000007 b 28 <hvc_dcc_put_chars+0x28> 8: ee100e11 mrc 14, 0, r0, cr0, cr1, {0} c: e3100202 tst r0, #536870912 ; 0x20000000 10: 1afffffc bne 8 <hvc_dcc_put_chars+0x8> 14: e7d10003 ldrb r0, [r1, r3] 18: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} 1c: 2afffffd bcs 18 <hvc_dcc_put_chars+0x18> 20: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} 24: e2833001 add r3, r3, #1 ; 0x1 28: e1530002 cmp r3, r2 2c: bafffff5 blt 8 <hvc_dcc_put_chars+0x8> 30: e1a00002 mov r0, r2 34: e12fff1e bx lr which looks better. I marked all the asm in this driver as volatile. Is that correct? > +#if defined(CONFIG_CPU_V7) > +static inline char __dcc_getchar(void) > +{ > + char __c; > + > + asm("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ > + bne get_wait \n\ > + mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" > + : "=r" (__c) : : "cc"); > + > + return __c; > +} > +#else > +static inline char __dcc_getchar(void) > +{ > + char __c; > + > + asm("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" > + : "=r" (__c)); > + > + return __c; > +} > +#endif > + > +#if defined(CONFIG_CPU_V7) > +static inline void __dcc_putchar(char c) > +{ > + asm("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ > + bcs put_wait \n\ > + mcr p14, 0, %0, c0, c5, 0 " > + : : "r" (c) : "cc"); > +} > +#else > +static inline void __dcc_putchar(char c) > +{ > + asm("mcr p14, 0, %0, c0, c5, 0 @ write a char" > + : /* no output register */ > + : "r" (c)); > +} > +#endif > + I don't think both the v7 and v6 functions are necessary. It seems I can get away with just the second version of __dcc_(get|put)char() on a v7. The mrc p14, 0, pc, c0, c1, 0 will assign the top 4 bits (31-28) to the condition codes NZCV on v7. It also looks like on an ARM11 (a v6) will also do the same thing if I read the manuals right. The test in the inline assembly is saying, wait for a character to be ready or wait for a character to be read then actually write a character or read one. The code in hvc_dcc_put_chars() is already doing the same thing, albeit in a slightly different form. Instead of getting the status bits put into the condition codes and looping with bne or bcs it will read the register, and it with bit 29 or bit 28 to see if it should wait and then continue with the writing/reading. I think you can just drop the looping for the v7 version of the functions and have this driver work on v6 and v7. Alternatively, you can make some function that says tx buffer is empty, rx buffer is full or something but I don't see how saving a couple instructions buys us much when we can have one driver for v6 and v7. I see that Tony Lindgren modified the DCC macros for v7 in commit 200b7a8 (ARM: 5884/1: arm: Fix DCC console for v7, 2010-01-19). I'm not sure why though, since it seems that a v6 and a v7 should really do the same thing by waiting for the buffers to be ready before filling them or reading them. Which probably means we can get low-level dcc debugging on all targets if I'm not mistaken. > +static int hvc_dcc_put_chars(uint32_t vt, const char *buf, int count) > +{ > + int i; > + > + for (i = 0; i < count; i++) { > + while (__dcc_getstatus() & DCC_STATUS_TX) > + cpu_relax(); > + > + __dcc_putchar((char)(buf[i] & 0xFF)); Is this & 0xFF and cast to char unnecessary? buf is a char array, and chars are always 8 bits. Can't we just do __dcc_putchar(buf[i])? > +static int hvc_dcc_get_chars(uint32_t vt, char *buf, int count) > +{ > + int i; > + > + for (i = 0; i < count; ++i) { > + int c = -1; > + > + if (__dcc_getstatus() & DCC_STATUS_RX) > + c = __dcc_getchar(); > + if (c < 0) > + break; > + buf[i] = c; > + } I think this for loop can be simplified. __dcc_getchar() returns a char. It never returns -1, so the check for c < 0 can't be taken if __dcc_getstatus() & DCC_STATUS_RX is true. The only case you break the loop in then is if __dcc_getstatus() & DCC_STATUS_RX is false. So you can have a simple if-else and assign buf[i] in the if branch and break in the else branch. -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support @ 2010-12-01 5:30 ` Stephen Boyd 0 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2010-12-01 5:30 UTC (permalink / raw) To: Daniel Walker Cc: Randy Dunlap, Mike Frysinger, Arnd Bergmann, Nicolas Pitre, Tony Lindgren, linux-arm-msm, Greg Kroah-Hartman, linux-kernel, FUJITA Tomonori, Andrew Morton, linuxppc-dev, Alan Cox On 11/30/2010 11:25 AM, Daniel Walker wrote: > @@ -682,6 +682,15 @@ config HVC_UDBG > select HVC_DRIVER > default n > > +config HVC_DCC > + bool "ARM JTAG DCC console" > + depends on ARM > + select HVC_DRIVER > + help > + This console uses the JTAG DCC on ARM to create a console under the HVC Looks like you added one too many spaces for indent here. > diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c > new file mode 100644 > index 0000000..6470f63 > --- /dev/null > +++ b/drivers/char/hvc_dcc.c > +static inline u32 __dcc_getstatus(void) > +{ > + u32 __ret; > + > + asm("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" > + : "=r" (__ret) : : "cc"); > + > + return __ret; > +} Without marking this asm volatile my compiler decides it can cache the value of __ret in a register and then check the value of it continually in hvc_dcc_put_chars() (I had to replace get_wait/put_wait with 1 and fixup the branch otherwise my disassembler barfed on __dcc_(get|put)char). 00000000 <hvc_dcc_put_chars>: 0: ee103e11 mrc 14, 0, r3, cr0, cr1, {0} 4: e3a0c000 mov ip, #0 ; 0x0 8: e2033202 and r3, r3, #536870912 ; 0x20000000 c: ea000006 b 2c <hvc_dcc_put_chars+0x2c> 10: e3530000 cmp r3, #0 ; 0x0 14: 1afffffd bne 10 <hvc_dcc_put_chars+0x10> 18: e7d1000c ldrb r0, [r1, ip] 1c: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} 20: 2afffffd bcs 1c <hvc_dcc_put_chars+0x1c> 24: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} 28: e28cc001 add ip, ip, #1 ; 0x1 2c: e15c0002 cmp ip, r2 30: bafffff6 blt 10 <hvc_dcc_put_chars+0x10> 34: e1a00002 mov r0, r2 38: e12fff1e bx lr As you can see, the value of the mrc is checked against DCC_STATUS_TX (bit 29) and then stored in r3 for later use. Marking this volatile produces the following: 00000000 <hvc_dcc_put_chars>: 0: e3a03000 mov r3, #0 ; 0x0 4: ea000007 b 28 <hvc_dcc_put_chars+0x28> 8: ee100e11 mrc 14, 0, r0, cr0, cr1, {0} c: e3100202 tst r0, #536870912 ; 0x20000000 10: 1afffffc bne 8 <hvc_dcc_put_chars+0x8> 14: e7d10003 ldrb r0, [r1, r3] 18: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} 1c: 2afffffd bcs 18 <hvc_dcc_put_chars+0x18> 20: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} 24: e2833001 add r3, r3, #1 ; 0x1 28: e1530002 cmp r3, r2 2c: bafffff5 blt 8 <hvc_dcc_put_chars+0x8> 30: e1a00002 mov r0, r2 34: e12fff1e bx lr which looks better. I marked all the asm in this driver as volatile. Is that correct? > +#if defined(CONFIG_CPU_V7) > +static inline char __dcc_getchar(void) > +{ > + char __c; > + > + asm("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ > + bne get_wait \n\ > + mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" > + : "=r" (__c) : : "cc"); > + > + return __c; > +} > +#else > +static inline char __dcc_getchar(void) > +{ > + char __c; > + > + asm("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" > + : "=r" (__c)); > + > + return __c; > +} > +#endif > + > +#if defined(CONFIG_CPU_V7) > +static inline void __dcc_putchar(char c) > +{ > + asm("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ > + bcs put_wait \n\ > + mcr p14, 0, %0, c0, c5, 0 " > + : : "r" (c) : "cc"); > +} > +#else > +static inline void __dcc_putchar(char c) > +{ > + asm("mcr p14, 0, %0, c0, c5, 0 @ write a char" > + : /* no output register */ > + : "r" (c)); > +} > +#endif > + I don't think both the v7 and v6 functions are necessary. It seems I can get away with just the second version of __dcc_(get|put)char() on a v7. The mrc p14, 0, pc, c0, c1, 0 will assign the top 4 bits (31-28) to the condition codes NZCV on v7. It also looks like on an ARM11 (a v6) will also do the same thing if I read the manuals right. The test in the inline assembly is saying, wait for a character to be ready or wait for a character to be read then actually write a character or read one. The code in hvc_dcc_put_chars() is already doing the same thing, albeit in a slightly different form. Instead of getting the status bits put into the condition codes and looping with bne or bcs it will read the register, and it with bit 29 or bit 28 to see if it should wait and then continue with the writing/reading. I think you can just drop the looping for the v7 version of the functions and have this driver work on v6 and v7. Alternatively, you can make some function that says tx buffer is empty, rx buffer is full or something but I don't see how saving a couple instructions buys us much when we can have one driver for v6 and v7. I see that Tony Lindgren modified the DCC macros for v7 in commit 200b7a8 (ARM: 5884/1: arm: Fix DCC console for v7, 2010-01-19). I'm not sure why though, since it seems that a v6 and a v7 should really do the same thing by waiting for the buffers to be ready before filling them or reading them. Which probably means we can get low-level dcc debugging on all targets if I'm not mistaken. > +static int hvc_dcc_put_chars(uint32_t vt, const char *buf, int count) > +{ > + int i; > + > + for (i = 0; i < count; i++) { > + while (__dcc_getstatus() & DCC_STATUS_TX) > + cpu_relax(); > + > + __dcc_putchar((char)(buf[i] & 0xFF)); Is this & 0xFF and cast to char unnecessary? buf is a char array, and chars are always 8 bits. Can't we just do __dcc_putchar(buf[i])? > +static int hvc_dcc_get_chars(uint32_t vt, char *buf, int count) > +{ > + int i; > + > + for (i = 0; i < count; ++i) { > + int c = -1; > + > + if (__dcc_getstatus() & DCC_STATUS_RX) > + c = __dcc_getchar(); > + if (c < 0) > + break; > + buf[i] = c; > + } I think this for loop can be simplified. __dcc_getchar() returns a char. It never returns -1, so the check for c < 0 can't be taken if __dcc_getstatus() & DCC_STATUS_RX is true. The only case you break the loop in then is if __dcc_getstatus() & DCC_STATUS_RX is false. So you can have a simple if-else and assign buf[i] in the if branch and break in the else branch. -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support 2010-12-01 5:30 ` Stephen Boyd @ 2010-12-01 18:54 ` Daniel Walker -1 siblings, 0 replies; 79+ messages in thread From: Daniel Walker @ 2010-12-01 18:54 UTC (permalink / raw) To: Stephen Boyd Cc: linux-kernel, linux-arm-msm, Tony Lindgren, Arnd Bergmann, Nicolas Pitre, Greg Kroah-Hartman, Mike Frysinger, Andrew Morton, Alan Cox, Randy Dunlap, FUJITA Tomonori, linuxppc-dev On Tue, 2010-11-30 at 21:30 -0800, Stephen Boyd wrote: > On 11/30/2010 11:25 AM, Daniel Walker wrote: > > @@ -682,6 +682,15 @@ config HVC_UDBG > > select HVC_DRIVER > > default n > > > > +config HVC_DCC > > + bool "ARM JTAG DCC console" > > + depends on ARM > > + select HVC_DRIVER > > + help > > + This console uses the JTAG DCC on ARM to create a console under the HVC > > Looks like you added one too many spaces for indent here. The first line is fine, but the other two might need an extra space. > > diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c > > new file mode 100644 > > index 0000000..6470f63 > > --- /dev/null > > +++ b/drivers/char/hvc_dcc.c > > +static inline u32 __dcc_getstatus(void) > > +{ > > + u32 __ret; > > + > > + asm("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" > > + : "=r" (__ret) : : "cc"); > > + > > + return __ret; > > +} > > Without marking this asm volatile my compiler decides it can cache the > value of __ret in a register and then check the value of it continually > in hvc_dcc_put_chars() (I had to replace get_wait/put_wait with 1 and > fixup the branch otherwise my disassembler barfed on __dcc_(get|put)char). > > > 00000000 <hvc_dcc_put_chars>: > 0: ee103e11 mrc 14, 0, r3, cr0, cr1, {0} > 4: e3a0c000 mov ip, #0 ; 0x0 > 8: e2033202 and r3, r3, #536870912 ; 0x20000000 > c: ea000006 b 2c <hvc_dcc_put_chars+0x2c> > 10: e3530000 cmp r3, #0 ; 0x0 > 14: 1afffffd bne 10 <hvc_dcc_put_chars+0x10> > 18: e7d1000c ldrb r0, [r1, ip] > 1c: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} > 20: 2afffffd bcs 1c <hvc_dcc_put_chars+0x1c> > 24: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} > 28: e28cc001 add ip, ip, #1 ; 0x1 > 2c: e15c0002 cmp ip, r2 > 30: bafffff6 blt 10 <hvc_dcc_put_chars+0x10> > 34: e1a00002 mov r0, r2 > 38: e12fff1e bx lr > > As you can see, the value of the mrc is checked against DCC_STATUS_TX > (bit 29) and then stored in r3 for later use. Marking this volatile > produces the following: > > 00000000 <hvc_dcc_put_chars>: > 0: e3a03000 mov r3, #0 ; 0x0 > 4: ea000007 b 28 <hvc_dcc_put_chars+0x28> > 8: ee100e11 mrc 14, 0, r0, cr0, cr1, {0} > c: e3100202 tst r0, #536870912 ; 0x20000000 > 10: 1afffffc bne 8 <hvc_dcc_put_chars+0x8> > 14: e7d10003 ldrb r0, [r1, r3] > 18: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} > 1c: 2afffffd bcs 18 <hvc_dcc_put_chars+0x18> > 20: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} > 24: e2833001 add r3, r3, #1 ; 0x1 > 28: e1530002 cmp r3, r2 > 2c: bafffff5 blt 8 <hvc_dcc_put_chars+0x8> > 30: e1a00002 mov r0, r2 > 34: e12fff1e bx lr > > which looks better. > > I marked all the asm in this driver as volatile. Is that correct? Are you talking about __dcc_getstatus only? I don't think adding volatile is going to hurt anything, if not having it causes problems. > > +#if defined(CONFIG_CPU_V7) > > +static inline char __dcc_getchar(void) > > +{ > > + char __c; > > + > > + asm("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ > > + bne get_wait \n\ > > + mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" > > + : "=r" (__c) : : "cc"); > > + > > + return __c; > > +} > > +#else > > +static inline char __dcc_getchar(void) > > +{ > > + char __c; > > + > > + asm("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" > > + : "=r" (__c)); > > + > > + return __c; > > +} > > +#endif > > + > > +#if defined(CONFIG_CPU_V7) > > +static inline void __dcc_putchar(char c) > > +{ > > + asm("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ > > + bcs put_wait \n\ > > + mcr p14, 0, %0, c0, c5, 0 " > > + : : "r" (c) : "cc"); > > +} > > +#else > > +static inline void __dcc_putchar(char c) > > +{ > > + asm("mcr p14, 0, %0, c0, c5, 0 @ write a char" > > + : /* no output register */ > > + : "r" (c)); > > +} > > +#endif > > + > > I don't think both the v7 and v6 functions are necessary. It seems I can > get away with just the second version of __dcc_(get|put)char() on a v7. > The mrc p14, 0, pc, c0, c1, 0 will assign the top 4 bits (31-28) to the > condition codes NZCV on v7. It also looks like on an ARM11 (a v6) will > also do the same thing if I read the manuals right. The test in the > inline assembly is saying, wait for a character to be ready or wait for > a character to be read then actually write a character or read one. The > code in hvc_dcc_put_chars() is already doing the same thing, albeit in a > slightly different form. Instead of getting the status bits put into the > condition codes and looping with bne or bcs it will read the register, > and it with bit 29 or bit 28 to see if it should wait and then continue > with the writing/reading. I think you can just drop the looping for the > v7 version of the functions and have this driver work on v6 and v7. We could maybe drop the looping for TX, but RX has no C based looping even tho for v7 it's recommended that we loop (presumably for v6 it's not recommended). > Alternatively, you can make some function that says tx buffer is empty, > rx buffer is full or something but I don't see how saving a couple > instructions buys us much when we can have one driver for v6 and v7. > > I see that Tony Lindgren modified the DCC macros for v7 in commit > 200b7a8 (ARM: 5884/1: arm: Fix DCC console for v7, 2010-01-19). I'm not > sure why though, since it seems that a v6 and a v7 should really do the > same thing by waiting for the buffers to be ready before filling them or > reading them. Which probably means we can get low-level dcc debugging on > all targets if I'm not mistaken. This is primarily why these function look they way they do. I'd like to hear from Tony, because he apparent didn't think they did the same thing. > > +static int hvc_dcc_put_chars(uint32_t vt, const char *buf, int count) > > +{ > > + int i; > > + > > + for (i = 0; i < count; i++) { > > + while (__dcc_getstatus() & DCC_STATUS_TX) > > + cpu_relax(); > > + > > + __dcc_putchar((char)(buf[i] & 0xFF)); > > Is this & 0xFF and cast to char unnecessary? buf is a char array, and > chars are always 8 bits. Can't we just do __dcc_putchar(buf[i])? Yeah, doesn't seem like it. > > +static int hvc_dcc_get_chars(uint32_t vt, char *buf, int count) > > +{ > > + int i; > > + > > + for (i = 0; i < count; ++i) { > > + int c = -1; > > + > > + if (__dcc_getstatus() & DCC_STATUS_RX) > > + c = __dcc_getchar(); > > + if (c < 0) > > + break; > > + buf[i] = c; > > + } > > I think this for loop can be simplified. __dcc_getchar() returns a char. > It never returns -1, so the check for c < 0 can't be taken if > __dcc_getstatus() & DCC_STATUS_RX is true. The only case you break the > loop in then is if __dcc_getstatus() & DCC_STATUS_RX is false. So you > can have a simple if-else and assign buf[i] in the if branch and break > in the else branch. > Like this? for (i = 0; i < count; ++i) { if (__dcc_getstatus() & DCC_STATUS_RX) buf[i] = __dcc_getchar(); else break; } It's a micro clean up I guess .. Daniel -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support @ 2010-12-01 18:54 ` Daniel Walker 0 siblings, 0 replies; 79+ messages in thread From: Daniel Walker @ 2010-12-01 18:54 UTC (permalink / raw) To: Stephen Boyd Cc: Randy Dunlap, Mike Frysinger, Arnd Bergmann, Nicolas Pitre, Tony Lindgren, linux-arm-msm, Greg Kroah-Hartman, linux-kernel, FUJITA Tomonori, Andrew Morton, linuxppc-dev, Alan Cox On Tue, 2010-11-30 at 21:30 -0800, Stephen Boyd wrote: > On 11/30/2010 11:25 AM, Daniel Walker wrote: > > @@ -682,6 +682,15 @@ config HVC_UDBG > > select HVC_DRIVER > > default n > > > > +config HVC_DCC > > + bool "ARM JTAG DCC console" > > + depends on ARM > > + select HVC_DRIVER > > + help > > + This console uses the JTAG DCC on ARM to create a console under the HVC > > Looks like you added one too many spaces for indent here. The first line is fine, but the other two might need an extra space. > > diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c > > new file mode 100644 > > index 0000000..6470f63 > > --- /dev/null > > +++ b/drivers/char/hvc_dcc.c > > +static inline u32 __dcc_getstatus(void) > > +{ > > + u32 __ret; > > + > > + asm("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" > > + : "=r" (__ret) : : "cc"); > > + > > + return __ret; > > +} > > Without marking this asm volatile my compiler decides it can cache the > value of __ret in a register and then check the value of it continually > in hvc_dcc_put_chars() (I had to replace get_wait/put_wait with 1 and > fixup the branch otherwise my disassembler barfed on __dcc_(get|put)char). > > > 00000000 <hvc_dcc_put_chars>: > 0: ee103e11 mrc 14, 0, r3, cr0, cr1, {0} > 4: e3a0c000 mov ip, #0 ; 0x0 > 8: e2033202 and r3, r3, #536870912 ; 0x20000000 > c: ea000006 b 2c <hvc_dcc_put_chars+0x2c> > 10: e3530000 cmp r3, #0 ; 0x0 > 14: 1afffffd bne 10 <hvc_dcc_put_chars+0x10> > 18: e7d1000c ldrb r0, [r1, ip] > 1c: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} > 20: 2afffffd bcs 1c <hvc_dcc_put_chars+0x1c> > 24: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} > 28: e28cc001 add ip, ip, #1 ; 0x1 > 2c: e15c0002 cmp ip, r2 > 30: bafffff6 blt 10 <hvc_dcc_put_chars+0x10> > 34: e1a00002 mov r0, r2 > 38: e12fff1e bx lr > > As you can see, the value of the mrc is checked against DCC_STATUS_TX > (bit 29) and then stored in r3 for later use. Marking this volatile > produces the following: > > 00000000 <hvc_dcc_put_chars>: > 0: e3a03000 mov r3, #0 ; 0x0 > 4: ea000007 b 28 <hvc_dcc_put_chars+0x28> > 8: ee100e11 mrc 14, 0, r0, cr0, cr1, {0} > c: e3100202 tst r0, #536870912 ; 0x20000000 > 10: 1afffffc bne 8 <hvc_dcc_put_chars+0x8> > 14: e7d10003 ldrb r0, [r1, r3] > 18: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} > 1c: 2afffffd bcs 18 <hvc_dcc_put_chars+0x18> > 20: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} > 24: e2833001 add r3, r3, #1 ; 0x1 > 28: e1530002 cmp r3, r2 > 2c: bafffff5 blt 8 <hvc_dcc_put_chars+0x8> > 30: e1a00002 mov r0, r2 > 34: e12fff1e bx lr > > which looks better. > > I marked all the asm in this driver as volatile. Is that correct? Are you talking about __dcc_getstatus only? I don't think adding volatile is going to hurt anything, if not having it causes problems. > > +#if defined(CONFIG_CPU_V7) > > +static inline char __dcc_getchar(void) > > +{ > > + char __c; > > + > > + asm("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ > > + bne get_wait \n\ > > + mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" > > + : "=r" (__c) : : "cc"); > > + > > + return __c; > > +} > > +#else > > +static inline char __dcc_getchar(void) > > +{ > > + char __c; > > + > > + asm("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" > > + : "=r" (__c)); > > + > > + return __c; > > +} > > +#endif > > + > > +#if defined(CONFIG_CPU_V7) > > +static inline void __dcc_putchar(char c) > > +{ > > + asm("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ > > + bcs put_wait \n\ > > + mcr p14, 0, %0, c0, c5, 0 " > > + : : "r" (c) : "cc"); > > +} > > +#else > > +static inline void __dcc_putchar(char c) > > +{ > > + asm("mcr p14, 0, %0, c0, c5, 0 @ write a char" > > + : /* no output register */ > > + : "r" (c)); > > +} > > +#endif > > + > > I don't think both the v7 and v6 functions are necessary. It seems I can > get away with just the second version of __dcc_(get|put)char() on a v7. > The mrc p14, 0, pc, c0, c1, 0 will assign the top 4 bits (31-28) to the > condition codes NZCV on v7. It also looks like on an ARM11 (a v6) will > also do the same thing if I read the manuals right. The test in the > inline assembly is saying, wait for a character to be ready or wait for > a character to be read then actually write a character or read one. The > code in hvc_dcc_put_chars() is already doing the same thing, albeit in a > slightly different form. Instead of getting the status bits put into the > condition codes and looping with bne or bcs it will read the register, > and it with bit 29 or bit 28 to see if it should wait and then continue > with the writing/reading. I think you can just drop the looping for the > v7 version of the functions and have this driver work on v6 and v7. We could maybe drop the looping for TX, but RX has no C based looping even tho for v7 it's recommended that we loop (presumably for v6 it's not recommended). > Alternatively, you can make some function that says tx buffer is empty, > rx buffer is full or something but I don't see how saving a couple > instructions buys us much when we can have one driver for v6 and v7. > > I see that Tony Lindgren modified the DCC macros for v7 in commit > 200b7a8 (ARM: 5884/1: arm: Fix DCC console for v7, 2010-01-19). I'm not > sure why though, since it seems that a v6 and a v7 should really do the > same thing by waiting for the buffers to be ready before filling them or > reading them. Which probably means we can get low-level dcc debugging on > all targets if I'm not mistaken. This is primarily why these function look they way they do. I'd like to hear from Tony, because he apparent didn't think they did the same thing. > > +static int hvc_dcc_put_chars(uint32_t vt, const char *buf, int count) > > +{ > > + int i; > > + > > + for (i = 0; i < count; i++) { > > + while (__dcc_getstatus() & DCC_STATUS_TX) > > + cpu_relax(); > > + > > + __dcc_putchar((char)(buf[i] & 0xFF)); > > Is this & 0xFF and cast to char unnecessary? buf is a char array, and > chars are always 8 bits. Can't we just do __dcc_putchar(buf[i])? Yeah, doesn't seem like it. > > +static int hvc_dcc_get_chars(uint32_t vt, char *buf, int count) > > +{ > > + int i; > > + > > + for (i = 0; i < count; ++i) { > > + int c = -1; > > + > > + if (__dcc_getstatus() & DCC_STATUS_RX) > > + c = __dcc_getchar(); > > + if (c < 0) > > + break; > > + buf[i] = c; > > + } > > I think this for loop can be simplified. __dcc_getchar() returns a char. > It never returns -1, so the check for c < 0 can't be taken if > __dcc_getstatus() & DCC_STATUS_RX is true. The only case you break the > loop in then is if __dcc_getstatus() & DCC_STATUS_RX is false. So you > can have a simple if-else and assign buf[i] in the if branch and break > in the else branch. > Like this? for (i = 0; i < count; ++i) { if (__dcc_getstatus() & DCC_STATUS_RX) buf[i] = __dcc_getchar(); else break; } It's a micro clean up I guess .. Daniel -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support 2010-12-01 18:54 ` Daniel Walker @ 2010-12-01 19:28 ` Greg KH -1 siblings, 0 replies; 79+ messages in thread From: Greg KH @ 2010-12-01 19:28 UTC (permalink / raw) To: Daniel Walker Cc: Stephen Boyd, linux-kernel, linux-arm-msm, Tony Lindgren, Arnd Bergmann, Nicolas Pitre, Mike Frysinger, Andrew Morton, Alan Cox, Randy Dunlap, FUJITA Tomonori, linuxppc-dev On Wed, Dec 01, 2010 at 10:54:56AM -0800, Daniel Walker wrote: > On Tue, 2010-11-30 at 21:30 -0800, Stephen Boyd wrote: > > On 11/30/2010 11:25 AM, Daniel Walker wrote: > > > @@ -682,6 +682,15 @@ config HVC_UDBG > > > select HVC_DRIVER > > > default n > > > > > > +config HVC_DCC > > > + bool "ARM JTAG DCC console" > > > + depends on ARM > > > + select HVC_DRIVER > > > + help > > > + This console uses the JTAG DCC on ARM to create a console under the HVC > > > > Looks like you added one too many spaces for indent here. > > The first line is fine, but the other two might need an extra space. For this, or any other changes you want, I'll gladly take a follow-on patch as this one is already in my tty-next tree. thanks, greg k-h ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support @ 2010-12-01 19:28 ` Greg KH 0 siblings, 0 replies; 79+ messages in thread From: Greg KH @ 2010-12-01 19:28 UTC (permalink / raw) To: Daniel Walker Cc: Randy Dunlap, Mike Frysinger, Arnd Bergmann, Nicolas Pitre, Tony Lindgren, linux-arm-msm, Stephen Boyd, linux-kernel, FUJITA Tomonori, Andrew Morton, linuxppc-dev, Alan Cox On Wed, Dec 01, 2010 at 10:54:56AM -0800, Daniel Walker wrote: > On Tue, 2010-11-30 at 21:30 -0800, Stephen Boyd wrote: > > On 11/30/2010 11:25 AM, Daniel Walker wrote: > > > @@ -682,6 +682,15 @@ config HVC_UDBG > > > select HVC_DRIVER > > > default n > > > > > > +config HVC_DCC > > > + bool "ARM JTAG DCC console" > > > + depends on ARM > > > + select HVC_DRIVER > > > + help > > > + This console uses the JTAG DCC on ARM to create a console under the HVC > > > > Looks like you added one too many spaces for indent here. > > The first line is fine, but the other two might need an extra space. For this, or any other changes you want, I'll gladly take a follow-on patch as this one is already in my tty-next tree. thanks, greg k-h ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH] hvc_dcc: Simplify assembly for v6 and v7 ARM 2010-12-01 19:28 ` Greg KH @ 2010-12-18 5:16 ` Stephen Boyd -1 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2010-12-18 5:16 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, Tony Lindgren, Arnd Bergmann, Nicolas Pitre, Daniel Walker The inline assembly differences for v6 vs. v7 in the hvc_dcc driver are purely optimizations. On a v7 processor, an mrc with the pc sets the condition codes to the 28-31 bits of the register being read. It just so happens that the TX/RX full bits the DCC driver is testing for are high enough in the register to be put into the condition codes. On a v6 processor, this "feature" isn't implemented and thus we have to do the usual read, mask, test operations to check for TX/RX full. Since we already test the RX/TX full bits before calling __dcc_getchar() and __dcc_putchar() we don't actually need to do anything special for v7 over v6. The only difference is in hvc_dcc_get_chars(). We would test RX full, poll RX full, and then read a character from the buffer, whereas now we will test RX full, read a character from the buffer, and then test RX full again for the second iteration of the loop. It doesn't seem possible for the buffer to go from full to empty between testing the RX full and reading a character. Therefore, replace the v7 versions with the v6 versions and everything works the same. While we're here, cleanup the for loops a bit and mark the inline assembly as volatile. Not marking it volatile causes GCC to cache the results of the status and RX buffer registers causing lockups. Cc: Tony Lindgren <tony@atomide.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Nicolas Pitre <nicolas.pitre@linaro.org> Cc: Daniel Walker <dwalker@codeaurora.org> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- drivers/char/hvc_dcc.c | 43 +++++++------------------------------------ 1 files changed, 7 insertions(+), 36 deletions(-) diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c index 6470f63..435f6fa 100644 --- a/drivers/char/hvc_dcc.c +++ b/drivers/char/hvc_dcc.c @@ -33,54 +33,29 @@ static inline u32 __dcc_getstatus(void) { u32 __ret; - - asm("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" + asm volatile("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" : "=r" (__ret) : : "cc"); return __ret; } -#if defined(CONFIG_CPU_V7) static inline char __dcc_getchar(void) { char __c; - asm("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ - bne get_wait \n\ - mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" - : "=r" (__c) : : "cc"); - - return __c; -} -#else -static inline char __dcc_getchar(void) -{ - char __c; - - asm("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" + asm volatile("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" : "=r" (__c)); return __c; } -#endif -#if defined(CONFIG_CPU_V7) -static inline void __dcc_putchar(char c) -{ - asm("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ - bcs put_wait \n\ - mcr p14, 0, %0, c0, c5, 0 " - : : "r" (c) : "cc"); -} -#else static inline void __dcc_putchar(char c) { - asm("mcr p14, 0, %0, c0, c5, 0 @ write a char" + asm volatile("mcr p14, 0, %0, c0, c5, 0 @ write a char" : /* no output register */ : "r" (c)); } -#endif static int hvc_dcc_put_chars(uint32_t vt, const char *buf, int count) { @@ -90,7 +65,7 @@ static int hvc_dcc_put_chars(uint32_t vt, const char *buf, int count) while (__dcc_getstatus() & DCC_STATUS_TX) cpu_relax(); - __dcc_putchar((char)(buf[i] & 0xFF)); + __dcc_putchar(buf[i]); } return count; @@ -100,15 +75,11 @@ static int hvc_dcc_get_chars(uint32_t vt, char *buf, int count) { int i; - for (i = 0; i < count; ++i) { - int c = -1; - + for (i = 0; i < count; ++i) if (__dcc_getstatus() & DCC_STATUS_RX) - c = __dcc_getchar(); - if (c < 0) + buf[i] = __dcc_getchar(); + else break; - buf[i] = c; - } return i; } -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply related [flat|nested] 79+ messages in thread
* [PATCH] hvc_dcc: Simplify assembly for v6 and v7 ARM @ 2010-12-18 5:16 ` Stephen Boyd 0 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2010-12-18 5:16 UTC (permalink / raw) To: linux-arm-kernel The inline assembly differences for v6 vs. v7 in the hvc_dcc driver are purely optimizations. On a v7 processor, an mrc with the pc sets the condition codes to the 28-31 bits of the register being read. It just so happens that the TX/RX full bits the DCC driver is testing for are high enough in the register to be put into the condition codes. On a v6 processor, this "feature" isn't implemented and thus we have to do the usual read, mask, test operations to check for TX/RX full. Since we already test the RX/TX full bits before calling __dcc_getchar() and __dcc_putchar() we don't actually need to do anything special for v7 over v6. The only difference is in hvc_dcc_get_chars(). We would test RX full, poll RX full, and then read a character from the buffer, whereas now we will test RX full, read a character from the buffer, and then test RX full again for the second iteration of the loop. It doesn't seem possible for the buffer to go from full to empty between testing the RX full and reading a character. Therefore, replace the v7 versions with the v6 versions and everything works the same. While we're here, cleanup the for loops a bit and mark the inline assembly as volatile. Not marking it volatile causes GCC to cache the results of the status and RX buffer registers causing lockups. Cc: Tony Lindgren <tony@atomide.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Nicolas Pitre <nicolas.pitre@linaro.org> Cc: Daniel Walker <dwalker@codeaurora.org> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- drivers/char/hvc_dcc.c | 43 +++++++------------------------------------ 1 files changed, 7 insertions(+), 36 deletions(-) diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c index 6470f63..435f6fa 100644 --- a/drivers/char/hvc_dcc.c +++ b/drivers/char/hvc_dcc.c @@ -33,54 +33,29 @@ static inline u32 __dcc_getstatus(void) { u32 __ret; - - asm("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" + asm volatile("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" : "=r" (__ret) : : "cc"); return __ret; } -#if defined(CONFIG_CPU_V7) static inline char __dcc_getchar(void) { char __c; - asm("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ - bne get_wait \n\ - mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" - : "=r" (__c) : : "cc"); - - return __c; -} -#else -static inline char __dcc_getchar(void) -{ - char __c; - - asm("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" + asm volatile("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" : "=r" (__c)); return __c; } -#endif -#if defined(CONFIG_CPU_V7) -static inline void __dcc_putchar(char c) -{ - asm("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ - bcs put_wait \n\ - mcr p14, 0, %0, c0, c5, 0 " - : : "r" (c) : "cc"); -} -#else static inline void __dcc_putchar(char c) { - asm("mcr p14, 0, %0, c0, c5, 0 @ write a char" + asm volatile("mcr p14, 0, %0, c0, c5, 0 @ write a char" : /* no output register */ : "r" (c)); } -#endif static int hvc_dcc_put_chars(uint32_t vt, const char *buf, int count) { @@ -90,7 +65,7 @@ static int hvc_dcc_put_chars(uint32_t vt, const char *buf, int count) while (__dcc_getstatus() & DCC_STATUS_TX) cpu_relax(); - __dcc_putchar((char)(buf[i] & 0xFF)); + __dcc_putchar(buf[i]); } return count; @@ -100,15 +75,11 @@ static int hvc_dcc_get_chars(uint32_t vt, char *buf, int count) { int i; - for (i = 0; i < count; ++i) { - int c = -1; - + for (i = 0; i < count; ++i) if (__dcc_getstatus() & DCC_STATUS_RX) - c = __dcc_getchar(); - if (c < 0) + buf[i] = __dcc_getchar(); + else break; - buf[i] = c; - } return i; } -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply related [flat|nested] 79+ messages in thread
* Re: [PATCH] hvc_dcc: Simplify assembly for v6 and v7 ARM 2010-12-18 5:16 ` Stephen Boyd @ 2010-12-20 17:51 ` Daniel Walker -1 siblings, 0 replies; 79+ messages in thread From: Daniel Walker @ 2010-12-20 17:51 UTC (permalink / raw) To: Stephen Boyd Cc: Greg Kroah-Hartman, linux-kernel, linux-arm-msm, linux-arm-kernel, Tony Lindgren, Arnd Bergmann, Nicolas Pitre On Fri, 2010-12-17 at 21:16 -0800, Stephen Boyd wrote: > The inline assembly differences for v6 vs. v7 in the hvc_dcc > driver are purely optimizations. On a v7 processor, an mrc with > the pc sets the condition codes to the 28-31 bits of the register > being read. It just so happens that the TX/RX full bits the DCC > driver is testing for are high enough in the register to be put > into the condition codes. On a v6 processor, this "feature" isn't > implemented and thus we have to do the usual read, mask, test > operations to check for TX/RX full. > > Since we already test the RX/TX full bits before calling > __dcc_getchar() and __dcc_putchar() we don't actually need to do > anything special for v7 over v6. The only difference is in > hvc_dcc_get_chars(). We would test RX full, poll RX full, and > then read a character from the buffer, whereas now we will test > RX full, read a character from the buffer, and then test RX full > again for the second iteration of the loop. It doesn't seem > possible for the buffer to go from full to empty between testing > the RX full and reading a character. Therefore, replace the v7 > versions with the v6 versions and everything works the same. > > While we're here, cleanup the for loops a bit and mark the inline > assembly as volatile. Not marking it volatile causes GCC to cache > the results of the status and RX buffer registers causing > lockups. I would expect to see three patches. One that adds volatile, which appears to be a good fix. Another patch that changes the assembly lines, and another that does the clean up. The last two are more controversial ones. Daniel -- Sent by an consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH] hvc_dcc: Simplify assembly for v6 and v7 ARM @ 2010-12-20 17:51 ` Daniel Walker 0 siblings, 0 replies; 79+ messages in thread From: Daniel Walker @ 2010-12-20 17:51 UTC (permalink / raw) To: linux-arm-kernel On Fri, 2010-12-17 at 21:16 -0800, Stephen Boyd wrote: > The inline assembly differences for v6 vs. v7 in the hvc_dcc > driver are purely optimizations. On a v7 processor, an mrc with > the pc sets the condition codes to the 28-31 bits of the register > being read. It just so happens that the TX/RX full bits the DCC > driver is testing for are high enough in the register to be put > into the condition codes. On a v6 processor, this "feature" isn't > implemented and thus we have to do the usual read, mask, test > operations to check for TX/RX full. > > Since we already test the RX/TX full bits before calling > __dcc_getchar() and __dcc_putchar() we don't actually need to do > anything special for v7 over v6. The only difference is in > hvc_dcc_get_chars(). We would test RX full, poll RX full, and > then read a character from the buffer, whereas now we will test > RX full, read a character from the buffer, and then test RX full > again for the second iteration of the loop. It doesn't seem > possible for the buffer to go from full to empty between testing > the RX full and reading a character. Therefore, replace the v7 > versions with the v6 versions and everything works the same. > > While we're here, cleanup the for loops a bit and mark the inline > assembly as volatile. Not marking it volatile causes GCC to cache > the results of the status and RX buffer registers causing > lockups. I would expect to see three patches. One that adds volatile, which appears to be a good fix. Another patch that changes the assembly lines, and another that does the clean up. The last two are more controversial ones. Daniel -- Sent by an consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH] hvc_dcc: Simplify assembly for v6 and v7 ARM 2010-12-20 17:51 ` Daniel Walker @ 2010-12-20 18:39 ` Stephen Boyd -1 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2010-12-20 18:39 UTC (permalink / raw) To: Daniel Walker Cc: Greg Kroah-Hartman, linux-kernel, linux-arm-msm, linux-arm-kernel, Tony Lindgren, Arnd Bergmann, Nicolas Pitre On 12/20/2010 09:51 AM, Daniel Walker wrote: > > I would expect to see three patches. One that adds volatile, which > appears to be a good fix. Another patch that changes the assembly lines, > and another that does the clean up. The last two are more controversial > ones Ok. I'll send a series later today to give some more time for feedback. -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH] hvc_dcc: Simplify assembly for v6 and v7 ARM @ 2010-12-20 18:39 ` Stephen Boyd 0 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2010-12-20 18:39 UTC (permalink / raw) To: linux-arm-kernel On 12/20/2010 09:51 AM, Daniel Walker wrote: > > I would expect to see three patches. One that adds volatile, which > appears to be a good fix. Another patch that changes the assembly lines, > and another that does the clean up. The last two are more controversial > ones Ok. I'll send a series later today to give some more time for feedback. -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH] hvc_dcc: Simplify assembly for v6 and v7 ARM 2010-12-20 18:39 ` Stephen Boyd @ 2010-12-20 18:46 ` Nicolas Pitre -1 siblings, 0 replies; 79+ messages in thread From: Nicolas Pitre @ 2010-12-20 18:46 UTC (permalink / raw) To: Stephen Boyd Cc: Daniel Walker, Greg Kroah-Hartman, linux-kernel, linux-arm-msm, linux-arm-kernel, Tony Lindgren, Arnd Bergmann On Mon, 20 Dec 2010, Stephen Boyd wrote: > On 12/20/2010 09:51 AM, Daniel Walker wrote: > > > > I would expect to see three patches. One that adds volatile, which > > appears to be a good fix. Another patch that changes the assembly lines, > > and another that does the clean up. The last two are more controversial > > ones > > Ok. I'll send a series later today to give some more time for feedback. I think you can do that right away. Splitting the patch will make that feedback easier to provide. Nicolas ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH] hvc_dcc: Simplify assembly for v6 and v7 ARM @ 2010-12-20 18:46 ` Nicolas Pitre 0 siblings, 0 replies; 79+ messages in thread From: Nicolas Pitre @ 2010-12-20 18:46 UTC (permalink / raw) To: linux-arm-kernel On Mon, 20 Dec 2010, Stephen Boyd wrote: > On 12/20/2010 09:51 AM, Daniel Walker wrote: > > > > I would expect to see three patches. One that adds volatile, which > > appears to be a good fix. Another patch that changes the assembly lines, > > and another that does the clean up. The last two are more controversial > > ones > > Ok. I'll send a series later today to give some more time for feedback. I think you can do that right away. Splitting the patch will make that feedback easier to provide. Nicolas ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH 0/3] hvc_dcc cleanups and fixes 2010-12-18 5:16 ` Stephen Boyd @ 2010-12-20 20:08 ` Stephen Boyd -1 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2010-12-20 20:08 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: linux-kernel, linux-arm-msm, linux-arm-kernel Here are the split patches. The first two patches cleanup and fix the hvc_dcc driver for my compiler. The final patch is more controversial, it removes the v6 and v7 differences in this driver. Stephen Boyd (3): hvc_dcc: Fix bad code generation by marking assembly volatile hvc_dcc: Simplify put_chars()/get_chars() loops hvc_dcc: Simplify assembly for v6 and v7 ARM drivers/char/hvc_dcc.c | 43 +++++++------------------------------------ 1 files changed, 7 insertions(+), 36 deletions(-) -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH 0/3] hvc_dcc cleanups and fixes @ 2010-12-20 20:08 ` Stephen Boyd 0 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2010-12-20 20:08 UTC (permalink / raw) To: linux-arm-kernel Here are the split patches. The first two patches cleanup and fix the hvc_dcc driver for my compiler. The final patch is more controversial, it removes the v6 and v7 differences in this driver. Stephen Boyd (3): hvc_dcc: Fix bad code generation by marking assembly volatile hvc_dcc: Simplify put_chars()/get_chars() loops hvc_dcc: Simplify assembly for v6 and v7 ARM drivers/char/hvc_dcc.c | 43 +++++++------------------------------------ 1 files changed, 7 insertions(+), 36 deletions(-) -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile 2010-12-20 20:08 ` Stephen Boyd @ 2010-12-20 20:08 ` Stephen Boyd -1 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2010-12-20 20:08 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, Tony Lindgren, Arnd Bergmann, Nicolas Pitre, Daniel Walker Without marking the asm __dcc_getstatus() volatile my compiler decides it can cache the value of __ret in a register and then check the value of it continually in hvc_dcc_put_chars() (I had to replace get_wait/put_wait with 1 and fixup the branch otherwise my disassembler barfed on __dcc_(get|put)char). 00000000 <hvc_dcc_put_chars>: 0: ee103e11 mrc 14, 0, r3, cr0, cr1, {0} 4: e3a0c000 mov ip, #0 ; 0x0 8: e2033202 and r3, r3, #536870912 ; 0x20000000 c: ea000006 b 2c <hvc_dcc_put_chars+0x2c> 10: e3530000 cmp r3, #0 ; 0x0 14: 1afffffd bne 10 <hvc_dcc_put_chars+0x10> 18: e7d1000c ldrb r0, [r1, ip] 1c: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} 20: 2afffffd bcs 1c <hvc_dcc_put_chars+0x1c> 24: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} 28: e28cc001 add ip, ip, #1 ; 0x1 2c: e15c0002 cmp ip, r2 30: bafffff6 blt 10 <hvc_dcc_put_chars+0x10> 34: e1a00002 mov r0, r2 38: e12fff1e bx lr As you can see, the value of the mrc is checked against DCC_STATUS_TX (bit 29) and then stored in r3 for later use. Marking the asm volatile produces the following: 00000000 <hvc_dcc_put_chars>: 0: e3a03000 mov r3, #0 ; 0x0 4: ea000007 b 28 <hvc_dcc_put_chars+0x28> 8: ee100e11 mrc 14, 0, r0, cr0, cr1, {0} c: e3100202 tst r0, #536870912 ; 0x20000000 10: 1afffffc bne 8 <hvc_dcc_put_chars+0x8> 14: e7d10003 ldrb r0, [r1, r3] 18: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} 1c: 2afffffd bcs 18 <hvc_dcc_put_chars+0x18> 20: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} 24: e2833001 add r3, r3, #1 ; 0x1 28: e1530002 cmp r3, r2 2c: bafffff5 blt 8 <hvc_dcc_put_chars+0x8> 30: e1a00002 mov r0, r2 34: e12fff1e bx lr which looks better and actually works. Mark all the inline assembly in this file as volatile since we don't want the compiler to optimize away these statements or move them around in any way. Cc: Tony Lindgren <tony@atomide.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Nicolas Pitre <nicolas.pitre@linaro.org> Cc: Daniel Walker <dwalker@codeaurora.org> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- drivers/char/hvc_dcc.c | 11 +++++------ 1 files changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c index 6470f63..155ec10 100644 --- a/drivers/char/hvc_dcc.c +++ b/drivers/char/hvc_dcc.c @@ -33,8 +33,7 @@ static inline u32 __dcc_getstatus(void) { u32 __ret; - - asm("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" + asm volatile("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" : "=r" (__ret) : : "cc"); return __ret; @@ -46,7 +45,7 @@ static inline char __dcc_getchar(void) { char __c; - asm("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ + asm volatile("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ bne get_wait \n\ mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" : "=r" (__c) : : "cc"); @@ -58,7 +57,7 @@ static inline char __dcc_getchar(void) { char __c; - asm("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" + asm volatile("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" : "=r" (__c)); return __c; @@ -68,7 +67,7 @@ static inline char __dcc_getchar(void) #if defined(CONFIG_CPU_V7) static inline void __dcc_putchar(char c) { - asm("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ + asm volatile("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ bcs put_wait \n\ mcr p14, 0, %0, c0, c5, 0 " : : "r" (c) : "cc"); @@ -76,7 +75,7 @@ static inline void __dcc_putchar(char c) #else static inline void __dcc_putchar(char c) { - asm("mcr p14, 0, %0, c0, c5, 0 @ write a char" + asm volatile("mcr p14, 0, %0, c0, c5, 0 @ write a char" : /* no output register */ : "r" (c)); } -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply related [flat|nested] 79+ messages in thread
* [PATCH 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile @ 2010-12-20 20:08 ` Stephen Boyd 0 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2010-12-20 20:08 UTC (permalink / raw) To: linux-arm-kernel Without marking the asm __dcc_getstatus() volatile my compiler decides it can cache the value of __ret in a register and then check the value of it continually in hvc_dcc_put_chars() (I had to replace get_wait/put_wait with 1 and fixup the branch otherwise my disassembler barfed on __dcc_(get|put)char). 00000000 <hvc_dcc_put_chars>: 0: ee103e11 mrc 14, 0, r3, cr0, cr1, {0} 4: e3a0c000 mov ip, #0 ; 0x0 8: e2033202 and r3, r3, #536870912 ; 0x20000000 c: ea000006 b 2c <hvc_dcc_put_chars+0x2c> 10: e3530000 cmp r3, #0 ; 0x0 14: 1afffffd bne 10 <hvc_dcc_put_chars+0x10> 18: e7d1000c ldrb r0, [r1, ip] 1c: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} 20: 2afffffd bcs 1c <hvc_dcc_put_chars+0x1c> 24: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} 28: e28cc001 add ip, ip, #1 ; 0x1 2c: e15c0002 cmp ip, r2 30: bafffff6 blt 10 <hvc_dcc_put_chars+0x10> 34: e1a00002 mov r0, r2 38: e12fff1e bx lr As you can see, the value of the mrc is checked against DCC_STATUS_TX (bit 29) and then stored in r3 for later use. Marking the asm volatile produces the following: 00000000 <hvc_dcc_put_chars>: 0: e3a03000 mov r3, #0 ; 0x0 4: ea000007 b 28 <hvc_dcc_put_chars+0x28> 8: ee100e11 mrc 14, 0, r0, cr0, cr1, {0} c: e3100202 tst r0, #536870912 ; 0x20000000 10: 1afffffc bne 8 <hvc_dcc_put_chars+0x8> 14: e7d10003 ldrb r0, [r1, r3] 18: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} 1c: 2afffffd bcs 18 <hvc_dcc_put_chars+0x18> 20: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} 24: e2833001 add r3, r3, #1 ; 0x1 28: e1530002 cmp r3, r2 2c: bafffff5 blt 8 <hvc_dcc_put_chars+0x8> 30: e1a00002 mov r0, r2 34: e12fff1e bx lr which looks better and actually works. Mark all the inline assembly in this file as volatile since we don't want the compiler to optimize away these statements or move them around in any way. Cc: Tony Lindgren <tony@atomide.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Nicolas Pitre <nicolas.pitre@linaro.org> Cc: Daniel Walker <dwalker@codeaurora.org> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- drivers/char/hvc_dcc.c | 11 +++++------ 1 files changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c index 6470f63..155ec10 100644 --- a/drivers/char/hvc_dcc.c +++ b/drivers/char/hvc_dcc.c @@ -33,8 +33,7 @@ static inline u32 __dcc_getstatus(void) { u32 __ret; - - asm("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" + asm volatile("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" : "=r" (__ret) : : "cc"); return __ret; @@ -46,7 +45,7 @@ static inline char __dcc_getchar(void) { char __c; - asm("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ + asm volatile("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ bne get_wait \n\ mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" : "=r" (__c) : : "cc"); @@ -58,7 +57,7 @@ static inline char __dcc_getchar(void) { char __c; - asm("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" + asm volatile("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" : "=r" (__c)); return __c; @@ -68,7 +67,7 @@ static inline char __dcc_getchar(void) #if defined(CONFIG_CPU_V7) static inline void __dcc_putchar(char c) { - asm("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ + asm volatile("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ bcs put_wait \n\ mcr p14, 0, %0, c0, c5, 0 " : : "r" (c) : "cc"); @@ -76,7 +75,7 @@ static inline void __dcc_putchar(char c) #else static inline void __dcc_putchar(char c) { - asm("mcr p14, 0, %0, c0, c5, 0 @ write a char" + asm volatile("mcr p14, 0, %0, c0, c5, 0 @ write a char" : /* no output register */ : "r" (c)); } -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply related [flat|nested] 79+ messages in thread
* Re: [PATCH 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile 2010-12-20 20:08 ` Stephen Boyd @ 2010-12-20 21:39 ` Nicolas Pitre -1 siblings, 0 replies; 79+ messages in thread From: Nicolas Pitre @ 2010-12-20 21:39 UTC (permalink / raw) To: Stephen Boyd Cc: Greg Kroah-Hartman, linux-kernel, linux-arm-msm, linux-arm-kernel, Tony Lindgren, Arnd Bergmann, Daniel Walker On Mon, 20 Dec 2010, Stephen Boyd wrote: > Without marking the asm __dcc_getstatus() volatile my compiler > decides it can cache the value of __ret in a register and then > check the value of it continually in hvc_dcc_put_chars() (I had > to replace get_wait/put_wait with 1 and fixup the branch > otherwise my disassembler barfed on __dcc_(get|put)char). > > 00000000 <hvc_dcc_put_chars>: > 0: ee103e11 mrc 14, 0, r3, cr0, cr1, {0} > 4: e3a0c000 mov ip, #0 ; 0x0 > 8: e2033202 and r3, r3, #536870912 ; 0x20000000 > c: ea000006 b 2c <hvc_dcc_put_chars+0x2c> > 10: e3530000 cmp r3, #0 ; 0x0 > 14: 1afffffd bne 10 <hvc_dcc_put_chars+0x10> > 18: e7d1000c ldrb r0, [r1, ip] > 1c: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} > 20: 2afffffd bcs 1c <hvc_dcc_put_chars+0x1c> > 24: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} > 28: e28cc001 add ip, ip, #1 ; 0x1 > 2c: e15c0002 cmp ip, r2 > 30: bafffff6 blt 10 <hvc_dcc_put_chars+0x10> > 34: e1a00002 mov r0, r2 > 38: e12fff1e bx lr > > As you can see, the value of the mrc is checked against > DCC_STATUS_TX (bit 29) and then stored in r3 for later use. > Marking the asm volatile produces the following: > > 00000000 <hvc_dcc_put_chars>: > 0: e3a03000 mov r3, #0 ; 0x0 > 4: ea000007 b 28 <hvc_dcc_put_chars+0x28> > 8: ee100e11 mrc 14, 0, r0, cr0, cr1, {0} > c: e3100202 tst r0, #536870912 ; 0x20000000 > 10: 1afffffc bne 8 <hvc_dcc_put_chars+0x8> > 14: e7d10003 ldrb r0, [r1, r3] > 18: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} > 1c: 2afffffd bcs 18 <hvc_dcc_put_chars+0x18> > 20: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} > 24: e2833001 add r3, r3, #1 ; 0x1 > 28: e1530002 cmp r3, r2 > 2c: bafffff5 blt 8 <hvc_dcc_put_chars+0x8> > 30: e1a00002 mov r0, r2 > 34: e12fff1e bx lr > > which looks better and actually works. Mark all the inline > assembly in this file as volatile since we don't want the > compiler to optimize away these statements or move them around > in any way. > > Cc: Tony Lindgren <tony@atomide.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Nicolas Pitre <nicolas.pitre@linaro.org> > Cc: Daniel Walker <dwalker@codeaurora.org> > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org> > --- > drivers/char/hvc_dcc.c | 11 +++++------ > 1 files changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c > index 6470f63..155ec10 100644 > --- a/drivers/char/hvc_dcc.c > +++ b/drivers/char/hvc_dcc.c > @@ -33,8 +33,7 @@ > static inline u32 __dcc_getstatus(void) > { > u32 __ret; > - > - asm("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" > + asm volatile("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" > : "=r" (__ret) : : "cc"); > > return __ret; > @@ -46,7 +45,7 @@ static inline char __dcc_getchar(void) > { > char __c; > > - asm("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ > + asm volatile("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ > bne get_wait \n\ > mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" > : "=r" (__c) : : "cc"); > @@ -58,7 +57,7 @@ static inline char __dcc_getchar(void) > { > char __c; > > - asm("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" > + asm volatile("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" > : "=r" (__c)); > > return __c; > @@ -68,7 +67,7 @@ static inline char __dcc_getchar(void) > #if defined(CONFIG_CPU_V7) > static inline void __dcc_putchar(char c) > { > - asm("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ > + asm volatile("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ > bcs put_wait \n\ > mcr p14, 0, %0, c0, c5, 0 " > : : "r" (c) : "cc"); > @@ -76,7 +75,7 @@ static inline void __dcc_putchar(char c) > #else > static inline void __dcc_putchar(char c) > { > - asm("mcr p14, 0, %0, c0, c5, 0 @ write a char" > + asm volatile("mcr p14, 0, %0, c0, c5, 0 @ write a char" > : /* no output register */ > : "r" (c)); > } > -- > Sent by an employee of the Qualcomm Innovation Center, Inc. > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. > ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile @ 2010-12-20 21:39 ` Nicolas Pitre 0 siblings, 0 replies; 79+ messages in thread From: Nicolas Pitre @ 2010-12-20 21:39 UTC (permalink / raw) To: linux-arm-kernel On Mon, 20 Dec 2010, Stephen Boyd wrote: > Without marking the asm __dcc_getstatus() volatile my compiler > decides it can cache the value of __ret in a register and then > check the value of it continually in hvc_dcc_put_chars() (I had > to replace get_wait/put_wait with 1 and fixup the branch > otherwise my disassembler barfed on __dcc_(get|put)char). > > 00000000 <hvc_dcc_put_chars>: > 0: ee103e11 mrc 14, 0, r3, cr0, cr1, {0} > 4: e3a0c000 mov ip, #0 ; 0x0 > 8: e2033202 and r3, r3, #536870912 ; 0x20000000 > c: ea000006 b 2c <hvc_dcc_put_chars+0x2c> > 10: e3530000 cmp r3, #0 ; 0x0 > 14: 1afffffd bne 10 <hvc_dcc_put_chars+0x10> > 18: e7d1000c ldrb r0, [r1, ip] > 1c: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} > 20: 2afffffd bcs 1c <hvc_dcc_put_chars+0x1c> > 24: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} > 28: e28cc001 add ip, ip, #1 ; 0x1 > 2c: e15c0002 cmp ip, r2 > 30: bafffff6 blt 10 <hvc_dcc_put_chars+0x10> > 34: e1a00002 mov r0, r2 > 38: e12fff1e bx lr > > As you can see, the value of the mrc is checked against > DCC_STATUS_TX (bit 29) and then stored in r3 for later use. > Marking the asm volatile produces the following: > > 00000000 <hvc_dcc_put_chars>: > 0: e3a03000 mov r3, #0 ; 0x0 > 4: ea000007 b 28 <hvc_dcc_put_chars+0x28> > 8: ee100e11 mrc 14, 0, r0, cr0, cr1, {0} > c: e3100202 tst r0, #536870912 ; 0x20000000 > 10: 1afffffc bne 8 <hvc_dcc_put_chars+0x8> > 14: e7d10003 ldrb r0, [r1, r3] > 18: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} > 1c: 2afffffd bcs 18 <hvc_dcc_put_chars+0x18> > 20: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} > 24: e2833001 add r3, r3, #1 ; 0x1 > 28: e1530002 cmp r3, r2 > 2c: bafffff5 blt 8 <hvc_dcc_put_chars+0x8> > 30: e1a00002 mov r0, r2 > 34: e12fff1e bx lr > > which looks better and actually works. Mark all the inline > assembly in this file as volatile since we don't want the > compiler to optimize away these statements or move them around > in any way. > > Cc: Tony Lindgren <tony@atomide.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Nicolas Pitre <nicolas.pitre@linaro.org> > Cc: Daniel Walker <dwalker@codeaurora.org> > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org> > --- > drivers/char/hvc_dcc.c | 11 +++++------ > 1 files changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c > index 6470f63..155ec10 100644 > --- a/drivers/char/hvc_dcc.c > +++ b/drivers/char/hvc_dcc.c > @@ -33,8 +33,7 @@ > static inline u32 __dcc_getstatus(void) > { > u32 __ret; > - > - asm("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" > + asm volatile("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" > : "=r" (__ret) : : "cc"); > > return __ret; > @@ -46,7 +45,7 @@ static inline char __dcc_getchar(void) > { > char __c; > > - asm("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ > + asm volatile("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ > bne get_wait \n\ > mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" > : "=r" (__c) : : "cc"); > @@ -58,7 +57,7 @@ static inline char __dcc_getchar(void) > { > char __c; > > - asm("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" > + asm volatile("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" > : "=r" (__c)); > > return __c; > @@ -68,7 +67,7 @@ static inline char __dcc_getchar(void) > #if defined(CONFIG_CPU_V7) > static inline void __dcc_putchar(char c) > { > - asm("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ > + asm volatile("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ > bcs put_wait \n\ > mcr p14, 0, %0, c0, c5, 0 " > : : "r" (c) : "cc"); > @@ -76,7 +75,7 @@ static inline void __dcc_putchar(char c) > #else > static inline void __dcc_putchar(char c) > { > - asm("mcr p14, 0, %0, c0, c5, 0 @ write a char" > + asm volatile("mcr p14, 0, %0, c0, c5, 0 @ write a char" > : /* no output register */ > : "r" (c)); > } > -- > Sent by an employee of the Qualcomm Innovation Center, Inc. > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. > ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile 2010-12-20 21:39 ` Nicolas Pitre @ 2011-01-02 9:00 ` Pavel Machek -1 siblings, 0 replies; 79+ messages in thread From: Pavel Machek @ 2011-01-02 9:00 UTC (permalink / raw) To: Nicolas Pitre Cc: Stephen Boyd, Greg Kroah-Hartman, linux-kernel, linux-arm-msm, linux-arm-kernel, Tony Lindgren, Arnd Bergmann, Daniel Walker Hi! > > diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c > > index 6470f63..155ec10 100644 > > --- a/drivers/char/hvc_dcc.c > > +++ b/drivers/char/hvc_dcc.c > > @@ -33,8 +33,7 @@ > > static inline u32 __dcc_getstatus(void) > > { > > u32 __ret; > > - > > - asm("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" > > + asm volatile("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" > > : "=r" (__ret) : : "cc"); > > > > return __ret; Is volatile needed here? If __dcc_getstatus() return value is discarded, we want assembly discarded, right? > > @@ -58,7 +57,7 @@ static inline char __dcc_getchar(void) > > { > > char __c; > > > > - asm("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" > > + asm volatile("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" > > : "=r" (__c)); > > > > return __c; Same here? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile @ 2011-01-02 9:00 ` Pavel Machek 0 siblings, 0 replies; 79+ messages in thread From: Pavel Machek @ 2011-01-02 9:00 UTC (permalink / raw) To: linux-arm-kernel Hi! > > diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c > > index 6470f63..155ec10 100644 > > --- a/drivers/char/hvc_dcc.c > > +++ b/drivers/char/hvc_dcc.c > > @@ -33,8 +33,7 @@ > > static inline u32 __dcc_getstatus(void) > > { > > u32 __ret; > > - > > - asm("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" > > + asm volatile("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" > > : "=r" (__ret) : : "cc"); > > > > return __ret; Is volatile needed here? If __dcc_getstatus() return value is discarded, we want assembly discarded, right? > > @@ -58,7 +57,7 @@ static inline char __dcc_getchar(void) > > { > > char __c; > > > > - asm("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" > > + asm volatile("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" > > : "=r" (__c)); > > > > return __c; Same here? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile 2011-01-02 9:00 ` Pavel Machek @ 2011-01-02 18:49 ` David Brown -1 siblings, 0 replies; 79+ messages in thread From: David Brown @ 2011-01-02 18:49 UTC (permalink / raw) To: Pavel Machek Cc: Nicolas Pitre, Stephen Boyd, Greg Kroah-Hartman, linux-kernel, linux-arm-msm, linux-arm-kernel, Tony Lindgren, Arnd Bergmann, Daniel Walker On Sun, Jan 02 2011, Pavel Machek wrote: >> > diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c >> > index 6470f63..155ec10 100644 >> > --- a/drivers/char/hvc_dcc.c >> > +++ b/drivers/char/hvc_dcc.c >> > @@ -33,8 +33,7 @@ >> > static inline u32 __dcc_getstatus(void) >> > { >> > u32 __ret; >> > - >> > - asm("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" >> > + asm volatile("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" >> > : "=r" (__ret) : : "cc"); >> > >> > return __ret; > > Is volatile needed here? If __dcc_getstatus() return value is > discarded, we want assembly discarded, right? That's not really the issue being fixed. Without the volatile, the compiler is free to cache and reuse a previously loaded status value. It is important that the status be read each time. I don't think there is a way of indicating that assembly needs to happen for each use, but that it is OK to discard if the value isn't used. 'volatile' is a bit overloaded. -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile @ 2011-01-02 18:49 ` David Brown 0 siblings, 0 replies; 79+ messages in thread From: David Brown @ 2011-01-02 18:49 UTC (permalink / raw) To: linux-arm-kernel On Sun, Jan 02 2011, Pavel Machek wrote: >> > diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c >> > index 6470f63..155ec10 100644 >> > --- a/drivers/char/hvc_dcc.c >> > +++ b/drivers/char/hvc_dcc.c >> > @@ -33,8 +33,7 @@ >> > static inline u32 __dcc_getstatus(void) >> > { >> > u32 __ret; >> > - >> > - asm("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" >> > + asm volatile("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" >> > : "=r" (__ret) : : "cc"); >> > >> > return __ret; > > Is volatile needed here? If __dcc_getstatus() return value is > discarded, we want assembly discarded, right? That's not really the issue being fixed. Without the volatile, the compiler is free to cache and reuse a previously loaded status value. It is important that the status be read each time. I don't think there is a way of indicating that assembly needs to happen for each use, but that it is OK to discard if the value isn't used. 'volatile' is a bit overloaded. -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile 2011-01-02 18:49 ` David Brown @ 2011-01-03 5:50 ` Pavel Machek -1 siblings, 0 replies; 79+ messages in thread From: Pavel Machek @ 2011-01-03 5:50 UTC (permalink / raw) To: David Brown Cc: Nicolas Pitre, Stephen Boyd, Greg Kroah-Hartman, linux-kernel, linux-arm-msm, linux-arm-kernel, Tony Lindgren, Arnd Bergmann, Daniel Walker On Sun 2011-01-02 10:49:32, David Brown wrote: > On Sun, Jan 02 2011, Pavel Machek wrote: > > >> > diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c > >> > index 6470f63..155ec10 100644 > >> > --- a/drivers/char/hvc_dcc.c > >> > +++ b/drivers/char/hvc_dcc.c > >> > @@ -33,8 +33,7 @@ > >> > static inline u32 __dcc_getstatus(void) > >> > { > >> > u32 __ret; > >> > - > >> > - asm("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" > >> > + asm volatile("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" > >> > : "=r" (__ret) : : "cc"); > >> > > >> > return __ret; > > > > Is volatile needed here? If __dcc_getstatus() return value is > > discarded, we want assembly discarded, right? > > That's not really the issue being fixed. Without the volatile, the > compiler is free to cache and reuse a previously loaded status value. > It is important that the status be read each time. > > I don't think there is a way of indicating that assembly needs to happen > for each use, but that it is OK to discard if the value isn't used. > 'volatile' is a bit overloaded. Ok, thanks for explanation. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile @ 2011-01-03 5:50 ` Pavel Machek 0 siblings, 0 replies; 79+ messages in thread From: Pavel Machek @ 2011-01-03 5:50 UTC (permalink / raw) To: linux-arm-kernel On Sun 2011-01-02 10:49:32, David Brown wrote: > On Sun, Jan 02 2011, Pavel Machek wrote: > > >> > diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c > >> > index 6470f63..155ec10 100644 > >> > --- a/drivers/char/hvc_dcc.c > >> > +++ b/drivers/char/hvc_dcc.c > >> > @@ -33,8 +33,7 @@ > >> > static inline u32 __dcc_getstatus(void) > >> > { > >> > u32 __ret; > >> > - > >> > - asm("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" > >> > + asm volatile("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" > >> > : "=r" (__ret) : : "cc"); > >> > > >> > return __ret; > > > > Is volatile needed here? If __dcc_getstatus() return value is > > discarded, we want assembly discarded, right? > > That's not really the issue being fixed. Without the volatile, the > compiler is free to cache and reuse a previously loaded status value. > It is important that the status be read each time. > > I don't think there is a way of indicating that assembly needs to happen > for each use, but that it is OK to discard if the value isn't used. > 'volatile' is a bit overloaded. Ok, thanks for explanation. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile 2010-12-20 21:39 ` Nicolas Pitre @ 2011-01-04 18:49 ` Tony Lindgren -1 siblings, 0 replies; 79+ messages in thread From: Tony Lindgren @ 2011-01-04 18:49 UTC (permalink / raw) To: Nicolas Pitre Cc: Stephen Boyd, Greg Kroah-Hartman, linux-kernel, linux-arm-msm, linux-arm-kernel, Arnd Bergmann, Daniel Walker * Nicolas Pitre <nico@fluxnic.net> [101220 13:38]: > On Mon, 20 Dec 2010, Stephen Boyd wrote: > > > Without marking the asm __dcc_getstatus() volatile my compiler > > decides it can cache the value of __ret in a register and then > > check the value of it continually in hvc_dcc_put_chars() (I had > > to replace get_wait/put_wait with 1 and fixup the branch > > otherwise my disassembler barfed on __dcc_(get|put)char). > > > > 00000000 <hvc_dcc_put_chars>: > > 0: ee103e11 mrc 14, 0, r3, cr0, cr1, {0} > > 4: e3a0c000 mov ip, #0 ; 0x0 > > 8: e2033202 and r3, r3, #536870912 ; 0x20000000 > > c: ea000006 b 2c <hvc_dcc_put_chars+0x2c> > > 10: e3530000 cmp r3, #0 ; 0x0 > > 14: 1afffffd bne 10 <hvc_dcc_put_chars+0x10> > > 18: e7d1000c ldrb r0, [r1, ip] > > 1c: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} > > 20: 2afffffd bcs 1c <hvc_dcc_put_chars+0x1c> > > 24: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} > > 28: e28cc001 add ip, ip, #1 ; 0x1 > > 2c: e15c0002 cmp ip, r2 > > 30: bafffff6 blt 10 <hvc_dcc_put_chars+0x10> > > 34: e1a00002 mov r0, r2 > > 38: e12fff1e bx lr > > > > As you can see, the value of the mrc is checked against > > DCC_STATUS_TX (bit 29) and then stored in r3 for later use. > > Marking the asm volatile produces the following: > > > > 00000000 <hvc_dcc_put_chars>: > > 0: e3a03000 mov r3, #0 ; 0x0 > > 4: ea000007 b 28 <hvc_dcc_put_chars+0x28> > > 8: ee100e11 mrc 14, 0, r0, cr0, cr1, {0} > > c: e3100202 tst r0, #536870912 ; 0x20000000 > > 10: 1afffffc bne 8 <hvc_dcc_put_chars+0x8> > > 14: e7d10003 ldrb r0, [r1, r3] > > 18: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} > > 1c: 2afffffd bcs 18 <hvc_dcc_put_chars+0x18> > > 20: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} > > 24: e2833001 add r3, r3, #1 ; 0x1 > > 28: e1530002 cmp r3, r2 > > 2c: bafffff5 blt 8 <hvc_dcc_put_chars+0x8> > > 30: e1a00002 mov r0, r2 > > 34: e12fff1e bx lr > > > > which looks better and actually works. Mark all the inline > > assembly in this file as volatile since we don't want the > > compiler to optimize away these statements or move them around > > in any way. > > > > Cc: Tony Lindgren <tony@atomide.com> > > Cc: Arnd Bergmann <arnd@arndb.de> > > Cc: Nicolas Pitre <nicolas.pitre@linaro.org> > > Cc: Daniel Walker <dwalker@codeaurora.org> > > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> > > Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org> Acked-by: Tony Lindgren <tony@atomide.com> ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile @ 2011-01-04 18:49 ` Tony Lindgren 0 siblings, 0 replies; 79+ messages in thread From: Tony Lindgren @ 2011-01-04 18:49 UTC (permalink / raw) To: linux-arm-kernel * Nicolas Pitre <nico@fluxnic.net> [101220 13:38]: > On Mon, 20 Dec 2010, Stephen Boyd wrote: > > > Without marking the asm __dcc_getstatus() volatile my compiler > > decides it can cache the value of __ret in a register and then > > check the value of it continually in hvc_dcc_put_chars() (I had > > to replace get_wait/put_wait with 1 and fixup the branch > > otherwise my disassembler barfed on __dcc_(get|put)char). > > > > 00000000 <hvc_dcc_put_chars>: > > 0: ee103e11 mrc 14, 0, r3, cr0, cr1, {0} > > 4: e3a0c000 mov ip, #0 ; 0x0 > > 8: e2033202 and r3, r3, #536870912 ; 0x20000000 > > c: ea000006 b 2c <hvc_dcc_put_chars+0x2c> > > 10: e3530000 cmp r3, #0 ; 0x0 > > 14: 1afffffd bne 10 <hvc_dcc_put_chars+0x10> > > 18: e7d1000c ldrb r0, [r1, ip] > > 1c: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} > > 20: 2afffffd bcs 1c <hvc_dcc_put_chars+0x1c> > > 24: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} > > 28: e28cc001 add ip, ip, #1 ; 0x1 > > 2c: e15c0002 cmp ip, r2 > > 30: bafffff6 blt 10 <hvc_dcc_put_chars+0x10> > > 34: e1a00002 mov r0, r2 > > 38: e12fff1e bx lr > > > > As you can see, the value of the mrc is checked against > > DCC_STATUS_TX (bit 29) and then stored in r3 for later use. > > Marking the asm volatile produces the following: > > > > 00000000 <hvc_dcc_put_chars>: > > 0: e3a03000 mov r3, #0 ; 0x0 > > 4: ea000007 b 28 <hvc_dcc_put_chars+0x28> > > 8: ee100e11 mrc 14, 0, r0, cr0, cr1, {0} > > c: e3100202 tst r0, #536870912 ; 0x20000000 > > 10: 1afffffc bne 8 <hvc_dcc_put_chars+0x8> > > 14: e7d10003 ldrb r0, [r1, r3] > > 18: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} > > 1c: 2afffffd bcs 18 <hvc_dcc_put_chars+0x18> > > 20: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} > > 24: e2833001 add r3, r3, #1 ; 0x1 > > 28: e1530002 cmp r3, r2 > > 2c: bafffff5 blt 8 <hvc_dcc_put_chars+0x8> > > 30: e1a00002 mov r0, r2 > > 34: e12fff1e bx lr > > > > which looks better and actually works. Mark all the inline > > assembly in this file as volatile since we don't want the > > compiler to optimize away these statements or move them around > > in any way. > > > > Cc: Tony Lindgren <tony@atomide.com> > > Cc: Arnd Bergmann <arnd@arndb.de> > > Cc: Nicolas Pitre <nicolas.pitre@linaro.org> > > Cc: Daniel Walker <dwalker@codeaurora.org> > > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> > > Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org> Acked-by: Tony Lindgren <tony@atomide.com> ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile 2010-12-20 20:08 ` Stephen Boyd @ 2010-12-20 21:49 ` Arnaud Lacombe -1 siblings, 0 replies; 79+ messages in thread From: Arnaud Lacombe @ 2010-12-20 21:49 UTC (permalink / raw) To: Stephen Boyd Cc: Greg Kroah-Hartman, linux-kernel, linux-arm-msm, linux-arm-kernel, Tony Lindgren, Arnd Bergmann, Nicolas Pitre, Daniel Walker Hi, On Mon, Dec 20, 2010 at 3:08 PM, Stephen Boyd <sboyd@codeaurora.org> wrote: > Without marking the asm __dcc_getstatus() volatile my compiler > decides [...] What compiler ? That might be a usefull information to know, espectially 5 years from now when tracing code history. There has been similar issue with gcc 4.5 recently. AFAIK, in the same idea, the final change has been to mark the asm volatile. Thanks, - Arnaud ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile @ 2010-12-20 21:49 ` Arnaud Lacombe 0 siblings, 0 replies; 79+ messages in thread From: Arnaud Lacombe @ 2010-12-20 21:49 UTC (permalink / raw) To: linux-arm-kernel Hi, On Mon, Dec 20, 2010 at 3:08 PM, Stephen Boyd <sboyd@codeaurora.org> wrote: > Without marking the asm __dcc_getstatus() volatile my compiler > decides [...] What compiler ? That might be a usefull information to know, espectially 5 years from now when tracing code history. There has been similar issue with gcc 4.5 recently. AFAIK, in the same idea, the final change has been to mark the asm volatile. Thanks, - Arnaud ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile 2010-12-20 21:49 ` Arnaud Lacombe @ 2010-12-20 21:52 ` Stephen Boyd -1 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2010-12-20 21:52 UTC (permalink / raw) To: Arnaud Lacombe Cc: Greg Kroah-Hartman, linux-kernel, linux-arm-msm, linux-arm-kernel, Tony Lindgren, Arnd Bergmann, Nicolas Pitre, Daniel Walker On 12/20/2010 01:49 PM, Arnaud Lacombe wrote: > Hi, > > On Mon, Dec 20, 2010 at 3:08 PM, Stephen Boyd <sboyd@codeaurora.org> wrote: >> Without marking the asm __dcc_getstatus() volatile my compiler >> decides [...] > What compiler ? That might be a usefull information to know, > espectially 5 years from now when tracing code history. There has been > similar issue with gcc 4.5 recently. AFAIK, in the same idea, the > final change has been to mark the asm volatile. Sure, we can replace "my compiler" with "my compiler (arm-eabi-gcc (GCC) 4.4.0)". -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile @ 2010-12-20 21:52 ` Stephen Boyd 0 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2010-12-20 21:52 UTC (permalink / raw) To: linux-arm-kernel On 12/20/2010 01:49 PM, Arnaud Lacombe wrote: > Hi, > > On Mon, Dec 20, 2010 at 3:08 PM, Stephen Boyd <sboyd@codeaurora.org> wrote: >> Without marking the asm __dcc_getstatus() volatile my compiler >> decides [...] > What compiler ? That might be a usefull information to know, > espectially 5 years from now when tracing code history. There has been > similar issue with gcc 4.5 recently. AFAIK, in the same idea, the > final change has been to mark the asm volatile. Sure, we can replace "my compiler" with "my compiler (arm-eabi-gcc (GCC) 4.4.0)". -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile 2010-12-20 21:52 ` Stephen Boyd @ 2010-12-20 22:10 ` Nicolas Pitre -1 siblings, 0 replies; 79+ messages in thread From: Nicolas Pitre @ 2010-12-20 22:10 UTC (permalink / raw) To: Stephen Boyd Cc: Arnaud Lacombe, Greg Kroah-Hartman, lkml, linux-arm-msm, linux-arm-kernel, Tony Lindgren, Arnd Bergmann, Daniel Walker On Mon, 20 Dec 2010, Stephen Boyd wrote: > On 12/20/2010 01:49 PM, Arnaud Lacombe wrote: > > Hi, > > > > On Mon, Dec 20, 2010 at 3:08 PM, Stephen Boyd <sboyd@codeaurora.org> wrote: > >> Without marking the asm __dcc_getstatus() volatile my compiler > >> decides [...] > > What compiler ? That might be a usefull information to know, > > espectially 5 years from now when tracing code history. There has been > > similar issue with gcc 4.5 recently. AFAIK, in the same idea, the > > final change has been to mark the asm volatile. > > Sure, we can replace "my compiler" with "my compiler (arm-eabi-gcc (GCC) > 4.4.0)". Compiler version doesn't matter -- this is a simple correctness issue. If an inline asm statement provides an output value then the compiler is free to cache that value in a register, unless the inline asm is marked so that the value may change from one invocation to another. Also, the compiler is free to eliminate the inline asm statement entirely as well if the output value provided by that inline asm is never used... unless if the inline asm is marked as having side effects. In both cases the volatile qualifier does indicate that the returned value may change (new status flag state) and that the asm code therein has side effects (e.g. pop a character off a FIFO). Sometimes the desired effect can be indicated by clever usage of parameter constraints, but in this case the volatile keyword is most appropriate. Nicolas ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile @ 2010-12-20 22:10 ` Nicolas Pitre 0 siblings, 0 replies; 79+ messages in thread From: Nicolas Pitre @ 2010-12-20 22:10 UTC (permalink / raw) To: linux-arm-kernel On Mon, 20 Dec 2010, Stephen Boyd wrote: > On 12/20/2010 01:49 PM, Arnaud Lacombe wrote: > > Hi, > > > > On Mon, Dec 20, 2010 at 3:08 PM, Stephen Boyd <sboyd@codeaurora.org> wrote: > >> Without marking the asm __dcc_getstatus() volatile my compiler > >> decides [...] > > What compiler ? That might be a usefull information to know, > > espectially 5 years from now when tracing code history. There has been > > similar issue with gcc 4.5 recently. AFAIK, in the same idea, the > > final change has been to mark the asm volatile. > > Sure, we can replace "my compiler" with "my compiler (arm-eabi-gcc (GCC) > 4.4.0)". Compiler version doesn't matter -- this is a simple correctness issue. If an inline asm statement provides an output value then the compiler is free to cache that value in a register, unless the inline asm is marked so that the value may change from one invocation to another. Also, the compiler is free to eliminate the inline asm statement entirely as well if the output value provided by that inline asm is never used... unless if the inline asm is marked as having side effects. In both cases the volatile qualifier does indicate that the returned value may change (new status flag state) and that the asm code therein has side effects (e.g. pop a character off a FIFO). Sometimes the desired effect can be indicated by clever usage of parameter constraints, but in this case the volatile keyword is most appropriate. Nicolas ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH 2/3] hvc_dcc: Simplify put_chars()/get_chars() loops 2010-12-20 20:08 ` Stephen Boyd @ 2010-12-20 20:08 ` Stephen Boyd -1 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2010-12-20 20:08 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, Daniel Walker Casting and anding with 0xff is unnecessary in hvc_dcc_put_chars() since buf is already a char[]. __dcc_get_char() can't return an int less than 0 since it only returns a char. Simplify the if statement in hvc_dcc_get_chars() to take this into account. Cc: Daniel Walker <dwalker@codeaurora.org> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- drivers/char/hvc_dcc.c | 12 ++++-------- 1 files changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c index 155ec10..ad23cc8 100644 --- a/drivers/char/hvc_dcc.c +++ b/drivers/char/hvc_dcc.c @@ -89,7 +89,7 @@ static int hvc_dcc_put_chars(uint32_t vt, const char *buf, int count) while (__dcc_getstatus() & DCC_STATUS_TX) cpu_relax(); - __dcc_putchar((char)(buf[i] & 0xFF)); + __dcc_putchar(buf[i]); } return count; @@ -99,15 +99,11 @@ static int hvc_dcc_get_chars(uint32_t vt, char *buf, int count) { int i; - for (i = 0; i < count; ++i) { - int c = -1; - + for (i = 0; i < count; ++i) if (__dcc_getstatus() & DCC_STATUS_RX) - c = __dcc_getchar(); - if (c < 0) + buf[i] = __dcc_getchar(); + else break; - buf[i] = c; - } return i; } -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply related [flat|nested] 79+ messages in thread
* [PATCH 2/3] hvc_dcc: Simplify put_chars()/get_chars() loops @ 2010-12-20 20:08 ` Stephen Boyd 0 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2010-12-20 20:08 UTC (permalink / raw) To: linux-arm-kernel Casting and anding with 0xff is unnecessary in hvc_dcc_put_chars() since buf is already a char[]. __dcc_get_char() can't return an int less than 0 since it only returns a char. Simplify the if statement in hvc_dcc_get_chars() to take this into account. Cc: Daniel Walker <dwalker@codeaurora.org> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- drivers/char/hvc_dcc.c | 12 ++++-------- 1 files changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c index 155ec10..ad23cc8 100644 --- a/drivers/char/hvc_dcc.c +++ b/drivers/char/hvc_dcc.c @@ -89,7 +89,7 @@ static int hvc_dcc_put_chars(uint32_t vt, const char *buf, int count) while (__dcc_getstatus() & DCC_STATUS_TX) cpu_relax(); - __dcc_putchar((char)(buf[i] & 0xFF)); + __dcc_putchar(buf[i]); } return count; @@ -99,15 +99,11 @@ static int hvc_dcc_get_chars(uint32_t vt, char *buf, int count) { int i; - for (i = 0; i < count; ++i) { - int c = -1; - + for (i = 0; i < count; ++i) if (__dcc_getstatus() & DCC_STATUS_RX) - c = __dcc_getchar(); - if (c < 0) + buf[i] = __dcc_getchar(); + else break; - buf[i] = c; - } return i; } -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply related [flat|nested] 79+ messages in thread
* [PATCH 3/3] hvc_dcc: Simplify assembly for v6 and v7 ARM 2010-12-20 20:08 ` Stephen Boyd @ 2010-12-20 20:08 ` Stephen Boyd -1 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2010-12-20 20:08 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, Tony Lindgren, Arnd Bergmann, Nicolas Pitre, Daniel Walker The inline assembly differences for v6 vs. v7 in the hvc_dcc driver are purely optimizations. On a v7 processor, an mrc with the pc sets the condition codes to the 28-31 bits of the register being read. It just so happens that the TX/RX full bits the DCC driver is testing for are high enough in the register to be put into the condition codes. On a v6 processor, this "feature" isn't implemented and thus we have to do the usual read, mask, test operations to check for TX/RX full. Since we already test the RX/TX full bits before calling __dcc_getchar() and __dcc_putchar() we don't actually need to do anything special for v7 over v6. The only difference is in hvc_dcc_get_chars(). We would test RX full, poll RX full, and then read a character from the buffer, whereas now we will test RX full, read a character from the buffer, and then test RX full again for the second iteration of the loop. It doesn't seem possible for the buffer to go from full to empty between testing the RX full and reading a character. Therefore, replace the v7 versions with the v6 versions and everything works the same. Cc: Tony Lindgren <tony@atomide.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Nicolas Pitre <nicolas.pitre@linaro.org> Cc: Daniel Walker <dwalker@codeaurora.org> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- drivers/char/hvc_dcc.c | 24 ------------------------ 1 files changed, 0 insertions(+), 24 deletions(-) diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c index ad23cc8..435f6fa 100644 --- a/drivers/char/hvc_dcc.c +++ b/drivers/char/hvc_dcc.c @@ -40,19 +40,6 @@ static inline u32 __dcc_getstatus(void) } -#if defined(CONFIG_CPU_V7) -static inline char __dcc_getchar(void) -{ - char __c; - - asm volatile("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ - bne get_wait \n\ - mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" - : "=r" (__c) : : "cc"); - - return __c; -} -#else static inline char __dcc_getchar(void) { char __c; @@ -62,24 +49,13 @@ static inline char __dcc_getchar(void) return __c; } -#endif -#if defined(CONFIG_CPU_V7) -static inline void __dcc_putchar(char c) -{ - asm volatile("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ - bcs put_wait \n\ - mcr p14, 0, %0, c0, c5, 0 " - : : "r" (c) : "cc"); -} -#else static inline void __dcc_putchar(char c) { asm volatile("mcr p14, 0, %0, c0, c5, 0 @ write a char" : /* no output register */ : "r" (c)); } -#endif static int hvc_dcc_put_chars(uint32_t vt, const char *buf, int count) { -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply related [flat|nested] 79+ messages in thread
* [PATCH 3/3] hvc_dcc: Simplify assembly for v6 and v7 ARM @ 2010-12-20 20:08 ` Stephen Boyd 0 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2010-12-20 20:08 UTC (permalink / raw) To: linux-arm-kernel The inline assembly differences for v6 vs. v7 in the hvc_dcc driver are purely optimizations. On a v7 processor, an mrc with the pc sets the condition codes to the 28-31 bits of the register being read. It just so happens that the TX/RX full bits the DCC driver is testing for are high enough in the register to be put into the condition codes. On a v6 processor, this "feature" isn't implemented and thus we have to do the usual read, mask, test operations to check for TX/RX full. Since we already test the RX/TX full bits before calling __dcc_getchar() and __dcc_putchar() we don't actually need to do anything special for v7 over v6. The only difference is in hvc_dcc_get_chars(). We would test RX full, poll RX full, and then read a character from the buffer, whereas now we will test RX full, read a character from the buffer, and then test RX full again for the second iteration of the loop. It doesn't seem possible for the buffer to go from full to empty between testing the RX full and reading a character. Therefore, replace the v7 versions with the v6 versions and everything works the same. Cc: Tony Lindgren <tony@atomide.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Nicolas Pitre <nicolas.pitre@linaro.org> Cc: Daniel Walker <dwalker@codeaurora.org> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- drivers/char/hvc_dcc.c | 24 ------------------------ 1 files changed, 0 insertions(+), 24 deletions(-) diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c index ad23cc8..435f6fa 100644 --- a/drivers/char/hvc_dcc.c +++ b/drivers/char/hvc_dcc.c @@ -40,19 +40,6 @@ static inline u32 __dcc_getstatus(void) } -#if defined(CONFIG_CPU_V7) -static inline char __dcc_getchar(void) -{ - char __c; - - asm volatile("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ - bne get_wait \n\ - mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" - : "=r" (__c) : : "cc"); - - return __c; -} -#else static inline char __dcc_getchar(void) { char __c; @@ -62,24 +49,13 @@ static inline char __dcc_getchar(void) return __c; } -#endif -#if defined(CONFIG_CPU_V7) -static inline void __dcc_putchar(char c) -{ - asm volatile("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ - bcs put_wait \n\ - mcr p14, 0, %0, c0, c5, 0 " - : : "r" (c) : "cc"); -} -#else static inline void __dcc_putchar(char c) { asm volatile("mcr p14, 0, %0, c0, c5, 0 @ write a char" : /* no output register */ : "r" (c)); } -#endif static int hvc_dcc_put_chars(uint32_t vt, const char *buf, int count) { -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply related [flat|nested] 79+ messages in thread
* Re: [PATCH 3/3] hvc_dcc: Simplify assembly for v6 and v7 ARM 2010-12-20 20:08 ` Stephen Boyd @ 2010-12-20 21:44 ` Nicolas Pitre -1 siblings, 0 replies; 79+ messages in thread From: Nicolas Pitre @ 2010-12-20 21:44 UTC (permalink / raw) To: Stephen Boyd Cc: Greg Kroah-Hartman, linux-kernel, linux-arm-msm, linux-arm-kernel, Tony Lindgren, Arnd Bergmann, Daniel Walker On Mon, 20 Dec 2010, Stephen Boyd wrote: > The inline assembly differences for v6 vs. v7 in the hvc_dcc > driver are purely optimizations. On a v7 processor, an mrc with > the pc sets the condition codes to the 28-31 bits of the register > being read. It just so happens that the TX/RX full bits the DCC > driver is testing for are high enough in the register to be put > into the condition codes. On a v6 processor, this "feature" isn't > implemented and thus we have to do the usual read, mask, test > operations to check for TX/RX full. > > Since we already test the RX/TX full bits before calling > __dcc_getchar() and __dcc_putchar() we don't actually need to do > anything special for v7 over v6. The only difference is in > hvc_dcc_get_chars(). We would test RX full, poll RX full, and > then read a character from the buffer, whereas now we will test > RX full, read a character from the buffer, and then test RX full > again for the second iteration of the loop. It doesn't seem > possible for the buffer to go from full to empty between testing > the RX full and reading a character. Therefore, replace the v7 > versions with the v6 versions and everything works the same. > > Cc: Tony Lindgren <tony@atomide.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Nicolas Pitre <nicolas.pitre@linaro.org> > Cc: Daniel Walker <dwalker@codeaurora.org> > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org> > --- > drivers/char/hvc_dcc.c | 24 ------------------------ > 1 files changed, 0 insertions(+), 24 deletions(-) > > diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c > index ad23cc8..435f6fa 100644 > --- a/drivers/char/hvc_dcc.c > +++ b/drivers/char/hvc_dcc.c > @@ -40,19 +40,6 @@ static inline u32 __dcc_getstatus(void) > } > > > -#if defined(CONFIG_CPU_V7) > -static inline char __dcc_getchar(void) > -{ > - char __c; > - > - asm volatile("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ > - bne get_wait \n\ > - mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" > - : "=r" (__c) : : "cc"); > - > - return __c; > -} > -#else > static inline char __dcc_getchar(void) > { > char __c; > @@ -62,24 +49,13 @@ static inline char __dcc_getchar(void) > > return __c; > } > -#endif > > -#if defined(CONFIG_CPU_V7) > -static inline void __dcc_putchar(char c) > -{ > - asm volatile("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ > - bcs put_wait \n\ > - mcr p14, 0, %0, c0, c5, 0 " > - : : "r" (c) : "cc"); > -} > -#else > static inline void __dcc_putchar(char c) > { > asm volatile("mcr p14, 0, %0, c0, c5, 0 @ write a char" > : /* no output register */ > : "r" (c)); > } > -#endif > > static int hvc_dcc_put_chars(uint32_t vt, const char *buf, int count) > { > -- > Sent by an employee of the Qualcomm Innovation Center, Inc. > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. > ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH 3/3] hvc_dcc: Simplify assembly for v6 and v7 ARM @ 2010-12-20 21:44 ` Nicolas Pitre 0 siblings, 0 replies; 79+ messages in thread From: Nicolas Pitre @ 2010-12-20 21:44 UTC (permalink / raw) To: linux-arm-kernel On Mon, 20 Dec 2010, Stephen Boyd wrote: > The inline assembly differences for v6 vs. v7 in the hvc_dcc > driver are purely optimizations. On a v7 processor, an mrc with > the pc sets the condition codes to the 28-31 bits of the register > being read. It just so happens that the TX/RX full bits the DCC > driver is testing for are high enough in the register to be put > into the condition codes. On a v6 processor, this "feature" isn't > implemented and thus we have to do the usual read, mask, test > operations to check for TX/RX full. > > Since we already test the RX/TX full bits before calling > __dcc_getchar() and __dcc_putchar() we don't actually need to do > anything special for v7 over v6. The only difference is in > hvc_dcc_get_chars(). We would test RX full, poll RX full, and > then read a character from the buffer, whereas now we will test > RX full, read a character from the buffer, and then test RX full > again for the second iteration of the loop. It doesn't seem > possible for the buffer to go from full to empty between testing > the RX full and reading a character. Therefore, replace the v7 > versions with the v6 versions and everything works the same. > > Cc: Tony Lindgren <tony@atomide.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Nicolas Pitre <nicolas.pitre@linaro.org> > Cc: Daniel Walker <dwalker@codeaurora.org> > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org> > --- > drivers/char/hvc_dcc.c | 24 ------------------------ > 1 files changed, 0 insertions(+), 24 deletions(-) > > diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c > index ad23cc8..435f6fa 100644 > --- a/drivers/char/hvc_dcc.c > +++ b/drivers/char/hvc_dcc.c > @@ -40,19 +40,6 @@ static inline u32 __dcc_getstatus(void) > } > > > -#if defined(CONFIG_CPU_V7) > -static inline char __dcc_getchar(void) > -{ > - char __c; > - > - asm volatile("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ > - bne get_wait \n\ > - mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" > - : "=r" (__c) : : "cc"); > - > - return __c; > -} > -#else > static inline char __dcc_getchar(void) > { > char __c; > @@ -62,24 +49,13 @@ static inline char __dcc_getchar(void) > > return __c; > } > -#endif > > -#if defined(CONFIG_CPU_V7) > -static inline void __dcc_putchar(char c) > -{ > - asm volatile("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ > - bcs put_wait \n\ > - mcr p14, 0, %0, c0, c5, 0 " > - : : "r" (c) : "cc"); > -} > -#else > static inline void __dcc_putchar(char c) > { > asm volatile("mcr p14, 0, %0, c0, c5, 0 @ write a char" > : /* no output register */ > : "r" (c)); > } > -#endif > > static int hvc_dcc_put_chars(uint32_t vt, const char *buf, int count) > { > -- > Sent by an employee of the Qualcomm Innovation Center, Inc. > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. > ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH 3/3] hvc_dcc: Simplify assembly for v6 and v7 ARM 2010-12-20 21:44 ` Nicolas Pitre @ 2011-01-04 18:52 ` Tony Lindgren -1 siblings, 0 replies; 79+ messages in thread From: Tony Lindgren @ 2011-01-04 18:52 UTC (permalink / raw) To: Nicolas Pitre Cc: Stephen Boyd, Greg Kroah-Hartman, linux-kernel, linux-arm-msm, linux-arm-kernel, Arnd Bergmann, Daniel Walker * Nicolas Pitre <nico@fluxnic.net> [101220 13:44]: > On Mon, 20 Dec 2010, Stephen Boyd wrote: > > > The inline assembly differences for v6 vs. v7 in the hvc_dcc > > driver are purely optimizations. On a v7 processor, an mrc with > > the pc sets the condition codes to the 28-31 bits of the register > > being read. It just so happens that the TX/RX full bits the DCC > > driver is testing for are high enough in the register to be put > > into the condition codes. On a v6 processor, this "feature" isn't > > implemented and thus we have to do the usual read, mask, test > > operations to check for TX/RX full. > > > > Since we already test the RX/TX full bits before calling > > __dcc_getchar() and __dcc_putchar() we don't actually need to do > > anything special for v7 over v6. The only difference is in > > hvc_dcc_get_chars(). We would test RX full, poll RX full, and > > then read a character from the buffer, whereas now we will test > > RX full, read a character from the buffer, and then test RX full > > again for the second iteration of the loop. It doesn't seem > > possible for the buffer to go from full to empty between testing > > the RX full and reading a character. Therefore, replace the v7 > > versions with the v6 versions and everything works the same. > > > > Cc: Tony Lindgren <tony@atomide.com> > > Cc: Arnd Bergmann <arnd@arndb.de> > > Cc: Nicolas Pitre <nicolas.pitre@linaro.org> > > Cc: Daniel Walker <dwalker@codeaurora.org> > > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> > > Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org> Acked-by: Tony Lindgren <tony@atomide.com> ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH 3/3] hvc_dcc: Simplify assembly for v6 and v7 ARM @ 2011-01-04 18:52 ` Tony Lindgren 0 siblings, 0 replies; 79+ messages in thread From: Tony Lindgren @ 2011-01-04 18:52 UTC (permalink / raw) To: linux-arm-kernel * Nicolas Pitre <nico@fluxnic.net> [101220 13:44]: > On Mon, 20 Dec 2010, Stephen Boyd wrote: > > > The inline assembly differences for v6 vs. v7 in the hvc_dcc > > driver are purely optimizations. On a v7 processor, an mrc with > > the pc sets the condition codes to the 28-31 bits of the register > > being read. It just so happens that the TX/RX full bits the DCC > > driver is testing for are high enough in the register to be put > > into the condition codes. On a v6 processor, this "feature" isn't > > implemented and thus we have to do the usual read, mask, test > > operations to check for TX/RX full. > > > > Since we already test the RX/TX full bits before calling > > __dcc_getchar() and __dcc_putchar() we don't actually need to do > > anything special for v7 over v6. The only difference is in > > hvc_dcc_get_chars(). We would test RX full, poll RX full, and > > then read a character from the buffer, whereas now we will test > > RX full, read a character from the buffer, and then test RX full > > again for the second iteration of the loop. It doesn't seem > > possible for the buffer to go from full to empty between testing > > the RX full and reading a character. Therefore, replace the v7 > > versions with the v6 versions and everything works the same. > > > > Cc: Tony Lindgren <tony@atomide.com> > > Cc: Arnd Bergmann <arnd@arndb.de> > > Cc: Nicolas Pitre <nicolas.pitre@linaro.org> > > Cc: Daniel Walker <dwalker@codeaurora.org> > > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> > > Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org> Acked-by: Tony Lindgren <tony@atomide.com> ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH 0/3] hvc_dcc cleanups and fixes 2010-12-20 20:08 ` Stephen Boyd @ 2011-01-06 1:49 ` Stephen Boyd -1 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2011-01-06 1:49 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: linux-kernel, linux-arm-msm, linux-arm-kernel Greg, On 12/20/2010 12:08 PM, Stephen Boyd wrote: > Here are the split patches. Should I resend these with the proper acks or can/have you picked these patches up? -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH 0/3] hvc_dcc cleanups and fixes @ 2011-01-06 1:49 ` Stephen Boyd 0 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2011-01-06 1:49 UTC (permalink / raw) To: linux-arm-kernel Greg, On 12/20/2010 12:08 PM, Stephen Boyd wrote: > Here are the split patches. Should I resend these with the proper acks or can/have you picked these patches up? -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH 0/3] hvc_dcc cleanups and fixes 2011-01-06 1:49 ` Stephen Boyd @ 2011-01-06 3:20 ` Greg KH -1 siblings, 0 replies; 79+ messages in thread From: Greg KH @ 2011-01-06 3:20 UTC (permalink / raw) To: Stephen Boyd; +Cc: linux-kernel, linux-arm-msm, linux-arm-kernel On Wed, Jan 05, 2011 at 05:49:17PM -0800, Stephen Boyd wrote: > Greg, > > On 12/20/2010 12:08 PM, Stephen Boyd wrote: > > Here are the split patches. > > Should I resend these with the proper acks or can/have you picked these > patches up? They are in my to-apply queue after .38-rc1 is out. thanks, greg k-h ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH 0/3] hvc_dcc cleanups and fixes @ 2011-01-06 3:20 ` Greg KH 0 siblings, 0 replies; 79+ messages in thread From: Greg KH @ 2011-01-06 3:20 UTC (permalink / raw) To: linux-arm-kernel On Wed, Jan 05, 2011 at 05:49:17PM -0800, Stephen Boyd wrote: > Greg, > > On 12/20/2010 12:08 PM, Stephen Boyd wrote: > > Here are the split patches. > > Should I resend these with the proper acks or can/have you picked these > patches up? They are in my to-apply queue after .38-rc1 is out. thanks, greg k-h ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH 0/3] hvc_dcc cleanups and fixes 2010-12-20 20:08 ` Stephen Boyd (?) @ 2011-02-03 22:17 ` Greg KH -1 siblings, 0 replies; 79+ messages in thread From: Greg KH @ 2011-02-03 22:17 UTC (permalink / raw) To: Stephen Boyd Cc: linux-arm-msm, Greg Kroah-Hartman, linux-kernel, linux-arm-kernel On Mon, Dec 20, 2010 at 12:08:35PM -0800, Stephen Boyd wrote: > Here are the split patches. > > The first two patches cleanup and fix the hvc_dcc driver for my > compiler. The final patch is more controversial, it removes the > v6 and v7 differences in this driver. > > Stephen Boyd (3): > hvc_dcc: Fix bad code generation by marking assembly volatile > hvc_dcc: Simplify put_chars()/get_chars() loops > hvc_dcc: Simplify assembly for v6 and v7 ARM Can you redo these against the linux-next tree as they don't apply anymore and resend them so I can apply them? thanks, greg k-h ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH 0/3] hvc_dcc cleanups and fixes @ 2011-02-03 22:17 ` Greg KH 0 siblings, 0 replies; 79+ messages in thread From: Greg KH @ 2011-02-03 22:17 UTC (permalink / raw) To: linux-arm-kernel On Mon, Dec 20, 2010 at 12:08:35PM -0800, Stephen Boyd wrote: > Here are the split patches. > > The first two patches cleanup and fix the hvc_dcc driver for my > compiler. The final patch is more controversial, it removes the > v6 and v7 differences in this driver. > > Stephen Boyd (3): > hvc_dcc: Fix bad code generation by marking assembly volatile > hvc_dcc: Simplify put_chars()/get_chars() loops > hvc_dcc: Simplify assembly for v6 and v7 ARM Can you redo these against the linux-next tree as they don't apply anymore and resend them so I can apply them? thanks, greg k-h ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH 0/3] hvc_dcc cleanups and fixes @ 2011-02-03 22:17 ` Greg KH 0 siblings, 0 replies; 79+ messages in thread From: Greg KH @ 2011-02-03 22:17 UTC (permalink / raw) To: Stephen Boyd Cc: Greg Kroah-Hartman, linux-kernel, linux-arm-msm, linux-arm-kernel On Mon, Dec 20, 2010 at 12:08:35PM -0800, Stephen Boyd wrote: > Here are the split patches. > > The first two patches cleanup and fix the hvc_dcc driver for my > compiler. The final patch is more controversial, it removes the > v6 and v7 differences in this driver. > > Stephen Boyd (3): > hvc_dcc: Fix bad code generation by marking assembly volatile > hvc_dcc: Simplify put_chars()/get_chars() loops > hvc_dcc: Simplify assembly for v6 and v7 ARM Can you redo these against the linux-next tree as they don't apply anymore and resend them so I can apply them? thanks, greg k-h ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH 0/3] hvc_dcc cleanups and fixes 2011-02-03 22:17 ` Greg KH @ 2011-02-03 23:19 ` Stephen Boyd -1 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2011-02-03 23:19 UTC (permalink / raw) To: Greg KH Cc: Stephen Boyd, Greg Kroah-Hartman, linux-kernel, linux-arm-msm, linux-arm-kernel > On Mon, Dec 20, 2010 at 12:08:35PM -0800, Stephen Boyd wrote: >> Here are the split patches. >> >> The first two patches cleanup and fix the hvc_dcc driver for my >> compiler. The final patch is more controversial, it removes the >> v6 and v7 differences in this driver. >> >> Stephen Boyd (3): >> hvc_dcc: Fix bad code generation by marking assembly volatile >> hvc_dcc: Simplify put_chars()/get_chars() loops >> hvc_dcc: Simplify assembly for v6 and v7 ARM > > Can you redo these against the linux-next tree as they don't apply > anymore and resend them so I can apply them? > Have you tried 'git am -3' on them? I assume it's the movement of the file that's causing the problem. Either way, I'll resend and collect the acks in a bit. ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH 0/3] hvc_dcc cleanups and fixes @ 2011-02-03 23:19 ` Stephen Boyd 0 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2011-02-03 23:19 UTC (permalink / raw) To: linux-arm-kernel > On Mon, Dec 20, 2010 at 12:08:35PM -0800, Stephen Boyd wrote: >> Here are the split patches. >> >> The first two patches cleanup and fix the hvc_dcc driver for my >> compiler. The final patch is more controversial, it removes the >> v6 and v7 differences in this driver. >> >> Stephen Boyd (3): >> hvc_dcc: Fix bad code generation by marking assembly volatile >> hvc_dcc: Simplify put_chars()/get_chars() loops >> hvc_dcc: Simplify assembly for v6 and v7 ARM > > Can you redo these against the linux-next tree as they don't apply > anymore and resend them so I can apply them? > Have you tried 'git am -3' on them? I assume it's the movement of the file that's causing the problem. Either way, I'll resend and collect the acks in a bit. ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH 0/3] hvc_dcc cleanups and fixes 2011-02-03 23:19 ` Stephen Boyd @ 2011-02-03 23:30 ` Greg KH -1 siblings, 0 replies; 79+ messages in thread From: Greg KH @ 2011-02-03 23:30 UTC (permalink / raw) To: Stephen Boyd; +Cc: Greg KH, linux-kernel, linux-arm-msm, linux-arm-kernel On Thu, Feb 03, 2011 at 03:19:07PM -0800, Stephen Boyd wrote: > > On Mon, Dec 20, 2010 at 12:08:35PM -0800, Stephen Boyd wrote: > >> Here are the split patches. > >> > >> The first two patches cleanup and fix the hvc_dcc driver for my > >> compiler. The final patch is more controversial, it removes the > >> v6 and v7 differences in this driver. > >> > >> Stephen Boyd (3): > >> hvc_dcc: Fix bad code generation by marking assembly volatile > >> hvc_dcc: Simplify put_chars()/get_chars() loops > >> hvc_dcc: Simplify assembly for v6 and v7 ARM > > > > Can you redo these against the linux-next tree as they don't apply > > anymore and resend them so I can apply them? > > > > Have you tried 'git am -3' on them? I assume it's the movement of the > file that's causing the problem. > > Either way, I'll resend and collect the acks in a bit. thanks, that would be great. greg k-h ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCH 0/3] hvc_dcc cleanups and fixes @ 2011-02-03 23:30 ` Greg KH 0 siblings, 0 replies; 79+ messages in thread From: Greg KH @ 2011-02-03 23:30 UTC (permalink / raw) To: linux-arm-kernel On Thu, Feb 03, 2011 at 03:19:07PM -0800, Stephen Boyd wrote: > > On Mon, Dec 20, 2010 at 12:08:35PM -0800, Stephen Boyd wrote: > >> Here are the split patches. > >> > >> The first two patches cleanup and fix the hvc_dcc driver for my > >> compiler. The final patch is more controversial, it removes the > >> v6 and v7 differences in this driver. > >> > >> Stephen Boyd (3): > >> hvc_dcc: Fix bad code generation by marking assembly volatile > >> hvc_dcc: Simplify put_chars()/get_chars() loops > >> hvc_dcc: Simplify assembly for v6 and v7 ARM > > > > Can you redo these against the linux-next tree as they don't apply > > anymore and resend them so I can apply them? > > > > Have you tried 'git am -3' on them? I assume it's the movement of the > file that's causing the problem. > > Either way, I'll resend and collect the acks in a bit. thanks, that would be great. greg k-h ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCHv2 0/3] hvc_dcc cleanups and fixes 2011-02-03 22:17 ` Greg KH @ 2011-02-03 23:48 ` Stephen Boyd -1 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2011-02-03 23:48 UTC (permalink / raw) To: Greg KH; +Cc: linux-kernel, linux-arm-msm, linux-arm-kernel Redone against tty-next and collected acks from Nicolas and Tony. Stephen Boyd (3): hvc_dcc: Fix bad code generation by marking assembly volatile hvc_dcc: Simplify put_chars()/get_chars() loops hvc_dcc: Simplify assembly for v6 and v7 ARM drivers/tty/hvc/hvc_dcc.c | 43 +++++++------------------------------------ 1 files changed, 7 insertions(+), 36 deletions(-) -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCHv2 0/3] hvc_dcc cleanups and fixes @ 2011-02-03 23:48 ` Stephen Boyd 0 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2011-02-03 23:48 UTC (permalink / raw) To: linux-arm-kernel Redone against tty-next and collected acks from Nicolas and Tony. Stephen Boyd (3): hvc_dcc: Fix bad code generation by marking assembly volatile hvc_dcc: Simplify put_chars()/get_chars() loops hvc_dcc: Simplify assembly for v6 and v7 ARM drivers/tty/hvc/hvc_dcc.c | 43 +++++++------------------------------------ 1 files changed, 7 insertions(+), 36 deletions(-) -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 79+ messages in thread
* [PATCHv2 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile 2011-02-03 23:48 ` Stephen Boyd @ 2011-02-03 23:48 ` Stephen Boyd -1 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2011-02-03 23:48 UTC (permalink / raw) To: Greg KH Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, Arnd Bergmann, Daniel Walker Without marking the asm __dcc_getstatus() volatile my compiler decides it can cache the value of __ret in a register and then check the value of it continually in hvc_dcc_put_chars() (I had to replace get_wait/put_wait with 1 and fixup the branch otherwise my disassembler barfed on __dcc_(get|put)char). 00000000 <hvc_dcc_put_chars>: 0: ee103e11 mrc 14, 0, r3, cr0, cr1, {0} 4: e3a0c000 mov ip, #0 ; 0x0 8: e2033202 and r3, r3, #536870912 ; 0x20000000 c: ea000006 b 2c <hvc_dcc_put_chars+0x2c> 10: e3530000 cmp r3, #0 ; 0x0 14: 1afffffd bne 10 <hvc_dcc_put_chars+0x10> 18: e7d1000c ldrb r0, [r1, ip] 1c: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} 20: 2afffffd bcs 1c <hvc_dcc_put_chars+0x1c> 24: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} 28: e28cc001 add ip, ip, #1 ; 0x1 2c: e15c0002 cmp ip, r2 30: bafffff6 blt 10 <hvc_dcc_put_chars+0x10> 34: e1a00002 mov r0, r2 38: e12fff1e bx lr As you can see, the value of the mrc is checked against DCC_STATUS_TX (bit 29) and then stored in r3 for later use. Marking the asm volatile produces the following: 00000000 <hvc_dcc_put_chars>: 0: e3a03000 mov r3, #0 ; 0x0 4: ea000007 b 28 <hvc_dcc_put_chars+0x28> 8: ee100e11 mrc 14, 0, r0, cr0, cr1, {0} c: e3100202 tst r0, #536870912 ; 0x20000000 10: 1afffffc bne 8 <hvc_dcc_put_chars+0x8> 14: e7d10003 ldrb r0, [r1, r3] 18: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} 1c: 2afffffd bcs 18 <hvc_dcc_put_chars+0x18> 20: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} 24: e2833001 add r3, r3, #1 ; 0x1 28: e1530002 cmp r3, r2 2c: bafffff5 blt 8 <hvc_dcc_put_chars+0x8> 30: e1a00002 mov r0, r2 34: e12fff1e bx lr which looks better and actually works. Mark all the inline assembly in this file as volatile since we don't want the compiler to optimize away these statements or move them around in any way. Acked-by: Tony Lindgren <tony@atomide.com> Cc: Arnd Bergmann <arnd@arndb.de> Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org> Cc: Daniel Walker <dwalker@codeaurora.org> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- drivers/tty/hvc/hvc_dcc.c | 11 +++++------ 1 files changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/tty/hvc/hvc_dcc.c b/drivers/tty/hvc/hvc_dcc.c index 6470f63..155ec10 100644 --- a/drivers/tty/hvc/hvc_dcc.c +++ b/drivers/tty/hvc/hvc_dcc.c @@ -33,8 +33,7 @@ static inline u32 __dcc_getstatus(void) { u32 __ret; - - asm("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" + asm volatile("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" : "=r" (__ret) : : "cc"); return __ret; @@ -46,7 +45,7 @@ static inline char __dcc_getchar(void) { char __c; - asm("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ + asm volatile("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ bne get_wait \n\ mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" : "=r" (__c) : : "cc"); @@ -58,7 +57,7 @@ static inline char __dcc_getchar(void) { char __c; - asm("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" + asm volatile("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" : "=r" (__c)); return __c; @@ -68,7 +67,7 @@ static inline char __dcc_getchar(void) #if defined(CONFIG_CPU_V7) static inline void __dcc_putchar(char c) { - asm("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ + asm volatile("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ bcs put_wait \n\ mcr p14, 0, %0, c0, c5, 0 " : : "r" (c) : "cc"); @@ -76,7 +75,7 @@ static inline void __dcc_putchar(char c) #else static inline void __dcc_putchar(char c) { - asm("mcr p14, 0, %0, c0, c5, 0 @ write a char" + asm volatile("mcr p14, 0, %0, c0, c5, 0 @ write a char" : /* no output register */ : "r" (c)); } -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply related [flat|nested] 79+ messages in thread
* [PATCHv2 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile @ 2011-02-03 23:48 ` Stephen Boyd 0 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2011-02-03 23:48 UTC (permalink / raw) To: linux-arm-kernel Without marking the asm __dcc_getstatus() volatile my compiler decides it can cache the value of __ret in a register and then check the value of it continually in hvc_dcc_put_chars() (I had to replace get_wait/put_wait with 1 and fixup the branch otherwise my disassembler barfed on __dcc_(get|put)char). 00000000 <hvc_dcc_put_chars>: 0: ee103e11 mrc 14, 0, r3, cr0, cr1, {0} 4: e3a0c000 mov ip, #0 ; 0x0 8: e2033202 and r3, r3, #536870912 ; 0x20000000 c: ea000006 b 2c <hvc_dcc_put_chars+0x2c> 10: e3530000 cmp r3, #0 ; 0x0 14: 1afffffd bne 10 <hvc_dcc_put_chars+0x10> 18: e7d1000c ldrb r0, [r1, ip] 1c: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} 20: 2afffffd bcs 1c <hvc_dcc_put_chars+0x1c> 24: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} 28: e28cc001 add ip, ip, #1 ; 0x1 2c: e15c0002 cmp ip, r2 30: bafffff6 blt 10 <hvc_dcc_put_chars+0x10> 34: e1a00002 mov r0, r2 38: e12fff1e bx lr As you can see, the value of the mrc is checked against DCC_STATUS_TX (bit 29) and then stored in r3 for later use. Marking the asm volatile produces the following: 00000000 <hvc_dcc_put_chars>: 0: e3a03000 mov r3, #0 ; 0x0 4: ea000007 b 28 <hvc_dcc_put_chars+0x28> 8: ee100e11 mrc 14, 0, r0, cr0, cr1, {0} c: e3100202 tst r0, #536870912 ; 0x20000000 10: 1afffffc bne 8 <hvc_dcc_put_chars+0x8> 14: e7d10003 ldrb r0, [r1, r3] 18: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} 1c: 2afffffd bcs 18 <hvc_dcc_put_chars+0x18> 20: ee000e15 mcr 14, 0, r0, cr0, cr5, {0} 24: e2833001 add r3, r3, #1 ; 0x1 28: e1530002 cmp r3, r2 2c: bafffff5 blt 8 <hvc_dcc_put_chars+0x8> 30: e1a00002 mov r0, r2 34: e12fff1e bx lr which looks better and actually works. Mark all the inline assembly in this file as volatile since we don't want the compiler to optimize away these statements or move them around in any way. Acked-by: Tony Lindgren <tony@atomide.com> Cc: Arnd Bergmann <arnd@arndb.de> Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org> Cc: Daniel Walker <dwalker@codeaurora.org> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- drivers/tty/hvc/hvc_dcc.c | 11 +++++------ 1 files changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/tty/hvc/hvc_dcc.c b/drivers/tty/hvc/hvc_dcc.c index 6470f63..155ec10 100644 --- a/drivers/tty/hvc/hvc_dcc.c +++ b/drivers/tty/hvc/hvc_dcc.c @@ -33,8 +33,7 @@ static inline u32 __dcc_getstatus(void) { u32 __ret; - - asm("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" + asm volatile("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg" : "=r" (__ret) : : "cc"); return __ret; @@ -46,7 +45,7 @@ static inline char __dcc_getchar(void) { char __c; - asm("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ + asm volatile("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ bne get_wait \n\ mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" : "=r" (__c) : : "cc"); @@ -58,7 +57,7 @@ static inline char __dcc_getchar(void) { char __c; - asm("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" + asm volatile("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" : "=r" (__c)); return __c; @@ -68,7 +67,7 @@ static inline char __dcc_getchar(void) #if defined(CONFIG_CPU_V7) static inline void __dcc_putchar(char c) { - asm("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ + asm volatile("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ bcs put_wait \n\ mcr p14, 0, %0, c0, c5, 0 " : : "r" (c) : "cc"); @@ -76,7 +75,7 @@ static inline void __dcc_putchar(char c) #else static inline void __dcc_putchar(char c) { - asm("mcr p14, 0, %0, c0, c5, 0 @ write a char" + asm volatile("mcr p14, 0, %0, c0, c5, 0 @ write a char" : /* no output register */ : "r" (c)); } -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply related [flat|nested] 79+ messages in thread
* [PATCHv2 2/3] hvc_dcc: Simplify put_chars()/get_chars() loops 2011-02-03 23:48 ` Stephen Boyd @ 2011-02-03 23:48 ` Stephen Boyd -1 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2011-02-03 23:48 UTC (permalink / raw) To: Greg KH; +Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, Daniel Walker Casting and anding with 0xff is unnecessary in hvc_dcc_put_chars() since buf is already a char[]. __dcc_get_char() can't return an int less than 0 since it only returns a char. Simplify the if statement in hvc_dcc_get_chars() to take this into account. Cc: Daniel Walker <dwalker@codeaurora.org> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- drivers/tty/hvc/hvc_dcc.c | 12 ++++-------- 1 files changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/tty/hvc/hvc_dcc.c b/drivers/tty/hvc/hvc_dcc.c index 155ec10..ad23cc8 100644 --- a/drivers/tty/hvc/hvc_dcc.c +++ b/drivers/tty/hvc/hvc_dcc.c @@ -89,7 +89,7 @@ static int hvc_dcc_put_chars(uint32_t vt, const char *buf, int count) while (__dcc_getstatus() & DCC_STATUS_TX) cpu_relax(); - __dcc_putchar((char)(buf[i] & 0xFF)); + __dcc_putchar(buf[i]); } return count; @@ -99,15 +99,11 @@ static int hvc_dcc_get_chars(uint32_t vt, char *buf, int count) { int i; - for (i = 0; i < count; ++i) { - int c = -1; - + for (i = 0; i < count; ++i) if (__dcc_getstatus() & DCC_STATUS_RX) - c = __dcc_getchar(); - if (c < 0) + buf[i] = __dcc_getchar(); + else break; - buf[i] = c; - } return i; } -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply related [flat|nested] 79+ messages in thread
* [PATCHv2 2/3] hvc_dcc: Simplify put_chars()/get_chars() loops @ 2011-02-03 23:48 ` Stephen Boyd 0 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2011-02-03 23:48 UTC (permalink / raw) To: linux-arm-kernel Casting and anding with 0xff is unnecessary in hvc_dcc_put_chars() since buf is already a char[]. __dcc_get_char() can't return an int less than 0 since it only returns a char. Simplify the if statement in hvc_dcc_get_chars() to take this into account. Cc: Daniel Walker <dwalker@codeaurora.org> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- drivers/tty/hvc/hvc_dcc.c | 12 ++++-------- 1 files changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/tty/hvc/hvc_dcc.c b/drivers/tty/hvc/hvc_dcc.c index 155ec10..ad23cc8 100644 --- a/drivers/tty/hvc/hvc_dcc.c +++ b/drivers/tty/hvc/hvc_dcc.c @@ -89,7 +89,7 @@ static int hvc_dcc_put_chars(uint32_t vt, const char *buf, int count) while (__dcc_getstatus() & DCC_STATUS_TX) cpu_relax(); - __dcc_putchar((char)(buf[i] & 0xFF)); + __dcc_putchar(buf[i]); } return count; @@ -99,15 +99,11 @@ static int hvc_dcc_get_chars(uint32_t vt, char *buf, int count) { int i; - for (i = 0; i < count; ++i) { - int c = -1; - + for (i = 0; i < count; ++i) if (__dcc_getstatus() & DCC_STATUS_RX) - c = __dcc_getchar(); - if (c < 0) + buf[i] = __dcc_getchar(); + else break; - buf[i] = c; - } return i; } -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply related [flat|nested] 79+ messages in thread
* [PATCHv2 3/3] hvc_dcc: Simplify assembly for v6 and v7 ARM 2011-02-03 23:48 ` Stephen Boyd @ 2011-02-03 23:48 ` Stephen Boyd -1 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2011-02-03 23:48 UTC (permalink / raw) To: Greg KH Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, Arnd Bergmann, Daniel Walker The inline assembly differences for v6 vs. v7 in the hvc_dcc driver are purely optimizations. On a v7 processor, an mrc with the pc sets the condition codes to the 28-31 bits of the register being read. It just so happens that the TX/RX full bits the DCC driver is testing for are high enough in the register to be put into the condition codes. On a v6 processor, this "feature" isn't implemented and thus we have to do the usual read, mask, test operations to check for TX/RX full. Since we already test the RX/TX full bits before calling __dcc_getchar() and __dcc_putchar() we don't actually need to do anything special for v7 over v6. The only difference is in hvc_dcc_get_chars(). We would test RX full, poll RX full, and then read a character from the buffer, whereas now we will test RX full, read a character from the buffer, and then test RX full again for the second iteration of the loop. It doesn't seem possible for the buffer to go from full to empty between testing the RX full and reading a character. Therefore, replace the v7 versions with the v6 versions and everything works the same. Acked-by: Tony Lindgren <tony@atomide.com> Cc: Arnd Bergmann <arnd@arndb.de> Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org> Cc: Daniel Walker <dwalker@codeaurora.org> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- drivers/tty/hvc/hvc_dcc.c | 24 ------------------------ 1 files changed, 0 insertions(+), 24 deletions(-) diff --git a/drivers/tty/hvc/hvc_dcc.c b/drivers/tty/hvc/hvc_dcc.c index ad23cc8..435f6fa 100644 --- a/drivers/tty/hvc/hvc_dcc.c +++ b/drivers/tty/hvc/hvc_dcc.c @@ -40,19 +40,6 @@ static inline u32 __dcc_getstatus(void) } -#if defined(CONFIG_CPU_V7) -static inline char __dcc_getchar(void) -{ - char __c; - - asm volatile("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ - bne get_wait \n\ - mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" - : "=r" (__c) : : "cc"); - - return __c; -} -#else static inline char __dcc_getchar(void) { char __c; @@ -62,24 +49,13 @@ static inline char __dcc_getchar(void) return __c; } -#endif -#if defined(CONFIG_CPU_V7) -static inline void __dcc_putchar(char c) -{ - asm volatile("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ - bcs put_wait \n\ - mcr p14, 0, %0, c0, c5, 0 " - : : "r" (c) : "cc"); -} -#else static inline void __dcc_putchar(char c) { asm volatile("mcr p14, 0, %0, c0, c5, 0 @ write a char" : /* no output register */ : "r" (c)); } -#endif static int hvc_dcc_put_chars(uint32_t vt, const char *buf, int count) { -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply related [flat|nested] 79+ messages in thread
* [PATCHv2 3/3] hvc_dcc: Simplify assembly for v6 and v7 ARM @ 2011-02-03 23:48 ` Stephen Boyd 0 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2011-02-03 23:48 UTC (permalink / raw) To: linux-arm-kernel The inline assembly differences for v6 vs. v7 in the hvc_dcc driver are purely optimizations. On a v7 processor, an mrc with the pc sets the condition codes to the 28-31 bits of the register being read. It just so happens that the TX/RX full bits the DCC driver is testing for are high enough in the register to be put into the condition codes. On a v6 processor, this "feature" isn't implemented and thus we have to do the usual read, mask, test operations to check for TX/RX full. Since we already test the RX/TX full bits before calling __dcc_getchar() and __dcc_putchar() we don't actually need to do anything special for v7 over v6. The only difference is in hvc_dcc_get_chars(). We would test RX full, poll RX full, and then read a character from the buffer, whereas now we will test RX full, read a character from the buffer, and then test RX full again for the second iteration of the loop. It doesn't seem possible for the buffer to go from full to empty between testing the RX full and reading a character. Therefore, replace the v7 versions with the v6 versions and everything works the same. Acked-by: Tony Lindgren <tony@atomide.com> Cc: Arnd Bergmann <arnd@arndb.de> Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org> Cc: Daniel Walker <dwalker@codeaurora.org> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- drivers/tty/hvc/hvc_dcc.c | 24 ------------------------ 1 files changed, 0 insertions(+), 24 deletions(-) diff --git a/drivers/tty/hvc/hvc_dcc.c b/drivers/tty/hvc/hvc_dcc.c index ad23cc8..435f6fa 100644 --- a/drivers/tty/hvc/hvc_dcc.c +++ b/drivers/tty/hvc/hvc_dcc.c @@ -40,19 +40,6 @@ static inline u32 __dcc_getstatus(void) } -#if defined(CONFIG_CPU_V7) -static inline char __dcc_getchar(void) -{ - char __c; - - asm volatile("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\ - bne get_wait \n\ - mrc p14, 0, %0, c0, c5, 0 @ read comms data reg" - : "=r" (__c) : : "cc"); - - return __c; -} -#else static inline char __dcc_getchar(void) { char __c; @@ -62,24 +49,13 @@ static inline char __dcc_getchar(void) return __c; } -#endif -#if defined(CONFIG_CPU_V7) -static inline void __dcc_putchar(char c) -{ - asm volatile("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\ - bcs put_wait \n\ - mcr p14, 0, %0, c0, c5, 0 " - : : "r" (c) : "cc"); -} -#else static inline void __dcc_putchar(char c) { asm volatile("mcr p14, 0, %0, c0, c5, 0 @ write a char" : /* no output register */ : "r" (c)); } -#endif static int hvc_dcc_put_chars(uint32_t vt, const char *buf, int count) { -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply related [flat|nested] 79+ messages in thread
* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support 2010-12-01 18:54 ` Daniel Walker @ 2010-12-01 20:20 ` Stephen Boyd -1 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2010-12-01 20:20 UTC (permalink / raw) To: Daniel Walker, Tony Lindgren Cc: linux-kernel, linux-arm-msm, Arnd Bergmann, Nicolas Pitre, Greg Kroah-Hartman, Mike Frysinger, Andrew Morton, Alan Cox, Randy Dunlap, FUJITA Tomonori, linuxppc-dev On 12/01/2010 10:54 AM, Daniel Walker wrote: > Are you talking about __dcc_getstatus only? I don't think adding > volatile is going to hurt anything, if not having it causes problems. > Just marking __dcc_getstatus volatile gives me 00000038 <hvc_dcc_get_chars>: 38: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} 3c: 1afffffd bne 38 <hvc_dcc_get_chars> 40: ee103e15 mrc 14, 0, r3, cr0, cr5, {0} 44: e3a00000 mov r0, #0 ; 0x0 48: e6ef3073 uxtb r3, r3 4c: ea000004 b 64 <hvc_dcc_get_chars+0x2c> 50: ee10ce11 mrc 14, 0, ip, cr0, cr1, {0} 54: e31c0101 tst ip, #1073741824 ; 0x40000000 58: 012fff1e bxeq lr 5c: e7c13000 strb r3, [r1, r0] 60: e2800001 add r0, r0, #1 ; 0x1 64: e1500002 cmp r0, r2 68: bafffff8 blt 50 <hvc_dcc_get_chars+0x18> 6c: e12fff1e bx lr Seems the compiler keeps the value of __dcc_getchar() in r3 for the duration of the loop. So we need to mark that one volatile too. I don't think __dcc_putchar() needs to be marked volatile but it probably doesn't hurt. > > We could maybe drop the looping for TX, but RX has no C based looping > even tho for v7 it's recommended that we loop (presumably for v6 it's > not recommended). > Definitely for TX since it seems like a redundant loop, but I agree RX code has changed. Instead of If RX buffer full Poll for RX buffer full Read character from RX buffer we would have If RX buffer full Read character from RX buffer which doesn't seem all that different assuming the RX buffer doesn't go from full to empty between the If and Poll steps. Hopefully Tony knows more. > Like this? > > for (i = 0; i < count; ++i) { > > if (__dcc_getstatus() & DCC_STATUS_RX) > buf[i] = __dcc_getchar(); > else > break; > } > > It's a micro clean up I guess .. Yes, it's much clearer that way. -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support @ 2010-12-01 20:20 ` Stephen Boyd 0 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2010-12-01 20:20 UTC (permalink / raw) To: Daniel Walker, Tony Lindgren Cc: Randy Dunlap, Mike Frysinger, Arnd Bergmann, Nicolas Pitre, linux-arm-msm, Greg Kroah-Hartman, linux-kernel, FUJITA Tomonori, Andrew Morton, linuxppc-dev, Alan Cox On 12/01/2010 10:54 AM, Daniel Walker wrote: > Are you talking about __dcc_getstatus only? I don't think adding > volatile is going to hurt anything, if not having it causes problems. > Just marking __dcc_getstatus volatile gives me 00000038 <hvc_dcc_get_chars>: 38: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0} 3c: 1afffffd bne 38 <hvc_dcc_get_chars> 40: ee103e15 mrc 14, 0, r3, cr0, cr5, {0} 44: e3a00000 mov r0, #0 ; 0x0 48: e6ef3073 uxtb r3, r3 4c: ea000004 b 64 <hvc_dcc_get_chars+0x2c> 50: ee10ce11 mrc 14, 0, ip, cr0, cr1, {0} 54: e31c0101 tst ip, #1073741824 ; 0x40000000 58: 012fff1e bxeq lr 5c: e7c13000 strb r3, [r1, r0] 60: e2800001 add r0, r0, #1 ; 0x1 64: e1500002 cmp r0, r2 68: bafffff8 blt 50 <hvc_dcc_get_chars+0x18> 6c: e12fff1e bx lr Seems the compiler keeps the value of __dcc_getchar() in r3 for the duration of the loop. So we need to mark that one volatile too. I don't think __dcc_putchar() needs to be marked volatile but it probably doesn't hurt. > > We could maybe drop the looping for TX, but RX has no C based looping > even tho for v7 it's recommended that we loop (presumably for v6 it's > not recommended). > Definitely for TX since it seems like a redundant loop, but I agree RX code has changed. Instead of If RX buffer full Poll for RX buffer full Read character from RX buffer we would have If RX buffer full Read character from RX buffer which doesn't seem all that different assuming the RX buffer doesn't go from full to empty between the If and Poll steps. Hopefully Tony knows more. > Like this? > > for (i = 0; i < count; ++i) { > > if (__dcc_getstatus() & DCC_STATUS_RX) > buf[i] = __dcc_getchar(); > else > break; > } > > It's a micro clean up I guess .. Yes, it's much clearer that way. -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support 2010-12-01 20:20 ` Stephen Boyd @ 2010-12-07 19:10 ` Stephen Boyd -1 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2010-12-07 19:10 UTC (permalink / raw) To: Tony Lindgren Cc: Daniel Walker, linux-kernel, linux-arm-msm, Arnd Bergmann, Nicolas Pitre, Greg Kroah-Hartman, Mike Frysinger, Andrew Morton, Alan Cox, Randy Dunlap, FUJITA Tomonori, linuxppc-dev On 12/01/2010 12:20 PM, Stephen Boyd wrote: > Definitely for TX since it seems like a redundant loop, but I agree RX > code has changed. Instead of > > If RX buffer full > Poll for RX buffer full > Read character from RX buffer > > we would have > > If RX buffer full > Read character from RX buffer > > which doesn't seem all that different assuming the RX buffer doesn't go > from full to empty between the If and Poll steps. Hopefully Tony knows more. > Tony, any thoughts? -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support @ 2010-12-07 19:10 ` Stephen Boyd 0 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2010-12-07 19:10 UTC (permalink / raw) To: Tony Lindgren Cc: Randy Dunlap, Daniel Walker, Mike Frysinger, Arnd Bergmann, Nicolas Pitre, linux-arm-msm, Greg Kroah-Hartman, linux-kernel, FUJITA Tomonori, Andrew Morton, linuxppc-dev, Alan Cox On 12/01/2010 12:20 PM, Stephen Boyd wrote: > Definitely for TX since it seems like a redundant loop, but I agree RX > code has changed. Instead of > > If RX buffer full > Poll for RX buffer full > Read character from RX buffer > > we would have > > If RX buffer full > Read character from RX buffer > > which doesn't seem all that different assuming the RX buffer doesn't go > from full to empty between the If and Poll steps. Hopefully Tony knows more. > Tony, any thoughts? -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support 2010-12-07 19:10 ` Stephen Boyd @ 2011-01-14 19:19 ` Tony Lindgren -1 siblings, 0 replies; 79+ messages in thread From: Tony Lindgren @ 2011-01-14 19:19 UTC (permalink / raw) To: Stephen Boyd Cc: Daniel Walker, linux-kernel, linux-arm-msm, Arnd Bergmann, Nicolas Pitre, Greg Kroah-Hartman, Mike Frysinger, Andrew Morton, Alan Cox, Randy Dunlap, FUJITA Tomonori, linuxppc-dev * Stephen Boyd <sboyd@codeaurora.org> [101207 11:00]: > On 12/01/2010 12:20 PM, Stephen Boyd wrote: > > Definitely for TX since it seems like a redundant loop, but I agree RX > > code has changed. Instead of > > > > If RX buffer full > > Poll for RX buffer full > > Read character from RX buffer > > > > we would have > > > > If RX buffer full > > Read character from RX buffer > > > > which doesn't seem all that different assuming the RX buffer doesn't go > > from full to empty between the If and Poll steps. Hopefully Tony knows more. > > > > Tony, any thoughts? Sorry for the delay, looks like I'm only 1 month behind with email.. Sounds like it should work to me. I can try it out if you point me to a patch. Regards, Tony ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support @ 2011-01-14 19:19 ` Tony Lindgren 0 siblings, 0 replies; 79+ messages in thread From: Tony Lindgren @ 2011-01-14 19:19 UTC (permalink / raw) To: Stephen Boyd Cc: Randy Dunlap, Daniel Walker, Mike Frysinger, Arnd Bergmann, Nicolas Pitre, linux-arm-msm, Greg Kroah-Hartman, linux-kernel, FUJITA Tomonori, Andrew Morton, linuxppc-dev, Alan Cox * Stephen Boyd <sboyd@codeaurora.org> [101207 11:00]: > On 12/01/2010 12:20 PM, Stephen Boyd wrote: > > Definitely for TX since it seems like a redundant loop, but I agree RX > > code has changed. Instead of > > > > If RX buffer full > > Poll for RX buffer full > > Read character from RX buffer > > > > we would have > > > > If RX buffer full > > Read character from RX buffer > > > > which doesn't seem all that different assuming the RX buffer doesn't go > > from full to empty between the If and Poll steps. Hopefully Tony knows more. > > > > Tony, any thoughts? Sorry for the delay, looks like I'm only 1 month behind with email.. Sounds like it should work to me. I can try it out if you point me to a patch. Regards, Tony ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support 2011-01-14 19:19 ` Tony Lindgren @ 2011-01-14 23:49 ` Stephen Boyd -1 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2011-01-14 23:49 UTC (permalink / raw) To: Tony Lindgren Cc: Daniel Walker, linux-kernel, linux-arm-msm, Arnd Bergmann, Nicolas Pitre, Greg Kroah-Hartman, Mike Frysinger, Andrew Morton, Alan Cox, Randy Dunlap, FUJITA Tomonori, linuxppc-dev On 01/14/2011 11:19 AM, Tony Lindgren wrote: > * Stephen Boyd <sboyd@codeaurora.org> [101207 11:00]: >> On 12/01/2010 12:20 PM, Stephen Boyd wrote: >>> Definitely for TX since it seems like a redundant loop, but I agree RX >>> code has changed. Instead of >>> >>> If RX buffer full >>> Poll for RX buffer full >>> Read character from RX buffer >>> >>> we would have >>> >>> If RX buffer full >>> Read character from RX buffer >>> >>> which doesn't seem all that different assuming the RX buffer doesn't go >>> from full to empty between the If and Poll steps. Hopefully Tony knows more. >>> >> >> Tony, any thoughts? > > Sorry for the delay, looks like I'm only 1 month behind with email.. > Sounds like it should work to me. I can try it out if you point me > to a patch. I think you acked the patches to make this change. You can test them out by applying the "hvc_dcc cleanups and fixes" patches (Message-Id: <1292875718-7980-1-git-send-email-sboyd@codeaurora.org>) on top of this patch. They were sent as a reply to this thread. Stephen -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support @ 2011-01-14 23:49 ` Stephen Boyd 0 siblings, 0 replies; 79+ messages in thread From: Stephen Boyd @ 2011-01-14 23:49 UTC (permalink / raw) To: Tony Lindgren Cc: Randy Dunlap, Daniel Walker, Mike Frysinger, Arnd Bergmann, Nicolas Pitre, linux-arm-msm, Greg Kroah-Hartman, linux-kernel, FUJITA Tomonori, Andrew Morton, linuxppc-dev, Alan Cox On 01/14/2011 11:19 AM, Tony Lindgren wrote: > * Stephen Boyd <sboyd@codeaurora.org> [101207 11:00]: >> On 12/01/2010 12:20 PM, Stephen Boyd wrote: >>> Definitely for TX since it seems like a redundant loop, but I agree RX >>> code has changed. Instead of >>> >>> If RX buffer full >>> Poll for RX buffer full >>> Read character from RX buffer >>> >>> we would have >>> >>> If RX buffer full >>> Read character from RX buffer >>> >>> which doesn't seem all that different assuming the RX buffer doesn't go >>> from full to empty between the If and Poll steps. Hopefully Tony knows more. >>> >> >> Tony, any thoughts? > > Sorry for the delay, looks like I'm only 1 month behind with email.. > Sounds like it should work to me. I can try it out if you point me > to a patch. I think you acked the patches to make this change. You can test them out by applying the "hvc_dcc cleanups and fixes" patches (Message-Id: <1292875718-7980-1-git-send-email-sboyd@codeaurora.org>) on top of this patch. They were sent as a reply to this thread. Stephen -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support 2010-11-30 19:25 ` Daniel Walker @ 2011-04-07 18:39 ` RONETIX - Asen Dimov -1 siblings, 0 replies; 79+ messages in thread From: RONETIX - Asen Dimov @ 2011-04-07 18:39 UTC (permalink / raw) To: Daniel Walker Cc: Randy Dunlap, Mike Frysinger, Arnd Bergmann, Nicolas Pitre, Tony Lindgren, linux-arm-msm, Greg Kroah-Hartman, linux-kernel, FUJITA Tomonori, Andrew Morton, linuxppc-dev, Alan Cox Hello, On 11/30/2010 09:25 PM, Daniel Walker wrote: > This driver adds a basic console that uses the arm JTAG > DCC to transfer data back and forth. It has support for > ARMv6 and ARMv7. > > This console is created under the HVC driver, and should be named > /dev/hvcX (or /dev/hvc0 for example). > > Cc: Tony Lindgren<tony@atomide.com> > Cc: Arnd Bergmann<arnd@arndb.de> > Cc: Nicolas Pitre<nico@fluxnic.net> > Cc: Greg Kroah-Hartman<gregkh@suse.de> > Cc: Mike Frysinger<vapier@gentoo.org> > Signed-off-by: Daniel Walker<dwalker@codeaurora.org> > --- > drivers/char/Kconfig | 9 +++ > drivers/char/Makefile | 1 + > drivers/char/hvc_dcc.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++ ... this DCC driver implements "one channel", but what about implementing "multiple channels". For example reserve few(3) bits for channel number, and two bits for carried data, then fill the rest bytes with with some data and send the word(32 bits) over DCC. On the Linux side writing on /dev/hvcX puts the number X as channel number, and on the other side the CPU emulator gets the data and redistribute it to TCP/IP socket. I have started write some code implementing this. Are there any one interested in this multiple channels, and are there any one started to work on it? Regards, Asen ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support @ 2011-04-07 18:39 ` RONETIX - Asen Dimov 0 siblings, 0 replies; 79+ messages in thread From: RONETIX - Asen Dimov @ 2011-04-07 18:39 UTC (permalink / raw) To: Daniel Walker Cc: linux-kernel, Randy Dunlap, Mike Frysinger, Arnd Bergmann, Nicolas Pitre, Tony Lindgren, linux-arm-msm, Greg Kroah-Hartman, FUJITA Tomonori, Andrew Morton, linuxppc-dev, Alan Cox Hello, On 11/30/2010 09:25 PM, Daniel Walker wrote: > This driver adds a basic console that uses the arm JTAG > DCC to transfer data back and forth. It has support for > ARMv6 and ARMv7. > > This console is created under the HVC driver, and should be named > /dev/hvcX (or /dev/hvc0 for example). > > Cc: Tony Lindgren<tony@atomide.com> > Cc: Arnd Bergmann<arnd@arndb.de> > Cc: Nicolas Pitre<nico@fluxnic.net> > Cc: Greg Kroah-Hartman<gregkh@suse.de> > Cc: Mike Frysinger<vapier@gentoo.org> > Signed-off-by: Daniel Walker<dwalker@codeaurora.org> > --- > drivers/char/Kconfig | 9 +++ > drivers/char/Makefile | 1 + > drivers/char/hvc_dcc.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++ ... this DCC driver implements "one channel", but what about implementing "multiple channels". For example reserve few(3) bits for channel number, and two bits for carried data, then fill the rest bytes with with some data and send the word(32 bits) over DCC. On the Linux side writing on /dev/hvcX puts the number X as channel number, and on the other side the CPU emulator gets the data and redistribute it to TCP/IP socket. I have started write some code implementing this. Are there any one interested in this multiple channels, and are there any one started to work on it? Regards, Asen ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support 2011-04-07 18:39 ` RONETIX - Asen Dimov @ 2011-04-07 18:57 ` Mike Frysinger -1 siblings, 0 replies; 79+ messages in thread From: Mike Frysinger @ 2011-04-07 18:57 UTC (permalink / raw) To: RONETIX - Asen Dimov Cc: Daniel Walker, linux-kernel, Randy Dunlap, Arnd Bergmann, Nicolas Pitre, Tony Lindgren, linux-arm-msm, Greg Kroah-Hartman, FUJITA Tomonori, Andrew Morton, linuxppc-dev, Alan Cox On Thu, Apr 7, 2011 at 14:39, RONETIX - Asen Dimov wrote: > On 11/30/2010 09:25 PM, Daniel Walker wrote: >> This driver adds a basic console that uses the arm JTAG >> DCC to transfer data back and forth. It has support for >> ARMv6 and ARMv7. >> >> This console is created under the HVC driver, and should be named >> /dev/hvcX (or /dev/hvc0 for example). >> >> drivers/char/Kconfig | 9 +++ >> drivers/char/Makefile | 1 + >> drivers/char/hvc_dcc.c | 133 >> ++++++++++++++++++++++++++++++++++++++++++++++++ > > ... > > this DCC driver implements "one channel", but what about implementing > "multiple channels". For example reserve few(3) bits for channel number, > and two bits for carried data, then fill the rest bytes with with some data > and send the word(32 bits) over DCC. On the Linux side writing on /dev/hvcX > puts the number X as channel number, and on the other side the CPU > emulator gets the data and redistribute it to TCP/IP socket. > > I have started write some code implementing this. Are there any one > interested > in this multiple channels, and are there any one started to work on it? this sort of multiplexing of the data stream sounds like the job for userspace ? or maybe a line discipline ? inserting structured data into the kernel driver doesnt sound right to me ... -mike ^ permalink raw reply [flat|nested] 79+ messages in thread
* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support @ 2011-04-07 18:57 ` Mike Frysinger 0 siblings, 0 replies; 79+ messages in thread From: Mike Frysinger @ 2011-04-07 18:57 UTC (permalink / raw) To: RONETIX - Asen Dimov Cc: Randy Dunlap, Daniel Walker, Arnd Bergmann, Nicolas Pitre, Tony Lindgren, linux-arm-msm, Greg Kroah-Hartman, linux-kernel, FUJITA Tomonori, Andrew Morton, linuxppc-dev, Alan Cox On Thu, Apr 7, 2011 at 14:39, RONETIX - Asen Dimov wrote: > On 11/30/2010 09:25 PM, Daniel Walker wrote: >> This driver adds a basic console that uses the arm JTAG >> DCC to transfer data back and forth. It has support for >> ARMv6 and ARMv7. >> >> This console is created under the HVC driver, and should be named >> /dev/hvcX (or /dev/hvc0 for example). >> >> =C2=A0drivers/char/Kconfig =C2=A0 | =C2=A0 =C2=A09 +++ >> =C2=A0drivers/char/Makefile =C2=A0| =C2=A0 =C2=A01 + >> =C2=A0drivers/char/hvc_dcc.c | =C2=A0133 >> ++++++++++++++++++++++++++++++++++++++++++++++++ > > ... > > this DCC driver implements "one channel", but what about implementing > "multiple channels". For example reserve few(3) bits for channel number, > and two bits for carried data, then fill the rest bytes with with some da= ta > and send the word(32 bits) over DCC. On the Linux side writing on /dev/hv= cX > puts the number X as channel number, and on the other side the CPU > emulator gets the data and redistribute it to TCP/IP socket. > > I have started write some code implementing this. Are there any one > interested > in this multiple channels, and are there any one started to work on it? this sort of multiplexing of the data stream sounds like the job for userspace ? or maybe a line discipline ? inserting structured data into the kernel driver doesnt sound right to me ... -mike ^ permalink raw reply [flat|nested] 79+ messages in thread
end of thread, other threads:[~2011-04-07 18:58 UTC | newest] Thread overview: 79+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-11-30 19:25 [PATCH] drivers: char: hvc: add arm JTAG DCC console support Daniel Walker 2010-11-30 19:25 ` Daniel Walker 2010-11-30 19:57 ` Nicolas Pitre 2010-11-30 19:57 ` Nicolas Pitre 2010-11-30 21:17 ` Arnd Bergmann 2010-11-30 21:17 ` Arnd Bergmann 2010-12-01 5:30 ` Stephen Boyd 2010-12-01 5:30 ` Stephen Boyd 2010-12-01 18:54 ` Daniel Walker 2010-12-01 18:54 ` Daniel Walker 2010-12-01 19:28 ` Greg KH 2010-12-01 19:28 ` Greg KH 2010-12-18 5:16 ` [PATCH] hvc_dcc: Simplify assembly for v6 and v7 ARM Stephen Boyd 2010-12-18 5:16 ` Stephen Boyd 2010-12-20 17:51 ` Daniel Walker 2010-12-20 17:51 ` Daniel Walker 2010-12-20 18:39 ` Stephen Boyd 2010-12-20 18:39 ` Stephen Boyd 2010-12-20 18:46 ` Nicolas Pitre 2010-12-20 18:46 ` Nicolas Pitre 2010-12-20 20:08 ` [PATCH 0/3] hvc_dcc cleanups and fixes Stephen Boyd 2010-12-20 20:08 ` Stephen Boyd 2010-12-20 20:08 ` [PATCH 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile Stephen Boyd 2010-12-20 20:08 ` Stephen Boyd 2010-12-20 21:39 ` Nicolas Pitre 2010-12-20 21:39 ` Nicolas Pitre 2011-01-02 9:00 ` Pavel Machek 2011-01-02 9:00 ` Pavel Machek 2011-01-02 18:49 ` David Brown 2011-01-02 18:49 ` David Brown 2011-01-03 5:50 ` Pavel Machek 2011-01-03 5:50 ` Pavel Machek 2011-01-04 18:49 ` Tony Lindgren 2011-01-04 18:49 ` Tony Lindgren 2010-12-20 21:49 ` Arnaud Lacombe 2010-12-20 21:49 ` Arnaud Lacombe 2010-12-20 21:52 ` Stephen Boyd 2010-12-20 21:52 ` Stephen Boyd 2010-12-20 22:10 ` Nicolas Pitre 2010-12-20 22:10 ` Nicolas Pitre 2010-12-20 20:08 ` [PATCH 2/3] hvc_dcc: Simplify put_chars()/get_chars() loops Stephen Boyd 2010-12-20 20:08 ` Stephen Boyd 2010-12-20 20:08 ` [PATCH 3/3] hvc_dcc: Simplify assembly for v6 and v7 ARM Stephen Boyd 2010-12-20 20:08 ` Stephen Boyd 2010-12-20 21:44 ` Nicolas Pitre 2010-12-20 21:44 ` Nicolas Pitre 2011-01-04 18:52 ` Tony Lindgren 2011-01-04 18:52 ` Tony Lindgren 2011-01-06 1:49 ` [PATCH 0/3] hvc_dcc cleanups and fixes Stephen Boyd 2011-01-06 1:49 ` Stephen Boyd 2011-01-06 3:20 ` Greg KH 2011-01-06 3:20 ` Greg KH 2011-02-03 22:17 ` Greg KH 2011-02-03 22:17 ` Greg KH 2011-02-03 22:17 ` Greg KH 2011-02-03 23:19 ` Stephen Boyd 2011-02-03 23:19 ` Stephen Boyd 2011-02-03 23:30 ` Greg KH 2011-02-03 23:30 ` Greg KH 2011-02-03 23:48 ` [PATCHv2 " Stephen Boyd 2011-02-03 23:48 ` Stephen Boyd 2011-02-03 23:48 ` [PATCHv2 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile Stephen Boyd 2011-02-03 23:48 ` Stephen Boyd 2011-02-03 23:48 ` [PATCHv2 2/3] hvc_dcc: Simplify put_chars()/get_chars() loops Stephen Boyd 2011-02-03 23:48 ` Stephen Boyd 2011-02-03 23:48 ` [PATCHv2 3/3] hvc_dcc: Simplify assembly for v6 and v7 ARM Stephen Boyd 2011-02-03 23:48 ` Stephen Boyd 2010-12-01 20:20 ` [PATCH] drivers: char: hvc: add arm JTAG DCC console support Stephen Boyd 2010-12-01 20:20 ` Stephen Boyd 2010-12-07 19:10 ` Stephen Boyd 2010-12-07 19:10 ` Stephen Boyd 2011-01-14 19:19 ` Tony Lindgren 2011-01-14 19:19 ` Tony Lindgren 2011-01-14 23:49 ` Stephen Boyd 2011-01-14 23:49 ` Stephen Boyd 2011-04-07 18:39 ` RONETIX - Asen Dimov 2011-04-07 18:39 ` RONETIX - Asen Dimov 2011-04-07 18:57 ` Mike Frysinger 2011-04-07 18:57 ` Mike Frysinger
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.