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
<[email protected]>
- 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
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 <[email protected]>
Cc: Marcel Partap <[email protected]>
Cc: Michael Scott <[email protected]>
Cc: Sebastian Reichel <[email protected]>
Signed-off-by: Tony Lindgren <[email protected]>
---
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
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 <[email protected]>
Cc: Marcel Partap <[email protected]>
Cc: Michael Scott <[email protected]>
Cc: Sebastian Reichel <[email protected]>
Reviewed-By: Sebastian Reichel <[email protected]>
Tested-by: Sebastian Reichel <[email protected]>
Acked-by: Lee Jones <[email protected]>
Signed-off-by: Tony Lindgren <[email protected]>
---
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
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 <[email protected]>
Cc: Marcel Partap <[email protected]>
Cc: Michael Scott <[email protected]>
Tested-by: Sebastian Reichel <[email protected]>
Signed-off-by: Tony Lindgren <[email protected]>
---
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
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 <[email protected]>
> Cc: Marcel Partap <[email protected]>
> Cc: Michael Scott <[email protected]>
> Cc: Sebastian Reichel <[email protected]>
> Signed-off-by: Tony Lindgren <[email protected]>
> ---
Thanks for looking into that further, I thought that might be
what was going on.
Reviewed-by: Charles Keepax <[email protected]>
Thanks,
Charles
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 <[email protected]>
> Cc: Marcel Partap <[email protected]>
> Cc: Michael Scott <[email protected]>
> Cc: Sebastian Reichel <[email protected]>
> Signed-off-by: Tony Lindgren <[email protected]>
> ---
> 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
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 <[email protected]>
> Cc: Marcel Partap <[email protected]>
> Cc: Michael Scott <[email protected]>
> Tested-by: Sebastian Reichel <[email protected]>
> Signed-off-by: Tony Lindgren <[email protected]>
> ---
> 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
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 <[email protected]>
> Cc: Marcel Partap <[email protected]>
> Cc: Michael Scott <[email protected]>
> Cc: Sebastian Reichel <[email protected]>
> Reviewed-By: Sebastian Reichel <[email protected]>
> Tested-by: Sebastian Reichel <[email protected]>
> Acked-by: Lee Jones <[email protected]>
> Signed-off-by: Tony Lindgren <[email protected]>
> ---
> 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