[PATCH v1 1/7] mfd: intel_soc_pmic_bxtwc: fix TMU interrupt index

[PATCH v1 1/7] mfd: intel_soc_pmic_bxtwc: fix TMU interrupt index

[sathyanarayanan.kuppuswamy]

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

TMU interrupts are registered as a separate interrupt chip, and
hence it should start its interrupt index(BXTWC_TMU_IRQ) number
from 0. But currently, BXTWC_TMU_IRQ is defined as part of enum
bxtwc_irqs_level2 and its index value is 11. Since this index
value is used when calculating .num_irqs of regmap_irq_chip_tmu,
it incorrectly reports number of irqs as 12 instead of actual
value of 1.

static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = {
	REGMAP_IRQ_REG(BXTWC_TMU_IRQ, 0, 0x06),
};

static struct regmap_irq_chip bxtwc_regmap_irq_chip_tmu = {
	.name = "bxtwc_irq_chip_tmu",
	.status_base = BXTWC_TMUIRQ,
	.mask_base = BXTWC_MTMUIRQ,
	.irqs = bxtwc_regmap_irqs_tmu,
	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_tmu),
	.num_regs = 1,
};

This patch fixes this issue by creating new enum of tmu irqs and
resetting its starting index to 0.

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 drivers/mfd/intel_soc_pmic_bxtwc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
index 699c8c7..bb18e20 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -94,7 +94,10 @@ enum bxtwc_irqs_level2 {
 	BXTWC_GPIO0_IRQ,
 	BXTWC_GPIO1_IRQ,
 	BXTWC_CRIT_IRQ,
-	BXTWC_TMU_IRQ,
+};
+
+enum bxtwc_irqs_tmu {
+	BXTWC_TMU_IRQ = 0,
 };
 
 static const struct regmap_irq bxtwc_regmap_irqs[] = {
-- 
2.7.4

[PATCH v1 1/7] mfd: intel_soc_pmic_bxtwc: fix TMU interrupt index

[sathyanarayanan.kuppuswamy]

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

TMU interrupts are registered as a separate interrupt chip, and
hence it should start its interrupt index(BXTWC_TMU_IRQ) number
from 0. But currently, BXTWC_TMU_IRQ is defined as part of enum
bxtwc_irqs_level2 and its index value is 11. Since this index
value is used when calculating .num_irqs of regmap_irq_chip_tmu,
it incorrectly reports number of irqs as 12 instead of actual
value of 1.

static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = {
	REGMAP_IRQ_REG(BXTWC_TMU_IRQ, 0, 0x06),
};

static struct regmap_irq_chip bxtwc_regmap_irq_chip_tmu = {
	.name = "bxtwc_irq_chip_tmu",
	.status_base = BXTWC_TMUIRQ,
	.mask_base = BXTWC_MTMUIRQ,
	.irqs = bxtwc_regmap_irqs_tmu,
	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_tmu),
	.num_regs = 1,
};

This patch fixes this issue by creating new enum of tmu irqs and
resetting its starting index to 0.

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 drivers/mfd/intel_soc_pmic_bxtwc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
index 699c8c7..bb18e20 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -94,7 +94,10 @@ enum bxtwc_irqs_level2 {
 	BXTWC_GPIO0_IRQ,
 	BXTWC_GPIO1_IRQ,
 	BXTWC_CRIT_IRQ,
-	BXTWC_TMU_IRQ,
+};
+
+enum bxtwc_irqs_tmu {
+	BXTWC_TMU_IRQ = 0,
 };
 
 static const struct regmap_irq bxtwc_regmap_irqs[] = {
-- 
2.7.4

[PATCH v1 2/7] mfd: intel_soc_pmic_bxtwc: remove thermal second level irqs

[sathyanarayanan.kuppuswamy]

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

Since all second level thermal irqs are consumed by the same
device(bxt_wcove_thermal), there is no need to expose them as separate
interrupts. We can just export only the first level irqs for thermal and
let the device(bxt_wcove_thermal) driver handle the second level irqs
based on thermal interrupt status register. Also, just using only the
first level irq will eliminate the bug involved in requesting only the
second level irq and not explicitly enable the first level irq. For
more info on this issue please read the details at,

https://lkml.org/lkml/2017/2/27/148

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 drivers/mfd/intel_soc_pmic_bxtwc.c | 32 ++++++++++++--------------------
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
index bb18e20..c08d514 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -83,10 +83,7 @@ enum bxtwc_irqs {
 
 enum bxtwc_irqs_level2 {
 	/* Level 2 */
-	BXTWC_THRM0_IRQ = 0,
-	BXTWC_THRM1_IRQ,
-	BXTWC_THRM2_IRQ,
-	BXTWC_BCU_IRQ,
+	BXTWC_BCU_IRQ = 0,
 	BXTWC_ADC_IRQ,
 	BXTWC_USBC_IRQ,
 	BXTWC_CHGR0_IRQ,
@@ -113,17 +110,14 @@ static const struct regmap_irq bxtwc_regmap_irqs[] = {
 };
 
 static const struct regmap_irq bxtwc_regmap_irqs_level2[] = {
-	REGMAP_IRQ_REG(BXTWC_THRM0_IRQ, 0, 0xff),
-	REGMAP_IRQ_REG(BXTWC_THRM1_IRQ, 1, 0xbf),
-	REGMAP_IRQ_REG(BXTWC_THRM2_IRQ, 2, 0xff),
-	REGMAP_IRQ_REG(BXTWC_BCU_IRQ, 3, 0x1f),
-	REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 4, 0xff),
-	REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 5, BIT(5)),
-	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 5, 0x1f),
-	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 6, 0x1f),
-	REGMAP_IRQ_REG(BXTWC_GPIO0_IRQ, 7, 0xff),
-	REGMAP_IRQ_REG(BXTWC_GPIO1_IRQ, 8, 0x3f),
-	REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 9, 0x03),
+	REGMAP_IRQ_REG(BXTWC_BCU_IRQ, 0, 0x1f),
+	REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 1, 0xff),
+	REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 2, BIT(5)),
+	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 2, 0x1f),
+	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 3, 0x1f),
+	REGMAP_IRQ_REG(BXTWC_GPIO0_IRQ, 4, 0xff),
+	REGMAP_IRQ_REG(BXTWC_GPIO1_IRQ, 5, 0x3f),
+	REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 6, 0x03),
 };
 
 static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = {
@@ -141,8 +135,8 @@ static struct regmap_irq_chip bxtwc_regmap_irq_chip = {
 
 static struct regmap_irq_chip bxtwc_regmap_irq_chip_level2 = {
 	.name = "bxtwc_irq_chip_level2",
-	.status_base = BXTWC_THRM0IRQ,
-	.mask_base = BXTWC_MTHRM0IRQ,
+	.status_base = BXTWC_BCUIRQ,
+	.mask_base = BXTWC_MBCUIRQ,
 	.irqs = bxtwc_regmap_irqs_level2,
 	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_level2),
 	.num_regs = 10,
@@ -176,9 +170,7 @@ static struct resource charger_resources[] = {
 };
 
 static struct resource thermal_resources[] = {
-	DEFINE_RES_IRQ(BXTWC_THRM0_IRQ),
-	DEFINE_RES_IRQ(BXTWC_THRM1_IRQ),
-	DEFINE_RES_IRQ(BXTWC_THRM2_IRQ),
+	DEFINE_RES_IRQ(BXTWC_THRM_LVL1_IRQ),
 };
 
 static struct resource bcu_resources[] = {
-- 
2.7.4

[PATCH v1 2/7] mfd: intel_soc_pmic_bxtwc: remove thermal second level irqs

[sathyanarayanan.kuppuswamy]

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

Since all second level thermal irqs are consumed by the same
device(bxt_wcove_thermal), there is no need to expose them as separate
interrupts. We can just export only the first level irqs for thermal and
let the device(bxt_wcove_thermal) driver handle the second level irqs
based on thermal interrupt status register. Also, just using only the
first level irq will eliminate the bug involved in requesting only the
second level irq and not explicitly enable the first level irq. For
more info on this issue please read the details at,

https://lkml.org/lkml/2017/2/27/148

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 drivers/mfd/intel_soc_pmic_bxtwc.c | 32 ++++++++++++--------------------
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
index bb18e20..c08d514 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -83,10 +83,7 @@ enum bxtwc_irqs {
 
 enum bxtwc_irqs_level2 {
 	/* Level 2 */
-	BXTWC_THRM0_IRQ = 0,
-	BXTWC_THRM1_IRQ,
-	BXTWC_THRM2_IRQ,
-	BXTWC_BCU_IRQ,
+	BXTWC_BCU_IRQ = 0,
 	BXTWC_ADC_IRQ,
 	BXTWC_USBC_IRQ,
 	BXTWC_CHGR0_IRQ,
@@ -113,17 +110,14 @@ static const struct regmap_irq bxtwc_regmap_irqs[] = {
 };
 
 static const struct regmap_irq bxtwc_regmap_irqs_level2[] = {
-	REGMAP_IRQ_REG(BXTWC_THRM0_IRQ, 0, 0xff),
-	REGMAP_IRQ_REG(BXTWC_THRM1_IRQ, 1, 0xbf),
-	REGMAP_IRQ_REG(BXTWC_THRM2_IRQ, 2, 0xff),
-	REGMAP_IRQ_REG(BXTWC_BCU_IRQ, 3, 0x1f),
-	REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 4, 0xff),
-	REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 5, BIT(5)),
-	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 5, 0x1f),
-	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 6, 0x1f),
-	REGMAP_IRQ_REG(BXTWC_GPIO0_IRQ, 7, 0xff),
-	REGMAP_IRQ_REG(BXTWC_GPIO1_IRQ, 8, 0x3f),
-	REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 9, 0x03),
+	REGMAP_IRQ_REG(BXTWC_BCU_IRQ, 0, 0x1f),
+	REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 1, 0xff),
+	REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 2, BIT(5)),
+	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 2, 0x1f),
+	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 3, 0x1f),
+	REGMAP_IRQ_REG(BXTWC_GPIO0_IRQ, 4, 0xff),
+	REGMAP_IRQ_REG(BXTWC_GPIO1_IRQ, 5, 0x3f),
+	REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 6, 0x03),
 };
 
 static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = {
@@ -141,8 +135,8 @@ static struct regmap_irq_chip bxtwc_regmap_irq_chip = {
 
 static struct regmap_irq_chip bxtwc_regmap_irq_chip_level2 = {
 	.name = "bxtwc_irq_chip_level2",
-	.status_base = BXTWC_THRM0IRQ,
-	.mask_base = BXTWC_MTHRM0IRQ,
+	.status_base = BXTWC_BCUIRQ,
+	.mask_base = BXTWC_MBCUIRQ,
 	.irqs = bxtwc_regmap_irqs_level2,
 	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_level2),
 	.num_regs = 10,
@@ -176,9 +170,7 @@ static struct resource charger_resources[] = {
 };
 
 static struct resource thermal_resources[] = {
-	DEFINE_RES_IRQ(BXTWC_THRM0_IRQ),
-	DEFINE_RES_IRQ(BXTWC_THRM1_IRQ),
-	DEFINE_RES_IRQ(BXTWC_THRM2_IRQ),
+	DEFINE_RES_IRQ(BXTWC_THRM_LVL1_IRQ),
 };
 
 static struct resource bcu_resources[] = {
-- 
2.7.4

[PATCH v1 3/7] thermal: intel_bxt_pmic_thermal: use first level PMIC thermal irq

[sathyanarayanan.kuppuswamy]

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

PMIC mfd driver only exports first level irq for thermal device.
But currently we are reading the irqs from the second level irq
chip, So this patch fixes this issue by adding support to use
first level PMIC thermal irq.

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 drivers/thermal/intel_bxt_pmic_thermal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/intel_bxt_pmic_thermal.c b/drivers/thermal/intel_bxt_pmic_thermal.c
index 0f19a39..ef6b322 100644
--- a/drivers/thermal/intel_bxt_pmic_thermal.c
+++ b/drivers/thermal/intel_bxt_pmic_thermal.c
@@ -241,7 +241,7 @@ static int pmic_thermal_probe(struct platform_device *pdev)
 	}
 
 	regmap = pmic->regmap;
-	regmap_irq_chip = pmic->irq_chip_data_level2;
+	regmap_irq_chip = pmic->irq_chip_data;
 
 	pmic_irq_count = 0;
 	while ((irq = platform_get_irq(pdev, pmic_irq_count)) != -ENXIO) {
-- 
2.7.4

[PATCH v1 3/7] thermal: intel_bxt_pmic_thermal: use first level PMIC thermal irq

[sathyanarayanan.kuppuswamy]

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

PMIC mfd driver only exports first level irq for thermal device.
But currently we are reading the irqs from the second level irq
chip, So this patch fixes this issue by adding support to use
first level PMIC thermal irq.

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 drivers/thermal/intel_bxt_pmic_thermal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/intel_bxt_pmic_thermal.c b/drivers/thermal/intel_bxt_pmic_thermal.c
index 0f19a39..ef6b322 100644
--- a/drivers/thermal/intel_bxt_pmic_thermal.c
+++ b/drivers/thermal/intel_bxt_pmic_thermal.c
@@ -241,7 +241,7 @@ static int pmic_thermal_probe(struct platform_device *pdev)
 	}
 
 	regmap = pmic->regmap;
-	regmap_irq_chip = pmic->irq_chip_data_level2;
+	regmap_irq_chip = pmic->irq_chip_data;
 
 	pmic_irq_count = 0;
 	while ((irq = platform_get_irq(pdev, pmic_irq_count)) != -ENXIO) {
-- 
2.7.4

[PATCH v1 4/7] mfd: intel_soc_pmic_bxtwc: remove second level irq for gpio device

[sathyanarayanan.kuppuswamy]

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

Currently all PMIC GPIO domain irqs are consumed by the same
device(bxt_wcove_gpio), so there is no need to export them as
separate interrupts. We can just export only the first level
GPIO irq(BXTWC_GPIO_LVL1_IRQ) as an irq resource and let the
GPIO device driver(bxt_wcove_gpio) handle the GPIO sub domain
irqs based on status value of GPIO level2 interrupt status
register. Also, just using only the first level irq will eliminate
the bug involved in requesting only the second level irq and not
explicitly enable the first level irq. For more info on this
issue please read the details at,

https://lkml.org/lkml/2017/2/27/148

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 drivers/mfd/intel_soc_pmic_bxtwc.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
index c08d514..dc8af1d 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -88,8 +88,6 @@ enum bxtwc_irqs_level2 {
 	BXTWC_USBC_IRQ,
 	BXTWC_CHGR0_IRQ,
 	BXTWC_CHGR1_IRQ,
-	BXTWC_GPIO0_IRQ,
-	BXTWC_GPIO1_IRQ,
 	BXTWC_CRIT_IRQ,
 };
 
@@ -115,8 +113,6 @@ static const struct regmap_irq bxtwc_regmap_irqs_level2[] = {
 	REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 2, BIT(5)),
 	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 2, 0x1f),
 	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 3, 0x1f),
-	REGMAP_IRQ_REG(BXTWC_GPIO0_IRQ, 4, 0xff),
-	REGMAP_IRQ_REG(BXTWC_GPIO1_IRQ, 5, 0x3f),
 	REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 6, 0x03),
 };
 
@@ -152,8 +148,7 @@ static struct regmap_irq_chip bxtwc_regmap_irq_chip_tmu = {
 };
 
 static struct resource gpio_resources[] = {
-	DEFINE_RES_IRQ_NAMED(BXTWC_GPIO0_IRQ, "GPIO0"),
-	DEFINE_RES_IRQ_NAMED(BXTWC_GPIO1_IRQ, "GPIO1"),
+	DEFINE_RES_IRQ_NAMED(BXTWC_GPIO_LVL1_IRQ, "GPIO"),
 };
 
 static struct resource adc_resources[] = {
-- 
2.7.4

[PATCH v1 4/7] mfd: intel_soc_pmic_bxtwc: remove second level irq for gpio device

[sathyanarayanan.kuppuswamy]

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

Currently all PMIC GPIO domain irqs are consumed by the same
device(bxt_wcove_gpio), so there is no need to export them as
separate interrupts. We can just export only the first level
GPIO irq(BXTWC_GPIO_LVL1_IRQ) as an irq resource and let the
GPIO device driver(bxt_wcove_gpio) handle the GPIO sub domain
irqs based on status value of GPIO level2 interrupt status
register. Also, just using only the first level irq will eliminate
the bug involved in requesting only the second level irq and not
explicitly enable the first level irq. For more info on this
issue please read the details at,

https://lkml.org/lkml/2017/2/27/148

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 drivers/mfd/intel_soc_pmic_bxtwc.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
index c08d514..dc8af1d 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -88,8 +88,6 @@ enum bxtwc_irqs_level2 {
 	BXTWC_USBC_IRQ,
 	BXTWC_CHGR0_IRQ,
 	BXTWC_CHGR1_IRQ,
-	BXTWC_GPIO0_IRQ,
-	BXTWC_GPIO1_IRQ,
 	BXTWC_CRIT_IRQ,
 };
 
@@ -115,8 +113,6 @@ static const struct regmap_irq bxtwc_regmap_irqs_level2[] = {
 	REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 2, BIT(5)),
 	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 2, 0x1f),
 	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 3, 0x1f),
-	REGMAP_IRQ_REG(BXTWC_GPIO0_IRQ, 4, 0xff),
-	REGMAP_IRQ_REG(BXTWC_GPIO1_IRQ, 5, 0x3f),
 	REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 6, 0x03),
 };
 
@@ -152,8 +148,7 @@ static struct regmap_irq_chip bxtwc_regmap_irq_chip_tmu = {
 };
 
 static struct resource gpio_resources[] = {
-	DEFINE_RES_IRQ_NAMED(BXTWC_GPIO0_IRQ, "GPIO0"),
-	DEFINE_RES_IRQ_NAMED(BXTWC_GPIO1_IRQ, "GPIO1"),
+	DEFINE_RES_IRQ_NAMED(BXTWC_GPIO_LVL1_IRQ, "GPIO"),
 };
 
 static struct resource adc_resources[] = {
-- 
2.7.4

[PATCH v1 5/7] gpio: gpio-wcove: use first level PMIC GPIO irq

[sathyanarayanan.kuppuswamy]

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

PMIC mfd driver only exports first level irq for GPIO device.
But currently we are reading the irqs from the second level irq
chip, So this patch fixes this issue by adding support to use
first level PMIC GPIO irq.

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 drivers/gpio/gpio-wcove.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-wcove.c b/drivers/gpio/gpio-wcove.c
index 97613de..0117106 100644
--- a/drivers/gpio/gpio-wcove.c
+++ b/drivers/gpio/gpio-wcove.c
@@ -399,7 +399,7 @@ static int wcove_gpio_probe(struct platform_device *pdev)
 	if (!wg)
 		return -ENOMEM;
 
-	wg->regmap_irq_chip = pmic->irq_chip_data_level2;
+	wg->regmap_irq_chip = pmic->irq_chip_data;
 
 	platform_set_drvdata(pdev, wg);
 
@@ -447,6 +447,16 @@ static int wcove_gpio_probe(struct platform_device *pdev)
 
 	gpiochip_set_nested_irqchip(&wg->chip, &wcove_irqchip, virq);
 
+	/* Enable GPIO0 interrupts */
+	ret = regmap_update_bits(wg->regmap, IRQ_MASK_BASE, 0xff, 0x00);
+	if (ret)
+		return ret;
+
+	/* Enable GPIO1 interrupts */
+	ret = regmap_update_bits(wg->regmap, IRQ_MASK_BASE + 1, 0xff, 0x00);
+	if (ret)
+		return ret;
+
 	return 0;
 }
 
-- 
2.7.4

[PATCH v1 5/7] gpio: gpio-wcove: use first level PMIC GPIO irq

[sathyanarayanan.kuppuswamy]

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

PMIC mfd driver only exports first level irq for GPIO device.
But currently we are reading the irqs from the second level irq
chip, So this patch fixes this issue by adding support to use
first level PMIC GPIO irq.

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 drivers/gpio/gpio-wcove.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-wcove.c b/drivers/gpio/gpio-wcove.c
index 97613de..0117106 100644
--- a/drivers/gpio/gpio-wcove.c
+++ b/drivers/gpio/gpio-wcove.c
@@ -399,7 +399,7 @@ static int wcove_gpio_probe(struct platform_device *pdev)
 	if (!wg)
 		return -ENOMEM;
 
-	wg->regmap_irq_chip = pmic->irq_chip_data_level2;
+	wg->regmap_irq_chip = pmic->irq_chip_data;
 
 	platform_set_drvdata(pdev, wg);
 
@@ -447,6 +447,16 @@ static int wcove_gpio_probe(struct platform_device *pdev)
 
 	gpiochip_set_nested_irqchip(&wg->chip, &wcove_irqchip, virq);
 
+	/* Enable GPIO0 interrupts */
+	ret = regmap_update_bits(wg->regmap, IRQ_MASK_BASE, 0xff, 0x00);
+	if (ret)
+		return ret;
+
+	/* Enable GPIO1 interrupts */
+	ret = regmap_update_bits(wg->regmap, IRQ_MASK_BASE + 1, 0xff, 0x00);
+	if (ret)
+		return ret;
+
 	return 0;
 }
 
-- 
2.7.4

[PATCH v1 6/7] mfd: intel_soc_pmic_bxtwc: use chained irqs for second level irq chips

[sathyanarayanan.kuppuswamy]

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

Whishkey cove PMIC has support to mask/unmask interrupts at two levels.
At first level we can mask/unmask interrupt domains like TMU, GPIO, ADC,
CHGR, BCU THERMAL and PWRBTN and at second level, it provides facility
to mask/unmask individual interrupts belong each of this domain. For
example, in case of TMU, at first level we have TMU interrupt domain,
and at second level we have two interrupts, wake alarm, system alarm that
belong to the TMU interrupt domain.

Currently, in this driver all first level irqs are registered as part of
irq chip(bxtwc_regmap_irq_chip). By default, after you register the irq
chip from your driver, all irqs in that chip will masked and can only be
enabled if that irq is requested using request_irq call. This is the
default Linux irq behavior model. And whenever a dependent device that
belongs to PMIC requests only the second level irq and not explicitly
unmask the first level irq, then in essence the second level irq will
still be disabled. For example, if TMU device driver request wake_alarm
irq and not explicitly unmask TMU level 1 irq then according to the default
Linux irq model,  wake_alarm irq will still be disabled. So the proper
solution to fix this issue is to use the chained irq chip concept. We
should chain all the second level chip irqs to the corresponding first
level irq. To do this, we need to create separate irq chips for every
group of second level irqs.

In case of TMU, when adding second level irq chip, instead of using pmic
irq we should use the corresponding first level irq. So the following
code will change from

ret = regmap_add_irq_chip(pmic->regmap, pmic->irq, ...)

to,

virq = regmap_irq_get_virq(&pmic->irq_chip_data, BXTWC_TMU_LVL1_IRQ);

ret = regmap_add_irq_chip(pmic->regmap, virq, ...)

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 drivers/mfd/intel_soc_pmic_bxtwc.c | 212 ++++++++++++++++++++++++++++++-------
 include/linux/mfd/intel_soc_pmic.h |   5 +
 2 files changed, 176 insertions(+), 41 deletions(-)

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
index dc8af1d..807eba3 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -81,18 +81,26 @@ enum bxtwc_irqs {
 	BXTWC_PWRBTN_IRQ,
 };
 
-enum bxtwc_irqs_level2 {
-	/* Level 2 */
+enum bxtwc_irqs_tmu {
+	BXTWC_TMU_IRQ = 0,
+};
+
+enum bxtwc_irqs_bcu {
 	BXTWC_BCU_IRQ = 0,
-	BXTWC_ADC_IRQ,
-	BXTWC_USBC_IRQ,
+};
+
+enum bxtwc_irqs_adc {
+	BXTWC_ADC_IRQ = 0,
+};
+
+enum bxtwc_irqs_chgr {
+	BXTWC_USBC_IRQ = 0,
 	BXTWC_CHGR0_IRQ,
 	BXTWC_CHGR1_IRQ,
-	BXTWC_CRIT_IRQ,
 };
 
-enum bxtwc_irqs_tmu {
-	BXTWC_TMU_IRQ = 0,
+enum bxtwc_irqs_crit {
+	BXTWC_CRIT_IRQ = 0,
 };
 
 static const struct regmap_irq bxtwc_regmap_irqs[] = {
@@ -107,17 +115,26 @@ static const struct regmap_irq bxtwc_regmap_irqs[] = {
 	REGMAP_IRQ_REG(BXTWC_PWRBTN_IRQ, 1, 0x03),
 };
 
-static const struct regmap_irq bxtwc_regmap_irqs_level2[] = {
+static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = {
+	REGMAP_IRQ_REG(BXTWC_TMU_IRQ, 0, 0x06),
+};
+
+static const struct regmap_irq bxtwc_regmap_irqs_bcu[] = {
 	REGMAP_IRQ_REG(BXTWC_BCU_IRQ, 0, 0x1f),
-	REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 1, 0xff),
-	REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 2, BIT(5)),
-	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 2, 0x1f),
-	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 3, 0x1f),
-	REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 6, 0x03),
 };
 
-static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = {
-	REGMAP_IRQ_REG(BXTWC_TMU_IRQ, 0, 0x06),
+static const struct regmap_irq bxtwc_regmap_irqs_adc[] = {
+	REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 0, 0xff),
+};
+
+static const struct regmap_irq bxtwc_regmap_irqs_chgr[] = {
+	REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 0, BIT(5)),
+	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 0, 0x1f),
+	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 1, 0x1f),
+};
+
+static const struct regmap_irq bxtwc_regmap_irqs_crit[] = {
+	REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 0, 0x03),
 };
 
 static struct regmap_irq_chip bxtwc_regmap_irq_chip = {
@@ -129,15 +146,6 @@ static struct regmap_irq_chip bxtwc_regmap_irq_chip = {
 	.num_regs = 2,
 };
 
-static struct regmap_irq_chip bxtwc_regmap_irq_chip_level2 = {
-	.name = "bxtwc_irq_chip_level2",
-	.status_base = BXTWC_BCUIRQ,
-	.mask_base = BXTWC_MBCUIRQ,
-	.irqs = bxtwc_regmap_irqs_level2,
-	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_level2),
-	.num_regs = 10,
-};
-
 static struct regmap_irq_chip bxtwc_regmap_irq_chip_tmu = {
 	.name = "bxtwc_irq_chip_tmu",
 	.status_base = BXTWC_TMUIRQ,
@@ -147,6 +155,42 @@ static struct regmap_irq_chip bxtwc_regmap_irq_chip_tmu = {
 	.num_regs = 1,
 };
 
+static struct regmap_irq_chip bxtwc_regmap_irq_chip_bcu = {
+	.name = "bxtwc_irq_chip_bcu",
+	.status_base = BXTWC_BCUIRQ,
+	.mask_base = BXTWC_MBCUIRQ,
+	.irqs = bxtwc_regmap_irqs_bcu,
+	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_bcu),
+	.num_regs = 1,
+};
+
+static struct regmap_irq_chip bxtwc_regmap_irq_chip_adc = {
+	.name = "bxtwc_irq_chip_adc",
+	.status_base = BXTWC_ADCIRQ,
+	.mask_base = BXTWC_MADCIRQ,
+	.irqs = bxtwc_regmap_irqs_adc,
+	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_adc),
+	.num_regs = 1,
+};
+
+static struct regmap_irq_chip bxtwc_regmap_irq_chip_chgr = {
+	.name = "bxtwc_irq_chip_chgr",
+	.status_base = BXTWC_CHGR0IRQ,
+	.mask_base = BXTWC_MCHGR0IRQ,
+	.irqs = bxtwc_regmap_irqs_chgr,
+	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_chgr),
+	.num_regs = 2,
+};
+
+static struct regmap_irq_chip bxtwc_regmap_irq_chip_crit = {
+	.name = "bxtwc_irq_chip_crit",
+	.status_base = BXTWC_CRITIRQ,
+	.mask_base = BXTWC_MCRITIRQ,
+	.irqs = bxtwc_regmap_irqs_crit,
+	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_crit),
+	.num_regs = 1,
+};
+
 static struct resource gpio_resources[] = {
 	DEFINE_RES_IRQ_NAMED(BXTWC_GPIO_LVL1_IRQ, "GPIO"),
 };
@@ -358,6 +402,34 @@ static const struct regmap_config bxtwc_regmap_config = {
 	.reg_read = regmap_ipc_byte_reg_read,
 };
 
+static int bxtwc_add_chained_irq_chip(struct intel_soc_pmic *pmic,
+				struct regmap_irq_chip_data *pdata,
+				int pirq,
+				int irq_flags,
+				const struct regmap_irq_chip *chip,
+				struct regmap_irq_chip_data **data,
+				int *irq)
+{
+	int ret;
+
+	ret = regmap_irq_get_virq(pdata, pirq);
+	if (ret < 0) {
+		dev_err(pmic->dev, "failed to get virtual interrupt=%d\n", ret);
+		return ret;
+	}
+
+	*irq = ret;
+
+	ret = regmap_add_irq_chip(pmic->regmap, *irq, irq_flags, 0,
+				  chip, data);
+	if (ret) {
+		dev_err(pmic->dev, "Failed to add %s irq chip\n", chip->name);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 static int bxtwc_probe(struct platform_device *pdev)
 {
 	int ret;
@@ -409,22 +481,71 @@ static int bxtwc_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	ret = regmap_add_irq_chip(pmic->regmap, pmic->irq,
-				  IRQF_ONESHOT | IRQF_SHARED,
-				  0, &bxtwc_regmap_irq_chip_level2,
-				  &pmic->irq_chip_data_level2);
+	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
+					 BXTWC_TMU_LVL1_IRQ,
+					 IRQF_ONESHOT,
+					 &bxtwc_regmap_irq_chip_tmu,
+					 &pmic->irq_chip_data_tmu,
+					 &pmic->tmu_irq);
 	if (ret) {
-		dev_err(&pdev->dev, "Failed to add secondary IRQ chip\n");
-		goto err_irq_chip_level2;
+		dev_err(&pdev->dev, "Failed to add TMU IRQ chip\n");
+		goto err_irq_chip_tmu;
 	}
 
-	ret = regmap_add_irq_chip(pmic->regmap, pmic->irq,
-				  IRQF_ONESHOT | IRQF_SHARED,
-				  0, &bxtwc_regmap_irq_chip_tmu,
-				  &pmic->irq_chip_data_tmu);
+	/* add chained irq handler for BCU irqs */
+	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
+					 BXTWC_BCU_LVL1_IRQ,
+					 IRQF_ONESHOT,
+					 &bxtwc_regmap_irq_chip_bcu,
+					 &pmic->irq_chip_data_bcu,
+					 &pmic->bcu_irq);
+
+
 	if (ret) {
-		dev_err(&pdev->dev, "Failed to add TMU IRQ chip\n");
-		goto err_irq_chip_tmu;
+		dev_err(&pdev->dev, "Failed to add BUC IRQ chip\n");
+		goto err_irq_chip_bcu;
+	}
+
+	/* add chained irq handler for ADC irqs */
+	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
+					 BXTWC_ADC_LVL1_IRQ,
+					 IRQF_ONESHOT,
+					 &bxtwc_regmap_irq_chip_adc,
+					 &pmic->irq_chip_data_adc,
+					 &pmic->adc_irq);
+
+
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to add ADC IRQ chip\n");
+		goto err_irq_chip_adc;
+	}
+
+	/* add chained irq handler for CHGR irqs */
+	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
+					 BXTWC_CHGR_LVL1_IRQ,
+					 IRQF_ONESHOT,
+					 &bxtwc_regmap_irq_chip_chgr,
+					 &pmic->irq_chip_data_chgr,
+					 &pmic->chgr_irq);
+
+
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to add CHGR IRQ chip\n");
+		goto err_irq_chip_chgr;
+	}
+
+	/* add chained irq handler for CRIT irqs */
+	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
+					 BXTWC_CRIT_LVL1_IRQ,
+					 IRQF_ONESHOT,
+					 &bxtwc_regmap_irq_chip_crit,
+					 &pmic->irq_chip_data_crit,
+					 &pmic->crit_irq);
+
+
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to add CRIT irq chip\n");
+		goto err_irq_chip_crit;
 	}
 
 	ret = mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE, bxt_wc_dev,
@@ -456,10 +577,16 @@ static int bxtwc_probe(struct platform_device *pdev)
 err_sysfs:
 	mfd_remove_devices(&pdev->dev);
 err_mfd:
-	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_tmu);
+	regmap_del_irq_chip(pmic->crit_irq, pmic->irq_chip_data_crit);
+err_irq_chip_crit:
+	regmap_del_irq_chip(pmic->chgr_irq, pmic->irq_chip_data_chgr);
+err_irq_chip_chgr:
+	regmap_del_irq_chip(pmic->adc_irq, pmic->irq_chip_data_adc);
+err_irq_chip_adc:
+	regmap_del_irq_chip(pmic->bcu_irq, pmic->irq_chip_data_bcu);
+err_irq_chip_bcu:
+	regmap_del_irq_chip(pmic->tmu_irq, pmic->irq_chip_data_tmu);
 err_irq_chip_tmu:
-	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_level2);
-err_irq_chip_level2:
 	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
 
 	return ret;
@@ -472,8 +599,11 @@ static int bxtwc_remove(struct platform_device *pdev)
 	sysfs_remove_group(&pdev->dev.kobj, &bxtwc_group);
 	mfd_remove_devices(&pdev->dev);
 	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
-	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_level2);
-	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_tmu);
+	regmap_del_irq_chip(pmic->tmu_irq, pmic->irq_chip_data_tmu);
+	regmap_del_irq_chip(pmic->bcu_irq, pmic->irq_chip_data_bcu);
+	regmap_del_irq_chip(pmic->adc_irq, pmic->irq_chip_data_adc);
+	regmap_del_irq_chip(pmic->chgr_irq, pmic->irq_chip_data_chgr);
+	regmap_del_irq_chip(pmic->crit_irq, pmic->irq_chip_data_crit);
 
 	return 0;
 }
diff --git a/include/linux/mfd/intel_soc_pmic.h b/include/linux/mfd/intel_soc_pmic.h
index 956caa0..63e1270 100644
--- a/include/linux/mfd/intel_soc_pmic.h
+++ b/include/linux/mfd/intel_soc_pmic.h
@@ -23,10 +23,15 @@
 
 struct intel_soc_pmic {
 	int irq;
+	int tmu_irq, bcu_irq, adc_irq, chgr_irq, crit_irq;
 	struct regmap *regmap;
 	struct regmap_irq_chip_data *irq_chip_data;
 	struct regmap_irq_chip_data *irq_chip_data_level2;
 	struct regmap_irq_chip_data *irq_chip_data_tmu;
+	struct regmap_irq_chip_data *irq_chip_data_bcu;
+	struct regmap_irq_chip_data *irq_chip_data_adc;
+	struct regmap_irq_chip_data *irq_chip_data_chgr;
+	struct regmap_irq_chip_data *irq_chip_data_crit;
 	struct device *dev;
 };
 
-- 
2.7.4

[PATCH v1 6/7] mfd: intel_soc_pmic_bxtwc: use chained irqs for second level irq chips

[sathyanarayanan.kuppuswamy]

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

Whishkey cove PMIC has support to mask/unmask interrupts at two levels.
At first level we can mask/unmask interrupt domains like TMU, GPIO, ADC,
CHGR, BCU THERMAL and PWRBTN and at second level, it provides facility
to mask/unmask individual interrupts belong each of this domain. For
example, in case of TMU, at first level we have TMU interrupt domain,
and at second level we have two interrupts, wake alarm, system alarm that
belong to the TMU interrupt domain.

Currently, in this driver all first level irqs are registered as part of
irq chip(bxtwc_regmap_irq_chip). By default, after you register the irq
chip from your driver, all irqs in that chip will masked and can only be
enabled if that irq is requested using request_irq call. This is the
default Linux irq behavior model. And whenever a dependent device that
belongs to PMIC requests only the second level irq and not explicitly
unmask the first level irq, then in essence the second level irq will
still be disabled. For example, if TMU device driver request wake_alarm
irq and not explicitly unmask TMU level 1 irq then according to the default
Linux irq model,  wake_alarm irq will still be disabled. So the proper
solution to fix this issue is to use the chained irq chip concept. We
should chain all the second level chip irqs to the corresponding first
level irq. To do this, we need to create separate irq chips for every
group of second level irqs.

In case of TMU, when adding second level irq chip, instead of using pmic
irq we should use the corresponding first level irq. So the following
code will change from

ret = regmap_add_irq_chip(pmic->regmap, pmic->irq, ...)

to,

virq = regmap_irq_get_virq(&pmic->irq_chip_data, BXTWC_TMU_LVL1_IRQ);

ret = regmap_add_irq_chip(pmic->regmap, virq, ...)

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 drivers/mfd/intel_soc_pmic_bxtwc.c | 212 ++++++++++++++++++++++++++++++-------
 include/linux/mfd/intel_soc_pmic.h |   5 +
 2 files changed, 176 insertions(+), 41 deletions(-)

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
index dc8af1d..807eba3 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -81,18 +81,26 @@ enum bxtwc_irqs {
 	BXTWC_PWRBTN_IRQ,
 };
 
-enum bxtwc_irqs_level2 {
-	/* Level 2 */
+enum bxtwc_irqs_tmu {
+	BXTWC_TMU_IRQ = 0,
+};
+
+enum bxtwc_irqs_bcu {
 	BXTWC_BCU_IRQ = 0,
-	BXTWC_ADC_IRQ,
-	BXTWC_USBC_IRQ,
+};
+
+enum bxtwc_irqs_adc {
+	BXTWC_ADC_IRQ = 0,
+};
+
+enum bxtwc_irqs_chgr {
+	BXTWC_USBC_IRQ = 0,
 	BXTWC_CHGR0_IRQ,
 	BXTWC_CHGR1_IRQ,
-	BXTWC_CRIT_IRQ,
 };
 
-enum bxtwc_irqs_tmu {
-	BXTWC_TMU_IRQ = 0,
+enum bxtwc_irqs_crit {
+	BXTWC_CRIT_IRQ = 0,
 };
 
 static const struct regmap_irq bxtwc_regmap_irqs[] = {
@@ -107,17 +115,26 @@ static const struct regmap_irq bxtwc_regmap_irqs[] = {
 	REGMAP_IRQ_REG(BXTWC_PWRBTN_IRQ, 1, 0x03),
 };
 
-static const struct regmap_irq bxtwc_regmap_irqs_level2[] = {
+static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = {
+	REGMAP_IRQ_REG(BXTWC_TMU_IRQ, 0, 0x06),
+};
+
+static const struct regmap_irq bxtwc_regmap_irqs_bcu[] = {
 	REGMAP_IRQ_REG(BXTWC_BCU_IRQ, 0, 0x1f),
-	REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 1, 0xff),
-	REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 2, BIT(5)),
-	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 2, 0x1f),
-	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 3, 0x1f),
-	REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 6, 0x03),
 };
 
-static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = {
-	REGMAP_IRQ_REG(BXTWC_TMU_IRQ, 0, 0x06),
+static const struct regmap_irq bxtwc_regmap_irqs_adc[] = {
+	REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 0, 0xff),
+};
+
+static const struct regmap_irq bxtwc_regmap_irqs_chgr[] = {
+	REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 0, BIT(5)),
+	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 0, 0x1f),
+	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 1, 0x1f),
+};
+
+static const struct regmap_irq bxtwc_regmap_irqs_crit[] = {
+	REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 0, 0x03),
 };
 
 static struct regmap_irq_chip bxtwc_regmap_irq_chip = {
@@ -129,15 +146,6 @@ static struct regmap_irq_chip bxtwc_regmap_irq_chip = {
 	.num_regs = 2,
 };
 
-static struct regmap_irq_chip bxtwc_regmap_irq_chip_level2 = {
-	.name = "bxtwc_irq_chip_level2",
-	.status_base = BXTWC_BCUIRQ,
-	.mask_base = BXTWC_MBCUIRQ,
-	.irqs = bxtwc_regmap_irqs_level2,
-	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_level2),
-	.num_regs = 10,
-};
-
 static struct regmap_irq_chip bxtwc_regmap_irq_chip_tmu = {
 	.name = "bxtwc_irq_chip_tmu",
 	.status_base = BXTWC_TMUIRQ,
@@ -147,6 +155,42 @@ static struct regmap_irq_chip bxtwc_regmap_irq_chip_tmu = {
 	.num_regs = 1,
 };
 
+static struct regmap_irq_chip bxtwc_regmap_irq_chip_bcu = {
+	.name = "bxtwc_irq_chip_bcu",
+	.status_base = BXTWC_BCUIRQ,
+	.mask_base = BXTWC_MBCUIRQ,
+	.irqs = bxtwc_regmap_irqs_bcu,
+	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_bcu),
+	.num_regs = 1,
+};
+
+static struct regmap_irq_chip bxtwc_regmap_irq_chip_adc = {
+	.name = "bxtwc_irq_chip_adc",
+	.status_base = BXTWC_ADCIRQ,
+	.mask_base = BXTWC_MADCIRQ,
+	.irqs = bxtwc_regmap_irqs_adc,
+	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_adc),
+	.num_regs = 1,
+};
+
+static struct regmap_irq_chip bxtwc_regmap_irq_chip_chgr = {
+	.name = "bxtwc_irq_chip_chgr",
+	.status_base = BXTWC_CHGR0IRQ,
+	.mask_base = BXTWC_MCHGR0IRQ,
+	.irqs = bxtwc_regmap_irqs_chgr,
+	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_chgr),
+	.num_regs = 2,
+};
+
+static struct regmap_irq_chip bxtwc_regmap_irq_chip_crit = {
+	.name = "bxtwc_irq_chip_crit",
+	.status_base = BXTWC_CRITIRQ,
+	.mask_base = BXTWC_MCRITIRQ,
+	.irqs = bxtwc_regmap_irqs_crit,
+	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_crit),
+	.num_regs = 1,
+};
+
 static struct resource gpio_resources[] = {
 	DEFINE_RES_IRQ_NAMED(BXTWC_GPIO_LVL1_IRQ, "GPIO"),
 };
@@ -358,6 +402,34 @@ static const struct regmap_config bxtwc_regmap_config = {
 	.reg_read = regmap_ipc_byte_reg_read,
 };
 
+static int bxtwc_add_chained_irq_chip(struct intel_soc_pmic *pmic,
+				struct regmap_irq_chip_data *pdata,
+				int pirq,
+				int irq_flags,
+				const struct regmap_irq_chip *chip,
+				struct regmap_irq_chip_data **data,
+				int *irq)
+{
+	int ret;
+
+	ret = regmap_irq_get_virq(pdata, pirq);
+	if (ret < 0) {
+		dev_err(pmic->dev, "failed to get virtual interrupt=%d\n", ret);
+		return ret;
+	}
+
+	*irq = ret;
+
+	ret = regmap_add_irq_chip(pmic->regmap, *irq, irq_flags, 0,
+				  chip, data);
+	if (ret) {
+		dev_err(pmic->dev, "Failed to add %s irq chip\n", chip->name);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 static int bxtwc_probe(struct platform_device *pdev)
 {
 	int ret;
@@ -409,22 +481,71 @@ static int bxtwc_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	ret = regmap_add_irq_chip(pmic->regmap, pmic->irq,
-				  IRQF_ONESHOT | IRQF_SHARED,
-				  0, &bxtwc_regmap_irq_chip_level2,
-				  &pmic->irq_chip_data_level2);
+	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
+					 BXTWC_TMU_LVL1_IRQ,
+					 IRQF_ONESHOT,
+					 &bxtwc_regmap_irq_chip_tmu,
+					 &pmic->irq_chip_data_tmu,
+					 &pmic->tmu_irq);
 	if (ret) {
-		dev_err(&pdev->dev, "Failed to add secondary IRQ chip\n");
-		goto err_irq_chip_level2;
+		dev_err(&pdev->dev, "Failed to add TMU IRQ chip\n");
+		goto err_irq_chip_tmu;
 	}
 
-	ret = regmap_add_irq_chip(pmic->regmap, pmic->irq,
-				  IRQF_ONESHOT | IRQF_SHARED,
-				  0, &bxtwc_regmap_irq_chip_tmu,
-				  &pmic->irq_chip_data_tmu);
+	/* add chained irq handler for BCU irqs */
+	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
+					 BXTWC_BCU_LVL1_IRQ,
+					 IRQF_ONESHOT,
+					 &bxtwc_regmap_irq_chip_bcu,
+					 &pmic->irq_chip_data_bcu,
+					 &pmic->bcu_irq);
+
+
 	if (ret) {
-		dev_err(&pdev->dev, "Failed to add TMU IRQ chip\n");
-		goto err_irq_chip_tmu;
+		dev_err(&pdev->dev, "Failed to add BUC IRQ chip\n");
+		goto err_irq_chip_bcu;
+	}
+
+	/* add chained irq handler for ADC irqs */
+	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
+					 BXTWC_ADC_LVL1_IRQ,
+					 IRQF_ONESHOT,
+					 &bxtwc_regmap_irq_chip_adc,
+					 &pmic->irq_chip_data_adc,
+					 &pmic->adc_irq);
+
+
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to add ADC IRQ chip\n");
+		goto err_irq_chip_adc;
+	}
+
+	/* add chained irq handler for CHGR irqs */
+	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
+					 BXTWC_CHGR_LVL1_IRQ,
+					 IRQF_ONESHOT,
+					 &bxtwc_regmap_irq_chip_chgr,
+					 &pmic->irq_chip_data_chgr,
+					 &pmic->chgr_irq);
+
+
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to add CHGR IRQ chip\n");
+		goto err_irq_chip_chgr;
+	}
+
+	/* add chained irq handler for CRIT irqs */
+	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
+					 BXTWC_CRIT_LVL1_IRQ,
+					 IRQF_ONESHOT,
+					 &bxtwc_regmap_irq_chip_crit,
+					 &pmic->irq_chip_data_crit,
+					 &pmic->crit_irq);
+
+
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to add CRIT irq chip\n");
+		goto err_irq_chip_crit;
 	}
 
 	ret = mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE, bxt_wc_dev,
@@ -456,10 +577,16 @@ static int bxtwc_probe(struct platform_device *pdev)
 err_sysfs:
 	mfd_remove_devices(&pdev->dev);
 err_mfd:
-	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_tmu);
+	regmap_del_irq_chip(pmic->crit_irq, pmic->irq_chip_data_crit);
+err_irq_chip_crit:
+	regmap_del_irq_chip(pmic->chgr_irq, pmic->irq_chip_data_chgr);
+err_irq_chip_chgr:
+	regmap_del_irq_chip(pmic->adc_irq, pmic->irq_chip_data_adc);
+err_irq_chip_adc:
+	regmap_del_irq_chip(pmic->bcu_irq, pmic->irq_chip_data_bcu);
+err_irq_chip_bcu:
+	regmap_del_irq_chip(pmic->tmu_irq, pmic->irq_chip_data_tmu);
 err_irq_chip_tmu:
-	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_level2);
-err_irq_chip_level2:
 	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
 
 	return ret;
@@ -472,8 +599,11 @@ static int bxtwc_remove(struct platform_device *pdev)
 	sysfs_remove_group(&pdev->dev.kobj, &bxtwc_group);
 	mfd_remove_devices(&pdev->dev);
 	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
-	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_level2);
-	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_tmu);
+	regmap_del_irq_chip(pmic->tmu_irq, pmic->irq_chip_data_tmu);
+	regmap_del_irq_chip(pmic->bcu_irq, pmic->irq_chip_data_bcu);
+	regmap_del_irq_chip(pmic->adc_irq, pmic->irq_chip_data_adc);
+	regmap_del_irq_chip(pmic->chgr_irq, pmic->irq_chip_data_chgr);
+	regmap_del_irq_chip(pmic->crit_irq, pmic->irq_chip_data_crit);
 
 	return 0;
 }
diff --git a/include/linux/mfd/intel_soc_pmic.h b/include/linux/mfd/intel_soc_pmic.h
index 956caa0..63e1270 100644
--- a/include/linux/mfd/intel_soc_pmic.h
+++ b/include/linux/mfd/intel_soc_pmic.h
@@ -23,10 +23,15 @@
 
 struct intel_soc_pmic {
 	int irq;
+	int tmu_irq, bcu_irq, adc_irq, chgr_irq, crit_irq;
 	struct regmap *regmap;
 	struct regmap_irq_chip_data *irq_chip_data;
 	struct regmap_irq_chip_data *irq_chip_data_level2;
 	struct regmap_irq_chip_data *irq_chip_data_tmu;
+	struct regmap_irq_chip_data *irq_chip_data_bcu;
+	struct regmap_irq_chip_data *irq_chip_data_adc;
+	struct regmap_irq_chip_data *irq_chip_data_chgr;
+	struct regmap_irq_chip_data *irq_chip_data_crit;
 	struct device *dev;
 };
 
-- 
2.7.4

[PATCH v1 7/7] platform: x86: intel_bxtwc_tmu: remove first level irq unmask

[sathyanarayanan.kuppuswamy]

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

Currently in WCOVE PMIC mfd driver, all second level irq chips
are chained to the respective first level irqs. So there is no
need for explicitly unmasking the first level irq in this
driver. This patches removes this level 1 irq unmask support.

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 drivers/platform/x86/intel_bxtwc_tmu.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/platform/x86/intel_bxtwc_tmu.c b/drivers/platform/x86/intel_bxtwc_tmu.c
index e202abd..ea865d4 100644
--- a/drivers/platform/x86/intel_bxtwc_tmu.c
+++ b/drivers/platform/x86/intel_bxtwc_tmu.c
@@ -92,10 +92,6 @@ static int bxt_wcove_tmu_probe(struct platform_device *pdev)
 	}
 	wctmu->irq = virq;
 
-	/* Enable TMU interrupts */
-	regmap_update_bits(wctmu->regmap, BXTWC_MIRQLVL1,
-				  BXTWC_MIRQLVL1_MTMU, 0);
-
 	/* Unmask TMU second level Wake & System alarm */
 	regmap_update_bits(wctmu->regmap, BXTWC_MTMUIRQ_REG,
 				  BXTWC_TMU_ALRM_MASK, 0);
-- 
2.7.4

[PATCH v1 7/7] platform: x86: intel_bxtwc_tmu: remove first level irq unmask

[sathyanarayanan.kuppuswamy]

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

Currently in WCOVE PMIC mfd driver, all second level irq chips
are chained to the respective first level irqs. So there is no
need for explicitly unmasking the first level irq in this
driver. This patches removes this level 1 irq unmask support.

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 drivers/platform/x86/intel_bxtwc_tmu.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/platform/x86/intel_bxtwc_tmu.c b/drivers/platform/x86/intel_bxtwc_tmu.c
index e202abd..ea865d4 100644
--- a/drivers/platform/x86/intel_bxtwc_tmu.c
+++ b/drivers/platform/x86/intel_bxtwc_tmu.c
@@ -92,10 +92,6 @@ static int bxt_wcove_tmu_probe(struct platform_device *pdev)
 	}
 	wctmu->irq = virq;
 
-	/* Enable TMU interrupts */
-	regmap_update_bits(wctmu->regmap, BXTWC_MIRQLVL1,
-				  BXTWC_MIRQLVL1_MTMU, 0);
-
 	/* Unmask TMU second level Wake & System alarm */
 	regmap_update_bits(wctmu->regmap, BXTWC_MTMUIRQ_REG,
 				  BXTWC_TMU_ALRM_MASK, 0);
-- 
2.7.4

Re: [PATCH v1 1/7] mfd: intel_soc_pmic_bxtwc: fix TMU interrupt index

[Lee Jones]

On Mon, 10 Apr 2017, sathyanarayanan.kuppuswamy@linux.intel.com wrote:

> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> 
> TMU interrupts are registered as a separate interrupt chip, and
> hence it should start its interrupt index(BXTWC_TMU_IRQ) number
> from 0. But currently, BXTWC_TMU_IRQ is defined as part of enum
> bxtwc_irqs_level2 and its index value is 11. Since this index
> value is used when calculating .num_irqs of regmap_irq_chip_tmu,
> it incorrectly reports number of irqs as 12 instead of actual
> value of 1.
> 
> static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = {
> 	REGMAP_IRQ_REG(BXTWC_TMU_IRQ, 0, 0x06),
> };
> 
> static struct regmap_irq_chip bxtwc_regmap_irq_chip_tmu = {
> 	.name = "bxtwc_irq_chip_tmu",
> 	.status_base = BXTWC_TMUIRQ,
> 	.mask_base = BXTWC_MTMUIRQ,
> 	.irqs = bxtwc_regmap_irqs_tmu,
> 	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_tmu),
> 	.num_regs = 1,
> };
> 
> This patch fixes this issue by creating new enum of tmu irqs and
> resetting its starting index to 0.
> 
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> ---
>  drivers/mfd/intel_soc_pmic_bxtwc.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Patch looks fine, but please remove the code segment from the commit
log.

For the code:

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
  
> diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
> index 699c8c7..bb18e20 100644
> --- a/drivers/mfd/intel_soc_pmic_bxtwc.c
> +++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
> @@ -94,7 +94,10 @@ enum bxtwc_irqs_level2 {
>  	BXTWC_GPIO0_IRQ,
>  	BXTWC_GPIO1_IRQ,
>  	BXTWC_CRIT_IRQ,
> -	BXTWC_TMU_IRQ,
> +};
> +
> +enum bxtwc_irqs_tmu {
> +	BXTWC_TMU_IRQ = 0,
>  };
>  
>  static const struct regmap_irq bxtwc_regmap_irqs[] = {

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

Re: [PATCH v1 2/7] mfd: intel_soc_pmic_bxtwc: remove thermal second level irqs

[Lee Jones]

On Mon, 10 Apr 2017, sathyanarayanan.kuppuswamy@linux.intel.com wrote:

> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> 
> Since all second level thermal irqs are consumed by the same
> device(bxt_wcove_thermal), there is no need to expose them as separate
> interrupts. We can just export only the first level irqs for thermal and
> let the device(bxt_wcove_thermal) driver handle the second level irqs
> based on thermal interrupt status register. Also, just using only the
> first level irq will eliminate the bug involved in requesting only the
> second level irq and not explicitly enable the first level irq. For
> more info on this issue please read the details at,
> 
> https://lkml.org/lkml/2017/2/27/148
> 
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> ---
>  drivers/mfd/intel_soc_pmic_bxtwc.c | 32 ++++++++++++--------------------
>  1 file changed, 12 insertions(+), 20 deletions(-)

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

> diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
> index bb18e20..c08d514 100644
> --- a/drivers/mfd/intel_soc_pmic_bxtwc.c
> +++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
> @@ -83,10 +83,7 @@ enum bxtwc_irqs {
>  
>  enum bxtwc_irqs_level2 {
>  	/* Level 2 */
> -	BXTWC_THRM0_IRQ = 0,
> -	BXTWC_THRM1_IRQ,
> -	BXTWC_THRM2_IRQ,
> -	BXTWC_BCU_IRQ,
> +	BXTWC_BCU_IRQ = 0,
>  	BXTWC_ADC_IRQ,
>  	BXTWC_USBC_IRQ,
>  	BXTWC_CHGR0_IRQ,
> @@ -113,17 +110,14 @@ static const struct regmap_irq bxtwc_regmap_irqs[] = {
>  };
>  
>  static const struct regmap_irq bxtwc_regmap_irqs_level2[] = {
> -	REGMAP_IRQ_REG(BXTWC_THRM0_IRQ, 0, 0xff),
> -	REGMAP_IRQ_REG(BXTWC_THRM1_IRQ, 1, 0xbf),
> -	REGMAP_IRQ_REG(BXTWC_THRM2_IRQ, 2, 0xff),
> -	REGMAP_IRQ_REG(BXTWC_BCU_IRQ, 3, 0x1f),
> -	REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 4, 0xff),
> -	REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 5, BIT(5)),
> -	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 5, 0x1f),
> -	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 6, 0x1f),
> -	REGMAP_IRQ_REG(BXTWC_GPIO0_IRQ, 7, 0xff),
> -	REGMAP_IRQ_REG(BXTWC_GPIO1_IRQ, 8, 0x3f),
> -	REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 9, 0x03),
> +	REGMAP_IRQ_REG(BXTWC_BCU_IRQ, 0, 0x1f),
> +	REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 1, 0xff),
> +	REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 2, BIT(5)),
> +	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 2, 0x1f),
> +	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 3, 0x1f),
> +	REGMAP_IRQ_REG(BXTWC_GPIO0_IRQ, 4, 0xff),
> +	REGMAP_IRQ_REG(BXTWC_GPIO1_IRQ, 5, 0x3f),
> +	REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 6, 0x03),
>  };
>  
>  static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = {
> @@ -141,8 +135,8 @@ static struct regmap_irq_chip bxtwc_regmap_irq_chip = {
>  
>  static struct regmap_irq_chip bxtwc_regmap_irq_chip_level2 = {
>  	.name = "bxtwc_irq_chip_level2",
> -	.status_base = BXTWC_THRM0IRQ,
> -	.mask_base = BXTWC_MTHRM0IRQ,
> +	.status_base = BXTWC_BCUIRQ,
> +	.mask_base = BXTWC_MBCUIRQ,
>  	.irqs = bxtwc_regmap_irqs_level2,
>  	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_level2),
>  	.num_regs = 10,
> @@ -176,9 +170,7 @@ static struct resource charger_resources[] = {
>  };
>  
>  static struct resource thermal_resources[] = {
> -	DEFINE_RES_IRQ(BXTWC_THRM0_IRQ),
> -	DEFINE_RES_IRQ(BXTWC_THRM1_IRQ),
> -	DEFINE_RES_IRQ(BXTWC_THRM2_IRQ),
> +	DEFINE_RES_IRQ(BXTWC_THRM_LVL1_IRQ),
>  };
>  
>  static struct resource bcu_resources[] = {

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

Re: [PATCH v1 4/7] mfd: intel_soc_pmic_bxtwc: remove second level irq for gpio device

[Lee Jones]

On Mon, 10 Apr 2017, sathyanarayanan.kuppuswamy@linux.intel.com wrote:

> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> 
> Currently all PMIC GPIO domain irqs are consumed by the same
> device(bxt_wcove_gpio), so there is no need to export them as
> separate interrupts. We can just export only the first level
> GPIO irq(BXTWC_GPIO_LVL1_IRQ) as an irq resource and let the
> GPIO device driver(bxt_wcove_gpio) handle the GPIO sub domain
> irqs based on status value of GPIO level2 interrupt status
> register. Also, just using only the first level irq will eliminate
> the bug involved in requesting only the second level irq and not
> explicitly enable the first level irq. For more info on this
> issue please read the details at,
> 
> https://lkml.org/lkml/2017/2/27/148
> 
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> ---
>  drivers/mfd/intel_soc_pmic_bxtwc.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
  
> diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
> index c08d514..dc8af1d 100644
> --- a/drivers/mfd/intel_soc_pmic_bxtwc.c
> +++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
> @@ -88,8 +88,6 @@ enum bxtwc_irqs_level2 {
>  	BXTWC_USBC_IRQ,
>  	BXTWC_CHGR0_IRQ,
>  	BXTWC_CHGR1_IRQ,
> -	BXTWC_GPIO0_IRQ,
> -	BXTWC_GPIO1_IRQ,
>  	BXTWC_CRIT_IRQ,
>  };
>  
> @@ -115,8 +113,6 @@ static const struct regmap_irq bxtwc_regmap_irqs_level2[] = {
>  	REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 2, BIT(5)),
>  	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 2, 0x1f),
>  	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 3, 0x1f),
> -	REGMAP_IRQ_REG(BXTWC_GPIO0_IRQ, 4, 0xff),
> -	REGMAP_IRQ_REG(BXTWC_GPIO1_IRQ, 5, 0x3f),
>  	REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 6, 0x03),
>  };
>  
> @@ -152,8 +148,7 @@ static struct regmap_irq_chip bxtwc_regmap_irq_chip_tmu = {
>  };
>  
>  static struct resource gpio_resources[] = {
> -	DEFINE_RES_IRQ_NAMED(BXTWC_GPIO0_IRQ, "GPIO0"),
> -	DEFINE_RES_IRQ_NAMED(BXTWC_GPIO1_IRQ, "GPIO1"),
> +	DEFINE_RES_IRQ_NAMED(BXTWC_GPIO_LVL1_IRQ, "GPIO"),
>  };
>  
>  static struct resource adc_resources[] = {

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

Re: [PATCH v1 6/7] mfd: intel_soc_pmic_bxtwc: use chained irqs for second level irq chips

[Lee Jones]

On Mon, 10 Apr 2017, sathyanarayanan.kuppuswamy@linux.intel.com wrote:

> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> 
> Whishkey cove PMIC has support to mask/unmask interrupts at two levels.
> At first level we can mask/unmask interrupt domains like TMU, GPIO, ADC,
> CHGR, BCU THERMAL and PWRBTN and at second level, it provides facility
> to mask/unmask individual interrupts belong each of this domain. For
> example, in case of TMU, at first level we have TMU interrupt domain,
> and at second level we have two interrupts, wake alarm, system alarm that
> belong to the TMU interrupt domain.
> 
> Currently, in this driver all first level irqs are registered as part of
> irq chip(bxtwc_regmap_irq_chip). By default, after you register the irq
> chip from your driver, all irqs in that chip will masked and can only be
> enabled if that irq is requested using request_irq call. This is the
> default Linux irq behavior model. And whenever a dependent device that
> belongs to PMIC requests only the second level irq and not explicitly
> unmask the first level irq, then in essence the second level irq will
> still be disabled. For example, if TMU device driver request wake_alarm
> irq and not explicitly unmask TMU level 1 irq then according to the default
> Linux irq model,  wake_alarm irq will still be disabled. So the proper
> solution to fix this issue is to use the chained irq chip concept. We
> should chain all the second level chip irqs to the corresponding first
> level irq. To do this, we need to create separate irq chips for every
> group of second level irqs.
> 
> In case of TMU, when adding second level irq chip, instead of using pmic
> irq we should use the corresponding first level irq. So the following
> code will change from
> 
> ret = regmap_add_irq_chip(pmic->regmap, pmic->irq, ...)
> 
> to,
> 
> virq = regmap_irq_get_virq(&pmic->irq_chip_data, BXTWC_TMU_LVL1_IRQ);
> 
> ret = regmap_add_irq_chip(pmic->regmap, virq, ...)
> 
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> ---
>  drivers/mfd/intel_soc_pmic_bxtwc.c | 212 ++++++++++++++++++++++++++++++-------
>  include/linux/mfd/intel_soc_pmic.h |   5 +
>  2 files changed, 176 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
> index dc8af1d..807eba3 100644
> --- a/drivers/mfd/intel_soc_pmic_bxtwc.c
> +++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
> @@ -81,18 +81,26 @@ enum bxtwc_irqs {
>  	BXTWC_PWRBTN_IRQ,
>  };
>  
> -enum bxtwc_irqs_level2 {
> -	/* Level 2 */
> +enum bxtwc_irqs_tmu {
> +	BXTWC_TMU_IRQ = 0,
> +};
> +
> +enum bxtwc_irqs_bcu {
>  	BXTWC_BCU_IRQ = 0,
> -	BXTWC_ADC_IRQ,
> -	BXTWC_USBC_IRQ,
> +};
> +
> +enum bxtwc_irqs_adc {
> +	BXTWC_ADC_IRQ = 0,
> +};
> +
> +enum bxtwc_irqs_chgr {
> +	BXTWC_USBC_IRQ = 0,
>  	BXTWC_CHGR0_IRQ,
>  	BXTWC_CHGR1_IRQ,
> -	BXTWC_CRIT_IRQ,
>  };
>  
> -enum bxtwc_irqs_tmu {
> -	BXTWC_TMU_IRQ = 0,
> +enum bxtwc_irqs_crit {
> +	BXTWC_CRIT_IRQ = 0,
>  };
>  
>  static const struct regmap_irq bxtwc_regmap_irqs[] = {
> @@ -107,17 +115,26 @@ static const struct regmap_irq bxtwc_regmap_irqs[] = {
>  	REGMAP_IRQ_REG(BXTWC_PWRBTN_IRQ, 1, 0x03),
>  };
>  
> -static const struct regmap_irq bxtwc_regmap_irqs_level2[] = {
> +static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = {
> +	REGMAP_IRQ_REG(BXTWC_TMU_IRQ, 0, 0x06),
> +};
> +
> +static const struct regmap_irq bxtwc_regmap_irqs_bcu[] = {
>  	REGMAP_IRQ_REG(BXTWC_BCU_IRQ, 0, 0x1f),
> -	REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 1, 0xff),
> -	REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 2, BIT(5)),
> -	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 2, 0x1f),
> -	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 3, 0x1f),
> -	REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 6, 0x03),
>  };
>  
> -static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = {
> -	REGMAP_IRQ_REG(BXTWC_TMU_IRQ, 0, 0x06),
> +static const struct regmap_irq bxtwc_regmap_irqs_adc[] = {
> +	REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 0, 0xff),
> +};
> +
> +static const struct regmap_irq bxtwc_regmap_irqs_chgr[] = {
> +	REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 0, BIT(5)),
> +	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 0, 0x1f),
> +	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 1, 0x1f),
> +};
> +
> +static const struct regmap_irq bxtwc_regmap_irqs_crit[] = {
> +	REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 0, 0x03),
>  };
>  
>  static struct regmap_irq_chip bxtwc_regmap_irq_chip = {
> @@ -129,15 +146,6 @@ static struct regmap_irq_chip bxtwc_regmap_irq_chip = {
>  	.num_regs = 2,
>  };
>  
> -static struct regmap_irq_chip bxtwc_regmap_irq_chip_level2 = {
> -	.name = "bxtwc_irq_chip_level2",
> -	.status_base = BXTWC_BCUIRQ,
> -	.mask_base = BXTWC_MBCUIRQ,
> -	.irqs = bxtwc_regmap_irqs_level2,
> -	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_level2),
> -	.num_regs = 10,
> -};
> -
>  static struct regmap_irq_chip bxtwc_regmap_irq_chip_tmu = {
>  	.name = "bxtwc_irq_chip_tmu",
>  	.status_base = BXTWC_TMUIRQ,
> @@ -147,6 +155,42 @@ static struct regmap_irq_chip bxtwc_regmap_irq_chip_tmu = {
>  	.num_regs = 1,
>  };
>  
> +static struct regmap_irq_chip bxtwc_regmap_irq_chip_bcu = {
> +	.name = "bxtwc_irq_chip_bcu",
> +	.status_base = BXTWC_BCUIRQ,
> +	.mask_base = BXTWC_MBCUIRQ,
> +	.irqs = bxtwc_regmap_irqs_bcu,
> +	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_bcu),
> +	.num_regs = 1,
> +};
> +
> +static struct regmap_irq_chip bxtwc_regmap_irq_chip_adc = {
> +	.name = "bxtwc_irq_chip_adc",
> +	.status_base = BXTWC_ADCIRQ,
> +	.mask_base = BXTWC_MADCIRQ,
> +	.irqs = bxtwc_regmap_irqs_adc,
> +	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_adc),
> +	.num_regs = 1,
> +};
> +
> +static struct regmap_irq_chip bxtwc_regmap_irq_chip_chgr = {
> +	.name = "bxtwc_irq_chip_chgr",
> +	.status_base = BXTWC_CHGR0IRQ,
> +	.mask_base = BXTWC_MCHGR0IRQ,
> +	.irqs = bxtwc_regmap_irqs_chgr,
> +	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_chgr),
> +	.num_regs = 2,
> +};
> +
> +static struct regmap_irq_chip bxtwc_regmap_irq_chip_crit = {
> +	.name = "bxtwc_irq_chip_crit",
> +	.status_base = BXTWC_CRITIRQ,
> +	.mask_base = BXTWC_MCRITIRQ,
> +	.irqs = bxtwc_regmap_irqs_crit,
> +	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_crit),
> +	.num_regs = 1,
> +};
> +
>  static struct resource gpio_resources[] = {
>  	DEFINE_RES_IRQ_NAMED(BXTWC_GPIO_LVL1_IRQ, "GPIO"),
>  };
> @@ -358,6 +402,34 @@ static const struct regmap_config bxtwc_regmap_config = {
>  	.reg_read = regmap_ipc_byte_reg_read,
>  };
>  
> +static int bxtwc_add_chained_irq_chip(struct intel_soc_pmic *pmic,
> +				struct regmap_irq_chip_data *pdata,

> +				int pirq,
> +				int irq_flags,

Nit: These do not need to be on separate lines.

> +				const struct regmap_irq_chip *chip,
> +				struct regmap_irq_chip_data **data,
> +				int *irq)
> +{
> +	int ret;
> +
> +	ret = regmap_irq_get_virq(pdata, pirq);
> +	if (ret < 0) {
> +		dev_err(pmic->dev, "failed to get virtual interrupt=%d\n", ret);

s/=/: /

> +		return ret;
> +	}
> +
> +	*irq = ret;
> +
> +	ret = regmap_add_irq_chip(pmic->regmap, *irq, irq_flags, 0,
> +				  chip, data);
> +	if (ret) {
> +		dev_err(pmic->dev, "Failed to add %s irq chip\n", chip->name);

s/irq/IRQ/

> +		return -ENODEV;

Why aren't you returning ret?

In fact, remove this line and ...

> +	}
> +
> +	return 0;

... return ret;

> +}
> +
>  static int bxtwc_probe(struct platform_device *pdev)
>  {
>  	int ret;
> @@ -409,22 +481,71 @@ static int bxtwc_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	ret = regmap_add_irq_chip(pmic->regmap, pmic->irq,
> -				  IRQF_ONESHOT | IRQF_SHARED,
> -				  0, &bxtwc_regmap_irq_chip_level2,
> -				  &pmic->irq_chip_data_level2);
> +	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
> +					 BXTWC_TMU_LVL1_IRQ,
> +					 IRQF_ONESHOT,
> +					 &bxtwc_regmap_irq_chip_tmu,
> +					 &pmic->irq_chip_data_tmu,
> +					 &pmic->tmu_irq);

Isn't there a generic API for chained IRQs already?

>  	if (ret) {
> -		dev_err(&pdev->dev, "Failed to add secondary IRQ chip\n");
> -		goto err_irq_chip_level2;
> +		dev_err(&pdev->dev, "Failed to add TMU IRQ chip\n");
> +		goto err_irq_chip_tmu;
>  	}
>  
> -	ret = regmap_add_irq_chip(pmic->regmap, pmic->irq,
> -				  IRQF_ONESHOT | IRQF_SHARED,
> -				  0, &bxtwc_regmap_irq_chip_tmu,
> -				  &pmic->irq_chip_data_tmu);
> +	/* add chained irq handler for BCU irqs */

Use proper grammar.

"Add chained IRQ handler for BCU IRQs"

> +	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
> +					 BXTWC_BCU_LVL1_IRQ,
> +					 IRQF_ONESHOT,
> +					 &bxtwc_regmap_irq_chip_bcu,
> +					 &pmic->irq_chip_data_bcu,
> +					 &pmic->bcu_irq);
> +
> +
>  	if (ret) {
> -		dev_err(&pdev->dev, "Failed to add TMU IRQ chip\n");
> -		goto err_irq_chip_tmu;
> +		dev_err(&pdev->dev, "Failed to add BUC IRQ chip\n");
> +		goto err_irq_chip_bcu;
> +	}
> +
> +	/* add chained irq handler for ADC irqs */

Grammar.

> +	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
> +					 BXTWC_ADC_LVL1_IRQ,
> +					 IRQF_ONESHOT,
> +					 &bxtwc_regmap_irq_chip_adc,
> +					 &pmic->irq_chip_data_adc,
> +					 &pmic->adc_irq);
> +
> +
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to add ADC IRQ chip\n");
> +		goto err_irq_chip_adc;
> +	}
> +
> +	/* add chained irq handler for CHGR irqs */
> +	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
> +					 BXTWC_CHGR_LVL1_IRQ,
> +					 IRQF_ONESHOT,
> +					 &bxtwc_regmap_irq_chip_chgr,
> +					 &pmic->irq_chip_data_chgr,
> +					 &pmic->chgr_irq);
> +
> +
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to add CHGR IRQ chip\n");
> +		goto err_irq_chip_chgr;
> +	}
> +
> +	/* add chained irq handler for CRIT irqs */
> +	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
> +					 BXTWC_CRIT_LVL1_IRQ,
> +					 IRQF_ONESHOT,
> +					 &bxtwc_regmap_irq_chip_crit,
> +					 &pmic->irq_chip_data_crit,
> +					 &pmic->crit_irq);
> +
> +
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to add CRIT irq chip\n");

s/irq/IRQ/

> +		goto err_irq_chip_crit;
>  	}
>  
>  	ret = mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE, bxt_wc_dev,
> @@ -456,10 +577,16 @@ static int bxtwc_probe(struct platform_device *pdev)
>  err_sysfs:
>  	mfd_remove_devices(&pdev->dev);
>  err_mfd:
> -	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_tmu);
> +	regmap_del_irq_chip(pmic->crit_irq, pmic->irq_chip_data_crit);
> +err_irq_chip_crit:
> +	regmap_del_irq_chip(pmic->chgr_irq, pmic->irq_chip_data_chgr);
> +err_irq_chip_chgr:
> +	regmap_del_irq_chip(pmic->adc_irq, pmic->irq_chip_data_adc);
> +err_irq_chip_adc:
> +	regmap_del_irq_chip(pmic->bcu_irq, pmic->irq_chip_data_bcu);
> +err_irq_chip_bcu:
> +	regmap_del_irq_chip(pmic->tmu_irq, pmic->irq_chip_data_tmu);
>  err_irq_chip_tmu:
> -	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_level2);
> -err_irq_chip_level2:
>  	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
>  
>  	return ret;
> @@ -472,8 +599,11 @@ static int bxtwc_remove(struct platform_device *pdev)
>  	sysfs_remove_group(&pdev->dev.kobj, &bxtwc_group);
>  	mfd_remove_devices(&pdev->dev);
>  	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
> -	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_level2);
> -	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_tmu);
> +	regmap_del_irq_chip(pmic->tmu_irq, pmic->irq_chip_data_tmu);
> +	regmap_del_irq_chip(pmic->bcu_irq, pmic->irq_chip_data_bcu);
> +	regmap_del_irq_chip(pmic->adc_irq, pmic->irq_chip_data_adc);
> +	regmap_del_irq_chip(pmic->chgr_irq, pmic->irq_chip_data_chgr);
> +	regmap_del_irq_chip(pmic->crit_irq, pmic->irq_chip_data_crit);
>  
>  	return 0;
>  }
> diff --git a/include/linux/mfd/intel_soc_pmic.h b/include/linux/mfd/intel_soc_pmic.h
> index 956caa0..63e1270 100644
> --- a/include/linux/mfd/intel_soc_pmic.h
> +++ b/include/linux/mfd/intel_soc_pmic.h
> @@ -23,10 +23,15 @@
>  
>  struct intel_soc_pmic {
>  	int irq;
> +	int tmu_irq, bcu_irq, adc_irq, chgr_irq, crit_irq;

Each attribute should be on a line of their own.

>  	struct regmap *regmap;
>  	struct regmap_irq_chip_data *irq_chip_data;
>  	struct regmap_irq_chip_data *irq_chip_data_level2;
>  	struct regmap_irq_chip_data *irq_chip_data_tmu;
> +	struct regmap_irq_chip_data *irq_chip_data_bcu;
> +	struct regmap_irq_chip_data *irq_chip_data_adc;
> +	struct regmap_irq_chip_data *irq_chip_data_chgr;
> +	struct regmap_irq_chip_data *irq_chip_data_crit;
>  	struct device *dev;
>  };
>  

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

Re: [PATCH v1 1/7] mfd: intel_soc_pmic_bxtwc: fix TMU interrupt index

[Sathyanarayanan Kuppuswamy Natarajan]

Hi Lee,

Thanks. Will remove the code segment in next version.

On Wed, Apr 12, 2017 at 3:45 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Mon, 10 Apr 2017, sathyanarayanan.kuppuswamy@linux.intel.com wrote:
>
>> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>>
>> TMU interrupts are registered as a separate interrupt chip, and
>> hence it should start its interrupt index(BXTWC_TMU_IRQ) number
>> from 0. But currently, BXTWC_TMU_IRQ is defined as part of enum
>> bxtwc_irqs_level2 and its index value is 11. Since this index
>> value is used when calculating .num_irqs of regmap_irq_chip_tmu,
>> it incorrectly reports number of irqs as 12 instead of actual
>> value of 1.
>>
>> static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = {
>>       REGMAP_IRQ_REG(BXTWC_TMU_IRQ, 0, 0x06),
>> };
>>
>> static struct regmap_irq_chip bxtwc_regmap_irq_chip_tmu = {
>>       .name = "bxtwc_irq_chip_tmu",
>>       .status_base = BXTWC_TMUIRQ,
>>       .mask_base = BXTWC_MTMUIRQ,
>>       .irqs = bxtwc_regmap_irqs_tmu,
>>       .num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_tmu),
>>       .num_regs = 1,
>> };
>>
>> This patch fixes this issue by creating new enum of tmu irqs and
>> resetting its starting index to 0.
>>
>> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>> ---
>>  drivers/mfd/intel_soc_pmic_bxtwc.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> Patch looks fine, but please remove the code segment from the commit
> log.
>
> For the code:
>
> For my own reference:
>   Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
>
>> diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
>> index 699c8c7..bb18e20 100644
>> --- a/drivers/mfd/intel_soc_pmic_bxtwc.c
>> +++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
>> @@ -94,7 +94,10 @@ enum bxtwc_irqs_level2 {
>>       BXTWC_GPIO0_IRQ,
>>       BXTWC_GPIO1_IRQ,
>>       BXTWC_CRIT_IRQ,
>> -     BXTWC_TMU_IRQ,
>> +};
>> +
>> +enum bxtwc_irqs_tmu {
>> +     BXTWC_TMU_IRQ = 0,
>>  };
>>
>>  static const struct regmap_irq bxtwc_regmap_irqs[] = {
>
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog



-- 
Sathya

Re: [PATCH v1 1/7] mfd: intel_soc_pmic_bxtwc: fix TMU interrupt index

[Sathyanarayanan Kuppuswamy Natarajan]

Hi Lee,

Thanks. Will remove the code segment in next version.

On Wed, Apr 12, 2017 at 3:45 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Mon, 10 Apr 2017, sathyanarayanan.kuppuswamy@linux.intel.com wrote:
>
>> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>>
>> TMU interrupts are registered as a separate interrupt chip, and
>> hence it should start its interrupt index(BXTWC_TMU_IRQ) number
>> from 0. But currently, BXTWC_TMU_IRQ is defined as part of enum
>> bxtwc_irqs_level2 and its index value is 11. Since this index
>> value is used when calculating .num_irqs of regmap_irq_chip_tmu,
>> it incorrectly reports number of irqs as 12 instead of actual
>> value of 1.
>>
>> static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = {
>>       REGMAP_IRQ_REG(BXTWC_TMU_IRQ, 0, 0x06),
>> };
>>
>> static struct regmap_irq_chip bxtwc_regmap_irq_chip_tmu = {
>>       .name = "bxtwc_irq_chip_tmu",
>>       .status_base = BXTWC_TMUIRQ,
>>       .mask_base = BXTWC_MTMUIRQ,
>>       .irqs = bxtwc_regmap_irqs_tmu,
>>       .num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_tmu),
>>       .num_regs = 1,
>> };
>>
>> This patch fixes this issue by creating new enum of tmu irqs and
>> resetting its starting index to 0.
>>
>> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>> ---
>>  drivers/mfd/intel_soc_pmic_bxtwc.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> Patch looks fine, but please remove the code segment from the commit
> log.
>
> For the code:
>
> For my own reference:
>   Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
>
>> diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
>> index 699c8c7..bb18e20 100644
>> --- a/drivers/mfd/intel_soc_pmic_bxtwc.c
>> +++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
>> @@ -94,7 +94,10 @@ enum bxtwc_irqs_level2 {
>>       BXTWC_GPIO0_IRQ,
>>       BXTWC_GPIO1_IRQ,
>>       BXTWC_CRIT_IRQ,
>> -     BXTWC_TMU_IRQ,
>> +};
>> +
>> +enum bxtwc_irqs_tmu {
>> +     BXTWC_TMU_IRQ = 0,
>>  };
>>
>>  static const struct regmap_irq bxtwc_regmap_irqs[] = {
>
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog



-- 
Sathya

Re: [PATCH v1 1/7] mfd: intel_soc_pmic_bxtwc: fix TMU interrupt index

[Lee Jones]

On Wed, 12 Apr 2017, Sathyanarayanan Kuppuswamy Natarajan wrote:

> Hi Lee,
> 
> Thanks. Will remove the code segment in next version.

Please always reply inline.  Top posting is frowned upon.

> On Wed, Apr 12, 2017 at 3:45 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Mon, 10 Apr 2017, sathyanarayanan.kuppuswamy@linux.intel.com wrote:
> >
> >> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> >>
> >> TMU interrupts are registered as a separate interrupt chip, and
> >> hence it should start its interrupt index(BXTWC_TMU_IRQ) number
> >> from 0. But currently, BXTWC_TMU_IRQ is defined as part of enum
> >> bxtwc_irqs_level2 and its index value is 11. Since this index
> >> value is used when calculating .num_irqs of regmap_irq_chip_tmu,
> >> it incorrectly reports number of irqs as 12 instead of actual
> >> value of 1.
> >>
> >> static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = {
> >>       REGMAP_IRQ_REG(BXTWC_TMU_IRQ, 0, 0x06),
> >> };
> >>
> >> static struct regmap_irq_chip bxtwc_regmap_irq_chip_tmu = {
> >>       .name = "bxtwc_irq_chip_tmu",
> >>       .status_base = BXTWC_TMUIRQ,
> >>       .mask_base = BXTWC_MTMUIRQ,
> >>       .irqs = bxtwc_regmap_irqs_tmu,
> >>       .num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_tmu),
> >>       .num_regs = 1,
> >> };
> >>
> >> This patch fixes this issue by creating new enum of tmu irqs and
> >> resetting its starting index to 0.
> >>
> >> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> >> ---
> >>  drivers/mfd/intel_soc_pmic_bxtwc.c | 5 ++++-
> >>  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > Patch looks fine, but please remove the code segment from the commit
> > log.
> >
> > For the code:
> >
> > For my own reference:
> >   Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> >
> >> diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
> >> index 699c8c7..bb18e20 100644
> >> --- a/drivers/mfd/intel_soc_pmic_bxtwc.c
> >> +++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
> >> @@ -94,7 +94,10 @@ enum bxtwc_irqs_level2 {
> >>       BXTWC_GPIO0_IRQ,
> >>       BXTWC_GPIO1_IRQ,
> >>       BXTWC_CRIT_IRQ,
> >> -     BXTWC_TMU_IRQ,
> >> +};
> >> +
> >> +enum bxtwc_irqs_tmu {
> >> +     BXTWC_TMU_IRQ = 0,
> >>  };
> >>
> >>  static const struct regmap_irq bxtwc_regmap_irqs[] = {
> >
> 
> 
> 

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

Re: [PATCH v1 1/7] mfd: intel_soc_pmic_bxtwc: fix TMU interrupt index

[Lee Jones]

On Wed, 12 Apr 2017, Sathyanarayanan Kuppuswamy Natarajan wrote:

> Hi Lee,
> 
> Thanks. Will remove the code segment in next version.

Please always reply inline.  Top posting is frowned upon.

> On Wed, Apr 12, 2017 at 3:45 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Mon, 10 Apr 2017, sathyanarayanan.kuppuswamy@linux.intel.com wrote:
> >
> >> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> >>
> >> TMU interrupts are registered as a separate interrupt chip, and
> >> hence it should start its interrupt index(BXTWC_TMU_IRQ) number
> >> from 0. But currently, BXTWC_TMU_IRQ is defined as part of enum
> >> bxtwc_irqs_level2 and its index value is 11. Since this index
> >> value is used when calculating .num_irqs of regmap_irq_chip_tmu,
> >> it incorrectly reports number of irqs as 12 instead of actual
> >> value of 1.
> >>
> >> static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = {
> >>       REGMAP_IRQ_REG(BXTWC_TMU_IRQ, 0, 0x06),
> >> };
> >>
> >> static struct regmap_irq_chip bxtwc_regmap_irq_chip_tmu = {
> >>       .name = "bxtwc_irq_chip_tmu",
> >>       .status_base = BXTWC_TMUIRQ,
> >>       .mask_base = BXTWC_MTMUIRQ,
> >>       .irqs = bxtwc_regmap_irqs_tmu,
> >>       .num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_tmu),
> >>       .num_regs = 1,
> >> };
> >>
> >> This patch fixes this issue by creating new enum of tmu irqs and
> >> resetting its starting index to 0.
> >>
> >> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> >> ---
> >>  drivers/mfd/intel_soc_pmic_bxtwc.c | 5 ++++-
> >>  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > Patch looks fine, but please remove the code segment from the commit
> > log.
> >
> > For the code:
> >
> > For my own reference:
> >   Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> >
> >> diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
> >> index 699c8c7..bb18e20 100644
> >> --- a/drivers/mfd/intel_soc_pmic_bxtwc.c
> >> +++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
> >> @@ -94,7 +94,10 @@ enum bxtwc_irqs_level2 {
> >>       BXTWC_GPIO0_IRQ,
> >>       BXTWC_GPIO1_IRQ,
> >>       BXTWC_CRIT_IRQ,
> >> -     BXTWC_TMU_IRQ,
> >> +};
> >> +
> >> +enum bxtwc_irqs_tmu {
> >> +     BXTWC_TMU_IRQ = 0,
> >>  };
> >>
> >>  static const struct regmap_irq bxtwc_regmap_irqs[] = {
> >
> 
> 
> 

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

Re: [PATCH v1 6/7] mfd: intel_soc_pmic_bxtwc: use chained irqs for second level irq chips

[sathyanarayanan kuppuswamy]

On 04/12/2017 04:53 AM, Lee Jones wrote:
> On Mon, 10 Apr 2017, sathyanarayanan.kuppuswamy@linux.intel.com wrote:
>
>> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>>
>> Whishkey cove PMIC has support to mask/unmask interrupts at two levels.
>> At first level we can mask/unmask interrupt domains like TMU, GPIO, ADC,
>> CHGR, BCU THERMAL and PWRBTN and at second level, it provides facility
>> to mask/unmask individual interrupts belong each of this domain. For
>> example, in case of TMU, at first level we have TMU interrupt domain,
>> and at second level we have two interrupts, wake alarm, system alarm that
>> belong to the TMU interrupt domain.
>>
>> Currently, in this driver all first level irqs are registered as part of
>> irq chip(bxtwc_regmap_irq_chip). By default, after you register the irq
>> chip from your driver, all irqs in that chip will masked and can only be
>> enabled if that irq is requested using request_irq call. This is the
>> default Linux irq behavior model. And whenever a dependent device that
>> belongs to PMIC requests only the second level irq and not explicitly
>> unmask the first level irq, then in essence the second level irq will
>> still be disabled. For example, if TMU device driver request wake_alarm
>> irq and not explicitly unmask TMU level 1 irq then according to the default
>> Linux irq model,  wake_alarm irq will still be disabled. So the proper
>> solution to fix this issue is to use the chained irq chip concept. We
>> should chain all the second level chip irqs to the corresponding first
>> level irq. To do this, we need to create separate irq chips for every
>> group of second level irqs.
>>
>> In case of TMU, when adding second level irq chip, instead of using pmic
>> irq we should use the corresponding first level irq. So the following
>> code will change from
>>
>> ret = regmap_add_irq_chip(pmic->regmap, pmic->irq, ...)
>>
>> to,
>>
>> virq = regmap_irq_get_virq(&pmic->irq_chip_data, BXTWC_TMU_LVL1_IRQ);
>>
>> ret = regmap_add_irq_chip(pmic->regmap, virq, ...)
>>
>> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>> ---
>>   drivers/mfd/intel_soc_pmic_bxtwc.c | 212 ++++++++++++++++++++++++++++++-------
>>   include/linux/mfd/intel_soc_pmic.h |   5 +
>>   2 files changed, 176 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
>> index dc8af1d..807eba3 100644
>> --- a/drivers/mfd/intel_soc_pmic_bxtwc.c
>> +++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
>> @@ -81,18 +81,26 @@ enum bxtwc_irqs {
>>   	BXTWC_PWRBTN_IRQ,
>>   };
>>   
>> -enum bxtwc_irqs_level2 {
>> -	/* Level 2 */
>> +enum bxtwc_irqs_tmu {
>> +	BXTWC_TMU_IRQ = 0,
>> +};
>> +
>> +enum bxtwc_irqs_bcu {
>>   	BXTWC_BCU_IRQ = 0,
>> -	BXTWC_ADC_IRQ,
>> -	BXTWC_USBC_IRQ,
>> +};
>> +
>> +enum bxtwc_irqs_adc {
>> +	BXTWC_ADC_IRQ = 0,
>> +};
>> +
>> +enum bxtwc_irqs_chgr {
>> +	BXTWC_USBC_IRQ = 0,
>>   	BXTWC_CHGR0_IRQ,
>>   	BXTWC_CHGR1_IRQ,
>> -	BXTWC_CRIT_IRQ,
>>   };
>>   
>> -enum bxtwc_irqs_tmu {
>> -	BXTWC_TMU_IRQ = 0,
>> +enum bxtwc_irqs_crit {
>> +	BXTWC_CRIT_IRQ = 0,
>>   };
>>   
>>   static const struct regmap_irq bxtwc_regmap_irqs[] = {
>> @@ -107,17 +115,26 @@ static const struct regmap_irq bxtwc_regmap_irqs[] = {
>>   	REGMAP_IRQ_REG(BXTWC_PWRBTN_IRQ, 1, 0x03),
>>   };
>>   
>> -static const struct regmap_irq bxtwc_regmap_irqs_level2[] = {
>> +static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = {
>> +	REGMAP_IRQ_REG(BXTWC_TMU_IRQ, 0, 0x06),
>> +};
>> +
>> +static const struct regmap_irq bxtwc_regmap_irqs_bcu[] = {
>>   	REGMAP_IRQ_REG(BXTWC_BCU_IRQ, 0, 0x1f),
>> -	REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 1, 0xff),
>> -	REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 2, BIT(5)),
>> -	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 2, 0x1f),
>> -	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 3, 0x1f),
>> -	REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 6, 0x03),
>>   };
>>   
>> -static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = {
>> -	REGMAP_IRQ_REG(BXTWC_TMU_IRQ, 0, 0x06),
>> +static const struct regmap_irq bxtwc_regmap_irqs_adc[] = {
>> +	REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 0, 0xff),
>> +};
>> +
>> +static const struct regmap_irq bxtwc_regmap_irqs_chgr[] = {
>> +	REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 0, BIT(5)),
>> +	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 0, 0x1f),
>> +	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 1, 0x1f),
>> +};
>> +
>> +static const struct regmap_irq bxtwc_regmap_irqs_crit[] = {
>> +	REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 0, 0x03),
>>   };
>>   
>>   static struct regmap_irq_chip bxtwc_regmap_irq_chip = {
>> @@ -129,15 +146,6 @@ static struct regmap_irq_chip bxtwc_regmap_irq_chip = {
>>   	.num_regs = 2,
>>   };
>>   
>> -static struct regmap_irq_chip bxtwc_regmap_irq_chip_level2 = {
>> -	.name = "bxtwc_irq_chip_level2",
>> -	.status_base = BXTWC_BCUIRQ,
>> -	.mask_base = BXTWC_MBCUIRQ,
>> -	.irqs = bxtwc_regmap_irqs_level2,
>> -	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_level2),
>> -	.num_regs = 10,
>> -};
>> -
>>   static struct regmap_irq_chip bxtwc_regmap_irq_chip_tmu = {
>>   	.name = "bxtwc_irq_chip_tmu",
>>   	.status_base = BXTWC_TMUIRQ,
>> @@ -147,6 +155,42 @@ static struct regmap_irq_chip bxtwc_regmap_irq_chip_tmu = {
>>   	.num_regs = 1,
>>   };
>>   
>> +static struct regmap_irq_chip bxtwc_regmap_irq_chip_bcu = {
>> +	.name = "bxtwc_irq_chip_bcu",
>> +	.status_base = BXTWC_BCUIRQ,
>> +	.mask_base = BXTWC_MBCUIRQ,
>> +	.irqs = bxtwc_regmap_irqs_bcu,
>> +	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_bcu),
>> +	.num_regs = 1,
>> +};
>> +
>> +static struct regmap_irq_chip bxtwc_regmap_irq_chip_adc = {
>> +	.name = "bxtwc_irq_chip_adc",
>> +	.status_base = BXTWC_ADCIRQ,
>> +	.mask_base = BXTWC_MADCIRQ,
>> +	.irqs = bxtwc_regmap_irqs_adc,
>> +	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_adc),
>> +	.num_regs = 1,
>> +};
>> +
>> +static struct regmap_irq_chip bxtwc_regmap_irq_chip_chgr = {
>> +	.name = "bxtwc_irq_chip_chgr",
>> +	.status_base = BXTWC_CHGR0IRQ,
>> +	.mask_base = BXTWC_MCHGR0IRQ,
>> +	.irqs = bxtwc_regmap_irqs_chgr,
>> +	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_chgr),
>> +	.num_regs = 2,
>> +};
>> +
>> +static struct regmap_irq_chip bxtwc_regmap_irq_chip_crit = {
>> +	.name = "bxtwc_irq_chip_crit",
>> +	.status_base = BXTWC_CRITIRQ,
>> +	.mask_base = BXTWC_MCRITIRQ,
>> +	.irqs = bxtwc_regmap_irqs_crit,
>> +	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_crit),
>> +	.num_regs = 1,
>> +};
>> +
>>   static struct resource gpio_resources[] = {
>>   	DEFINE_RES_IRQ_NAMED(BXTWC_GPIO_LVL1_IRQ, "GPIO"),
>>   };
>> @@ -358,6 +402,34 @@ static const struct regmap_config bxtwc_regmap_config = {
>>   	.reg_read = regmap_ipc_byte_reg_read,
>>   };
>>   
>> +static int bxtwc_add_chained_irq_chip(struct intel_soc_pmic *pmic,
>> +				struct regmap_irq_chip_data *pdata,
>> +				int pirq,
>> +				int irq_flags,
> Nit: These do not need to be on separate lines.
Will fix it in next version.
>
>> +				const struct regmap_irq_chip *chip,
>> +				struct regmap_irq_chip_data **data,
>> +				int *irq)
>> +{
>> +	int ret;
>> +
>> +	ret = regmap_irq_get_virq(pdata, pirq);
>> +	if (ret < 0) {
>> +		dev_err(pmic->dev, "failed to get virtual interrupt=%d\n", ret);
> s/=/: /
Will fix it in next version.
>
>> +		return ret;
>> +	}
>> +
>> +	*irq = ret;
>> +
>> +	ret = regmap_add_irq_chip(pmic->regmap, *irq, irq_flags, 0,
>> +				  chip, data);
>> +	if (ret) {
>> +		dev_err(pmic->dev, "Failed to add %s irq chip\n", chip->name);
> s/irq/IRQ/
Will fix it in next version.
>
>> +		return -ENODEV;
> Why aren't you returning ret?
>
> In fact, remove this line and ...
>
>> +	}
>> +
>> +	return 0;
> ... return ret;
>
>> +}
>> +
>>   static int bxtwc_probe(struct platform_device *pdev)
>>   {
>>   	int ret;
>> @@ -409,22 +481,71 @@ static int bxtwc_probe(struct platform_device *pdev)
>>   		return ret;
>>   	}
>>   
>> -	ret = regmap_add_irq_chip(pmic->regmap, pmic->irq,
>> -				  IRQF_ONESHOT | IRQF_SHARED,
>> -				  0, &bxtwc_regmap_irq_chip_level2,
>> -				  &pmic->irq_chip_data_level2);
>> +	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
>> +					 BXTWC_TMU_LVL1_IRQ,
>> +					 IRQF_ONESHOT,
>> +					 &bxtwc_regmap_irq_chip_tmu,
>> +					 &pmic->irq_chip_data_tmu,
>> +					 &pmic->tmu_irq);
> Isn't there a generic API for chained IRQs already?
I don't think we have regmap add IRQ chip API for nested IRQ chips. May 
be we can create one now ? I think we have something similar in gpio 
domain (gpiochip_irqchip_add_nested).
>
>>   	if (ret) {
>> -		dev_err(&pdev->dev, "Failed to add secondary IRQ chip\n");
>> -		goto err_irq_chip_level2;
>> +		dev_err(&pdev->dev, "Failed to add TMU IRQ chip\n");
>> +		goto err_irq_chip_tmu;
>>   	}
>>   
>> -	ret = regmap_add_irq_chip(pmic->regmap, pmic->irq,
>> -				  IRQF_ONESHOT | IRQF_SHARED,
>> -				  0, &bxtwc_regmap_irq_chip_tmu,
>> -				  &pmic->irq_chip_data_tmu);
>> +	/* add chained irq handler for BCU irqs */
> Use proper grammar.
>
> "Add chained IRQ handler for BCU IRQs"
will fix it next version.
>
>> +	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
>> +					 BXTWC_BCU_LVL1_IRQ,
>> +					 IRQF_ONESHOT,
>> +					 &bxtwc_regmap_irq_chip_bcu,
>> +					 &pmic->irq_chip_data_bcu,
>> +					 &pmic->bcu_irq);
>> +
>> +
>>   	if (ret) {
>> -		dev_err(&pdev->dev, "Failed to add TMU IRQ chip\n");
>> -		goto err_irq_chip_tmu;
>> +		dev_err(&pdev->dev, "Failed to add BUC IRQ chip\n");
>> +		goto err_irq_chip_bcu;
>> +	}
>> +
>> +	/* add chained irq handler for ADC irqs */
> Grammar.
will fix it next version.
>
>> +	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
>> +					 BXTWC_ADC_LVL1_IRQ,
>> +					 IRQF_ONESHOT,
>> +					 &bxtwc_regmap_irq_chip_adc,
>> +					 &pmic->irq_chip_data_adc,
>> +					 &pmic->adc_irq);
>> +
>> +
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "Failed to add ADC IRQ chip\n");
>> +		goto err_irq_chip_adc;
>> +	}
>> +
>> +	/* add chained irq handler for CHGR irqs */
>> +	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
>> +					 BXTWC_CHGR_LVL1_IRQ,
>> +					 IRQF_ONESHOT,
>> +					 &bxtwc_regmap_irq_chip_chgr,
>> +					 &pmic->irq_chip_data_chgr,
>> +					 &pmic->chgr_irq);
>> +
>> +
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "Failed to add CHGR IRQ chip\n");
>> +		goto err_irq_chip_chgr;
>> +	}
>> +
>> +	/* add chained irq handler for CRIT irqs */
>> +	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
>> +					 BXTWC_CRIT_LVL1_IRQ,
>> +					 IRQF_ONESHOT,
>> +					 &bxtwc_regmap_irq_chip_crit,
>> +					 &pmic->irq_chip_data_crit,
>> +					 &pmic->crit_irq);
>> +
>> +
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "Failed to add CRIT irq chip\n");
> s/irq/IRQ/
will fix it next version.
>
>> +		goto err_irq_chip_crit;
>>   	}
>>   
>>   	ret = mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE, bxt_wc_dev,
>> @@ -456,10 +577,16 @@ static int bxtwc_probe(struct platform_device *pdev)
>>   err_sysfs:
>>   	mfd_remove_devices(&pdev->dev);
>>   err_mfd:
>> -	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_tmu);
>> +	regmap_del_irq_chip(pmic->crit_irq, pmic->irq_chip_data_crit);
>> +err_irq_chip_crit:
>> +	regmap_del_irq_chip(pmic->chgr_irq, pmic->irq_chip_data_chgr);
>> +err_irq_chip_chgr:
>> +	regmap_del_irq_chip(pmic->adc_irq, pmic->irq_chip_data_adc);
>> +err_irq_chip_adc:
>> +	regmap_del_irq_chip(pmic->bcu_irq, pmic->irq_chip_data_bcu);
>> +err_irq_chip_bcu:
>> +	regmap_del_irq_chip(pmic->tmu_irq, pmic->irq_chip_data_tmu);
>>   err_irq_chip_tmu:
>> -	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_level2);
>> -err_irq_chip_level2:
>>   	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
>>   
>>   	return ret;
>> @@ -472,8 +599,11 @@ static int bxtwc_remove(struct platform_device *pdev)
>>   	sysfs_remove_group(&pdev->dev.kobj, &bxtwc_group);
>>   	mfd_remove_devices(&pdev->dev);
>>   	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
>> -	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_level2);
>> -	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_tmu);
>> +	regmap_del_irq_chip(pmic->tmu_irq, pmic->irq_chip_data_tmu);
>> +	regmap_del_irq_chip(pmic->bcu_irq, pmic->irq_chip_data_bcu);
>> +	regmap_del_irq_chip(pmic->adc_irq, pmic->irq_chip_data_adc);
>> +	regmap_del_irq_chip(pmic->chgr_irq, pmic->irq_chip_data_chgr);
>> +	regmap_del_irq_chip(pmic->crit_irq, pmic->irq_chip_data_crit);
>>   
>>   	return 0;
>>   }
>> diff --git a/include/linux/mfd/intel_soc_pmic.h b/include/linux/mfd/intel_soc_pmic.h
>> index 956caa0..63e1270 100644
>> --- a/include/linux/mfd/intel_soc_pmic.h
>> +++ b/include/linux/mfd/intel_soc_pmic.h
>> @@ -23,10 +23,15 @@
>>   
>>   struct intel_soc_pmic {
>>   	int irq;
>> +	int tmu_irq, bcu_irq, adc_irq, chgr_irq, crit_irq;
> Each attribute should be on a line of their own.
ok. will fix it next version.
>
>>   	struct regmap *regmap;
>>   	struct regmap_irq_chip_data *irq_chip_data;
>>   	struct regmap_irq_chip_data *irq_chip_data_level2;
>>   	struct regmap_irq_chip_data *irq_chip_data_tmu;
>> +	struct regmap_irq_chip_data *irq_chip_data_bcu;
>> +	struct regmap_irq_chip_data *irq_chip_data_adc;
>> +	struct regmap_irq_chip_data *irq_chip_data_chgr;
>> +	struct regmap_irq_chip_data *irq_chip_data_crit;
>>   	struct device *dev;
>>   };
>>   

-- 
Sathyanarayanan Kuppuswamy
Android kernel developer

Re: [PATCH v1 6/7] mfd: intel_soc_pmic_bxtwc: use chained irqs for second level irq chips

[sathyanarayanan kuppuswamy]

On 04/12/2017 04:53 AM, Lee Jones wrote:
> On Mon, 10 Apr 2017, sathyanarayanan.kuppuswamy@linux.intel.com wrote:
>
>> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>>
>> Whishkey cove PMIC has support to mask/unmask interrupts at two levels.
>> At first level we can mask/unmask interrupt domains like TMU, GPIO, ADC,
>> CHGR, BCU THERMAL and PWRBTN and at second level, it provides facility
>> to mask/unmask individual interrupts belong each of this domain. For
>> example, in case of TMU, at first level we have TMU interrupt domain,
>> and at second level we have two interrupts, wake alarm, system alarm that
>> belong to the TMU interrupt domain.
>>
>> Currently, in this driver all first level irqs are registered as part of
>> irq chip(bxtwc_regmap_irq_chip). By default, after you register the irq
>> chip from your driver, all irqs in that chip will masked and can only be
>> enabled if that irq is requested using request_irq call. This is the
>> default Linux irq behavior model. And whenever a dependent device that
>> belongs to PMIC requests only the second level irq and not explicitly
>> unmask the first level irq, then in essence the second level irq will
>> still be disabled. For example, if TMU device driver request wake_alarm
>> irq and not explicitly unmask TMU level 1 irq then according to the default
>> Linux irq model,  wake_alarm irq will still be disabled. So the proper
>> solution to fix this issue is to use the chained irq chip concept. We
>> should chain all the second level chip irqs to the corresponding first
>> level irq. To do this, we need to create separate irq chips for every
>> group of second level irqs.
>>
>> In case of TMU, when adding second level irq chip, instead of using pmic
>> irq we should use the corresponding first level irq. So the following
>> code will change from
>>
>> ret = regmap_add_irq_chip(pmic->regmap, pmic->irq, ...)
>>
>> to,
>>
>> virq = regmap_irq_get_virq(&pmic->irq_chip_data, BXTWC_TMU_LVL1_IRQ);
>>
>> ret = regmap_add_irq_chip(pmic->regmap, virq, ...)
>>
>> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>> ---
>>   drivers/mfd/intel_soc_pmic_bxtwc.c | 212 ++++++++++++++++++++++++++++++-------
>>   include/linux/mfd/intel_soc_pmic.h |   5 +
>>   2 files changed, 176 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
>> index dc8af1d..807eba3 100644
>> --- a/drivers/mfd/intel_soc_pmic_bxtwc.c
>> +++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
>> @@ -81,18 +81,26 @@ enum bxtwc_irqs {
>>   	BXTWC_PWRBTN_IRQ,
>>   };
>>   
>> -enum bxtwc_irqs_level2 {
>> -	/* Level 2 */
>> +enum bxtwc_irqs_tmu {
>> +	BXTWC_TMU_IRQ = 0,
>> +};
>> +
>> +enum bxtwc_irqs_bcu {
>>   	BXTWC_BCU_IRQ = 0,
>> -	BXTWC_ADC_IRQ,
>> -	BXTWC_USBC_IRQ,
>> +};
>> +
>> +enum bxtwc_irqs_adc {
>> +	BXTWC_ADC_IRQ = 0,
>> +};
>> +
>> +enum bxtwc_irqs_chgr {
>> +	BXTWC_USBC_IRQ = 0,
>>   	BXTWC_CHGR0_IRQ,
>>   	BXTWC_CHGR1_IRQ,
>> -	BXTWC_CRIT_IRQ,
>>   };
>>   
>> -enum bxtwc_irqs_tmu {
>> -	BXTWC_TMU_IRQ = 0,
>> +enum bxtwc_irqs_crit {
>> +	BXTWC_CRIT_IRQ = 0,
>>   };
>>   
>>   static const struct regmap_irq bxtwc_regmap_irqs[] = {
>> @@ -107,17 +115,26 @@ static const struct regmap_irq bxtwc_regmap_irqs[] = {
>>   	REGMAP_IRQ_REG(BXTWC_PWRBTN_IRQ, 1, 0x03),
>>   };
>>   
>> -static const struct regmap_irq bxtwc_regmap_irqs_level2[] = {
>> +static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = {
>> +	REGMAP_IRQ_REG(BXTWC_TMU_IRQ, 0, 0x06),
>> +};
>> +
>> +static const struct regmap_irq bxtwc_regmap_irqs_bcu[] = {
>>   	REGMAP_IRQ_REG(BXTWC_BCU_IRQ, 0, 0x1f),
>> -	REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 1, 0xff),
>> -	REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 2, BIT(5)),
>> -	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 2, 0x1f),
>> -	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 3, 0x1f),
>> -	REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 6, 0x03),
>>   };
>>   
>> -static const struct regmap_irq bxtwc_regmap_irqs_tmu[] = {
>> -	REGMAP_IRQ_REG(BXTWC_TMU_IRQ, 0, 0x06),
>> +static const struct regmap_irq bxtwc_regmap_irqs_adc[] = {
>> +	REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 0, 0xff),
>> +};
>> +
>> +static const struct regmap_irq bxtwc_regmap_irqs_chgr[] = {
>> +	REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 0, BIT(5)),
>> +	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 0, 0x1f),
>> +	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 1, 0x1f),
>> +};
>> +
>> +static const struct regmap_irq bxtwc_regmap_irqs_crit[] = {
>> +	REGMAP_IRQ_REG(BXTWC_CRIT_IRQ, 0, 0x03),
>>   };
>>   
>>   static struct regmap_irq_chip bxtwc_regmap_irq_chip = {
>> @@ -129,15 +146,6 @@ static struct regmap_irq_chip bxtwc_regmap_irq_chip = {
>>   	.num_regs = 2,
>>   };
>>   
>> -static struct regmap_irq_chip bxtwc_regmap_irq_chip_level2 = {
>> -	.name = "bxtwc_irq_chip_level2",
>> -	.status_base = BXTWC_BCUIRQ,
>> -	.mask_base = BXTWC_MBCUIRQ,
>> -	.irqs = bxtwc_regmap_irqs_level2,
>> -	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_level2),
>> -	.num_regs = 10,
>> -};
>> -
>>   static struct regmap_irq_chip bxtwc_regmap_irq_chip_tmu = {
>>   	.name = "bxtwc_irq_chip_tmu",
>>   	.status_base = BXTWC_TMUIRQ,
>> @@ -147,6 +155,42 @@ static struct regmap_irq_chip bxtwc_regmap_irq_chip_tmu = {
>>   	.num_regs = 1,
>>   };
>>   
>> +static struct regmap_irq_chip bxtwc_regmap_irq_chip_bcu = {
>> +	.name = "bxtwc_irq_chip_bcu",
>> +	.status_base = BXTWC_BCUIRQ,
>> +	.mask_base = BXTWC_MBCUIRQ,
>> +	.irqs = bxtwc_regmap_irqs_bcu,
>> +	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_bcu),
>> +	.num_regs = 1,
>> +};
>> +
>> +static struct regmap_irq_chip bxtwc_regmap_irq_chip_adc = {
>> +	.name = "bxtwc_irq_chip_adc",
>> +	.status_base = BXTWC_ADCIRQ,
>> +	.mask_base = BXTWC_MADCIRQ,
>> +	.irqs = bxtwc_regmap_irqs_adc,
>> +	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_adc),
>> +	.num_regs = 1,
>> +};
>> +
>> +static struct regmap_irq_chip bxtwc_regmap_irq_chip_chgr = {
>> +	.name = "bxtwc_irq_chip_chgr",
>> +	.status_base = BXTWC_CHGR0IRQ,
>> +	.mask_base = BXTWC_MCHGR0IRQ,
>> +	.irqs = bxtwc_regmap_irqs_chgr,
>> +	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_chgr),
>> +	.num_regs = 2,
>> +};
>> +
>> +static struct regmap_irq_chip bxtwc_regmap_irq_chip_crit = {
>> +	.name = "bxtwc_irq_chip_crit",
>> +	.status_base = BXTWC_CRITIRQ,
>> +	.mask_base = BXTWC_MCRITIRQ,
>> +	.irqs = bxtwc_regmap_irqs_crit,
>> +	.num_irqs = ARRAY_SIZE(bxtwc_regmap_irqs_crit),
>> +	.num_regs = 1,
>> +};
>> +
>>   static struct resource gpio_resources[] = {
>>   	DEFINE_RES_IRQ_NAMED(BXTWC_GPIO_LVL1_IRQ, "GPIO"),
>>   };
>> @@ -358,6 +402,34 @@ static const struct regmap_config bxtwc_regmap_config = {
>>   	.reg_read = regmap_ipc_byte_reg_read,
>>   };
>>   
>> +static int bxtwc_add_chained_irq_chip(struct intel_soc_pmic *pmic,
>> +				struct regmap_irq_chip_data *pdata,
>> +				int pirq,
>> +				int irq_flags,
> Nit: These do not need to be on separate lines.
Will fix it in next version.
>
>> +				const struct regmap_irq_chip *chip,
>> +				struct regmap_irq_chip_data **data,
>> +				int *irq)
>> +{
>> +	int ret;
>> +
>> +	ret = regmap_irq_get_virq(pdata, pirq);
>> +	if (ret < 0) {
>> +		dev_err(pmic->dev, "failed to get virtual interrupt=%d\n", ret);
> s/=/: /
Will fix it in next version.
>
>> +		return ret;
>> +	}
>> +
>> +	*irq = ret;
>> +
>> +	ret = regmap_add_irq_chip(pmic->regmap, *irq, irq_flags, 0,
>> +				  chip, data);
>> +	if (ret) {
>> +		dev_err(pmic->dev, "Failed to add %s irq chip\n", chip->name);
> s/irq/IRQ/
Will fix it in next version.
>
>> +		return -ENODEV;
> Why aren't you returning ret?
>
> In fact, remove this line and ...
>
>> +	}
>> +
>> +	return 0;
> ... return ret;
>
>> +}
>> +
>>   static int bxtwc_probe(struct platform_device *pdev)
>>   {
>>   	int ret;
>> @@ -409,22 +481,71 @@ static int bxtwc_probe(struct platform_device *pdev)
>>   		return ret;
>>   	}
>>   
>> -	ret = regmap_add_irq_chip(pmic->regmap, pmic->irq,
>> -				  IRQF_ONESHOT | IRQF_SHARED,
>> -				  0, &bxtwc_regmap_irq_chip_level2,
>> -				  &pmic->irq_chip_data_level2);
>> +	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
>> +					 BXTWC_TMU_LVL1_IRQ,
>> +					 IRQF_ONESHOT,
>> +					 &bxtwc_regmap_irq_chip_tmu,
>> +					 &pmic->irq_chip_data_tmu,
>> +					 &pmic->tmu_irq);
> Isn't there a generic API for chained IRQs already?
I don't think we have regmap add IRQ chip API for nested IRQ chips. May 
be we can create one now ? I think we have something similar in gpio 
domain (gpiochip_irqchip_add_nested).
>
>>   	if (ret) {
>> -		dev_err(&pdev->dev, "Failed to add secondary IRQ chip\n");
>> -		goto err_irq_chip_level2;
>> +		dev_err(&pdev->dev, "Failed to add TMU IRQ chip\n");
>> +		goto err_irq_chip_tmu;
>>   	}
>>   
>> -	ret = regmap_add_irq_chip(pmic->regmap, pmic->irq,
>> -				  IRQF_ONESHOT | IRQF_SHARED,
>> -				  0, &bxtwc_regmap_irq_chip_tmu,
>> -				  &pmic->irq_chip_data_tmu);
>> +	/* add chained irq handler for BCU irqs */
> Use proper grammar.
>
> "Add chained IRQ handler for BCU IRQs"
will fix it next version.
>
>> +	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
>> +					 BXTWC_BCU_LVL1_IRQ,
>> +					 IRQF_ONESHOT,
>> +					 &bxtwc_regmap_irq_chip_bcu,
>> +					 &pmic->irq_chip_data_bcu,
>> +					 &pmic->bcu_irq);
>> +
>> +
>>   	if (ret) {
>> -		dev_err(&pdev->dev, "Failed to add TMU IRQ chip\n");
>> -		goto err_irq_chip_tmu;
>> +		dev_err(&pdev->dev, "Failed to add BUC IRQ chip\n");
>> +		goto err_irq_chip_bcu;
>> +	}
>> +
>> +	/* add chained irq handler for ADC irqs */
> Grammar.
will fix it next version.
>
>> +	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
>> +					 BXTWC_ADC_LVL1_IRQ,
>> +					 IRQF_ONESHOT,
>> +					 &bxtwc_regmap_irq_chip_adc,
>> +					 &pmic->irq_chip_data_adc,
>> +					 &pmic->adc_irq);
>> +
>> +
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "Failed to add ADC IRQ chip\n");
>> +		goto err_irq_chip_adc;
>> +	}
>> +
>> +	/* add chained irq handler for CHGR irqs */
>> +	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
>> +					 BXTWC_CHGR_LVL1_IRQ,
>> +					 IRQF_ONESHOT,
>> +					 &bxtwc_regmap_irq_chip_chgr,
>> +					 &pmic->irq_chip_data_chgr,
>> +					 &pmic->chgr_irq);
>> +
>> +
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "Failed to add CHGR IRQ chip\n");
>> +		goto err_irq_chip_chgr;
>> +	}
>> +
>> +	/* add chained irq handler for CRIT irqs */
>> +	ret = bxtwc_add_chained_irq_chip(pmic, pmic->irq_chip_data,
>> +					 BXTWC_CRIT_LVL1_IRQ,
>> +					 IRQF_ONESHOT,
>> +					 &bxtwc_regmap_irq_chip_crit,
>> +					 &pmic->irq_chip_data_crit,
>> +					 &pmic->crit_irq);
>> +
>> +
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "Failed to add CRIT irq chip\n");
> s/irq/IRQ/
will fix it next version.
>
>> +		goto err_irq_chip_crit;
>>   	}
>>   
>>   	ret = mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE, bxt_wc_dev,
>> @@ -456,10 +577,16 @@ static int bxtwc_probe(struct platform_device *pdev)
>>   err_sysfs:
>>   	mfd_remove_devices(&pdev->dev);
>>   err_mfd:
>> -	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_tmu);
>> +	regmap_del_irq_chip(pmic->crit_irq, pmic->irq_chip_data_crit);
>> +err_irq_chip_crit:
>> +	regmap_del_irq_chip(pmic->chgr_irq, pmic->irq_chip_data_chgr);
>> +err_irq_chip_chgr:
>> +	regmap_del_irq_chip(pmic->adc_irq, pmic->irq_chip_data_adc);
>> +err_irq_chip_adc:
>> +	regmap_del_irq_chip(pmic->bcu_irq, pmic->irq_chip_data_bcu);
>> +err_irq_chip_bcu:
>> +	regmap_del_irq_chip(pmic->tmu_irq, pmic->irq_chip_data_tmu);
>>   err_irq_chip_tmu:
>> -	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_level2);
>> -err_irq_chip_level2:
>>   	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
>>   
>>   	return ret;
>> @@ -472,8 +599,11 @@ static int bxtwc_remove(struct platform_device *pdev)
>>   	sysfs_remove_group(&pdev->dev.kobj, &bxtwc_group);
>>   	mfd_remove_devices(&pdev->dev);
>>   	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data);
>> -	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_level2);
>> -	regmap_del_irq_chip(pmic->irq, pmic->irq_chip_data_tmu);
>> +	regmap_del_irq_chip(pmic->tmu_irq, pmic->irq_chip_data_tmu);
>> +	regmap_del_irq_chip(pmic->bcu_irq, pmic->irq_chip_data_bcu);
>> +	regmap_del_irq_chip(pmic->adc_irq, pmic->irq_chip_data_adc);
>> +	regmap_del_irq_chip(pmic->chgr_irq, pmic->irq_chip_data_chgr);
>> +	regmap_del_irq_chip(pmic->crit_irq, pmic->irq_chip_data_crit);
>>   
>>   	return 0;
>>   }
>> diff --git a/include/linux/mfd/intel_soc_pmic.h b/include/linux/mfd/intel_soc_pmic.h
>> index 956caa0..63e1270 100644
>> --- a/include/linux/mfd/intel_soc_pmic.h
>> +++ b/include/linux/mfd/intel_soc_pmic.h
>> @@ -23,10 +23,15 @@
>>   
>>   struct intel_soc_pmic {
>>   	int irq;
>> +	int tmu_irq, bcu_irq, adc_irq, chgr_irq, crit_irq;
> Each attribute should be on a line of their own.
ok. will fix it next version.
>
>>   	struct regmap *regmap;
>>   	struct regmap_irq_chip_data *irq_chip_data;
>>   	struct regmap_irq_chip_data *irq_chip_data_level2;
>>   	struct regmap_irq_chip_data *irq_chip_data_tmu;
>> +	struct regmap_irq_chip_data *irq_chip_data_bcu;
>> +	struct regmap_irq_chip_data *irq_chip_data_adc;
>> +	struct regmap_irq_chip_data *irq_chip_data_chgr;
>> +	struct regmap_irq_chip_data *irq_chip_data_crit;
>>   	struct device *dev;
>>   };
>>   

-- 
Sathyanarayanan Kuppuswamy
Android kernel developer