Hello Andrew, Russell,
These are small cleanups that I keep resending since Aug. Andrew, can you
please take them for the time being?
The rationale is the same:
During KDB FIQ patches review Russell mentioned that I should not
introduce another FIQ_START. It seems that in v3.6-rc the FIQ_START issue
was somewhat band-aided, i.e. machines don't necessary need to define this
stuff any longer, but I also read the background of the issue, and Russell
once said that he don't want the FIQ subsystem to mess with genirq.
So, the patch set is pretty straightforward:
- Get rid of FIQ_START. Nobody but platform-specific code (and
platform-specific drivers) should know the details about which interrupt
can be routed to FIQ and which cannot;
- Remove disable/enable_fiq() calls from the FIQ subsys.
Thanks!
--
arch/arm/include/asm/fiq.h | 2 -
arch/arm/include/asm/mach/irq.h | 7 +++-
arch/arm/kernel/fiq.c | 37 +++++--------------
arch/arm/kernel/irq.c | 2 -
arch/arm/mach-omap1/include/mach/irqs.h | 4 --
arch/arm/mach-rpc/dma.c | 4 +-
arch/arm/mach-rpc/include/mach/irqs.h | 8 ++--
arch/arm/mach-rpc/irq.c | 21 ++---------
arch/arm/mach-s3c24xx/include/mach/irqs.h | 3 --
arch/arm/plat-mxc/avic.c | 5 ---
arch/arm/plat-mxc/include/mach/irqs.h | 2 -
arch/arm/plat-mxc/tzic.c | 5 ---
arch/arm/plat-s3c24xx/irq.c | 6 +--
.../media/platform/soc_camera/mx1_camera.c | 6 +--
sound/soc/fsl/imx-pcm-fiq.c | 4 +-
15 files changed, 31 insertions(+), 85 deletions(-)
mach-rps registers FIQ controller with genirq, which makes no sense:
these FIQs cannot be routed to IRQs, so there is no need to register
it with genirq.
This effectively makes FIQ_START irrelevant.
Signed-off-by: Anton Vorontsov <[email protected]>
---
arch/arm/mach-rpc/dma.c | 4 ++--
arch/arm/mach-rpc/include/mach/irqs.h | 5 +++++
arch/arm/mach-rpc/irq.c | 19 ++++---------------
3 files changed, 11 insertions(+), 17 deletions(-)
diff --git a/arch/arm/mach-rpc/dma.c b/arch/arm/mach-rpc/dma.c
index 85883b2..4a525be 100644
--- a/arch/arm/mach-rpc/dma.c
+++ b/arch/arm/mach-rpc/dma.c
@@ -289,13 +289,13 @@ static void floppy_enable_dma(unsigned int chan, dma_t *dma)
set_fiq_handler(fiqhandler_start, fiqhandler_length);
set_fiq_regs(®s);
- enable_fiq(fdma->fiq);
+ iomd_unmask_fiq(fdma->fiq);
}
static void floppy_disable_dma(unsigned int chan, dma_t *dma)
{
struct floppy_dma *fdma = container_of(dma, struct floppy_dma, dma);
- disable_fiq(fdma->fiq);
+ iomd_mask_fiq(fdma->fiq);
release_fiq(&fh);
}
diff --git a/arch/arm/mach-rpc/include/mach/irqs.h b/arch/arm/mach-rpc/include/mach/irqs.h
index 6868e17..f27ead1 100644
--- a/arch/arm/mach-rpc/include/mach/irqs.h
+++ b/arch/arm/mach-rpc/include/mach/irqs.h
@@ -37,6 +37,11 @@
#define FIQ_EXPANSIONCARD 6
#define FIQ_FORCE 7
+#ifndef __ASSEMBLY__
+extern void iomd_mask_fiq(int fiq);
+extern void iomd_unmask_fiq(int fiq);
+#endif
+
/*
* This is the offset of the FIQ "IRQ" numbers
*/
diff --git a/arch/arm/mach-rpc/irq.c b/arch/arm/mach-rpc/irq.c
index 3e4fa84..a4221b3 100644
--- a/arch/arm/mach-rpc/irq.c
+++ b/arch/arm/mach-rpc/irq.c
@@ -89,30 +89,24 @@ static struct irq_chip iomd_dma_chip = {
.irq_unmask = iomd_unmask_irq_dma,
};
-static void iomd_mask_irq_fiq(struct irq_data *d)
+void iomd_mask_fiq(int fiq)
{
unsigned int val, mask;
- mask = 1 << (d->irq & 7);
+ mask = 1 << (fiq & 7);
val = iomd_readb(IOMD_FIQMASK);
iomd_writeb(val & ~mask, IOMD_FIQMASK);
}
-static void iomd_unmask_irq_fiq(struct irq_data *d)
+void iomd_unmask_fiq(int fiq)
{
unsigned int val, mask;
- mask = 1 << (d->irq & 7);
+ mask = 1 << (fiq & 7);
val = iomd_readb(IOMD_FIQMASK);
iomd_writeb(val | mask, IOMD_FIQMASK);
}
-static struct irq_chip iomd_fiq_chip = {
- .irq_ack = iomd_mask_irq_fiq,
- .irq_mask = iomd_mask_irq_fiq,
- .irq_unmask = iomd_unmask_irq_fiq,
-};
-
extern unsigned char rpc_default_fiq_start, rpc_default_fiq_end;
void __init rpc_init_irq(void)
@@ -155,11 +149,6 @@ void __init rpc_init_irq(void)
handle_level_irq);
set_irq_flags(irq, flags);
break;
-
- case 64 ... 71:
- irq_set_chip(irq, &iomd_fiq_chip);
- set_irq_flags(irq, IRQF_VALID);
- break;
}
}
--
1.8.0
We're about to remove FIQ_START mess, so move the platform-specific
detail inside platform-specific s3c24xx_set_fiq().
Signed-off-by: Anton Vorontsov <[email protected]>
Acked-by: Kukjin Kim <[email protected]>
---
arch/arm/plat-s3c24xx/irq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/plat-s3c24xx/irq.c b/arch/arm/plat-s3c24xx/irq.c
index fe57bbb..e4e9567 100644
--- a/arch/arm/plat-s3c24xx/irq.c
+++ b/arch/arm/plat-s3c24xx/irq.c
@@ -503,7 +503,7 @@ int s3c24xx_set_fiq(unsigned int irq, bool on)
unsigned offs;
if (on) {
- offs = irq - FIQ_START;
+ offs = irq - IRQ_EINT0;
if (offs > 31)
return -EINVAL;
--
1.8.0
There are no users left, so these can be removed.
Signed-off-by: Anton Vorontsov <[email protected]>
---
arch/arm/include/asm/fiq.h | 2 --
arch/arm/kernel/fiq.c | 15 ---------------
2 files changed, 17 deletions(-)
diff --git a/arch/arm/include/asm/fiq.h b/arch/arm/include/asm/fiq.h
index d493d0b..a293be4 100644
--- a/arch/arm/include/asm/fiq.h
+++ b/arch/arm/include/asm/fiq.h
@@ -36,8 +36,6 @@ struct fiq_handler {
extern int claim_fiq(struct fiq_handler *f);
extern void release_fiq(struct fiq_handler *f);
extern void set_fiq_handler(void *start, unsigned int length);
-extern void enable_fiq(int fiq);
-extern void disable_fiq(int fiq);
/* helpers defined in fiqasm.S: */
extern void __set_fiq_regs(unsigned long const *regs);
diff --git a/arch/arm/kernel/fiq.c b/arch/arm/kernel/fiq.c
index 2adda11..29b93b8 100644
--- a/arch/arm/kernel/fiq.c
+++ b/arch/arm/kernel/fiq.c
@@ -122,28 +122,13 @@ void release_fiq(struct fiq_handler *f)
while (current_fiq->fiq_op(current_fiq->dev_id, 0));
}
-static int fiq_start;
-
-void enable_fiq(int fiq)
-{
- enable_irq(fiq + fiq_start);
-}
-
-void disable_fiq(int fiq)
-{
- disable_irq(fiq + fiq_start);
-}
-
EXPORT_SYMBOL(set_fiq_handler);
EXPORT_SYMBOL(__set_fiq_regs); /* defined in fiqasm.S */
EXPORT_SYMBOL(__get_fiq_regs); /* defined in fiqasm.S */
EXPORT_SYMBOL(claim_fiq);
EXPORT_SYMBOL(release_fiq);
-EXPORT_SYMBOL(enable_fiq);
-EXPORT_SYMBOL(disable_fiq);
void __init init_FIQ(int start)
{
no_fiq_insn = *(unsigned long *)0xffff001c;
- fiq_start = start;
}
--
1.8.0
The driver uses platform-specific mxc_set_irq_fiq() with the VIRQ cookie
passed to it, so it's pretty clear that the driver is absolutely sure
that the FIQ is routed via platform-specific IC, and that the cookie can
be used to mask/unmask FIQs. So, let's switch to the genirq routines,
since we're about to remove FIQ-specific variants.
Signed-off-by: Anton Vorontsov <[email protected]>
Acked-by: Sascha Hauer <[email protected]>
---
drivers/media/platform/soc_camera/mx1_camera.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/soc_camera/mx1_camera.c b/drivers/media/platform/soc_camera/mx1_camera.c
index bbe7099..4855a46 100644
--- a/drivers/media/platform/soc_camera/mx1_camera.c
+++ b/drivers/media/platform/soc_camera/mx1_camera.c
@@ -801,7 +801,7 @@ static int __init mx1_camera_probe(struct platform_device *pdev)
set_fiq_regs(®s);
mxc_set_irq_fiq(irq, 1);
- enable_fiq(irq);
+ enable_irq(irq);
pcdev->soc_host.drv_name = DRIVER_NAME;
pcdev->soc_host.ops = &mx1_soc_camera_host_ops;
@@ -817,7 +817,7 @@ static int __init mx1_camera_probe(struct platform_device *pdev)
return 0;
exit_free_irq:
- disable_fiq(irq);
+ disable_irq(irq);
mxc_set_irq_fiq(irq, 0);
release_fiq(&fh);
exit_free_dma:
@@ -842,7 +842,7 @@ static int __exit mx1_camera_remove(struct platform_device *pdev)
struct resource *res;
imx_dma_free(pcdev->dma_chan);
- disable_fiq(pcdev->irq);
+ disable_irq(pcdev->irq);
mxc_set_irq_fiq(pcdev->irq, 0);
release_fiq(&fh);
--
1.8.0
RPC:
FIQ_START is irrelevant nowadays, the arch uses platform-specific
iomd_{,un}mask_fiq() calls.
OMAP1:
The only user of FIQs is MACH_AMS_DELTA, and in particular its
drivers/input/serio/ams_delta_serio.c driver. The driver does not rely on
the FIQ interrupts directly, instead it uses a "deferred fiq" interrupt
(raised after a buffer filled by the FIQ routine).
The FIQ handling routines are not using disable_fiq() or enable_fiq()
stuff, so it currently does not use FIQ_START at all -- the asm code uses
OMAP_IH1_BASE and twiddles the bits itself.
S3C:
The only user of FIQs on s3c24xx is spi-s3c24xx driver. The driver works
with interrupt controller directly (via S3C24XX_VA_IRQ base address), and
does not use IRQ subsystem to mask/unmask IRQs.
MXC:
Users now use enable/disable_irq() routines. The drivers rely on a
correctly passed VIRQ cookie anyway, so FIQ_START becomes irrelevant.
Signed-off-by: Anton Vorontsov <[email protected]>
---
arch/arm/include/asm/mach/irq.h | 2 +-
arch/arm/kernel/fiq.c | 2 +-
arch/arm/mach-omap1/include/mach/irqs.h | 4 ----
arch/arm/mach-rpc/include/mach/irqs.h | 5 -----
arch/arm/mach-rpc/irq.c | 2 +-
arch/arm/mach-s3c24xx/include/mach/irqs.h | 3 ---
arch/arm/plat-mxc/avic.c | 2 +-
arch/arm/plat-mxc/include/mach/irqs.h | 2 --
arch/arm/plat-mxc/tzic.c | 2 +-
arch/arm/plat-s3c24xx/irq.c | 2 +-
10 files changed, 6 insertions(+), 20 deletions(-)
diff --git a/arch/arm/include/asm/mach/irq.h b/arch/arm/include/asm/mach/irq.h
index 15cb035..febe495 100644
--- a/arch/arm/include/asm/mach/irq.h
+++ b/arch/arm/include/asm/mach/irq.h
@@ -17,7 +17,7 @@ struct seq_file;
/*
* This is internal. Do not use it.
*/
-extern void init_FIQ(int);
+extern void init_FIQ(void);
extern int show_fiq_list(struct seq_file *, int);
#ifdef CONFIG_MULTI_IRQ_HANDLER
diff --git a/arch/arm/kernel/fiq.c b/arch/arm/kernel/fiq.c
index 29b93b8..bd369c5 100644
--- a/arch/arm/kernel/fiq.c
+++ b/arch/arm/kernel/fiq.c
@@ -128,7 +128,7 @@ EXPORT_SYMBOL(__get_fiq_regs); /* defined in fiqasm.S */
EXPORT_SYMBOL(claim_fiq);
EXPORT_SYMBOL(release_fiq);
-void __init init_FIQ(int start)
+void __init init_FIQ(void)
{
no_fiq_insn = *(unsigned long *)0xffff001c;
}
diff --git a/arch/arm/mach-omap1/include/mach/irqs.h b/arch/arm/mach-omap1/include/mach/irqs.h
index 729992d..f254bb6 100644
--- a/arch/arm/mach-omap1/include/mach/irqs.h
+++ b/arch/arm/mach-omap1/include/mach/irqs.h
@@ -261,8 +261,4 @@
#include <mach/hardware.h>
-#ifdef CONFIG_FIQ
-#define FIQ_START 1024
-#endif
-
#endif
diff --git a/arch/arm/mach-rpc/include/mach/irqs.h b/arch/arm/mach-rpc/include/mach/irqs.h
index f27ead1..2536543 100644
--- a/arch/arm/mach-rpc/include/mach/irqs.h
+++ b/arch/arm/mach-rpc/include/mach/irqs.h
@@ -42,9 +42,4 @@ extern void iomd_mask_fiq(int fiq);
extern void iomd_unmask_fiq(int fiq);
#endif
-/*
- * This is the offset of the FIQ "IRQ" numbers
- */
-#define FIQ_START 64
-
#define NR_IRQS 128
diff --git a/arch/arm/mach-rpc/irq.c b/arch/arm/mach-rpc/irq.c
index a4221b3..07770c8 100644
--- a/arch/arm/mach-rpc/irq.c
+++ b/arch/arm/mach-rpc/irq.c
@@ -152,6 +152,6 @@ void __init rpc_init_irq(void)
}
}
- init_FIQ(FIQ_START);
+ init_FIQ();
}
diff --git a/arch/arm/mach-s3c24xx/include/mach/irqs.h b/arch/arm/mach-s3c24xx/include/mach/irqs.h
index b7a9f4d..7d66d41 100644
--- a/arch/arm/mach-s3c24xx/include/mach/irqs.h
+++ b/arch/arm/mach-s3c24xx/include/mach/irqs.h
@@ -209,7 +209,4 @@
#define IRQ_S3C244X_AC97 IRQ_S3C2443_AC97
#endif
-/* Our FIQs are routable from IRQ_EINT0 to IRQ_ADCPARENT */
-#define FIQ_START IRQ_EINT0
-
#endif /* __ASM_ARCH_IRQ_H */
diff --git a/arch/arm/plat-mxc/avic.c b/arch/arm/plat-mxc/avic.c
index cbd55c3..19701ec 100644
--- a/arch/arm/plat-mxc/avic.c
+++ b/arch/arm/plat-mxc/avic.c
@@ -218,7 +218,7 @@ void __init mxc_init_irq(void __iomem *irqbase)
#ifdef CONFIG_FIQ
/* Initialize FIQ */
- init_FIQ(FIQ_START);
+ init_FIQ();
#endif
printk(KERN_INFO "MXC IRQ initialized\n");
diff --git a/arch/arm/plat-mxc/include/mach/irqs.h b/arch/arm/plat-mxc/include/mach/irqs.h
index d73f5e8..2ec942f 100644
--- a/arch/arm/plat-mxc/include/mach/irqs.h
+++ b/arch/arm/plat-mxc/include/mach/irqs.h
@@ -13,8 +13,6 @@
extern int imx_irq_set_priority(unsigned char irq, unsigned char prio);
-/* all normal IRQs can be FIQs */
-#define FIQ_START 0
/* switch between IRQ and FIQ */
extern int mxc_set_irq_fiq(unsigned int irq, unsigned int type);
diff --git a/arch/arm/plat-mxc/tzic.c b/arch/arm/plat-mxc/tzic.c
index 3ed1adb..d09b573 100644
--- a/arch/arm/plat-mxc/tzic.c
+++ b/arch/arm/plat-mxc/tzic.c
@@ -193,7 +193,7 @@ void __init tzic_init_irq(void __iomem *irqbase)
#ifdef CONFIG_FIQ
/* Initialize FIQ */
- init_FIQ(FIQ_START);
+ init_FIQ();
#endif
pr_info("TrustZone Interrupt Controller (TZIC) initialized\n");
diff --git a/arch/arm/plat-s3c24xx/irq.c b/arch/arm/plat-s3c24xx/irq.c
index e4e9567..e0de92a 100644
--- a/arch/arm/plat-s3c24xx/irq.c
+++ b/arch/arm/plat-s3c24xx/irq.c
@@ -533,7 +533,7 @@ void __init s3c24xx_init_irq(void)
int i;
#ifdef CONFIG_FIQ
- init_FIQ(FIQ_START);
+ init_FIQ();
#endif
irqdbf("s3c2410_init_irq: clearing interrupt status flags\n");
--
1.8.0
The function only saves initial arch-specific "no FIQ" instruction, by
placing the code into set_fiq_handler() we can:
- Have less code and logic in the platform-specific files;
- Have the code that manages FIQ vector overwriting in one place, i.e.
don't spread the logic around.
p.s. Also, I noticed that we saved no_fiq_insn w/o bothering about
!CONFIG_CPU_USE_DOMAINS case, but set_fiq_handler() handles this case
specifically. I wonder, was that on purpose, e.g. it does not matter for
init_FIQ(), or it was just overlooked? In the latter case, the patch fixes
the issue.
Signed-off-by: Anton Vorontsov <[email protected]>
---
arch/arm/include/asm/mach/irq.h | 2 --
arch/arm/kernel/fiq.c | 17 ++++++++---------
arch/arm/mach-rpc/irq.c | 2 --
arch/arm/plat-mxc/avic.c | 3 ---
arch/arm/plat-mxc/tzic.c | 3 ---
arch/arm/plat-s3c24xx/irq.c | 2 --
6 files changed, 8 insertions(+), 21 deletions(-)
diff --git a/arch/arm/include/asm/mach/irq.h b/arch/arm/include/asm/mach/irq.h
index 8be5ba9..6e70ae3 100644
--- a/arch/arm/include/asm/mach/irq.h
+++ b/arch/arm/include/asm/mach/irq.h
@@ -18,10 +18,8 @@ struct seq_file;
* This is internal. Do not use it.
*/
#ifdef CONFIG_FIQ
-extern void init_FIQ(void);
extern void show_fiq_list(struct seq_file *, int);
#else
-static inline void init_FIQ(void) {}
static inline void show_fiq_list(struct seq_file *p, int prec) {}
#endif
diff --git a/arch/arm/kernel/fiq.c b/arch/arm/kernel/fiq.c
index 9bf3a60..3602df6 100644
--- a/arch/arm/kernel/fiq.c
+++ b/arch/arm/kernel/fiq.c
@@ -49,6 +49,7 @@
#include <asm/mach/irq.h>
static unsigned long no_fiq_insn;
+static int got_no_fiq_insn;
/* Default reacquire function
* - we always relinquish FIQ control
@@ -78,11 +79,14 @@ void show_fiq_list(struct seq_file *p, int prec)
void set_fiq_handler(void *start, unsigned int length)
{
-#if defined(CONFIG_CPU_USE_DOMAINS)
- memcpy((void *)0xffff001c, start, length);
-#else
- memcpy(vectors_page + 0x1c, start, length);
+ unsigned long *addr = (void *)0xffff001c;
+
+#ifndef CONFIG_CPU_USE_DOMAINS
+ addr = vectors_page + 0x1c;
#endif
+ if (!cmpxchg(&got_no_fiq_insn, 0, 1))
+ no_fiq_insn = *addr;
+ memcpy(addr, start, length);
flush_icache_range(0xffff001c, 0xffff001c + length);
if (!vectors_high())
flush_icache_range(0x1c, 0x1c + length);
@@ -126,8 +130,3 @@ EXPORT_SYMBOL(__set_fiq_regs); /* defined in fiqasm.S */
EXPORT_SYMBOL(__get_fiq_regs); /* defined in fiqasm.S */
EXPORT_SYMBOL(claim_fiq);
EXPORT_SYMBOL(release_fiq);
-
-void __init init_FIQ(void)
-{
- no_fiq_insn = *(unsigned long *)0xffff001c;
-}
diff --git a/arch/arm/mach-rpc/irq.c b/arch/arm/mach-rpc/irq.c
index 07770c8..2d915ba 100644
--- a/arch/arm/mach-rpc/irq.c
+++ b/arch/arm/mach-rpc/irq.c
@@ -151,7 +151,5 @@ void __init rpc_init_irq(void)
break;
}
}
-
- init_FIQ();
}
diff --git a/arch/arm/plat-mxc/avic.c b/arch/arm/plat-mxc/avic.c
index 426980c..b173dc6 100644
--- a/arch/arm/plat-mxc/avic.c
+++ b/arch/arm/plat-mxc/avic.c
@@ -216,8 +216,5 @@ void __init mxc_init_irq(void __iomem *irqbase)
for (i = 0; i < 8; i++)
__raw_writel(0, avic_base + AVIC_NIPRIORITY(i));
- /* Initialize FIQ */
- init_FIQ();
-
printk(KERN_INFO "MXC IRQ initialized\n");
}
diff --git a/arch/arm/plat-mxc/tzic.c b/arch/arm/plat-mxc/tzic.c
index 8a5a633..889267c 100644
--- a/arch/arm/plat-mxc/tzic.c
+++ b/arch/arm/plat-mxc/tzic.c
@@ -191,9 +191,6 @@ void __init tzic_init_irq(void __iomem *irqbase)
for (i = 0; i < 4; i++, irq_base += 32)
tzic_init_gc(i, irq_base);
- /* Initialize FIQ */
- init_FIQ();
-
pr_info("TrustZone Interrupt Controller (TZIC) initialized\n");
}
diff --git a/arch/arm/plat-s3c24xx/irq.c b/arch/arm/plat-s3c24xx/irq.c
index 531d6a4..66b8856 100644
--- a/arch/arm/plat-s3c24xx/irq.c
+++ b/arch/arm/plat-s3c24xx/irq.c
@@ -532,8 +532,6 @@ void __init s3c24xx_init_irq(void)
int irqno;
int i;
- init_FIQ();
-
irqdbf("s3c2410_init_irq: clearing interrupt status flags\n");
/* first, clear all interrupts pending... */
--
1.8.0
The return value is never checked, so we can turn show_fiq_list() into
returning void.
Signed-off-by: Anton Vorontsov <[email protected]>
---
arch/arm/include/asm/mach/irq.h | 4 ++--
arch/arm/kernel/fiq.c | 4 +---
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/arch/arm/include/asm/mach/irq.h b/arch/arm/include/asm/mach/irq.h
index 420d211..8be5ba9 100644
--- a/arch/arm/include/asm/mach/irq.h
+++ b/arch/arm/include/asm/mach/irq.h
@@ -19,10 +19,10 @@ struct seq_file;
*/
#ifdef CONFIG_FIQ
extern void init_FIQ(void);
-extern int show_fiq_list(struct seq_file *, int);
+extern void show_fiq_list(struct seq_file *, int);
#else
static inline void init_FIQ(void) {}
-static inline int show_fiq_list(struct seq_file *p, int prec) { return 0; }
+static inline void show_fiq_list(struct seq_file *p, int prec) {}
#endif
#ifdef CONFIG_MULTI_IRQ_HANDLER
diff --git a/arch/arm/kernel/fiq.c b/arch/arm/kernel/fiq.c
index 7be2e74..9bf3a60 100644
--- a/arch/arm/kernel/fiq.c
+++ b/arch/arm/kernel/fiq.c
@@ -69,13 +69,11 @@ static struct fiq_handler default_owner = {
static struct fiq_handler *current_fiq = &default_owner;
-int show_fiq_list(struct seq_file *p, int prec)
+void show_fiq_list(struct seq_file *p, int prec)
{
if (current_fiq != &default_owner)
seq_printf(p, "%*s: %s\n", prec, "FIQ",
current_fiq->name);
-
- return 0;
}
void set_fiq_handler(void *start, unsigned int length)
--
1.8.0
Simply removes ugly #ifdefs from C code.
Signed-off-by: Anton Vorontsov <[email protected]>
---
arch/arm/include/asm/mach/irq.h | 5 +++++
arch/arm/kernel/irq.c | 2 --
arch/arm/plat-mxc/avic.c | 2 --
arch/arm/plat-mxc/tzic.c | 2 --
arch/arm/plat-s3c24xx/irq.c | 2 --
5 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/arch/arm/include/asm/mach/irq.h b/arch/arm/include/asm/mach/irq.h
index febe495..420d211 100644
--- a/arch/arm/include/asm/mach/irq.h
+++ b/arch/arm/include/asm/mach/irq.h
@@ -17,8 +17,13 @@ struct seq_file;
/*
* This is internal. Do not use it.
*/
+#ifdef CONFIG_FIQ
extern void init_FIQ(void);
extern int show_fiq_list(struct seq_file *, int);
+#else
+static inline void init_FIQ(void) {}
+static inline int show_fiq_list(struct seq_file *p, int prec) { return 0; }
+#endif
#ifdef CONFIG_MULTI_IRQ_HANDLER
extern void (*handle_arch_irq)(struct pt_regs *);
diff --git a/arch/arm/kernel/irq.c b/arch/arm/kernel/irq.c
index 8961650..00911e5 100644
--- a/arch/arm/kernel/irq.c
+++ b/arch/arm/kernel/irq.c
@@ -45,9 +45,7 @@ unsigned long irq_err_count;
int arch_show_interrupts(struct seq_file *p, int prec)
{
-#ifdef CONFIG_FIQ
show_fiq_list(p, prec);
-#endif
#ifdef CONFIG_SMP
show_ipi_list(p, prec);
#endif
diff --git a/arch/arm/plat-mxc/avic.c b/arch/arm/plat-mxc/avic.c
index 19701ec..426980c 100644
--- a/arch/arm/plat-mxc/avic.c
+++ b/arch/arm/plat-mxc/avic.c
@@ -216,10 +216,8 @@ void __init mxc_init_irq(void __iomem *irqbase)
for (i = 0; i < 8; i++)
__raw_writel(0, avic_base + AVIC_NIPRIORITY(i));
-#ifdef CONFIG_FIQ
/* Initialize FIQ */
init_FIQ();
-#endif
printk(KERN_INFO "MXC IRQ initialized\n");
}
diff --git a/arch/arm/plat-mxc/tzic.c b/arch/arm/plat-mxc/tzic.c
index d09b573..8a5a633 100644
--- a/arch/arm/plat-mxc/tzic.c
+++ b/arch/arm/plat-mxc/tzic.c
@@ -191,10 +191,8 @@ void __init tzic_init_irq(void __iomem *irqbase)
for (i = 0; i < 4; i++, irq_base += 32)
tzic_init_gc(i, irq_base);
-#ifdef CONFIG_FIQ
/* Initialize FIQ */
init_FIQ();
-#endif
pr_info("TrustZone Interrupt Controller (TZIC) initialized\n");
}
diff --git a/arch/arm/plat-s3c24xx/irq.c b/arch/arm/plat-s3c24xx/irq.c
index e0de92a..531d6a4 100644
--- a/arch/arm/plat-s3c24xx/irq.c
+++ b/arch/arm/plat-s3c24xx/irq.c
@@ -532,9 +532,7 @@ void __init s3c24xx_init_irq(void)
int irqno;
int i;
-#ifdef CONFIG_FIQ
init_FIQ();
-#endif
irqdbf("s3c2410_init_irq: clearing interrupt status flags\n");
--
1.8.0
The patch fixes the following sparse warnings:
CHECK arch/arm/kernel/fiq.c
arch/arm/kernel/fiq.c:71:6: warning: symbol 'show_fiq_list' was not declared. Should it be static?
arch/arm/kernel/fiq.c:129:13: warning: symbol 'init_FIQ' was not declared. Should it be static?
Signed-off-by: Anton Vorontsov <[email protected]>
---
arch/arm/kernel/fiq.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/kernel/fiq.c b/arch/arm/kernel/fiq.c
index bd369c5..7be2e74 100644
--- a/arch/arm/kernel/fiq.c
+++ b/arch/arm/kernel/fiq.c
@@ -46,6 +46,7 @@
#include <asm/fiq.h>
#include <asm/irq.h>
#include <asm/traps.h>
+#include <asm/mach/irq.h>
static unsigned long no_fiq_insn;
--
1.8.0
The driver uses platform-specific mxc_set_irq_fiq() with the VIRQ cookie
passed to it, so it's pretty clear that the driver is absolutely sure
that the FIQ is routed via platform-specific IC, and that the cookie can
be used to mask/unmask FIQs. So, let's switch to the genirq routines,
since we're about to remove FIQ-specific variants.
Signed-off-by: Anton Vorontsov <[email protected]>
Acked-by: Sascha Hauer <[email protected]>
Acked-by: Mark Brown <[email protected]>
---
sound/soc/fsl/imx-pcm-fiq.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/fsl/imx-pcm-fiq.c b/sound/soc/fsl/imx-pcm-fiq.c
index 22c6130..8f1a4a6 100644
--- a/sound/soc/fsl/imx-pcm-fiq.c
+++ b/sound/soc/fsl/imx-pcm-fiq.c
@@ -139,7 +139,7 @@ static int snd_imx_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
hrtimer_start(&iprtd->hrt, ns_to_ktime(iprtd->poll_time_ns),
HRTIMER_MODE_REL);
if (++fiq_enable == 1)
- enable_fiq(imx_pcm_fiq);
+ enable_irq(imx_pcm_fiq);
break;
@@ -149,7 +149,7 @@ static int snd_imx_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
atomic_set(&iprtd->running, 0);
if (--fiq_enable == 0)
- disable_fiq(imx_pcm_fiq);
+ disable_irq(imx_pcm_fiq);
break;
default:
--
1.8.0
> The function only saves initial arch-specific "no FIQ" instruction, by
> placing the code into set_fiq_handler() we can:
>
> - Have less code and logic in the platform-specific files;
> - Have the code that manages FIQ vector overwriting in one place, i.e.
> don't spread the logic around.
>
> p.s. Also, I noticed that we saved no_fiq_insn w/o bothering about
> !CONFIG_CPU_USE_DOMAINS case, but set_fiq_handler() handles this case
> specifically. I wonder, was that on purpose, e.g. it does not matter for
> init_FIQ(), or it was just overlooked? In the latter case, the patch fixes
> the issue.
...
> diff --git a/arch/arm/kernel/fiq.c b/arch/arm/kernel/fiq.c
> index 9bf3a60..3602df6 100644
> --- a/arch/arm/kernel/fiq.c
> +++ b/arch/arm/kernel/fiq.c
> @@ -49,6 +49,7 @@
> #include <asm/mach/irq.h>
>
> static unsigned long no_fiq_insn;
> +static int got_no_fiq_insn;
>
> /* Default reacquire function
> * - we always relinquish FIQ control
> @@ -78,11 +79,14 @@ void show_fiq_list(struct seq_file *p, int prec)
>
> void set_fiq_handler(void *start, unsigned int length)
> {
> -#if defined(CONFIG_CPU_USE_DOMAINS)
> - memcpy((void *)0xffff001c, start, length);
> -#else
> - memcpy(vectors_page + 0x1c, start, length);
> + unsigned long *addr = (void *)0xffff001c;
> +
> +#ifndef CONFIG_CPU_USE_DOMAINS
> + addr = vectors_page + 0x1c;
> #endif
> + if (!cmpxchg(&got_no_fiq_insn, 0, 1))
> + no_fiq_insn = *addr;
> + memcpy(addr, start, length);
> flush_icache_range(0xffff001c, 0xffff001c + length);
> if (!vectors_high())
> flush_icache_range(0x1c, 0x1c + length);
> @@ -126,8 +130,3 @@ EXPORT_SYMBOL(__set_fiq_regs); /* defined in fiqasm.S */
> EXPORT_SYMBOL(__get_fiq_regs); /* defined in fiqasm.S */
> EXPORT_SYMBOL(claim_fiq);
> EXPORT_SYMBOL(release_fiq);
> -
> -void __init init_FIQ(void)
> -{
> - no_fiq_insn = *(unsigned long *)0xffff001c;
it seems that this is wrong. In this case we have an uninitialized variable and
sequential call claim_fiq and release_fiq could be fatal. FIXME please.
---
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On Fri, Nov 23, 2012 at 07:40:30AM +0400, Alexander Shiyan wrote:
[...]
> > static unsigned long no_fiq_insn;
> > +static int got_no_fiq_insn;
> > @@ -78,11 +79,14 @@ void show_fiq_list(struct seq_file *p, int prec)
> >
> > void set_fiq_handler(void *start, unsigned int length)
> > {
> > -#if defined(CONFIG_CPU_USE_DOMAINS)
> > - memcpy((void *)0xffff001c, start, length);
> > -#else
> > - memcpy(vectors_page + 0x1c, start, length);
> > + unsigned long *addr = (void *)0xffff001c;
> > +
> > +#ifndef CONFIG_CPU_USE_DOMAINS
> > + addr = vectors_page + 0x1c;
> > #endif
> > + if (!cmpxchg(&got_no_fiq_insn, 0, 1))
> > + no_fiq_insn = *addr;
> > + memcpy(addr, start, length);
> > flush_icache_range(0xffff001c, 0xffff001c + length);
> > if (!vectors_high())
> > flush_icache_range(0x1c, 0x1c + length);
> > @@ -126,8 +130,3 @@ EXPORT_SYMBOL(__set_fiq_regs); /* defined in fiqasm.S */
> > -
> > -void __init init_FIQ(void)
> > -{
> > - no_fiq_insn = *(unsigned long *)0xffff001c;
>
> it seems that this is wrong. In this case we have an uninitialized variable and
> sequential call claim_fiq and release_fiq could be fatal. FIXME please.
Um... I don't think I understand, can you please elaborate?
Thanks,
Anton.
> On Fri, Nov 23, 2012 at 07:40:30AM +0400, Alexander Shiyan wrote:
> [...]
> > > static unsigned long no_fiq_insn;
> > > +static int got_no_fiq_insn;
> > > @@ -78,11 +79,14 @@ void show_fiq_list(struct seq_file *p, int prec)
> > >
> > > void set_fiq_handler(void *start, unsigned int length)
> > > {
> > > -#if defined(CONFIG_CPU_USE_DOMAINS)
> > > - memcpy((void *)0xffff001c, start, length);
> > > -#else
> > > - memcpy(vectors_page + 0x1c, start, length);
> > > + unsigned long *addr = (void *)0xffff001c;
> > > +
> > > +#ifndef CONFIG_CPU_USE_DOMAINS
> > > + addr = vectors_page + 0x1c;
> > > #endif
> > > + if (!cmpxchg(&got_no_fiq_insn, 0, 1))
> > > + no_fiq_insn = *addr;
> > > + memcpy(addr, start, length);
> > > flush_icache_range(0xffff001c, 0xffff001c + length);
> > > if (!vectors_high())
> > > flush_icache_range(0x1c, 0x1c + length);
> > > @@ -126,8 +130,3 @@ EXPORT_SYMBOL(__set_fiq_regs); /* defined in fiqasm.S */
> > > -
> > > -void __init init_FIQ(void)
> > > -{
> > > - no_fiq_insn = *(unsigned long *)0xffff001c;
> >
> > it seems that this is wrong. In this case we have an uninitialized variable and
> > sequential call claim_fiq and release_fiq could be fatal. FIXME please.
>
> Um... I don't think I understand, can you please elaborate?
OK, I'll try to explain it.
At the end of the release_fiq function we have a call fiq_op. For the default
handler - is a fiq_def_op function, and we call this function with the option
"relinquish = 0", i.e. we want to restore old fiq_handler. But if we do not call
set_fiq_handler never before, we will have an uninitialized no_fiq_insn variable.
---
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On Fri, Nov 23, 2012 at 10:27:51AM +0400, Alexander Shiyan wrote:
> > On Fri, Nov 23, 2012 at 07:40:30AM +0400, Alexander Shiyan wrote:
> > [...]
> > > > static unsigned long no_fiq_insn;
> > > > +static int got_no_fiq_insn;
> > > > @@ -78,11 +79,14 @@ void show_fiq_list(struct seq_file *p, int prec)
> > > >
> > > > void set_fiq_handler(void *start, unsigned int length)
> > > > {
> > > > -#if defined(CONFIG_CPU_USE_DOMAINS)
> > > > - memcpy((void *)0xffff001c, start, length);
> > > > -#else
> > > > - memcpy(vectors_page + 0x1c, start, length);
> > > > + unsigned long *addr = (void *)0xffff001c;
> > > > +
> > > > +#ifndef CONFIG_CPU_USE_DOMAINS
> > > > + addr = vectors_page + 0x1c;
> > > > #endif
> > > > + if (!cmpxchg(&got_no_fiq_insn, 0, 1))
> > > > + no_fiq_insn = *addr;
> > > > + memcpy(addr, start, length);
> > > > flush_icache_range(0xffff001c, 0xffff001c + length);
> > > > if (!vectors_high())
> > > > flush_icache_range(0x1c, 0x1c + length);
> > > > @@ -126,8 +130,3 @@ EXPORT_SYMBOL(__set_fiq_regs); /* defined in fiqasm.S */
> > > > -
> > > > -void __init init_FIQ(void)
> > > > -{
> > > > - no_fiq_insn = *(unsigned long *)0xffff001c;
> > >
> > > it seems that this is wrong. In this case we have an uninitialized variable and
> > > sequential call claim_fiq and release_fiq could be fatal. FIXME please.
> >
> > Um... I don't think I understand, can you please elaborate?
>
> OK, I'll try to explain it.
> At the end of the release_fiq function we have a call fiq_op. For the default
> handler - is a fiq_def_op function, and we call this function with the option
> "relinquish = 0", i.e. we want to restore old fiq_handler. But if we do not call
> set_fiq_handler never before, we will have an uninitialized no_fiq_insn variable.
It should not matter when or in what order anyone calls the
set_fiq_handler(), since it stores "no FIQ instruction" into no_fiq_insn
at its first invocation:
if (!cmpxchg(&got_no_fiq_insn, 0, 1))
no_fiq_insn = *addr;
If we never called set_fiq_handler() before, during release_fiq() we'll:
1. Copy the initial instruction from the vector page to 'no_fiq_insn';
2. Copy the initial instruction from 'no_fiq_insn' to the vector page;
So no_fiq_insn gets initialized, then we just instantly write the same
value back.
Thanks,
Anton.
> On Fri, Nov 23, 2012 at 10:27:51AM +0400, Alexander Shiyan wrote:
> > > On Fri, Nov 23, 2012 at 07:40:30AM +0400, Alexander Shiyan wrote:
> > > [...]
> > > > > static unsigned long no_fiq_insn;
> > > > > +static int got_no_fiq_insn;
> > > > > @@ -78,11 +79,14 @@ void show_fiq_list(struct seq_file *p, int prec)
> > > > >
> > > > > void set_fiq_handler(void *start, unsigned int length)
> > > > > {
> > > > > -#if defined(CONFIG_CPU_USE_DOMAINS)
> > > > > - memcpy((void *)0xffff001c, start, length);
> > > > > -#else
> > > > > - memcpy(vectors_page + 0x1c, start, length);
> > > > > + unsigned long *addr = (void *)0xffff001c;
> > > > > +
> > > > > +#ifndef CONFIG_CPU_USE_DOMAINS
> > > > > + addr = vectors_page + 0x1c;
> > > > > #endif
> > > > > + if (!cmpxchg(&got_no_fiq_insn, 0, 1))
> > > > > + no_fiq_insn = *addr;
> > > > > + memcpy(addr, start, length);
> > > > > flush_icache_range(0xffff001c, 0xffff001c + length);
> > > > > if (!vectors_high())
> > > > > flush_icache_range(0x1c, 0x1c + length);
> > > > > @@ -126,8 +130,3 @@ EXPORT_SYMBOL(__set_fiq_regs); /* defined in fiqasm.S */
> > > > > -
> > > > > -void __init init_FIQ(void)
> > > > > -{
> > > > > - no_fiq_insn = *(unsigned long *)0xffff001c;
> > > >
> > > > it seems that this is wrong. In this case we have an uninitialized variable and
> > > > sequential call claim_fiq and release_fiq could be fatal. FIXME please.
> > >
> > > Um... I don't think I understand, can you please elaborate?
> >
> > OK, I'll try to explain it.
> > At the end of the release_fiq function we have a call fiq_op. For the default
> > handler - is a fiq_def_op function, and we call this function with the option
> > "relinquish = 0", i.e. we want to restore old fiq_handler. But if we do not call
> > set_fiq_handler never before, we will have an uninitialized no_fiq_insn variable.
>
> It should not matter when or in what order anyone calls the
> set_fiq_handler(), since it stores "no FIQ instruction" into no_fiq_insn
> at its first invocation:
>
> if (!cmpxchg(&got_no_fiq_insn, 0, 1))
> no_fiq_insn = *addr;
>
> If we never called set_fiq_handler() before, during release_fiq() we'll:
>
> 1. Copy the initial instruction from the vector page to 'no_fiq_insn';
> 2. Copy the initial instruction from 'no_fiq_insn' to the vector page;
>
> So no_fiq_insn gets initialized, then we just instantly write the same
> value back.
got_no_fiq_insn also not initialized.
I think as a whole on this issue requires additional comments from Russell
as the author of implementation.
PS: In any case, I can reproduce it and check it out if you send me a patchset
to my email as an attachment.
---
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On Fri, Nov 23, 2012 at 11:36:01AM +0400, Alexander Shiyan wrote:
[...]
> got_no_fiq_insn also not initialized.
It is static, so by definition it is initialized to 0.
> I think as a whole on this issue requires additional comments from Russell
> as the author of implementation.
>
> PS: In any case, I can reproduce it and check it out if you send me a patchset
> to my email as an attachment.
Sent privately.
Thanks,
Anton.
On Thu, Nov 22, 2012 at 11:51:38PM -0800, Anton Vorontsov wrote:
> On Fri, Nov 23, 2012 at 11:36:01AM +0400, Alexander Shiyan wrote:
> [...]
> > got_no_fiq_insn also not initialized.
>
> It is static, so by definition it is initialized to 0.
>
> > I think as a whole on this issue requires additional comments from Russell
> > as the author of implementation.
> >
> > PS: In any case, I can reproduce it and check it out if you send me a patchset
> > to my email as an attachment.
>
> Sent privately.
Andrew, Arnd, just FYI, it works for Alexander.
----- Forwarded message from Alexander Shiyan <[email protected]> -----
Date: Tue, 27 Nov 2012 12:42:50 +0400
From: Alexander Shiyan <[email protected]>
To: Anton Vorontsov <[email protected]>
Subject: Re[2]: FIQ
> Hello Alexander,
>
> On Tue, Nov 27, 2012 at 12:14:28PM +0400, Alexander Shiyan wrote:
> > Sorry, I lost the original message to the correct answer.
> > Today I had a test of removal init_FIQ.
> > The test took place on a custom board with a custom ARM CLPS711X OSS audio
> > driver, which uses the FIQ interrupts. Everything works fine.
>
> Nice! Thanks a lot for testing!
>
> Can you please reply to the original thread, with the
>
> Tested-by: Alexander Shiyan <[email protected]>
> ?
>
> This will help pushing the series forward, plus will give you a credit for
> your efforts.
I am lost thread in my mailbox, you can just copy/paste my reply and ping for Arnd.