[PATCH 1/2] ARM: shmobile: irqpin: add a DT property to enable masking on parent

[PATCH 1/2] ARM: shmobile: irqpin: add a DT property to enable masking on parent

[Guennadi Liakhovetski]

To disable spurious interrupts, that get triggered on certain hardware, the
irqpin driver masks them on the parent interrupt controller. To specify
such broken devices a .control_parent parameter can be provided in the
platform data. In the DT case we need a property, to do the same.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski-Mmb7MZpHnFY@public.gmane.org>
---

These two patches simply split the earlier "ARM: shmobile: irqpin: fix 
handling of spurious interrupts in DT case" patch into two parts. 
Otherwise no change.

 .../interrupt-controller/renesas,intc-irqpin.txt   |    2 ++
 drivers/irqchip/irq-renesas-intc-irqpin.c          |    7 +++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
index c6f09b7..152b10a 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
@@ -11,3 +11,5 @@ Optional properties:
   resource allocation properties
 - sense-bitfield-width: width of a single sense bitfield in the SENSE register,
   if different from the default 4 bits
+- control-parent: disable and enable interrupts on the parent interrupt
+  controller, needed for some broken implementations
diff --git a/drivers/irqchip/irq-renesas-intc-irqpin.c b/drivers/irqchip/irq-renesas-intc-irqpin.c
index 4aca1b2..82cec63 100644
--- a/drivers/irqchip/irq-renesas-intc-irqpin.c
+++ b/drivers/irqchip/irq-renesas-intc-irqpin.c
@@ -348,11 +348,14 @@ static int intc_irqpin_probe(struct platform_device *pdev)
 	}
 
 	/* deal with driver instance configuration */
-	if (pdata)
+	if (pdata) {
 		memcpy(&p->config, pdata, sizeof(*pdata));
-	else
+	} else {
 		of_property_read_u32(pdev->dev.of_node, "sense-bitfield-width",
 				     &p->config.sense_bitfield_width);
+		p->config.control_parent = of_property_read_bool(pdev->dev.of_node,
+								 "control-parent");
+	}
 	if (!p->config.sense_bitfield_width)
 		p->config.sense_bitfield_width = 4; /* default to 4 bits */
 
-- 
1.7.2.5

[PATCH 2/2] ARM: shmobile: irqpin: map spurious interrupts in DT case

[Guennadi Liakhovetski]

In the non-DT case all interrupts get mapped statically during probing,
therefore, if a spurious interrupt arrives, it can easily be mapped back
to hardware registers and bits and handled. In the DT case interrupts are
mapped only when a device, using that interrupt is instantiated from DT.
So, spurious interrupts occur unmapped and thus cannot be handled properly.
This patch fixes this by mapping such interrupts as they occur.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/irqchip/irq-renesas-intc-irqpin.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/irqchip/irq-renesas-intc-irqpin.c b/drivers/irqchip/irq-renesas-intc-irqpin.c
index 82cec63..e62d76d 100644
--- a/drivers/irqchip/irq-renesas-intc-irqpin.c
+++ b/drivers/irqchip/irq-renesas-intc-irqpin.c
@@ -71,6 +71,7 @@ struct intc_irqpin_priv {
 	struct intc_irqpin_iomem iomem[INTC_IRQPIN_REG_NR];
 	struct intc_irqpin_irq irq[INTC_IRQPIN_MAX];
 	struct renesas_intc_irqpin_config config;
+	unsigned int min_irq;
 	unsigned int number_of_irqs;
 	struct platform_device *pdev;
 	struct irq_chip irq_chip;
@@ -274,6 +275,10 @@ static irqreturn_t intc_irqpin_irq_handler(int irq, void *dev_id)
 	struct intc_irqpin_priv *p = i->p;
 	unsigned long bit;
 
+	if (!i->domain_irq)
+		/* unmapped: spurious IRQ, map it now */
+		irq_create_mapping(p->irq_domain, irq - p->min_irq);
+
 	intc_irqpin_dbg(i, "demux1");
 	bit = intc_irqpin_hwirq_mask(p, INTC_IRQPIN_REG_SOURCE, i->hw_irq);
 
@@ -372,6 +377,7 @@ static int intc_irqpin_probe(struct platform_device *pdev)
 		}
 	}
 
+	p->min_irq = INT_MAX;
 	/* allow any number of IRQs between 1 and INTC_IRQPIN_MAX */
 	for (k = 0; k < INTC_IRQPIN_MAX; k++) {
 		irq = platform_get_resource(pdev, IORESOURCE_IRQ, k);
@@ -380,6 +386,8 @@ static int intc_irqpin_probe(struct platform_device *pdev)
 
 		p->irq[k].p = p;
 		p->irq[k].requested_irq = irq->start;
+		if (p->min_irq > irq->start)
+			p->min_irq = irq->start;
 	}
 
 	p->number_of_irqs = k;
-- 
1.7.2.5

Re: [PATCH 2/2] ARM: shmobile: irqpin: map spurious interrupts in DT case

[Grant Likely]

On Fri, 24 May 2013 11:13:07 +0200 (CEST), Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> In the non-DT case all interrupts get mapped statically during probing,
> therefore, if a spurious interrupt arrives, it can easily be mapped back
> to hardware registers and bits and handled. In the DT case interrupts are
> mapped only when a device, using that interrupt is instantiated from DT.
> So, spurious interrupts occur unmapped and thus cannot be handled properly.
> This patch fixes this by mapping such interrupts as they occur.

That sounds like a bad approach. If the driver really requires the irqs
to be mapped, then it should do all of them when the controller is set
up. Mapping at interrupt handling time is absolutely the wrong time to
do it.

Also, why is mapping the irq necessary to handle it? You don't want to
call generic_handle_irq() on a bad interrupt because there won't be
anything listening for it. First of all, that particular irq should be
entirely disabled if at all possible. Second, if it isn't possible to
disable it, then the handler function should clear the state and get out
of the handler as quickly as possible.

g.

> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>  drivers/irqchip/irq-renesas-intc-irqpin.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-renesas-intc-irqpin.c b/drivers/irqchip/irq-renesas-intc-irqpin.c
> index 82cec63..e62d76d 100644
> --- a/drivers/irqchip/irq-renesas-intc-irqpin.c
> +++ b/drivers/irqchip/irq-renesas-intc-irqpin.c
> @@ -71,6 +71,7 @@ struct intc_irqpin_priv {
>  	struct intc_irqpin_iomem iomem[INTC_IRQPIN_REG_NR];
>  	struct intc_irqpin_irq irq[INTC_IRQPIN_MAX];
>  	struct renesas_intc_irqpin_config config;
> +	unsigned int min_irq;
>  	unsigned int number_of_irqs;
>  	struct platform_device *pdev;
>  	struct irq_chip irq_chip;
> @@ -274,6 +275,10 @@ static irqreturn_t intc_irqpin_irq_handler(int irq, void *dev_id)
>  	struct intc_irqpin_priv *p = i->p;
>  	unsigned long bit;
>  
> +	if (!i->domain_irq)
> +		/* unmapped: spurious IRQ, map it now */
> +		irq_create_mapping(p->irq_domain, irq - p->min_irq);
> +
>  	intc_irqpin_dbg(i, "demux1");
>  	bit = intc_irqpin_hwirq_mask(p, INTC_IRQPIN_REG_SOURCE, i->hw_irq);
>  
> @@ -372,6 +377,7 @@ static int intc_irqpin_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	p->min_irq = INT_MAX;
>  	/* allow any number of IRQs between 1 and INTC_IRQPIN_MAX */
>  	for (k = 0; k < INTC_IRQPIN_MAX; k++) {
>  		irq = platform_get_resource(pdev, IORESOURCE_IRQ, k);
> @@ -380,6 +386,8 @@ static int intc_irqpin_probe(struct platform_device *pdev)
>  
>  		p->irq[k].p = p;
>  		p->irq[k].requested_irq = irq->start;
> +		if (p->min_irq > irq->start)
> +			p->min_irq = irq->start;
>  	}
>  
>  	p->number_of_irqs = k;
> -- 
> 1.7.2.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

Re: [PATCH 1/2] ARM: shmobile: irqpin: add a DT property to enable masking on parent

[Simon Horman]

On Fri, May 24, 2013 at 11:13:03AM +0200, Guennadi Liakhovetski wrote:
> To disable spurious interrupts, that get triggered on certain hardware, the
> irqpin driver masks them on the parent interrupt controller. To specify
> such broken devices a .control_parent parameter can be provided in the
> platform data. In the DT case we need a property, to do the same.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
> 
> These two patches simply split the earlier "ARM: shmobile: irqpin: fix 
> handling of spurious interrupts in DT case" patch into two parts. 
> Otherwise no change.

Magnus,

could you please review this series?

> 
>  .../interrupt-controller/renesas,intc-irqpin.txt   |    2 ++
>  drivers/irqchip/irq-renesas-intc-irqpin.c          |    7 +++++--
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
> index c6f09b7..152b10a 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
> +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
> @@ -11,3 +11,5 @@ Optional properties:
>    resource allocation properties
>  - sense-bitfield-width: width of a single sense bitfield in the SENSE register,
>    if different from the default 4 bits
> +- control-parent: disable and enable interrupts on the parent interrupt
> +  controller, needed for some broken implementations
> diff --git a/drivers/irqchip/irq-renesas-intc-irqpin.c b/drivers/irqchip/irq-renesas-intc-irqpin.c
> index 4aca1b2..82cec63 100644
> --- a/drivers/irqchip/irq-renesas-intc-irqpin.c
> +++ b/drivers/irqchip/irq-renesas-intc-irqpin.c
> @@ -348,11 +348,14 @@ static int intc_irqpin_probe(struct platform_device *pdev)
>  	}
>  
>  	/* deal with driver instance configuration */
> -	if (pdata)
> +	if (pdata) {
>  		memcpy(&p->config, pdata, sizeof(*pdata));
> -	else
> +	} else {
>  		of_property_read_u32(pdev->dev.of_node, "sense-bitfield-width",
>  				     &p->config.sense_bitfield_width);
> +		p->config.control_parent = of_property_read_bool(pdev->dev.of_node,
> +								 "control-parent");
> +	}
>  	if (!p->config.sense_bitfield_width)
>  		p->config.sense_bitfield_width = 4; /* default to 4 bits */
>  
> -- 
> 1.7.2.5
> 

Re: [PATCH 1/2] ARM: shmobile: irqpin: add a DT property to enable masking on parent

[Magnus Damm]

On Fri, May 24, 2013 at 6:13 PM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> To disable spurious interrupts, that get triggered on certain hardware, the
> irqpin driver masks them on the parent interrupt controller. To specify
> such broken devices a .control_parent parameter can be provided in the
> platform data. In the DT case we need a property, to do the same.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>
> These two patches simply split the earlier "ARM: shmobile: irqpin: fix
> handling of spurious interrupts in DT case" patch into two parts.
> Otherwise no change.

I'm fine with this portion of the patch series. Thanks for your help!

Acked-by: Magnus Damm <damm@opensource.se>

Re: [PATCH 1/2] ARM: shmobile: irqpin: add a DT property to enable masking on parent

[Simon Horman]

On Tue, Jun 11, 2013 at 06:37:24PM +0900, Magnus Damm wrote:
> On Fri, May 24, 2013 at 6:13 PM, Guennadi Liakhovetski
> <g.liakhovetski-Mmb7MZpHnFY@public.gmane.org> wrote:
> > To disable spurious interrupts, that get triggered on certain hardware, the
> > irqpin driver masks them on the parent interrupt controller. To specify
> > such broken devices a .control_parent parameter can be provided in the
> > platform data. In the DT case we need a property, to do the same.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski-Mmb7MZpHnFY@public.gmane.org>
> > ---
> >
> > These two patches simply split the earlier "ARM: shmobile: irqpin: fix
> > handling of spurious interrupts in DT case" patch into two parts.
> > Otherwise no change.
> 
> I'm fine with this portion of the patch series. Thanks for your help!
>
> Acked-by: Magnus Damm <damm-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>

I believe that this patch depends on
"irqchip: renesas-intc-irqpin: DT binding for sense bitfield width"
which Arnd has requested some modifications to.

"I think you should add documentation here about how the two interrupt
cells are to be interpreted, to allow people to fill the values from a data
sheet or board schematic."
http://www.spinics.net/lists/linux-sh/msg20202.html

Please post a fresh series that:

1. Includes a fresh version of "irqchip: renesas-intc-irqpin: DT binding
   for sense bitfield width" with Arnd's concerns addressed.

2. Includes this patch with Magnus's Ack.

Please be sure to CC Arnd on the new series.

Re: [PATCH 1/2] ARM: shmobile: irqpin: add a DT property to enable masking on parent

[Simon Horman]

On Wed, Jun 12, 2013 at 05:38:50PM +0900, Simon Horman wrote:
> On Tue, Jun 11, 2013 at 06:37:24PM +0900, Magnus Damm wrote:
> > On Fri, May 24, 2013 at 6:13 PM, Guennadi Liakhovetski
> > <g.liakhovetski@gmx.de> wrote:
> > > To disable spurious interrupts, that get triggered on certain hardware, the
> > > irqpin driver masks them on the parent interrupt controller. To specify
> > > such broken devices a .control_parent parameter can be provided in the
> > > platform data. In the DT case we need a property, to do the same.
> > >
> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > ---
> > >
> > > These two patches simply split the earlier "ARM: shmobile: irqpin: fix
> > > handling of spurious interrupts in DT case" patch into two parts.
> > > Otherwise no change.
> > 
> > I'm fine with this portion of the patch series. Thanks for your help!
> >
> > Acked-by: Magnus Damm <damm@opensource.se>
> 
> I believe that this patch depends on
> "irqchip: renesas-intc-irqpin: DT binding for sense bitfield width"
> which Arnd has requested some modifications to.
> 
> "I think you should add documentation here about how the two interrupt
> cells are to be interpreted, to allow people to fill the values from a data
> sheet or board schematic."
> http://www.spinics.net/lists/linux-sh/msg20202.html
> 
> Please post a fresh series that:
> 
> 1. Includes a fresh version of "irqchip: renesas-intc-irqpin: DT binding
>    for sense bitfield width" with Arnd's concerns addressed.
> 
> 2. Includes this patch with Magnus's Ack.
> 
> Please be sure to CC Arnd on the new series.

Hi Guennadi,

I see that you reposted "irqchip: renesas-intc-irqpin: DT binding
for sense bitfield width", which I have queued-up, without this patch.

Please rebase this patch and repost it with Magnus's Ack.