Re: [PATCH 1/3] qe-uart: modify qe-uart to adapt both powerpc and arm
On Mon, 2014-10-13 at 11:30 -0500, Timur Tabi wrote:
On Fri, Oct 10, 2014 at 1:05 PM, Scott Wood scottw...@freescale.com wrote:
There are many changes in here that ought to be separate patches with
separate justification.
Also, some of the QE changes seem to be reasonable cleanup, but not
related to making the code work on ARM.
I agree with Scott. This patch already makes significant code
changes, so you should have one patch that just makes the
out_be32-iowrite32be changes. Changes to the QE library should NOT
be in the same patch as changes to ucc_uart.c.
In addition, changes like this:
- iprop = of_get_property(np, port-number, NULL);
- if (!iprop) {
+ ret = of_property_read_u32_index(np, port-number, 0, val);
+ if (ret) {
should be changed to remove the OF dependency. If you're going to
replace of_get_property, replace it with device_property_read_u32(),
to remove the OF dependency.
diff --git a/arch/arm/include/asm/delay.h b/arch/arm/include/asm/delay.h
index dff714d..a932f99 100644
--- a/arch/arm/include/asm/delay.h
+++ b/arch/arm/include/asm/delay.h
@@ -57,6 +57,22 @@ extern void __bad_udelay(void);
__const_udelay((n) * UDELAY_MULT)) :\
__udelay(n))
+#define spin_event_timeout(condition, timeout, delay)
\
+({
\
+ typeof(condition) __ret;
\
+ int i = 0;
\
+ while (!(__ret = (condition)) (i++ timeout)) {
\
+ if (delay)
\
+ udelay(delay);
\
+ else
\
+ cpu_relax();
\
+ udelay(1);
\
+ }
\
This will delay too long if delay is used.
Shouldn't ARM have a version of tb_ticks_since() by now?
There's get_cycles(), but it's not clear to me whether loops_per_jiffy
is OK to use with get_cycles() on 32-bit ARM. Is avoiding the udelay
worth making this non-generic?
+ if (!__ret)
\
+ __ret = (condition);
\
+ __ret;
\
Timur, do you remember why that final if (!__ret) __ret = (condition);
is needed?
powerpc: Fix spin_event_timeout() to be robust over context switches
Current implementation of spin_event_timeout can be interrupted by an
IRQ or context switch after testing the condition, but before checking
the timeout. This can cause the loop to report a timeout when the
condition actually became true in the middle.
This patch adds one final check of the condition upon exit of the loop
if the last test of the condition was still false.
OK, so this shouldn't be needed in the udelay version, since an
interrupt shouldn't cause a significant difference in the timeout count.
-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/3] qe-uart: modify qe-uart to adapt both powerpc and arm
On Fri, Oct 10, 2014 at 1:05 PM, Scott Wood scottw...@freescale.com wrote:
There are many changes in here that ought to be separate patches with
separate justification.
Also, some of the QE changes seem to be reasonable cleanup, but not
related to making the code work on ARM.
I agree with Scott. This patch already makes significant code
changes, so you should have one patch that just makes the
out_be32-iowrite32be changes. Changes to the QE library should NOT
be in the same patch as changes to ucc_uart.c.
In addition, changes like this:
- iprop = of_get_property(np, port-number, NULL);
- if (!iprop) {
+ ret = of_property_read_u32_index(np, port-number, 0, val);
+ if (ret) {
should be changed to remove the OF dependency. If you're going to
replace of_get_property, replace it with device_property_read_u32(),
to remove the OF dependency.
diff --git a/arch/arm/include/asm/delay.h b/arch/arm/include/asm/delay.h
index dff714d..a932f99 100644
--- a/arch/arm/include/asm/delay.h
+++ b/arch/arm/include/asm/delay.h
@@ -57,6 +57,22 @@ extern void __bad_udelay(void);
__const_udelay((n) * UDELAY_MULT)) :\
__udelay(n))
+#define spin_event_timeout(condition, timeout, delay)
\
+({
\
+ typeof(condition) __ret;
\
+ int i = 0;
\
+ while (!(__ret = (condition)) (i++ timeout)) {
\
+ if (delay)
\
+ udelay(delay);
\
+ else
\
+ cpu_relax();
\
+ udelay(1);
\
+ }
\
This will delay too long if delay is used.
Shouldn't ARM have a version of tb_ticks_since() by now?
+ if (!__ret)
\
+ __ret = (condition);
\
+ __ret;
\
Timur, do you remember why that final if (!__ret) __ret = (condition);
is needed?
powerpc: Fix spin_event_timeout() to be robust over context switches
Current implementation of spin_event_timeout can be interrupted by an
IRQ or context switch after testing the condition, but before checking
the timeout. This can cause the loop to report a timeout when the
condition actually became true in the middle.
This patch adds one final check of the condition upon exit of the loop
if the last test of the condition was still false.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/3] qe-uart: modify qe-uart to adapt both powerpc and arm
qe has been supported by arm board ls1021, qe-uart need
to be supported by ls1021.
modify the code to make qe-uart can work on both powerpc
and ls1021.
Signed-off-by: Zhao Qiang b45...@freescale.com
---
arch/arm/include/asm/delay.h | 16
arch/arm/include/asm/io.h | 28 +++
arch/arm/include/asm/irq.h| 2 +
arch/arm/kernel/irq.c | 7 ++
drivers/soc/qe/Kconfig| 1 -
drivers/soc/qe/qe.c | 63 ---
drivers/soc/qe/qe_common.c| 2 +-
drivers/soc/qe/qe_ic.c| 7 +-
drivers/soc/qe/qe_io.c| 53 ++---
drivers/soc/qe/ucc_slow.c | 40 +-
drivers/tty/serial/ucc_uart.c | 176 +-
include/linux/fsl/qe.h| 21 +
12 files changed, 245 insertions(+), 171 deletions(-)
diff --git a/arch/arm/include/asm/delay.h b/arch/arm/include/asm/delay.h
index dff714d..a932f99 100644
--- a/arch/arm/include/asm/delay.h
+++ b/arch/arm/include/asm/delay.h
@@ -57,6 +57,22 @@ extern void __bad_udelay(void);
__const_udelay((n) * UDELAY_MULT)) :\
__udelay(n))
+#define spin_event_timeout(condition, timeout, delay)
\
+({
\
+ typeof(condition) __ret; \
+ int i = 0; \
+ while (!(__ret = (condition)) (i++ timeout)) {\
+ if (delay) \
+ udelay(delay); \
+ else \
+ cpu_relax(); \
+ udelay(1); \
+ } \
+ if (!__ret)\
+ __ret = (condition); \
+ __ret; \
+})
+
/* Loop-based definitions for assembly code. */
extern void __loop_delay(unsigned long loops);
extern void __loop_udelay(unsigned long usecs);
diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index d070741..4bec694 100644
--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -206,6 +206,34 @@ extern int pci_ioremap_io(unsigned int offset, phys_addr_t
phys_addr);
#endif
#endif
+/* access ports */
+#define setbits32(_addr, _v) iowrite32be(ioread32be(_addr) | (_v), (_addr))
+#define clrbits32(_addr, _v) iowrite32be(ioread32be(_addr) ~(_v), (_addr))
+
+#define setbits16(_addr, _v) iowrite16be(ioread16be(_addr) | (_v), (_addr))
+#define clrbits16(_addr, _v) iowrite16be(ioread16be(_addr) ~(_v), (_addr))
+
+#define setbits8(_addr, _v) iowrite8(ioread8(_addr) | (_v), (_addr))
+#define clrbits8(_addr, _v) iowrite8(ioread8(_addr) ~(_v), (_addr))
+
+/* Clear and set bits in one shot. These macros can be used to clear and
+ * set multiple bits in a register using a single read-modify-write. These
+ * macros can also be used to set a multiple-bit bit pattern using a mask,
+ * by specifying the mask in the 'clear' parameter and the new bit pattern
+ * in the 'set' parameter.
+ */
+
+#define clrsetbits_be32(addr, clear, set) \
+ iowrite32be((ioread32be(addr) ~(clear)) | (set), (addr))
+#define clrsetbits_le32(addr, clear, set) \
+ iowrite32le((ioread32le(addr) ~(clear)) | (set), (addr))
+#define clrsetbits_be16(addr, clear, set) \
+ iowrite16be((ioread16be(addr) ~(clear)) | (set), (addr))
+#define clrsetbits_le16(addr, clear, set) \
+ iowrite16le((ioread16le(addr) ~(clear)) | (set), (addr))
+#define clrsetbits_8(addr, clear, set) \
+ iowrite8((ioread8(addr) ~(clear)) | (set), (addr))
+
/*
* IO port access primitives
* -
diff --git a/arch/arm/include/asm/irq.h b/arch/arm/include/asm/irq.h
index 53c15de..4358904 100644
--- a/arch/arm/include/asm/irq.h
+++ b/arch/arm/include/asm/irq.h
@@ -30,6 +30,8 @@ extern void asm_do_IRQ(unsigned int, struct pt_regs *);
void handle_IRQ(unsigned int, struct pt_regs *);
void init_IRQ(void);
+extern irq_hw_number_t virq_to_hw(unsigned int virq);
+
#ifdef CONFIG_MULTI_IRQ_HANDLER
extern void (*handle_arch_irq)(struct pt_regs *);
extern void set_handle_irq(void (*handle_irq)(struct pt_regs *));
diff --git a/arch/arm/kernel/irq.c b/arch/arm/kernel/irq.c
index 9723d17..afa204a 100644
--- a/arch/arm/kernel/irq.c
+++ b/arch/arm/kernel/irq.c
@@ -121,6 +121,13 @@ void __init init_IRQ(void)
machine_desc-init_irq();
}
+irq_hw_number_t virq_to_hw(unsigned int virq)
+{
+ struct irq_data *irq_data =
Re: [PATCH 1/3] qe-uart: modify qe-uart to adapt both powerpc and arm
On Fri, 2014-10-10 at 14:47 +0800, Zhao Qiang wrote:
qe has been supported by arm board ls1021, qe-uart need
to be supported by ls1021.
modify the code to make qe-uart can work on both powerpc
and ls1021.
Signed-off-by: Zhao Qiang b45...@freescale.com
---
arch/arm/include/asm/delay.h | 16
arch/arm/include/asm/io.h | 28 +++
arch/arm/include/asm/irq.h| 2 +
arch/arm/kernel/irq.c | 7 ++
drivers/soc/qe/Kconfig| 1 -
drivers/soc/qe/qe.c | 63 ---
drivers/soc/qe/qe_common.c| 2 +-
drivers/soc/qe/qe_ic.c| 7 +-
drivers/soc/qe/qe_io.c| 53 ++---
drivers/soc/qe/ucc_slow.c | 40 +-
drivers/tty/serial/ucc_uart.c | 176
+-
include/linux/fsl/qe.h| 21 +
12 files changed, 245 insertions(+), 171 deletions(-)
This patch obviously depends on the patches to relocate QE support, but
there's no mention of that dependency above.
There are many changes in here that ought to be separate patches with
separate justification.
Also, some of the QE changes seem to be reasonable cleanup, but not
related to making the code work on ARM.
diff --git a/arch/arm/include/asm/delay.h b/arch/arm/include/asm/delay.h
index dff714d..a932f99 100644
--- a/arch/arm/include/asm/delay.h
+++ b/arch/arm/include/asm/delay.h
@@ -57,6 +57,22 @@ extern void __bad_udelay(void);
__const_udelay((n) * UDELAY_MULT)) :\
__udelay(n))
+#define spin_event_timeout(condition, timeout, delay)
\
+({
\
+ typeof(condition) __ret; \
+ int i = 0; \
+ while (!(__ret = (condition)) (i++ timeout)) {\
+ if (delay) \
+ udelay(delay); \
+ else \
+ cpu_relax(); \
+ udelay(1); \
+ } \
This will delay too long if delay is used.
How about:
delay = delay ? delay : 1; \
while (!(__ret = (condition))) {\
if (i timeout)\
break; \
i += delay; \
udelay(delay); \
} \
Also, once the dependency on timebase is removed, how about making this
generic rather than just PPC+ARM?
+ if (!__ret)\
+ __ret = (condition); \
+ __ret; \
Timur, do you remember why that final if (!__ret) __ret = (condition);
is needed?
+})
+
/* Loop-based definitions for assembly code. */
extern void __loop_delay(unsigned long loops);
extern void __loop_udelay(unsigned long usecs);
diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index d070741..4bec694 100644
--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -206,6 +206,34 @@ extern int pci_ioremap_io(unsigned int offset,
phys_addr_t phys_addr);
#endif
#endif
+/* access ports */
+#define setbits32(_addr, _v) iowrite32be(ioread32be(_addr) | (_v), (_addr))
+#define clrbits32(_addr, _v) iowrite32be(ioread32be(_addr) ~(_v), (_addr))
+
+#define setbits16(_addr, _v) iowrite16be(ioread16be(_addr) | (_v), (_addr))
+#define clrbits16(_addr, _v) iowrite16be(ioread16be(_addr) ~(_v), (_addr))
+
+#define setbits8(_addr, _v) iowrite8(ioread8(_addr) | (_v), (_addr))
+#define clrbits8(_addr, _v) iowrite8(ioread8(_addr) ~(_v), (_addr))
This should also be a separate patch, though I don't think implicit big
endian is going to fly in arch/arm (it was a mistake in arch/powerpc).
Rename to setbits_be32 etc as is used in U-Boot.
+/* Clear and set bits in one shot. These macros can be used to clear and
+ * set multiple bits in a register using a single read-modify-write. These
+ * macros can also be used to set a multiple-bit bit pattern using a mask,
+ * by specifying the mask in the 'clear' parameter and the new bit pattern
+ * in the 'set' parameter.
+ */
+
+#define clrsetbits_be32(addr, clear, set) \
+ iowrite32be((ioread32be(addr) ~(clear)) | (set), (addr))
+#define clrsetbits_le32(addr, clear, set) \
+ iowrite32le((ioread32le(addr) ~(clear)) |
