linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 02/10] ARM: plat-s3c24xx: Don't use FIQ_START
  2012-10-15 21:49 [PATCH resend 0/10] Get rid of FIQ_START/enable/disable_fiq() + some FIQ cleanups Anton Vorontsov
@ 2012-10-15 21:51 ` Anton Vorontsov
  0 siblings, 0 replies; 19+ messages in thread
From: Anton Vorontsov @ 2012-10-15 21:51 UTC (permalink / raw)
  To: linux-arm-kernel

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 <anton.vorontsov@linaro.org>
Acked-by: Kukjin Kim <kgene.kim@samsung.com>
---
 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.7.12.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread
* [PATCH resend 0/10] Get rid of FIQ_START/enable/disable_fiq() + some FIQ cleanups
@ 2012-11-23  0:38 Anton Vorontsov
  2012-11-23  0:49 ` [PATCH 01/10] ARM: mach-rpc: Don't register FIQs with genirq Anton Vorontsov
                   ` (9 more replies)
  0 siblings, 10 replies; 19+ messages in thread
From: Anton Vorontsov @ 2012-11-23  0:38 UTC (permalink / raw)
  To: linux-arm-kernel

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(-)

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [PATCH 01/10] ARM: mach-rpc: Don't register FIQs with genirq
  2012-11-23  0:38 [PATCH resend 0/10] Get rid of FIQ_START/enable/disable_fiq() + some FIQ cleanups Anton Vorontsov
@ 2012-11-23  0:49 ` Anton Vorontsov
  2012-11-23  0:49 ` [PATCH 02/10] ARM: plat-s3c24xx: Don't use FIQ_START Anton Vorontsov
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Anton Vorontsov @ 2012-11-23  0:49 UTC (permalink / raw)
  To: linux-arm-kernel

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 <anton.vorontsov@linaro.org>
---
 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(&regs);
-	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

^ permalink raw reply related	[flat|nested] 19+ messages in thread
* [PATCH 02/10] ARM: plat-s3c24xx: Don't use FIQ_START
  2012-11-23  0:38 [PATCH resend 0/10] Get rid of FIQ_START/enable/disable_fiq() + some FIQ cleanups Anton Vorontsov
  2012-11-23  0:49 ` [PATCH 01/10] ARM: mach-rpc: Don't register FIQs with genirq Anton Vorontsov
@ 2012-11-23  0:49 ` Anton Vorontsov
  2012-11-23  0:49 ` [PATCH 03/10] [media] mx1_camera: Don't use {en,dis}able_fiq() calls Anton Vorontsov
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Anton Vorontsov @ 2012-11-23  0:49 UTC (permalink / raw)
  To: linux-arm-kernel

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 <anton.vorontsov@linaro.org>
Acked-by: Kukjin Kim <kgene.kim@samsung.com>
---
 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread
* [PATCH 03/10] [media] mx1_camera: Don't use {en,dis}able_fiq() calls
  2012-11-23  0:38 [PATCH resend 0/10] Get rid of FIQ_START/enable/disable_fiq() + some FIQ cleanups Anton Vorontsov
  2012-11-23  0:49 ` [PATCH 01/10] ARM: mach-rpc: Don't register FIQs with genirq Anton Vorontsov
  2012-11-23  0:49 ` [PATCH 02/10] ARM: plat-s3c24xx: Don't use FIQ_START Anton Vorontsov
@ 2012-11-23  0:49 ` Anton Vorontsov
  2012-11-23  0:49 ` [PATCH 04/10] ASoC: imx: " Anton Vorontsov
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Anton Vorontsov @ 2012-11-23  0:49 UTC (permalink / raw)
  To: linux-arm-kernel

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 <anton.vorontsov@linaro.org>
Acked-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 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(&regs);
 
 	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

^ permalink raw reply related	[flat|nested] 19+ messages in thread
* [PATCH 04/10] ASoC: imx: Don't use {en,dis}able_fiq() calls
  2012-11-23  0:38 [PATCH resend 0/10] Get rid of FIQ_START/enable/disable_fiq() + some FIQ cleanups Anton Vorontsov
                   ` (2 preceding siblings ...)
  2012-11-23  0:49 ` [PATCH 03/10] [media] mx1_camera: Don't use {en,dis}able_fiq() calls Anton Vorontsov
@ 2012-11-23  0:49 ` Anton Vorontsov
  2012-11-23  0:49 ` [PATCH 05/10] ARM: FIQ: Remove enable_fiq() and disable_fiq() calls Anton Vorontsov
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Anton Vorontsov @ 2012-11-23  0:49 UTC (permalink / raw)
  To: linux-arm-kernel

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 <anton.vorontsov@linaro.org>
Acked-by: Sascha Hauer <s.hauer@pengutronix.de>
Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread
* [PATCH 05/10] ARM: FIQ: Remove enable_fiq() and disable_fiq() calls
  2012-11-23  0:38 [PATCH resend 0/10] Get rid of FIQ_START/enable/disable_fiq() + some FIQ cleanups Anton Vorontsov
                   ` (3 preceding siblings ...)
  2012-11-23  0:49 ` [PATCH 04/10] ASoC: imx: " Anton Vorontsov
@ 2012-11-23  0:49 ` Anton Vorontsov
  2012-11-23  0:49 ` [PATCH 06/10] ARM: FIQ: Remove FIQ_START Anton Vorontsov
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Anton Vorontsov @ 2012-11-23  0:49 UTC (permalink / raw)
  To: linux-arm-kernel

There are no users left, so these can be removed.

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread
* [PATCH 06/10] ARM: FIQ: Remove FIQ_START
  2012-11-23  0:38 [PATCH resend 0/10] Get rid of FIQ_START/enable/disable_fiq() + some FIQ cleanups Anton Vorontsov
                   ` (4 preceding siblings ...)
  2012-11-23  0:49 ` [PATCH 05/10] ARM: FIQ: Remove enable_fiq() and disable_fiq() calls Anton Vorontsov
@ 2012-11-23  0:49 ` Anton Vorontsov
  2012-11-23  0:50 ` [PATCH 07/10] ARM: FIQ: Should include asm/mach/irq.h Anton Vorontsov
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Anton Vorontsov @ 2012-11-23  0:49 UTC (permalink / raw)
  To: linux-arm-kernel

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 <anton.vorontsov@linaro.org>
---
 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread
* [PATCH 07/10] ARM: FIQ: Should include asm/mach/irq.h
  2012-11-23  0:38 [PATCH resend 0/10] Get rid of FIQ_START/enable/disable_fiq() + some FIQ cleanups Anton Vorontsov
                   ` (5 preceding siblings ...)
  2012-11-23  0:49 ` [PATCH 06/10] ARM: FIQ: Remove FIQ_START Anton Vorontsov
@ 2012-11-23  0:50 ` Anton Vorontsov
  2012-11-23  0:50 ` [PATCH 08/10] ARM: FIQ: Implement !CONFIG_FIQ stubs Anton Vorontsov
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Anton Vorontsov @ 2012-11-23  0:50 UTC (permalink / raw)
  To: linux-arm-kernel

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 <anton.vorontsov@linaro.org>
---
 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread
* [PATCH 08/10] ARM: FIQ: Implement !CONFIG_FIQ stubs
  2012-11-23  0:38 [PATCH resend 0/10] Get rid of FIQ_START/enable/disable_fiq() + some FIQ cleanups Anton Vorontsov
                   ` (6 preceding siblings ...)
  2012-11-23  0:50 ` [PATCH 07/10] ARM: FIQ: Should include asm/mach/irq.h Anton Vorontsov
@ 2012-11-23  0:50 ` Anton Vorontsov
  2012-11-23  0:50 ` [PATCH 09/10] ARM: FIQ: Make show_fiq_list() return void Anton Vorontsov
  2012-11-23  0:50 ` [PATCH 10/10] ARM: FIQ: Get rid of init_FIQ() Anton Vorontsov
  9 siblings, 0 replies; 19+ messages in thread
From: Anton Vorontsov @ 2012-11-23  0:50 UTC (permalink / raw)
  To: linux-arm-kernel

Simply removes ugly #ifdefs from C code.

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread
* [PATCH 09/10] ARM: FIQ: Make show_fiq_list() return void
  2012-11-23  0:38 [PATCH resend 0/10] Get rid of FIQ_START/enable/disable_fiq() + some FIQ cleanups Anton Vorontsov
                   ` (7 preceding siblings ...)
  2012-11-23  0:50 ` [PATCH 08/10] ARM: FIQ: Implement !CONFIG_FIQ stubs Anton Vorontsov
@ 2012-11-23  0:50 ` Anton Vorontsov
  2012-11-23  0:50 ` [PATCH 10/10] ARM: FIQ: Get rid of init_FIQ() Anton Vorontsov
  9 siblings, 0 replies; 19+ messages in thread
From: Anton Vorontsov @ 2012-11-23  0:50 UTC (permalink / raw)
  To: linux-arm-kernel

The return value is never checked, so we can turn show_fiq_list() into
returning void.

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread
* [PATCH 10/10] ARM: FIQ: Get rid of init_FIQ()
  2012-11-23  0:38 [PATCH resend 0/10] Get rid of FIQ_START/enable/disable_fiq() + some FIQ cleanups Anton Vorontsov
                   ` (8 preceding siblings ...)
  2012-11-23  0:50 ` [PATCH 09/10] ARM: FIQ: Make show_fiq_list() return void Anton Vorontsov
@ 2012-11-23  0:50 ` Anton Vorontsov
  2012-11-23  3:40   ` Alexander Shiyan
  9 siblings, 1 reply; 19+ messages in thread
From: Anton Vorontsov @ 2012-11-23  0:50 UTC (permalink / raw)
  To: linux-arm-kernel

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 <anton.vorontsov@linaro.org>
---
 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread
* Re: [PATCH 10/10] ARM: FIQ: Get rid of init_FIQ()
  2012-11-23  0:50 ` [PATCH 10/10] ARM: FIQ: Get rid of init_FIQ() Anton Vorontsov
@ 2012-11-23  3:40   ` Alexander Shiyan
  2012-11-23  5:53     ` Anton Vorontsov
  0 siblings, 1 reply; 19+ messages in thread
From: Alexander Shiyan @ 2012-11-23  3:40 UTC (permalink / raw)
  To: linux-arm-kernel

> 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.

---

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [PATCH 10/10] ARM: FIQ: Get rid of init_FIQ()
  2012-11-23  3:40   ` Alexander Shiyan
@ 2012-11-23  5:53     ` Anton Vorontsov
  2012-11-23  6:27       ` Re[2]: " Alexander Shiyan
  0 siblings, 1 reply; 19+ messages in thread
From: Anton Vorontsov @ 2012-11-23  5:53 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 19+ messages in thread
* Re[2]: [PATCH 10/10] ARM: FIQ: Get rid of init_FIQ()
  2012-11-23  5:53     ` Anton Vorontsov
@ 2012-11-23  6:27       ` Alexander Shiyan
  2012-11-23  6:50         ` Anton Vorontsov
  0 siblings, 1 reply; 19+ messages in thread
From: Alexander Shiyan @ 2012-11-23  6:27 UTC (permalink / raw)
  To: linux-arm-kernel

> 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.

---

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [PATCH 10/10] ARM: FIQ: Get rid of init_FIQ()
  2012-11-23  6:27       ` Re[2]: " Alexander Shiyan
@ 2012-11-23  6:50         ` Anton Vorontsov
  2012-11-23  7:36           ` Re[2]: " Alexander Shiyan
  0 siblings, 1 reply; 19+ messages in thread
From: Anton Vorontsov @ 2012-11-23  6:50 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 19+ messages in thread
* Re[2]: [PATCH 10/10] ARM: FIQ: Get rid of init_FIQ()
  2012-11-23  6:50         ` Anton Vorontsov
@ 2012-11-23  7:36           ` Alexander Shiyan
  2012-11-23  7:51             ` Anton Vorontsov
  0 siblings, 1 reply; 19+ messages in thread
From: Alexander Shiyan @ 2012-11-23  7:36 UTC (permalink / raw)
  To: linux-arm-kernel

> 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.


---

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [PATCH 10/10] ARM: FIQ: Get rid of init_FIQ()
  2012-11-23  7:36           ` Re[2]: " Alexander Shiyan
@ 2012-11-23  7:51             ` Anton Vorontsov
  2012-11-27  9:05               ` Anton Vorontsov
  0 siblings, 1 reply; 19+ messages in thread
From: Anton Vorontsov @ 2012-11-23  7:51 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [PATCH 10/10] ARM: FIQ: Get rid of init_FIQ()
  2012-11-23  7:51             ` Anton Vorontsov
@ 2012-11-27  9:05               ` Anton Vorontsov
  0 siblings, 0 replies; 19+ messages in thread
From: Anton Vorontsov @ 2012-11-27  9:05 UTC (permalink / raw)
  To: linux-arm-kernel

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 <shc_work@mail.ru> -----
Date: Tue, 27 Nov 2012 12:42:50 +0400
From: Alexander Shiyan <shc_work@mail.ru>
To: Anton Vorontsov <anton.vorontsov@linaro.org>
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 <shc_work@mail.ru>
> ?
> 
> 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.

^ permalink raw reply	[flat|nested] 19+ messages in thread
end of thread, other threads:[~2012-11-27  9:05 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-23  0:38 [PATCH resend 0/10] Get rid of FIQ_START/enable/disable_fiq() + some FIQ cleanups Anton Vorontsov
2012-11-23  0:49 ` [PATCH 01/10] ARM: mach-rpc: Don't register FIQs with genirq Anton Vorontsov
2012-11-23  0:49 ` [PATCH 02/10] ARM: plat-s3c24xx: Don't use FIQ_START Anton Vorontsov
2012-11-23  0:49 ` [PATCH 03/10] [media] mx1_camera: Don't use {en,dis}able_fiq() calls Anton Vorontsov
2012-11-23  0:49 ` [PATCH 04/10] ASoC: imx: " Anton Vorontsov
2012-11-23  0:49 ` [PATCH 05/10] ARM: FIQ: Remove enable_fiq() and disable_fiq() calls Anton Vorontsov
2012-11-23  0:49 ` [PATCH 06/10] ARM: FIQ: Remove FIQ_START Anton Vorontsov
2012-11-23  0:50 ` [PATCH 07/10] ARM: FIQ: Should include asm/mach/irq.h Anton Vorontsov
2012-11-23  0:50 ` [PATCH 08/10] ARM: FIQ: Implement !CONFIG_FIQ stubs Anton Vorontsov
2012-11-23  0:50 ` [PATCH 09/10] ARM: FIQ: Make show_fiq_list() return void Anton Vorontsov
2012-11-23  0:50 ` [PATCH 10/10] ARM: FIQ: Get rid of init_FIQ() Anton Vorontsov
2012-11-23  3:40   ` Alexander Shiyan
2012-11-23  5:53     ` Anton Vorontsov
2012-11-23  6:27       ` Re[2]: " Alexander Shiyan
2012-11-23  6:50         ` Anton Vorontsov
2012-11-23  7:36           ` Re[2]: " Alexander Shiyan
2012-11-23  7:51             ` Anton Vorontsov
2012-11-27  9:05               ` Anton Vorontsov
  -- strict thread matches above, loose matches on Subject: below --
2012-10-15 21:49 [PATCH resend 0/10] Get rid of FIQ_START/enable/disable_fiq() + some FIQ cleanups Anton Vorontsov
2012-10-15 21:51 ` [PATCH 02/10] ARM: plat-s3c24xx: Don't use FIQ_START Anton Vorontsov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).