[PATCH 0/9] Get rid of FIQ_START/enable/disable_fiq() + some FIQ cleanups

[PATCH 0/9] Get rid of FIQ_START/enable/disable_fiq() + some FIQ cleanups

[Anton Vorontsov]

Hello Russell,

During KDB FIQ patches review you 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 you
once said that you don't want the FIQ subsystem to mess with genirq.

It is surely makes sense as FIQs are arch-specific, plus FIQs are special
in that way that platform makers are free to connects device directly
to the FIQ line, avoiding IC routing, and then enable_fiq(<IRQ>) would
look awkward, at the best.

So, given that, it seems that the only entity that knows for sure how
the particular FIQ is routed is whoever claims the FIQ and fills-in the
fiq_handler struct. It might make sense to introduce disable/enable
callbacks in the fiq_handler struct, and then prototypes for disable/
enable FIQ calls would look like this:

	enable_fiq(struct fiq_handler *fh);
	disable_fiq(struct fiq_handler *fh);

I.e. completely abstracted from the genirq.

Although, to not duplicate IC code, I think it's pretty legitimate to
call genirq functions iff the caller knows for sure that FIQ comes from
IRQ line and it is is routed through the well-known platform IC.

In that case (which is a majority), the typical user would do this:

	static int irq_line;

	void foo_enable_fiq(struct fiq_handler *fh)
	{
		enable_irq(irq_line);
	}

	static struct fiq_handler foo_fiq = {
		.name = "foo",
		.enable_fiq = foo_enable_fiq,
	};

	claim_fiq(&foo_fiq);
	enable_fiq(&foo_fiq);

Obviously, it's needless indirection, so I don't do this. If we ever
pass 'foo_fiq' to some device, which does not know all the routing
details, then it would make sense to introduce it, but not now.

So, the patch set is pretty straightforward:

- Get rid of FIQ_START. Nobody but platform-specific code (and 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 (the calls can
  be reintroduced w/ new prototypes when/if needed).

Does the approach make sense?

Thanks!

--
 arch/arm/include/asm/fiq.h                |    2 --
 arch/arm/include/asm/mach/irq.h           |    9 +++++++--
 arch/arm/kernel/fiq.c                     |   22 +++-------------------
 arch/arm/kernel/irq.c                     |    2 --
 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                  |    4 +---
 arch/arm/plat-mxc/include/mach/irqs.h     |    2 --
 arch/arm/plat-mxc/tzic.c                  |    4 +---
 arch/arm/plat-omap/include/plat/irqs.h    |    4 ----
 arch/arm/plat-s3c24xx/irq.c               |    6 ++----
 drivers/media/video/mx1_camera.c          |    6 +++---
 sound/soc/fsl/imx-pcm-fiq.c               |    4 ++--
 15 files changed, 30 insertions(+), 71 deletions(-)

-- 
Anton Vorontsov
Email: cbouatmailru@gmail.com

[PATCH 0/9] Get rid of FIQ_START/enable/disable_fiq() + some FIQ cleanups

[Anton Vorontsov]

Hello Russell,

During KDB FIQ patches review you 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 you
once said that you don't want the FIQ subsystem to mess with genirq.

It is surely makes sense as FIQs are arch-specific, plus FIQs are special
in that way that platform makers are free to connects device directly
to the FIQ line, avoiding IC routing, and then enable_fiq(<IRQ>) would
look awkward, at the best.

So, given that, it seems that the only entity that knows for sure how
the particular FIQ is routed is whoever claims the FIQ and fills-in the
fiq_handler struct. It might make sense to introduce disable/enable
callbacks in the fiq_handler struct, and then prototypes for disable/
enable FIQ calls would look like this:

	enable_fiq(struct fiq_handler *fh);
	disable_fiq(struct fiq_handler *fh);

I.e. completely abstracted from the genirq.

Although, to not duplicate IC code, I think it's pretty legitimate to
call genirq functions iff the caller knows for sure that FIQ comes from
IRQ line and it is is routed through the well-known platform IC.

In that case (which is a majority), the typical user would do this:

	static int irq_line;

	void foo_enable_fiq(struct fiq_handler *fh)
	{
		enable_irq(irq_line);
	}

	static struct fiq_handler foo_fiq = {
		.name = "foo",
		.enable_fiq = foo_enable_fiq,
	};

	claim_fiq(&foo_fiq);
	enable_fiq(&foo_fiq);

Obviously, it's needless indirection, so I don't do this. If we ever
pass 'foo_fiq' to some device, which does not know all the routing
details, then it would make sense to introduce it, but not now.

So, the patch set is pretty straightforward:

- Get rid of FIQ_START. Nobody but platform-specific code (and 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 (the calls can
  be reintroduced w/ new prototypes when/if needed).

Does the approach make sense?

Thanks!

--
 arch/arm/include/asm/fiq.h                |    2 --
 arch/arm/include/asm/mach/irq.h           |    9 +++++++--
 arch/arm/kernel/fiq.c                     |   22 +++-------------------
 arch/arm/kernel/irq.c                     |    2 --
 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                  |    4 +---
 arch/arm/plat-mxc/include/mach/irqs.h     |    2 --
 arch/arm/plat-mxc/tzic.c                  |    4 +---
 arch/arm/plat-omap/include/plat/irqs.h    |    4 ----
 arch/arm/plat-s3c24xx/irq.c               |    6 ++----
 drivers/media/video/mx1_camera.c          |    6 +++---
 sound/soc/fsl/imx-pcm-fiq.c               |    4 ++--
 15 files changed, 30 insertions(+), 71 deletions(-)

-- 
Anton Vorontsov
Email: cbouatmailru at gmail.com

[PATCH 1/9] ARM: mach-rpc: Don't register FIQs with genirq

[Anton Vorontsov]

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

[PATCH 1/9] ARM: mach-rpc: Don't register FIQs with genirq

[Anton Vorontsov]

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

[PATCH 2/9] ARM: plat-s3c24xx: Don't use FIQ_START

[Anton Vorontsov]

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

[PATCH 2/9] ARM: plat-s3c24xx: Don't use FIQ_START

[Anton Vorontsov]

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

RE: [PATCH 2/9] ARM: plat-s3c24xx: Don't use FIQ_START

[Kukjin Kim]

Anton Vorontsov wrote:
> 
> 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>

BTW, how was going on the 'change FIQ_START to a variable' patch from Shawn
Guo?
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-June/106486.html

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.


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

[PATCH 2/9] ARM: plat-s3c24xx: Don't use FIQ_START

[Kukjin Kim]

Anton Vorontsov wrote:
> 
> 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>

BTW, how was going on the 'change FIQ_START to a variable' patch from Shawn
Guo?
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-June/106486.html

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.


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

Re: [PATCH 2/9] ARM: plat-s3c24xx: Don't use FIQ_START

[Anton Vorontsov]

On Wed, Aug 08, 2012 at 07:47:26PM +0900, Kukjin Kim wrote:
> Anton Vorontsov wrote:
> > 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>

Thanks!

> BTW, how was going on the 'change FIQ_START to a variable' patch from Shawn
> Guo?
> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-June/106486.html

It's in Linus' tree already. My patch set is just a next step in
the same direction.

-- 
Anton Vorontsov
Email: cbouatmailru@gmail.com

[PATCH 2/9] ARM: plat-s3c24xx: Don't use FIQ_START

[Anton Vorontsov]

On Wed, Aug 08, 2012 at 07:47:26PM +0900, Kukjin Kim wrote:
> Anton Vorontsov wrote:
> > 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>

Thanks!

> BTW, how was going on the 'change FIQ_START to a variable' patch from Shawn
> Guo?
> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-June/106486.html

It's in Linus' tree already. My patch set is just a next step in
the same direction.

-- 
Anton Vorontsov
Email: cbouatmailru at gmail.com

[PATCH 3/9] [media] mx1_camera: Don't use {en,dis}able_fiq() calls

[Anton Vorontsov]

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>
---
 drivers/media/video/mx1_camera.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/video/mx1_camera.c b/drivers/media/video/mx1_camera.c
index d2e6f82..1536d09 100644
--- a/drivers/media/video/mx1_camera.c
+++ b/drivers/media/video/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.7.10.4

[PATCH 3/9] [media] mx1_camera: Don't use {en,dis}able_fiq() calls

[Anton Vorontsov]

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>
---
 drivers/media/video/mx1_camera.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/video/mx1_camera.c b/drivers/media/video/mx1_camera.c
index d2e6f82..1536d09 100644
--- a/drivers/media/video/mx1_camera.c
+++ b/drivers/media/video/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.7.10.4

Re: [PATCH 3/9] [media] mx1_camera: Don't use {en,dis}able_fiq() calls

[Sascha Hauer]

On Sun, Aug 05, 2012 at 04:03:33PM -0700, Anton Vorontsov wrote:
> 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/video/mx1_camera.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/video/mx1_camera.c b/drivers/media/video/mx1_camera.c
> index d2e6f82..1536d09 100644
> --- a/drivers/media/video/mx1_camera.c
> +++ b/drivers/media/video/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.7.10.4
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 3/9] [media] mx1_camera: Don't use {en,dis}able_fiq() calls

[Sascha Hauer]

On Sun, Aug 05, 2012 at 04:03:33PM -0700, Anton Vorontsov wrote:
> 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/video/mx1_camera.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/video/mx1_camera.c b/drivers/media/video/mx1_camera.c
> index d2e6f82..1536d09 100644
> --- a/drivers/media/video/mx1_camera.c
> +++ b/drivers/media/video/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.7.10.4
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Anton Vorontsov]

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>
---
 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 ee27ba3..993e37d 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.7.10.4

[PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Anton Vorontsov]

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>
---
 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 ee27ba3..993e37d 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.7.10.4

Re: [PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Matt Sealey]

On Sun, Aug 5, 2012 at 6:03 PM, Anton Vorontsov
<anton.vorontsov@linaro.org> wrote:
> 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.

I have a semi-related question about this;

Firstly, I was planning on (re-)submitting a patch for the
arch/arm/plat-mxc/ssi-fiq.S code which made it build in ARM mode
(since the code isn't Thumb compatible for various reasons) as it was
a blocker for a Thumb2-compiled kernel. Since the code was only needed
on ARM-capable processors it wouldn't cause a problem. Sascha signed
off on this a long while back and I've been testing it on all my
internal kernel versions, and I don't see any ill effects (that said I
don't have an i.MX28 or so to really verify it, I can't see why it
would not work). I realise this is redundant right now, anyway, since
it's only really enabled on imx_v4_v5 configs and they don't support
Thumb2 kernels anyway. What might be worth submitting is a switch to
add the ".arm" directive anyway simply for correctness since it could
never be compiled for Thumb anyway. We all know what gnu as is like.

Looking at it again on the back of these patches, I noticed the
ssi-fiq.S code is compiled in when SND_IMX_SOC is enabled - of course,
it's only needed in the kernel if SND_SOC_IMX_PCM_FIQ is enabled (the
code that uses the FIQ stuff is only compiled then) but here on the
Efika MX builds it's being built, and I noticed it when it broke my
build because of the above. I'm therefore going to submit a patch
which changes the ifdef SND_SOC_IMX to ifdef SND_SOC_IMX_PCM_FIQ so
it's not compiled in unless absolutely necessary. However, there was a
rumble that this code may disappear or be reworked in the future
making this also quite redundant. Since it's not in the
imx_v6_v7_defconfig anyway, I'm sure this only didn't get noticed
because nobody's building Thumb2 kernels and nobody is trying configs
with audio enabled anyway..

This begs the question, could there be something somewhere hidden deep
in the kernel that is enabled by default or in some config somewhere
that requires "select FIQ" in it's Kconfig entry, but isn't being
enabled? On i.MX the only thing turning it on is that code, but other
ARM arch enable it by default. Since things have been moved to more
generic routines I can't in my mind guarantee that such a patch would
be well tested here since I would be relying on symbols missing or
defines not there anymore.. I have no real way to ensure that it would
work on boards I don't have.

So, is the first patch (ssi-fiq.S .arm) worth it? I think the
SND_SOC_IMX_PCM_FIQ patch is worth it for v6_v7 systems anyway, but
maybe this should have been caught sooner, so should I update the
defconfig to enable some kind of audio bus support (Babbage has it in
the DT so is a needful thing for testing, I figure..)? And does anyone
forsee any problems with that option changing and the only "user" of
"FIQ" in the Kconfigs going away now all the FIQ-specific symbols went
away outside of the generic irq subsystem?

I will probably throw out all three today anyway, but I thought it
would make a good discussion anyway.

-- 
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.

[PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Matt Sealey]

On Sun, Aug 5, 2012 at 6:03 PM, Anton Vorontsov
<anton.vorontsov@linaro.org> wrote:
> 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.

I have a semi-related question about this;

Firstly, I was planning on (re-)submitting a patch for the
arch/arm/plat-mxc/ssi-fiq.S code which made it build in ARM mode
(since the code isn't Thumb compatible for various reasons) as it was
a blocker for a Thumb2-compiled kernel. Since the code was only needed
on ARM-capable processors it wouldn't cause a problem. Sascha signed
off on this a long while back and I've been testing it on all my
internal kernel versions, and I don't see any ill effects (that said I
don't have an i.MX28 or so to really verify it, I can't see why it
would not work). I realise this is redundant right now, anyway, since
it's only really enabled on imx_v4_v5 configs and they don't support
Thumb2 kernels anyway. What might be worth submitting is a switch to
add the ".arm" directive anyway simply for correctness since it could
never be compiled for Thumb anyway. We all know what gnu as is like.

Looking at it again on the back of these patches, I noticed the
ssi-fiq.S code is compiled in when SND_IMX_SOC is enabled - of course,
it's only needed in the kernel if SND_SOC_IMX_PCM_FIQ is enabled (the
code that uses the FIQ stuff is only compiled then) but here on the
Efika MX builds it's being built, and I noticed it when it broke my
build because of the above. I'm therefore going to submit a patch
which changes the ifdef SND_SOC_IMX to ifdef SND_SOC_IMX_PCM_FIQ so
it's not compiled in unless absolutely necessary. However, there was a
rumble that this code may disappear or be reworked in the future
making this also quite redundant. Since it's not in the
imx_v6_v7_defconfig anyway, I'm sure this only didn't get noticed
because nobody's building Thumb2 kernels and nobody is trying configs
with audio enabled anyway..

This begs the question, could there be something somewhere hidden deep
in the kernel that is enabled by default or in some config somewhere
that requires "select FIQ" in it's Kconfig entry, but isn't being
enabled? On i.MX the only thing turning it on is that code, but other
ARM arch enable it by default. Since things have been moved to more
generic routines I can't in my mind guarantee that such a patch would
be well tested here since I would be relying on symbols missing or
defines not there anymore.. I have no real way to ensure that it would
work on boards I don't have.

So, is the first patch (ssi-fiq.S .arm) worth it? I think the
SND_SOC_IMX_PCM_FIQ patch is worth it for v6_v7 systems anyway, but
maybe this should have been caught sooner, so should I update the
defconfig to enable some kind of audio bus support (Babbage has it in
the DT so is a needful thing for testing, I figure..)? And does anyone
forsee any problems with that option changing and the only "user" of
"FIQ" in the Kconfigs going away now all the FIQ-specific symbols went
away outside of the generic irq subsystem?

I will probably throw out all three today anyway, but I thought it
would make a good discussion anyway.

-- 
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.

Re: [PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Mark Brown]

On Mon, Aug 06, 2012 at 10:19:50AM -0500, Matt Sealey wrote:

> it's not compiled in unless absolutely necessary. However, there was a
> rumble that this code may disappear or be reworked in the future
> making this also quite redundant. Since it's not in the

There's no point in the FIQ driver if the DMA drivers are supported.

[PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Mark Brown]

On Mon, Aug 06, 2012 at 10:19:50AM -0500, Matt Sealey wrote:

> it's not compiled in unless absolutely necessary. However, there was a
> rumble that this code may disappear or be reworked in the future
> making this also quite redundant. Since it's not in the

There's no point in the FIQ driver if the DMA drivers are supported.

Re: [PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Matt Sealey]

On Mon, Aug 6, 2012 at 10:49 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Mon, Aug 06, 2012 at 10:19:50AM -0500, Matt Sealey wrote:
>
>> it's not compiled in unless absolutely necessary. However, there was a
>> rumble that this code may disappear or be reworked in the future
>> making this also quite redundant. Since it's not in the
>
> There's no point in the FIQ driver if the DMA drivers are supported.

http://patchwork.ozlabs.org/patch/128853/

Russell's sage advise; "It's worth pointing out that people end up
using FIQs for certain things
because the hardware requires you to do it.  So if a platform is using
them, they're probably not
doing it out of choice, but are doing it because it's a baseline
requirement to get something working."

I don't recall Sascha's response to this, I thought it was on the ML
but it may have been in private,
but something is broken on the MX27/28 SSI FIFO which means FIQ is the
only way to get reliable
audio since the DMA unit cannot get accurate alarms (this seems a
common bug in Freescale
processors :) - I'll let him elaborate since I've never even seen an
MX28 let alone booted one, and
our MX27 board never got tested without the FIQ code if I recall correctly.

So that needs to stay, the issue here is why did nobody catch
ssi-fiq.S breaking in testing MX51
Babbage and building a Thumb2 kernel, for example? Why did nobody
notice it was building when
configuring for MX3/5/6 boards (which do actually have working SSI and
DMA, as you assume, and
do not need this code, nor build the imx-pcm-dma-fiq part of the
driver anyway)? I'm willing to fix all
of the above, but if there's an obvious deficiency in testing at some
point we need to fix that too..

Of course if Sascha says the fiq dma hack can disappear forever that's
absolutely fine, I'm also
willing to be the one to delete it... :)

-- 
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.

[PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Matt Sealey]

On Mon, Aug 6, 2012 at 10:49 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Mon, Aug 06, 2012 at 10:19:50AM -0500, Matt Sealey wrote:
>
>> it's not compiled in unless absolutely necessary. However, there was a
>> rumble that this code may disappear or be reworked in the future
>> making this also quite redundant. Since it's not in the
>
> There's no point in the FIQ driver if the DMA drivers are supported.

http://patchwork.ozlabs.org/patch/128853/

Russell's sage advise; "It's worth pointing out that people end up
using FIQs for certain things
because the hardware requires you to do it.  So if a platform is using
them, they're probably not
doing it out of choice, but are doing it because it's a baseline
requirement to get something working."

I don't recall Sascha's response to this, I thought it was on the ML
but it may have been in private,
but something is broken on the MX27/28 SSI FIFO which means FIQ is the
only way to get reliable
audio since the DMA unit cannot get accurate alarms (this seems a
common bug in Freescale
processors :) - I'll let him elaborate since I've never even seen an
MX28 let alone booted one, and
our MX27 board never got tested without the FIQ code if I recall correctly.

So that needs to stay, the issue here is why did nobody catch
ssi-fiq.S breaking in testing MX51
Babbage and building a Thumb2 kernel, for example? Why did nobody
notice it was building when
configuring for MX3/5/6 boards (which do actually have working SSI and
DMA, as you assume, and
do not need this code, nor build the imx-pcm-dma-fiq part of the
driver anyway)? I'm willing to fix all
of the above, but if there's an obvious deficiency in testing at some
point we need to fix that too..

Of course if Sascha says the fiq dma hack can disappear forever that's
absolutely fine, I'm also
willing to be the one to delete it... :)

-- 
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.

Re: [PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Mark Brown]

On Mon, Aug 06, 2012 at 01:09:26PM -0500, Matt Sealey wrote:

> So that needs to stay, the issue here is why did nobody catch
> ssi-fiq.S breaking in testing MX51
> Babbage and building a Thumb2 kernel, for example? Why did nobody
> notice it was building when
> configuring for MX3/5/6 boards (which do actually have working SSI and
> DMA, as you assume, and
> do not need this code, nor build the imx-pcm-dma-fiq part of the
> driver anyway)? I'm willing to fix all
> of the above, but if there's an obvious deficiency in testing at some
> point we need to fix that too..

As far as I can tell nobody's really running much up to date mainline on
older i.MX processors, all the work is going on the new stuff and most
of the board are on either vendor BSPs or older kernels.

[PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Mark Brown]

On Mon, Aug 06, 2012 at 01:09:26PM -0500, Matt Sealey wrote:

> So that needs to stay, the issue here is why did nobody catch
> ssi-fiq.S breaking in testing MX51
> Babbage and building a Thumb2 kernel, for example? Why did nobody
> notice it was building when
> configuring for MX3/5/6 boards (which do actually have working SSI and
> DMA, as you assume, and
> do not need this code, nor build the imx-pcm-dma-fiq part of the
> driver anyway)? I'm willing to fix all
> of the above, but if there's an obvious deficiency in testing at some
> point we need to fix that too..

As far as I can tell nobody's really running much up to date mainline on
older i.MX processors, all the work is going on the new stuff and most
of the board are on either vendor BSPs or older kernels.

Re: [PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Robert Schwebel]

On Mon, Aug 06, 2012 at 08:37:34PM +0100, Mark Brown wrote:
> As far as I can tell nobody's really running much up to date mainline
> on older i.MX processors, all the work is going on the new stuff and
> most of the board are on either vendor BSPs or older kernels.

That's not true; we still run MX25, MX27, MX35, MX28 on mainline in
active projects.

rsc
-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Robert Schwebel]

On Mon, Aug 06, 2012 at 08:37:34PM +0100, Mark Brown wrote:
> As far as I can tell nobody's really running much up to date mainline
> on older i.MX processors, all the work is going on the new stuff and
> most of the board are on either vendor BSPs or older kernels.

That's not true; we still run MX25, MX27, MX35, MX28 on mainline in
active projects.

rsc
-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Matt Sealey]

On Mon, Aug 6, 2012 at 3:16 PM, Robert Schwebel
<r.schwebel@pengutronix.de> wrote:
> On Mon, Aug 06, 2012 at 08:37:34PM +0100, Mark Brown wrote:
>> As far as I can tell nobody's really running much up to date mainline
>> on older i.MX processors, all the work is going on the new stuff and
>> most of the board are on either vendor BSPs or older kernels.
>
> That's not true; we still run MX25, MX27, MX35, MX28 on mainline in
> active projects.

I think Shawn Guo (FSL/Linaro) would also disagree, since he's just
posted a large amount of MXS patches to fix up the board for device
trees, and Arnd is pulling them.

Work is ongoing, it would be awful to delete something people really
relied on or were in progress fixing (ahem). If they get up to audio
in the near future the audio FIQ stuff would just end up being
resubmitted almost verbatim... that seems like unnecessary churn.

As far as I know, the FIQ usage is quite valid for the processor it
needs to run on (MX21/27/28, right?) in the modes it runs in (AC97 on
these processors, and maybe MX35 too), and I'm just trying to figure
out what the steps are for

* making sure it doesn't get built for architectures/variants it's
certainly NOT required on (imx_v6_v7_defconfig, if it actually enabled
audio, that is - in fact, audio should be enabled as more one of the
boards supported defines it in the device tree) which would amount to
two seperate patches, one for the defconfig, one for the CONFIG item.

I did note that SND_SOC_EUKREA_TLV320 enables FIQ but it also depends
on MX51 which makes me think this need to be split, too, so that MX51
boards don't have it but MX2/MX3 do.

* if it is built then it's built as ARM code (redundant, as previously
stated, but would have stopped me from swearing at my build box when I
hit the issue yet again) which could be a patch or could be ignored.
I'd rather discuss this here than clutter the ML with several respins
of a patch, let's get an opinion first - to .arm or no to .arm?

* make sure there's no weird FIQ stuff floating around that has so far
relied on SND_SOC_IMX_PCM_FIQ doing select FIQ before I make it not
compile in for other boards (since otherwise no i.MX processor selects
FIQ if they're not using that driver, it could be simply coincidence
nobody noticed). I don't want to be the one submitting a patch I can't
test which mysteriously breaks every MX28 on the planet (since Rob,
Shawn, Sascha might be the ones swearing at me instead)

* making sure someone's actually testing audio as above... and
where/if/when the MX28 audio stuff is going in the future und so
weiter..

I guess I need Sascha to pipe up and tell me what the code is needed
for exactly again.. and someone to test the result of the changes..

-- 
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.

[PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Matt Sealey]

On Mon, Aug 6, 2012 at 3:16 PM, Robert Schwebel
<r.schwebel@pengutronix.de> wrote:
> On Mon, Aug 06, 2012 at 08:37:34PM +0100, Mark Brown wrote:
>> As far as I can tell nobody's really running much up to date mainline
>> on older i.MX processors, all the work is going on the new stuff and
>> most of the board are on either vendor BSPs or older kernels.
>
> That's not true; we still run MX25, MX27, MX35, MX28 on mainline in
> active projects.

I think Shawn Guo (FSL/Linaro) would also disagree, since he's just
posted a large amount of MXS patches to fix up the board for device
trees, and Arnd is pulling them.

Work is ongoing, it would be awful to delete something people really
relied on or were in progress fixing (ahem). If they get up to audio
in the near future the audio FIQ stuff would just end up being
resubmitted almost verbatim... that seems like unnecessary churn.

As far as I know, the FIQ usage is quite valid for the processor it
needs to run on (MX21/27/28, right?) in the modes it runs in (AC97 on
these processors, and maybe MX35 too), and I'm just trying to figure
out what the steps are for

* making sure it doesn't get built for architectures/variants it's
certainly NOT required on (imx_v6_v7_defconfig, if it actually enabled
audio, that is - in fact, audio should be enabled as more one of the
boards supported defines it in the device tree) which would amount to
two seperate patches, one for the defconfig, one for the CONFIG item.

I did note that SND_SOC_EUKREA_TLV320 enables FIQ but it also depends
on MX51 which makes me think this need to be split, too, so that MX51
boards don't have it but MX2/MX3 do.

* if it is built then it's built as ARM code (redundant, as previously
stated, but would have stopped me from swearing at my build box when I
hit the issue yet again) which could be a patch or could be ignored.
I'd rather discuss this here than clutter the ML with several respins
of a patch, let's get an opinion first - to .arm or no to .arm?

* make sure there's no weird FIQ stuff floating around that has so far
relied on SND_SOC_IMX_PCM_FIQ doing select FIQ before I make it not
compile in for other boards (since otherwise no i.MX processor selects
FIQ if they're not using that driver, it could be simply coincidence
nobody noticed). I don't want to be the one submitting a patch I can't
test which mysteriously breaks every MX28 on the planet (since Rob,
Shawn, Sascha might be the ones swearing at me instead)

* making sure someone's actually testing audio as above... and
where/if/when the MX28 audio stuff is going in the future und so
weiter..

I guess I need Sascha to pipe up and tell me what the code is needed
for exactly again.. and someone to test the result of the changes..

-- 
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.

Re: [PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Mark Brown]

On Mon, Aug 06, 2012 at 03:39:50PM -0500, Matt Sealey wrote:
> On Mon, Aug 6, 2012 at 3:16 PM, Robert Schwebel

> > That's not true; we still run MX25, MX27, MX35, MX28 on mainline in
> > active projects.

> I think Shawn Guo (FSL/Linaro) would also disagree, since he's just
> posted a large amount of MXS patches to fix up the board for device
> trees, and Arnd is pulling them.

MXS != i.MX.

> As far as I know, the FIQ usage is quite valid for the processor it
> needs to run on (MX21/27/28, right?) in the modes it runs in (AC97 on
> these processors, and maybe MX35 too), and I'm just trying to figure
> out what the steps are for

Oh, ick.  What is the problem that means people want to use the FIQ on
these processors?  There's been i.MX2x audio DMA since forever, it had
DMA in mainline before any of the later i.MXs due to them having SDMA.
The original mainline i.MX audio support was done on i.MX27 and didn't
have any issue here, this is the fist I've heard of a problem.

> I did note that SND_SOC_EUKREA_TLV320 enables FIQ but it also depends
> on MX51 which makes me think this need to be split, too, so that MX51
> boards don't have it but MX2/MX3 do.

Is that not bitrot due to it being there before SDMA support was?

[PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Mark Brown]

On Mon, Aug 06, 2012 at 03:39:50PM -0500, Matt Sealey wrote:
> On Mon, Aug 6, 2012 at 3:16 PM, Robert Schwebel

> > That's not true; we still run MX25, MX27, MX35, MX28 on mainline in
> > active projects.

> I think Shawn Guo (FSL/Linaro) would also disagree, since he's just
> posted a large amount of MXS patches to fix up the board for device
> trees, and Arnd is pulling them.

MXS != i.MX.

> As far as I know, the FIQ usage is quite valid for the processor it
> needs to run on (MX21/27/28, right?) in the modes it runs in (AC97 on
> these processors, and maybe MX35 too), and I'm just trying to figure
> out what the steps are for

Oh, ick.  What is the problem that means people want to use the FIQ on
these processors?  There's been i.MX2x audio DMA since forever, it had
DMA in mainline before any of the later i.MXs due to them having SDMA.
The original mainline i.MX audio support was done on i.MX27 and didn't
have any issue here, this is the fist I've heard of a problem.

> I did note that SND_SOC_EUKREA_TLV320 enables FIQ but it also depends
> on MX51 which makes me think this need to be split, too, so that MX51
> boards don't have it but MX2/MX3 do.

Is that not bitrot due to it being there before SDMA support was?

Re: [PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Matt Sealey]

On Mon, Aug 6, 2012 at 4:41 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Mon, Aug 06, 2012 at 03:39:50PM -0500, Matt Sealey wrote:
>> On Mon, Aug 6, 2012 at 3:16 PM, Robert Schwebel
>
>> > That's not true; we still run MX25, MX27, MX35, MX28 on mainline in
>> > active projects.
>
>> I think Shawn Guo (FSL/Linaro) would also disagree, since he's just
>> posted a large amount of MXS patches to fix up the board for device
>> trees, and Arnd is pulling them.
>
> MXS != i.MX.
>
>> As far as I know, the FIQ usage is quite valid for the processor it
>> needs to run on (MX21/27/28, right?) in the modes it runs in (AC97 on
>> these processors, and maybe MX35 too), and I'm just trying to figure
>> out what the steps are for
>
> Oh, ick.  What is the problem that means people want to use the FIQ on
> these processors?  There's been i.MX2x audio DMA since forever, it had
> DMA in mainline before any of the later i.MXs due to them having SDMA.
> The original mainline i.MX audio support was done on i.MX27 and didn't
> have any issue here, this is the fist I've heard of a problem.

For SSI in I2S mode, I think it works great, AC97 not so much. It's
the AC97 mode
the FIQ stuff is there to fix. It seems a bunch of boards use AC97
codecs (for no
good reason)

>> I did note that SND_SOC_EUKREA_TLV320 enables FIQ but it also depends
>> on MX51 which makes me think this need to be split, too, so that MX51
>> boards don't have it but MX2/MX3 do.
>
> Is that not bitrot due to it being there before SDMA support was?

Possibly. I'm going to wait for Sascha to explain it again.. if AC97
was the predicate
for FIQ being required, it doesn't make any sense for a codec that says in it's
description that it's I2S. It may not be bitrot but pure,
unadulterated zeal on the part
of whoever wrote that Kconfig snippet - enable everything, in case it
fails (actually
that kind of thing is why I fear for nuking the building of this
driver since if FIQ stuff
is silently required by something else and all that kept it in there
was some badly
thought out or bitrotted Kconfig entries... I don't want to cause a
world of hurt).

-- 
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.

[PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Matt Sealey]

On Mon, Aug 6, 2012 at 4:41 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Mon, Aug 06, 2012 at 03:39:50PM -0500, Matt Sealey wrote:
>> On Mon, Aug 6, 2012 at 3:16 PM, Robert Schwebel
>
>> > That's not true; we still run MX25, MX27, MX35, MX28 on mainline in
>> > active projects.
>
>> I think Shawn Guo (FSL/Linaro) would also disagree, since he's just
>> posted a large amount of MXS patches to fix up the board for device
>> trees, and Arnd is pulling them.
>
> MXS != i.MX.
>
>> As far as I know, the FIQ usage is quite valid for the processor it
>> needs to run on (MX21/27/28, right?) in the modes it runs in (AC97 on
>> these processors, and maybe MX35 too), and I'm just trying to figure
>> out what the steps are for
>
> Oh, ick.  What is the problem that means people want to use the FIQ on
> these processors?  There's been i.MX2x audio DMA since forever, it had
> DMA in mainline before any of the later i.MXs due to them having SDMA.
> The original mainline i.MX audio support was done on i.MX27 and didn't
> have any issue here, this is the fist I've heard of a problem.

For SSI in I2S mode, I think it works great, AC97 not so much. It's
the AC97 mode
the FIQ stuff is there to fix. It seems a bunch of boards use AC97
codecs (for no
good reason)

>> I did note that SND_SOC_EUKREA_TLV320 enables FIQ but it also depends
>> on MX51 which makes me think this need to be split, too, so that MX51
>> boards don't have it but MX2/MX3 do.
>
> Is that not bitrot due to it being there before SDMA support was?

Possibly. I'm going to wait for Sascha to explain it again.. if AC97
was the predicate
for FIQ being required, it doesn't make any sense for a codec that says in it's
description that it's I2S. It may not be bitrot but pure,
unadulterated zeal on the part
of whoever wrote that Kconfig snippet - enable everything, in case it
fails (actually
that kind of thing is why I fear for nuking the building of this
driver since if FIQ stuff
is silently required by something else and all that kept it in there
was some badly
thought out or bitrotted Kconfig entries... I don't want to cause a
world of hurt).

-- 
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.

Re: [PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Sascha Hauer]

On Mon, Aug 06, 2012 at 10:41:22PM +0100, Mark Brown wrote:
> On Mon, Aug 06, 2012 at 03:39:50PM -0500, Matt Sealey wrote:
> > On Mon, Aug 6, 2012 at 3:16 PM, Robert Schwebel
> 
> > > That's not true; we still run MX25, MX27, MX35, MX28 on mainline in
> > > active projects.
> 
> > I think Shawn Guo (FSL/Linaro) would also disagree, since he's just
> > posted a large amount of MXS patches to fix up the board for device
> > trees, and Arnd is pulling them.
> 
> MXS != i.MX.
> 
> > As far as I know, the FIQ usage is quite valid for the processor it
> > needs to run on (MX21/27/28, right?) in the modes it runs in (AC97 on
> > these processors, and maybe MX35 too), and I'm just trying to figure
> > out what the steps are for
> 
> Oh, ick.  What is the problem that means people want to use the FIQ on
> these processors?  There's been i.MX2x audio DMA since forever, it had
> DMA in mainline before any of the later i.MXs due to them having SDMA.
> The original mainline i.MX audio support was done on i.MX27 and didn't
> have any issue here, this is the fist I've heard of a problem.

Originally the FIQ support was only used because there hasn't been SDMA
support available at that time.
Nowadays the FIQ support is necessary only for AC97. The AC97 support in
the SSI unit is buggy: It does not allow you to select the slots you
want to receive. At least the wm9712 codec always sends (apart from the
stereo data) data in slot (I think it is) 12. You find this data mixed
in your audio stream. The FIQ driver skips this data to get a valid
audio stream.

One other way to solve this would be to use dma here and to filter out
the data afterwards.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Sascha Hauer]

On Mon, Aug 06, 2012 at 10:41:22PM +0100, Mark Brown wrote:
> On Mon, Aug 06, 2012 at 03:39:50PM -0500, Matt Sealey wrote:
> > On Mon, Aug 6, 2012 at 3:16 PM, Robert Schwebel
> 
> > > That's not true; we still run MX25, MX27, MX35, MX28 on mainline in
> > > active projects.
> 
> > I think Shawn Guo (FSL/Linaro) would also disagree, since he's just
> > posted a large amount of MXS patches to fix up the board for device
> > trees, and Arnd is pulling them.
> 
> MXS != i.MX.
> 
> > As far as I know, the FIQ usage is quite valid for the processor it
> > needs to run on (MX21/27/28, right?) in the modes it runs in (AC97 on
> > these processors, and maybe MX35 too), and I'm just trying to figure
> > out what the steps are for
> 
> Oh, ick.  What is the problem that means people want to use the FIQ on
> these processors?  There's been i.MX2x audio DMA since forever, it had
> DMA in mainline before any of the later i.MXs due to them having SDMA.
> The original mainline i.MX audio support was done on i.MX27 and didn't
> have any issue here, this is the fist I've heard of a problem.

Originally the FIQ support was only used because there hasn't been SDMA
support available at that time.
Nowadays the FIQ support is necessary only for AC97. The AC97 support in
the SSI unit is buggy: It does not allow you to select the slots you
want to receive. At least the wm9712 codec always sends (apart from the
stereo data) data in slot (I think it is) 12. You find this data mixed
in your audio stream. The FIQ driver skips this data to get a valid
audio stream.

One other way to solve this would be to use dma here and to filter out
the data afterwards.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Mark Brown]

On Tue, Aug 07, 2012 at 08:35:58AM +0200, Sascha Hauer wrote:

> Nowadays the FIQ support is necessary only for AC97. The AC97 support in
> the SSI unit is buggy: It does not allow you to select the slots you
> want to receive. At least the wm9712 codec always sends (apart from the
> stereo data) data in slot (I think it is) 12. You find this data mixed
> in your audio stream. The FIQ driver skips this data to get a valid
> audio stream.

Right, any device with GPIO support will do this - it's how GPIO works
in AC'97.

> One other way to solve this would be to use dma here and to filter out
> the data afterwards.

Yup.  That's probably more sane but also more work to implement.

[PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Mark Brown]

On Tue, Aug 07, 2012 at 08:35:58AM +0200, Sascha Hauer wrote:

> Nowadays the FIQ support is necessary only for AC97. The AC97 support in
> the SSI unit is buggy: It does not allow you to select the slots you
> want to receive. At least the wm9712 codec always sends (apart from the
> stereo data) data in slot (I think it is) 12. You find this data mixed
> in your audio stream. The FIQ driver skips this data to get a valid
> audio stream.

Right, any device with GPIO support will do this - it's how GPIO works
in AC'97.

> One other way to solve this would be to use dma here and to filter out
> the data afterwards.

Yup.  That's probably more sane but also more work to implement.

Re: [PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Shawn Guo]

On Mon, Aug 06, 2012 at 03:39:50PM -0500, Matt Sealey wrote:
> * make sure there's no weird FIQ stuff floating around that has so far
> relied on SND_SOC_IMX_PCM_FIQ doing select FIQ before I make it not

Acked on changing SND_IMX_SOC to SND_SOC_IMX_PCM_FIQ in
arch/arm/plat-mxc/Makefile.

> compile in for other boards (since otherwise no i.MX processor selects
> FIQ if they're not using that driver, it could be simply coincidence
> nobody noticed). I don't want to be the one submitting a patch I can't
> test which mysteriously breaks every MX28 on the planet (since Rob,
> Shawn, Sascha might be the ones swearing at me instead)
> 
Though i.MX23 and i.MX28 are named in i.MX family, they are actually
a different architecture from IMX/MXC.  It's MXS, nothing to do with
SSI.  Folder sound/soc/mxs/ is the one for i.MX28.

-- 
Regards,
Shawn

[PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Shawn Guo]

On Mon, Aug 06, 2012 at 03:39:50PM -0500, Matt Sealey wrote:
> * make sure there's no weird FIQ stuff floating around that has so far
> relied on SND_SOC_IMX_PCM_FIQ doing select FIQ before I make it not

Acked on changing SND_IMX_SOC to SND_SOC_IMX_PCM_FIQ in
arch/arm/plat-mxc/Makefile.

> compile in for other boards (since otherwise no i.MX processor selects
> FIQ if they're not using that driver, it could be simply coincidence
> nobody noticed). I don't want to be the one submitting a patch I can't
> test which mysteriously breaks every MX28 on the planet (since Rob,
> Shawn, Sascha might be the ones swearing at me instead)
> 
Though i.MX23 and i.MX28 are named in i.MX family, they are actually
a different architecture from IMX/MXC.  It's MXS, nothing to do with
SSI.  Folder sound/soc/mxs/ is the one for i.MX28.

-- 
Regards,
Shawn

Re: [PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Dave Martin]

On Mon, Aug 06, 2012 at 10:19:50AM -0500, Matt Sealey wrote:
> On Sun, Aug 5, 2012 at 6:03 PM, Anton Vorontsov
> <anton.vorontsov@linaro.org> wrote:
> > 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.
> 
> I have a semi-related question about this;
> 
> Firstly, I was planning on (re-)submitting a patch for the
> arch/arm/plat-mxc/ssi-fiq.S code which made it build in ARM mode
> (since the code isn't Thumb compatible for various reasons) as it was
> a blocker for a Thumb2-compiled kernel. Since the code was only needed
> on ARM-capable processors it wouldn't cause a problem. Sascha signed
> off on this a long while back and I've been testing it on all my
> internal kernel versions, and I don't see any ill effects (that said I
> don't have an i.MX28 or so to really verify it, I can't see why it
> would not work). I realise this is redundant right now, anyway, since
> it's only really enabled on imx_v4_v5 configs and they don't support
> Thumb2 kernels anyway. What might be worth submitting is a switch to
> add the ".arm" directive anyway simply for correctness since it could
> never be compiled for Thumb anyway. We all know what gnu as is like.
> 
> Looking at it again on the back of these patches, I noticed the
> ssi-fiq.S code is compiled in when SND_IMX_SOC is enabled - of course,
> it's only needed in the kernel if SND_SOC_IMX_PCM_FIQ is enabled (the
> code that uses the FIQ stuff is only compiled then) but here on the
> Efika MX builds it's being built, and I noticed it when it broke my
> build because of the above. I'm therefore going to submit a patch
> which changes the ifdef SND_SOC_IMX to ifdef SND_SOC_IMX_PCM_FIQ so
> it's not compiled in unless absolutely necessary. However, there was a
> rumble that this code may disappear or be reworked in the future
> making this also quite redundant. Since it's not in the
> imx_v6_v7_defconfig anyway, I'm sure this only didn't get noticed
> because nobody's building Thumb2 kernels and nobody is trying configs
> with audio enabled anyway..
> 
> This begs the question, could there be something somewhere hidden deep
> in the kernel that is enabled by default or in some config somewhere
> that requires "select FIQ" in it's Kconfig entry, but isn't being
> enabled? On i.MX the only thing turning it on is that code, but other
> ARM arch enable it by default. Since things have been moved to more
> generic routines I can't in my mind guarantee that such a patch would
> be well tested here since I would be relying on symbols missing or
> defines not there anymore.. I have no real way to ensure that it would
> work on boards I don't have.
> 
> So, is the first patch (ssi-fiq.S .arm) worth it? I think the
> SND_SOC_IMX_PCM_FIQ patch is worth it for v6_v7 systems anyway, but
> maybe this should have been caught sooner, so should I update the
> defconfig to enable some kind of audio bus support (Babbage has it in
> the DT so is a needful thing for testing, I figure..)? And does anyone
> forsee any problems with that option changing and the only "user" of
> "FIQ" in the Kconfigs going away now all the FIQ-specific symbols went
> away outside of the generic irq subsystem?

I hit this issue some time ago when I was trying to do a Thumb2 build
of this kernel.

I don't remember having to fix the generic FIQ code just for this,
but I had a patch somewhere to swap r13 with another register in
ssi-fiq.S.  I think that use of r13 in ways not permitted for Thumb
was the only problem I found.  I can try to dig it out if you want.

In any case, it didn't seem worth pushing at the time, because it
seemed that the SSI FIQ code was not relevant to any v7 or later
platform -- so building that code for Thumb presumably doesn't make
sense.

Cheers
---Dave

[PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Dave Martin]

On Mon, Aug 06, 2012 at 10:19:50AM -0500, Matt Sealey wrote:
> On Sun, Aug 5, 2012 at 6:03 PM, Anton Vorontsov
> <anton.vorontsov@linaro.org> wrote:
> > 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.
> 
> I have a semi-related question about this;
> 
> Firstly, I was planning on (re-)submitting a patch for the
> arch/arm/plat-mxc/ssi-fiq.S code which made it build in ARM mode
> (since the code isn't Thumb compatible for various reasons) as it was
> a blocker for a Thumb2-compiled kernel. Since the code was only needed
> on ARM-capable processors it wouldn't cause a problem. Sascha signed
> off on this a long while back and I've been testing it on all my
> internal kernel versions, and I don't see any ill effects (that said I
> don't have an i.MX28 or so to really verify it, I can't see why it
> would not work). I realise this is redundant right now, anyway, since
> it's only really enabled on imx_v4_v5 configs and they don't support
> Thumb2 kernels anyway. What might be worth submitting is a switch to
> add the ".arm" directive anyway simply for correctness since it could
> never be compiled for Thumb anyway. We all know what gnu as is like.
> 
> Looking at it again on the back of these patches, I noticed the
> ssi-fiq.S code is compiled in when SND_IMX_SOC is enabled - of course,
> it's only needed in the kernel if SND_SOC_IMX_PCM_FIQ is enabled (the
> code that uses the FIQ stuff is only compiled then) but here on the
> Efika MX builds it's being built, and I noticed it when it broke my
> build because of the above. I'm therefore going to submit a patch
> which changes the ifdef SND_SOC_IMX to ifdef SND_SOC_IMX_PCM_FIQ so
> it's not compiled in unless absolutely necessary. However, there was a
> rumble that this code may disappear or be reworked in the future
> making this also quite redundant. Since it's not in the
> imx_v6_v7_defconfig anyway, I'm sure this only didn't get noticed
> because nobody's building Thumb2 kernels and nobody is trying configs
> with audio enabled anyway..
> 
> This begs the question, could there be something somewhere hidden deep
> in the kernel that is enabled by default or in some config somewhere
> that requires "select FIQ" in it's Kconfig entry, but isn't being
> enabled? On i.MX the only thing turning it on is that code, but other
> ARM arch enable it by default. Since things have been moved to more
> generic routines I can't in my mind guarantee that such a patch would
> be well tested here since I would be relying on symbols missing or
> defines not there anymore.. I have no real way to ensure that it would
> work on boards I don't have.
> 
> So, is the first patch (ssi-fiq.S .arm) worth it? I think the
> SND_SOC_IMX_PCM_FIQ patch is worth it for v6_v7 systems anyway, but
> maybe this should have been caught sooner, so should I update the
> defconfig to enable some kind of audio bus support (Babbage has it in
> the DT so is a needful thing for testing, I figure..)? And does anyone
> forsee any problems with that option changing and the only "user" of
> "FIQ" in the Kconfigs going away now all the FIQ-specific symbols went
> away outside of the generic irq subsystem?

I hit this issue some time ago when I was trying to do a Thumb2 build
of this kernel.

I don't remember having to fix the generic FIQ code just for this,
but I had a patch somewhere to swap r13 with another register in
ssi-fiq.S.  I think that use of r13 in ways not permitted for Thumb
was the only problem I found.  I can try to dig it out if you want.

In any case, it didn't seem worth pushing at the time, because it
seemed that the SSI FIQ code was not relevant to any v7 or later
platform -- so building that code for Thumb presumably doesn't make
sense.

Cheers
---Dave

Re: [PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Sascha Hauer]

On Sun, Aug 05, 2012 at 04:03:34PM -0700, Anton Vorontsov wrote:
> 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>

> ---
>  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 ee27ba3..993e37d 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.7.10.4
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 4/9] ASoC: imx: Don't use {en,dis}able_fiq() calls

[Sascha Hauer]

On Sun, Aug 05, 2012 at 04:03:34PM -0700, Anton Vorontsov wrote:
> 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>

> ---
>  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 ee27ba3..993e37d 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.7.10.4
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 5/9] ARM: FIQ: Remove enable_fiq() and disable_fiq() calls

[Anton Vorontsov]

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

[PATCH 5/9] ARM: FIQ: Remove enable_fiq() and disable_fiq() calls

[Anton Vorontsov]

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

[PATCH 6/9] ARM: FIQ: Remove FIQ_START

[Anton Vorontsov]

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 "deffered 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-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-omap/include/plat/irqs.h    |    4 ----
 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-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-omap/include/plat/irqs.h b/arch/arm/plat-omap/include/plat/irqs.h
index 37bbbbb..d2b91e3c 100644
--- a/arch/arm/plat-omap/include/plat/irqs.h
+++ b/arch/arm/plat-omap/include/plat/irqs.h
@@ -446,8 +446,4 @@
 
 #include <mach/hardware.h>
 
-#ifdef CONFIG_FIQ
-#define FIQ_START		1024
-#endif
-
 #endif
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.7.10.4

[PATCH 6/9] ARM: FIQ: Remove FIQ_START

[Anton Vorontsov]

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 "deffered 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-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-omap/include/plat/irqs.h    |    4 ----
 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-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-omap/include/plat/irqs.h b/arch/arm/plat-omap/include/plat/irqs.h
index 37bbbbb..d2b91e3c 100644
--- a/arch/arm/plat-omap/include/plat/irqs.h
+++ b/arch/arm/plat-omap/include/plat/irqs.h
@@ -446,8 +446,4 @@
 
 #include <mach/hardware.h>
 
-#ifdef CONFIG_FIQ
-#define FIQ_START		1024
-#endif
-
 #endif
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.7.10.4

[PATCH 7/9] ARM: FIQ: Should include asm/mach/irq.h

[Anton Vorontsov]

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

[PATCH 7/9] ARM: FIQ: Should include asm/mach/irq.h

[Anton Vorontsov]

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

[PATCH 8/9] ARM: FIQ: Implement !CONFIG_FIQ stubs

[Anton Vorontsov]

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 16cedb4..8013ad9 100644
--- a/arch/arm/kernel/irq.c
+++ b/arch/arm/kernel/irq.c
@@ -44,9 +44,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.7.10.4

[PATCH 8/9] ARM: FIQ: Implement !CONFIG_FIQ stubs

[Anton Vorontsov]

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 16cedb4..8013ad9 100644
--- a/arch/arm/kernel/irq.c
+++ b/arch/arm/kernel/irq.c
@@ -44,9 +44,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.7.10.4

[PATCH 9/9] ARM: FIQ: Make show_fiq_list() return void

[Anton Vorontsov]

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

[PATCH 9/9] ARM: FIQ: Make show_fiq_list() return void

[Anton Vorontsov]

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

Re: [PATCH 0/9] Get rid of FIQ_START/enable/disable_fiq() + some FIQ cleanups

[Anton Vorontsov]

Hello Russell,

On Sun, Aug 05, 2012 at 04:02:38PM -0700, Anton Vorontsov wrote:
> During KDB FIQ patches review you 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 you
> once said that you don't want the FIQ subsystem to mess with genirq.

Just wonder if you had a chance to look into this patch set...

Plus, I also realized that maybe it's a good time to get rid of
init_FIQ() altogether? Maybe there are some not so obvious reasons for
its existence, but if not, does the following patch on top of this
series makes sense?

(Also, I noticed that we save no_fiq_insn w/o bothering about
!CONFIG_CPU_USE_DOMAINS case, but set_fiq_handler() has a special
handling for this. I wonder, was that on purpose, e.g. it does not
matter for init_FIQ(), or it was just overlooked? In latter case,
the patch fixes the issue.)

- - - -
[PATCH] ARM: FIQ: Get rid of init_FIQ()

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.
  not spread the logic around (both code placement wise and execution
  time wise).

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

[PATCH 0/9] Get rid of FIQ_START/enable/disable_fiq() + some FIQ cleanups

[Anton Vorontsov]

Hello Russell,

On Sun, Aug 05, 2012 at 04:02:38PM -0700, Anton Vorontsov wrote:
> During KDB FIQ patches review you 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 you
> once said that you don't want the FIQ subsystem to mess with genirq.

Just wonder if you had a chance to look into this patch set...

Plus, I also realized that maybe it's a good time to get rid of
init_FIQ() altogether? Maybe there are some not so obvious reasons for
its existence, but if not, does the following patch on top of this
series makes sense?

(Also, I noticed that we save no_fiq_insn w/o bothering about
!CONFIG_CPU_USE_DOMAINS case, but set_fiq_handler() has a special
handling for this. I wonder, was that on purpose, e.g. it does not
matter for init_FIQ(), or it was just overlooked? In latter case,
the patch fixes the issue.)

- - - -
[PATCH] ARM: FIQ: Get rid of init_FIQ()

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.
  not spread the logic around (both code placement wise and execution
  time wise).

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