[PATCHv3 0/3] CPCAP PMIC IRQ fix and related changes

[PATCHv3 0/3] CPCAP PMIC IRQ fix and related changes

[Tony Lindgren]

Hi all,

Here's v3 set of fixes to make CPCAP PMIC interrupts work reliably when
used with multiple drivers. While working on the ADC, charger and
USB PHY drivers I noticed that the PMIC interrupt to the SoC would
eventually stop working.

All these can wait for v4.12 merge window as these issues don't show
up currently. I'll send the related interrupt triggering dts change
separately.

Regards,

Tony

Changes since v2:

- Replaced first two regmap_irq related patches with proper interrupt
  triggering configuration after noticing the dts configuration did
  not get passed in

- Dropped regmap from the patch series subject line

Changes since v1:

- Updated regap-irq patch to use out_runtime_put in regmap_irq_thread
  also if pm_runtime_get() fails

- Clarify patch description for regmap-irq changes to make clear
  this is an issue with the CPCAP PMIC and not SoC GPIO edge/level
  handling based on comments from Charles Keepax
  <ckeepax@opensource.wolfsonmicro.com>

- Collected acks

Tony Lindgren (3):
  mfd: cpcap: Fix interrupt to use level interrupt
  mfd: cpcap: Use ack_invert interrupts
  mfd: cpcap: Fix bad use of IRQ sense register

 drivers/mfd/motorola-cpcap.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

-- 
2.12.2

[PATCH 1/3] mfd: cpcap: Fix interrupt to use level interrupt

[Tony Lindgren]

I made a mistake assuming the device tree configuration for interrupt
triggering was somehow passed to the SPI device but it's not.

In the Motorola Linux kernel tree CPCAP PMIC is configured as a rising
edge triggered interrupt, but then then it's interrupt handler keeps
looping until the GPIO line goes down. So the CPCAP interrupt is clearly
a level interrupt and not an edge interrupt.

Earlier when I tried to configure it as level interrupt using the
device tree, I did not account that the triggering only gets passed
to the SPI core and it also needs to be specified in the CPCAP driver
when we do devm_regmap_add_irq_chip().

Fixes: 56e1d40d3bea ("mfd: cpcap: Add minimal support")
Cc: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Cc: Marcel Partap <mpartap@gmx.net>
Cc: Michael Scott <michael.scott@linaro.org>
Cc: Sebastian Reichel <sre@kernel.org>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/mfd/motorola-cpcap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
--- a/drivers/mfd/motorola-cpcap.c
+++ b/drivers/mfd/motorola-cpcap.c
@@ -126,7 +126,7 @@ static int cpcap_init_irq_chip(struct cpcap_ddata *cpcap, int irq_chip,
 
 	ret = devm_regmap_add_irq_chip(&cpcap->spi->dev, cpcap->regmap,
 				       cpcap->spi->irq,
-				       IRQF_TRIGGER_RISING |
+				       irq_get_trigger_type(cpcap->spi->irq) |
 				       IRQF_SHARED, -1,
 				       chip, &cpcap->irqdata[irq_chip]);
 	if (ret) {
-- 
2.12.2

[PATCH 2/3] mfd: cpcap: Use ack_invert interrupts

[Tony Lindgren]

We should use ack_invert as the int_read_and_clear() in the Motorola
kernel tree does "ireg_val & ~mreg_val" before writing to the mask
register.

Fixes: 56e1d40d3bea ("mfd: cpcap: Add minimal support")
Cc: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Cc: Marcel Partap <mpartap@gmx.net>
Cc: Michael Scott <michael.scott@linaro.org>
Tested-by: Sebastian Reichel <sre@kernel.org>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/mfd/motorola-cpcap.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
--- a/drivers/mfd/motorola-cpcap.c
+++ b/drivers/mfd/motorola-cpcap.c
@@ -71,6 +71,7 @@ static struct regmap_irq_chip cpcap_irq_chip[CPCAP_NR_IRQ_CHIPS] = {
 		.ack_base = CPCAP_REG_MI1,
 		.mask_base = CPCAP_REG_MIM1,
 		.use_ack = true,
+		.ack_invert = true,
 	},
 	{
 		.name = "cpcap-m2",
@@ -79,6 +80,7 @@ static struct regmap_irq_chip cpcap_irq_chip[CPCAP_NR_IRQ_CHIPS] = {
 		.ack_base = CPCAP_REG_MI2,
 		.mask_base = CPCAP_REG_MIM2,
 		.use_ack = true,
+		.ack_invert = true,
 	},
 	{
 		.name = "cpcap1-4",
@@ -88,6 +90,7 @@ static struct regmap_irq_chip cpcap_irq_chip[CPCAP_NR_IRQ_CHIPS] = {
 		.mask_base = CPCAP_REG_INTM1,
 		.type_base = CPCAP_REG_INTS1,
 		.use_ack = true,
+		.ack_invert = true,
 	},
 };
 
-- 
2.12.2

[PATCH 3/3] mfd: cpcap: Fix bad use of IRQ sense register

[Tony Lindgren]

The cpcap INTS registers are for getting the value of the line,
not for configuring the type.

Fixes: 56e1d40d3bea ("mfd: cpcap: Add minimal support")
Cc: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Cc: Marcel Partap <mpartap@gmx.net>
Cc: Michael Scott <michael.scott@linaro.org>
Cc: Sebastian Reichel <sre@kernel.org>
Reviewed-By: Sebastian Reichel <sre@kernel.org>
Tested-by: Sebastian Reichel <sre@kernel.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/mfd/motorola-cpcap.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
--- a/drivers/mfd/motorola-cpcap.c
+++ b/drivers/mfd/motorola-cpcap.c
@@ -88,7 +88,6 @@ static struct regmap_irq_chip cpcap_irq_chip[CPCAP_NR_IRQ_CHIPS] = {
 		.status_base = CPCAP_REG_INT1,
 		.ack_base = CPCAP_REG_INT1,
 		.mask_base = CPCAP_REG_INTM1,
-		.type_base = CPCAP_REG_INTS1,
 		.use_ack = true,
 		.ack_invert = true,
 	},
-- 
2.12.2

Re: [PATCH 1/3] mfd: cpcap: Fix interrupt to use level interrupt

[Charles Keepax]

On Mon, Apr 03, 2017 at 08:15:54PM -0700, Tony Lindgren wrote:
> I made a mistake assuming the device tree configuration for interrupt
> triggering was somehow passed to the SPI device but it's not.
> 
> In the Motorola Linux kernel tree CPCAP PMIC is configured as a rising
> edge triggered interrupt, but then then it's interrupt handler keeps
> looping until the GPIO line goes down. So the CPCAP interrupt is clearly
> a level interrupt and not an edge interrupt.
> 
> Earlier when I tried to configure it as level interrupt using the
> device tree, I did not account that the triggering only gets passed
> to the SPI core and it also needs to be specified in the CPCAP driver
> when we do devm_regmap_add_irq_chip().
> 
> Fixes: 56e1d40d3bea ("mfd: cpcap: Add minimal support")
> Cc: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> Cc: Marcel Partap <mpartap@gmx.net>
> Cc: Michael Scott <michael.scott@linaro.org>
> Cc: Sebastian Reichel <sre@kernel.org>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---

Thanks for looking into that further, I thought that might be
what was going on.

Reviewed-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>

Thanks,
Charles

Re: [PATCH 1/3] mfd: cpcap: Fix interrupt to use level interrupt

[Lee Jones]

On Mon, 03 Apr 2017, Tony Lindgren wrote:

> I made a mistake assuming the device tree configuration for interrupt
> triggering was somehow passed to the SPI device but it's not.
> 
> In the Motorola Linux kernel tree CPCAP PMIC is configured as a rising
> edge triggered interrupt, but then then it's interrupt handler keeps
> looping until the GPIO line goes down. So the CPCAP interrupt is clearly
> a level interrupt and not an edge interrupt.
> 
> Earlier when I tried to configure it as level interrupt using the
> device tree, I did not account that the triggering only gets passed
> to the SPI core and it also needs to be specified in the CPCAP driver
> when we do devm_regmap_add_irq_chip().
> 
> Fixes: 56e1d40d3bea ("mfd: cpcap: Add minimal support")
> Cc: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> Cc: Marcel Partap <mpartap@gmx.net>
> Cc: Michael Scott <michael.scott@linaro.org>
> Cc: Sebastian Reichel <sre@kernel.org>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>  drivers/mfd/motorola-cpcap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

> diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
> --- a/drivers/mfd/motorola-cpcap.c
> +++ b/drivers/mfd/motorola-cpcap.c
> @@ -126,7 +126,7 @@ static int cpcap_init_irq_chip(struct cpcap_ddata *cpcap, int irq_chip,
>  
>  	ret = devm_regmap_add_irq_chip(&cpcap->spi->dev, cpcap->regmap,
>  				       cpcap->spi->irq,
> -				       IRQF_TRIGGER_RISING |
> +				       irq_get_trigger_type(cpcap->spi->irq) |
>  				       IRQF_SHARED, -1,
>  				       chip, &cpcap->irqdata[irq_chip]);
>  	if (ret) {

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 2/3] mfd: cpcap: Use ack_invert interrupts

[Lee Jones]

On Mon, 03 Apr 2017, Tony Lindgren wrote:

> We should use ack_invert as the int_read_and_clear() in the Motorola
> kernel tree does "ireg_val & ~mreg_val" before writing to the mask
> register.
> 
> Fixes: 56e1d40d3bea ("mfd: cpcap: Add minimal support")
> Cc: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> Cc: Marcel Partap <mpartap@gmx.net>
> Cc: Michael Scott <michael.scott@linaro.org>
> Tested-by: Sebastian Reichel <sre@kernel.org>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>  drivers/mfd/motorola-cpcap.c | 3 +++
>  1 file changed, 3 insertions(+)

Applied, thanks.

> diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
> --- a/drivers/mfd/motorola-cpcap.c
> +++ b/drivers/mfd/motorola-cpcap.c
> @@ -71,6 +71,7 @@ static struct regmap_irq_chip cpcap_irq_chip[CPCAP_NR_IRQ_CHIPS] = {
>  		.ack_base = CPCAP_REG_MI1,
>  		.mask_base = CPCAP_REG_MIM1,
>  		.use_ack = true,
> +		.ack_invert = true,
>  	},
>  	{
>  		.name = "cpcap-m2",
> @@ -79,6 +80,7 @@ static struct regmap_irq_chip cpcap_irq_chip[CPCAP_NR_IRQ_CHIPS] = {
>  		.ack_base = CPCAP_REG_MI2,
>  		.mask_base = CPCAP_REG_MIM2,
>  		.use_ack = true,
> +		.ack_invert = true,
>  	},
>  	{
>  		.name = "cpcap1-4",
> @@ -88,6 +90,7 @@ static struct regmap_irq_chip cpcap_irq_chip[CPCAP_NR_IRQ_CHIPS] = {
>  		.mask_base = CPCAP_REG_INTM1,
>  		.type_base = CPCAP_REG_INTS1,
>  		.use_ack = true,
> +		.ack_invert = true,
>  	},
>  };
>  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 3/3] mfd: cpcap: Fix bad use of IRQ sense register

[Lee Jones]

On Mon, 03 Apr 2017, Tony Lindgren wrote:

> The cpcap INTS registers are for getting the value of the line,
> not for configuring the type.
> 
> Fixes: 56e1d40d3bea ("mfd: cpcap: Add minimal support")
> Cc: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> Cc: Marcel Partap <mpartap@gmx.net>
> Cc: Michael Scott <michael.scott@linaro.org>
> Cc: Sebastian Reichel <sre@kernel.org>
> Reviewed-By: Sebastian Reichel <sre@kernel.org>
> Tested-by: Sebastian Reichel <sre@kernel.org>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>  drivers/mfd/motorola-cpcap.c | 1 -
>  1 file changed, 1 deletion(-)

Applied, thanks.

> diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
> --- a/drivers/mfd/motorola-cpcap.c
> +++ b/drivers/mfd/motorola-cpcap.c
> @@ -88,7 +88,6 @@ static struct regmap_irq_chip cpcap_irq_chip[CPCAP_NR_IRQ_CHIPS] = {
>  		.status_base = CPCAP_REG_INT1,
>  		.ack_base = CPCAP_REG_INT1,
>  		.mask_base = CPCAP_REG_INTM1,
> -		.type_base = CPCAP_REG_INTS1,
>  		.use_ack = true,
>  		.ack_invert = true,
>  	},

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog