Re: [PATCH 2/3] mfd: max8997: handle IRQs using regmap
From: Krzysztof Kozlowski <hidden>
Date: 2014-03-11 14:43:30
Also in:
linux-leds, lkml
On Tue, 2014-03-11 at 14:58 +0100, Robert Baldyga wrote:
This patch modifies mfd driver to use regmap for handling interrupts. It allows to simplify irq handling process. This modifications needed to make small changes in function drivers, which use interrupts. Signed-off-by: Robert Baldyga <redacted> --- drivers/extcon/extcon-max8997.c | 3 +- drivers/mfd/Makefile | 2 +- drivers/mfd/max8997-irq.c | 373 ----------------------------------- drivers/mfd/max8997.c | 91 ++++++++- drivers/rtc/rtc-max8997.c | 2 +- include/linux/mfd/max8997-private.h | 63 +++++- 6 files changed, 147 insertions(+), 387 deletions(-) delete mode 100644 drivers/mfd/max8997-irq.c
Add "select REGMAP_IRQ" in Kconfig. (...)
quoted hunk ↗ jump to hunk
diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c index ca6f310..9b6edfa 100644 --- a/drivers/mfd/max8997.c +++ b/drivers/mfd/max8997.c@@ -64,6 +64,49 @@ static const struct regmap_config max8997_regmap_config = { .max_register = MAX8997_REG_PMIC_END, }; +static const struct regmap_irq max8997_irqs[] = { + /* PMIC_INT1 interrupts */ + { .reg_offset = 0, .mask = PMIC_INT1_PWRONR_MASK, }, + { .reg_offset = 0, .mask = PMIC_INT1_PWRONF_MASK, }, + { .reg_offset = 0, .mask = PMIC_INT1_PWRON1SEC_MASK, }, + { .reg_offset = 0, .mask = PMIC_INT1_JIGONR_MASK, }, + { .reg_offset = 0, .mask = PMIC_INT1_JIGONF_MASK, }, + { .reg_offset = 0, .mask = PMIC_INT1_LOWBAT2_MASK, }, + { .reg_offset = 0, .mask = PMIC_INT1_LOWBAT1_MASK, }, + /* PMIC_INT2 interrupts */ + { .reg_offset = 1, .mask = PMIC_INT2_JIGR_MASK, }, + { .reg_offset = 1, .mask = PMIC_INT2_JIGF_MASK, }, + { .reg_offset = 1, .mask = PMIC_INT2_MR_MASK, }, + { .reg_offset = 1, .mask = PMIC_INT2_DVS1OK_MASK, }, + { .reg_offset = 1, .mask = PMIC_INT2_DVS2OK_MASK, }, + { .reg_offset = 1, .mask = PMIC_INT2_DVS3OK_MASK, }, + { .reg_offset = 1, .mask = PMIC_INT2_DVS4OK_MASK, }, + /* PMIC_INT3 interrupts */ + { .reg_offset = 2, .mask = PMIC_INT3_CHGINS_MASK, }, + { .reg_offset = 2, .mask = PMIC_INT3_CHGRM_MASK, }, + { .reg_offset = 2, .mask = PMIC_INT3_DCINOVP_MASK, }, + { .reg_offset = 2, .mask = PMIC_INT3_TOPOFFR_MASK, }, + { .reg_offset = 2, .mask = PMIC_INT3_CHGRSTF_MASK, }, + { .reg_offset = 2, .mask = PMIC_INT3_MBCHGTMEXPD_MASK, }, + /* PMIC_INT4 interrupts */ + { .reg_offset = 3, .mask = PMIC_INT4_RTC60S_MASK, }, + { .reg_offset = 3, .mask = PMIC_INT4_RTCA1_MASK, }, + { .reg_offset = 3, .mask = PMIC_INT4_RTCA2_MASK, }, + { .reg_offset = 3, .mask = PMIC_INT4_SMPL_INT_MASK, }, + { .reg_offset = 3, .mask = PMIC_INT4_RTC1S_MASK, }, + { .reg_offset = 3, .mask = PMIC_INT4_WTSR_MASK, }, +}; + +static const struct regmap_irq_chip max8997_irq_chip = { + .name = "max8997", + .status_base = MAX8997_REG_INT1, + .mask_base = MAX8997_REG_INT1MSK, + .mask_invert = false, + .num_regs = 4, + .irqs = max8997_irqs, + .num_irqs = ARRAY_SIZE(max8997_irqs), +}; + static const struct regmap_config max8997_regmap_rtc_config = { .reg_bits = 8, .val_bits = 8,@@ -82,6 +125,31 @@ static const struct regmap_config max8997_regmap_muic_config = { .max_register = MAX8997_MUIC_REG_END, }; +static const struct regmap_irq max8997_irqs_muic[] = { + /* MUIC_INT1 interrupts */ + { .reg_offset = 0, .mask = MUIC_INT1_ADC_MASK, }, + { .reg_offset = 0, .mask = MUIC_INT1_ADCLow_MASK, }, + { .reg_offset = 0, .mask = MUIC_INT1_ADCError_MASK, }, + /* MUIC_INT2 interrupts */ + { .reg_offset = 1, .mask = MUIC_INT2_ChgTyp_MASK, }, + { .reg_offset = 1, .mask = MUIC_INT2_ChgDetRun_MASK, }, + { .reg_offset = 1, .mask = MUIC_INT2_DCDTmr_MASK, }, + { .reg_offset = 1, .mask = MUIC_INT2_DBChg_MASK, }, + { .reg_offset = 1, .mask = MUIC_INT2_VBVolt_MASK, }, + /* MUIC_INT3 interrupts */ + { .reg_offset = 2, .mask = MUIC_INT3_OVP_MASK, }, +}; + +static const struct regmap_irq_chip max8997_irq_chip_muic = { + .name = "max8997-muic", + .status_base = MAX8997_MUIC_REG_INT1, + .mask_base = MAX8997_MUIC_REG_INTMASK1, + .mask_invert = true, + .num_regs = 3, + .irqs = max8997_irqs_muic, + .num_irqs = ARRAY_SIZE(max8997_irqs_muic), +}; + /* * Only the common platform data elements for max8997 are parsed here from the * device tree. Other sub-modules of max8997 such as pmic, rtc and others have@@ -173,6 +241,15 @@ static int max8997_i2c_probe(struct i2c_client *i2c, return ret; } + ret = regmap_add_irq_chip(max8997->regmap, max8997->irq, + IRQF_ONESHOT | IRQF_SHARED | + IRQF_TRIGGER_FALLING, 0, + &max8997_irq_chip, &max8997->irq_data); + if (ret) { + dev_err(max8997->dev, "failed to add irq chip: %d\n", ret); + return ret; + } +
Add regmap_del_irq_chip() on driver remove and where needed on probe error paths.
quoted hunk ↗ jump to hunk
max8997->regmap_rtc = devm_regmap_init_i2c(max8997->rtc, &max8997_regmap_rtc_config); if (IS_ERR(max8997->regmap_rtc)) {@@ -200,9 +277,17 @@ static int max8997_i2c_probe(struct i2c_client *i2c, goto err_regmap; } - pm_runtime_set_active(max8997->dev); + ret = regmap_add_irq_chip(max8997->regmap_muic, max8997->irq, + IRQF_ONESHOT | IRQF_SHARED | + IRQF_TRIGGER_FALLING, 0, + &max8997_irq_chip_muic, + &max8997->irq_data_muic); + if (ret) { + dev_err(max8997->dev, "failed to add irq chip: %d\n", ret); + return ret; + } - max8997_irq_init(max8997); + pm_runtime_set_active(max8997->dev); ret = mfd_add_devices(max8997->dev, -1, max8997_devs, ARRAY_SIZE(max8997_devs),@@ -464,7 +549,7 @@ static int max8997_resume(struct device *dev) if (device_may_wakeup(dev)) irq_set_irq_wake(max8997->irq, 0); - return max8997_irq_resume(max8997); + return 0; }
Actually you're changing a little driver logic here. I think the max8997_irq_resume() was here on purpose - to process any interrupts and clear interrupt registers (e.g. MUIC from) after resuming. Is it working properly during suspend/resume? Is it working when the MUIC interrupt is the wake up source?
quoted hunk ↗ jump to hunk
static const struct dev_pm_ops max8997_pm = {diff --git a/drivers/rtc/rtc-max8997.c b/drivers/rtc/rtc-max8997.c index b866f7d5..22769ea 100644 --- a/drivers/rtc/rtc-max8997.c +++ b/drivers/rtc/rtc-max8997.c@@ -494,7 +494,7 @@ static int max8997_rtc_probe(struct platform_device *pdev) return ret; } - virq = irq_create_mapping(max8997->irq_domain, MAX8997_PMICIRQ_RTCA1); + virq = regmap_irq_get_virq(max8997->irq_data, MAX8997_PMICIRQ_RTCA1); if (!virq) { dev_err(&pdev->dev, "Failed to create mapping alarm IRQ\n"); ret = -ENXIO;diff --git a/include/linux/mfd/max8997-private.h b/include/linux/mfd/max8997-private.h index ea80ef8..af2f0aa 100644 --- a/include/linux/mfd/max8997-private.h +++ b/include/linux/mfd/max8997-private.h@@ -333,6 +333,48 @@ enum max8997_irq_source { MAX8997_IRQ_GROUP_NR, }; +#define PMIC_INT1_PWRONR_MASK (0x1 << 0) +#define PMIC_INT1_PWRONF_MASK (0x1 << 1) +#define PMIC_INT1_PWRON1SEC_MASK (0x1 << 3) +#define PMIC_INT1_JIGONR_MASK (0x1 << 4) +#define PMIC_INT1_JIGONF_MASK (0x1 << 5) +#define PMIC_INT1_LOWBAT2_MASK (0x1 << 6) +#define PMIC_INT1_LOWBAT1_MASK (0x1 << 7) + +#define PMIC_INT2_JIGR_MASK (0x1 << 0) +#define PMIC_INT2_JIGF_MASK (0x1 << 1) +#define PMIC_INT2_MR_MASK (0x1 << 2) +#define PMIC_INT2_DVS1OK_MASK (0x1 << 3) +#define PMIC_INT2_DVS2OK_MASK (0x1 << 4) +#define PMIC_INT2_DVS3OK_MASK (0x1 << 5) +#define PMIC_INT2_DVS4OK_MASK (0x1 << 6) + +#define PMIC_INT3_CHGINS_MASK (0x1 << 0) +#define PMIC_INT3_CHGRM_MASK (0x1 << 1) +#define PMIC_INT3_DCINOVP_MASK (0x1 << 2) +#define PMIC_INT3_TOPOFFR_MASK (0x1 << 3) +#define PMIC_INT3_CHGRSTF_MASK (0x1 << 5) +#define PMIC_INT3_MBCHGTMEXPD_MASK (0x1 << 7) + +#define PMIC_INT4_RTC60S_MASK (0x1 << 0) +#define PMIC_INT4_RTCA1_MASK (0x1 << 1) +#define PMIC_INT4_RTCA2_MASK (0x1 << 2) +#define PMIC_INT4_SMPL_INT_MASK (0x1 << 3) +#define PMIC_INT4_RTC1S_MASK (0x1 << 4) +#define PMIC_INT4_WTSR_MASK (0x1 << 5) + +#define MUIC_INT1_ADC_MASK (0x1 << 0) +#define MUIC_INT1_ADCLow_MASK (0x1 << 1) +#define MUIC_INT1_ADCError_MASK (0x1 << 2) + +#define MUIC_INT2_ChgTyp_MASK (0x1 << 0) +#define MUIC_INT2_ChgDetRun_MASK (0x1 << 1) +#define MUIC_INT2_DCDTmr_MASK (0x1 << 2) +#define MUIC_INT2_DBChg_MASK (0x1 << 3) +#define MUIC_INT2_VBVolt_MASK (0x1 << 4)
You changed here naming convention to CamelCase. Any reasons?
quoted hunk ↗ jump to hunk
+ +#define MUIC_INT3_OVP_MASK (0x1 << 2) + enum max8997_irq { MAX8997_PMICIRQ_PWRONR, MAX8997_PMICIRQ_PWRONF,@@ -364,19 +406,23 @@ enum max8997_irq { MAX8997_PMICIRQ_RTC1S, MAX8997_PMICIRQ_WTSR, - MAX8997_MUICIRQ_ADCError, - MAX8997_MUICIRQ_ADCLow, + MAX8997_PMICIRQ_NR, +}; + +enum max8997_irq_muic { MAX8997_MUICIRQ_ADC, + MAX8997_MUICIRQ_ADCLow, + MAX8997_MUICIRQ_ADCError, - MAX8997_MUICIRQ_VBVolt, - MAX8997_MUICIRQ_DBChg, - MAX8997_MUICIRQ_DCDTmr, - MAX8997_MUICIRQ_ChgDetRun, MAX8997_MUICIRQ_ChgTyp, + MAX8997_MUICIRQ_ChgDetRun, + MAX8997_MUICIRQ_DCDTmr, + MAX8997_MUICIRQ_DBChg, + MAX8997_MUICIRQ_VBVolt, MAX8997_MUICIRQ_OVP, - MAX8997_IRQ_NR, + MAX8997_MUCIRQ_NR, };
Camel shows up once again.
quoted hunk ↗ jump to hunk
#define MAX8997_NUM_GPIO 12@@ -397,9 +443,10 @@ struct max8997_dev { struct regmap *regmap_haptic; struct regmap *regmap_muic; + struct regmap_irq_chip_data *irq_data; + struct regmap_irq_chip_data *irq_data_muic; int irq; int ono; - struct irq_domain *irq_domain; struct mutex irqlock; int irq_masks_cur[MAX8997_IRQ_GROUP_NR]; int irq_masks_cache[MAX8997_IRQ_GROUP_NR];