Re: [PATCH 2/3] mfd: max8997: handle IRQs using regmap

From: Krzysztof Kozlowski
Date: Tue Mar 11 2014 - 10:43:38 EST

Next message: Santosh Shilimkar: "Re: [PATCH 2/3] soc: keystone: add QMSS driver"
Previous message: Alexander Gordeev: "Re: [PATCH] percpu_ida: Handle out-of-tags gracefully"
In reply to: Robert Baldyga: "[PATCH 2/3] mfd: max8997: handle IRQs using regmap"
Next in thread: Robert Baldyga: "[PATCH 3/3] mfd: max8997: move regmap handling to function drivers"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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 <r.baldyga@xxxxxxxxxxx>
> ---
> 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.

(...)

> 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.

> 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?

> 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?

> +
> +#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.

>
> #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];

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Santosh Shilimkar: "Re: [PATCH 2/3] soc: keystone: add QMSS driver"
Previous message: Alexander Gordeev: "Re: [PATCH] percpu_ida: Handle out-of-tags gracefully"
In reply to: Robert Baldyga: "[PATCH 2/3] mfd: max8997: handle IRQs using regmap"
Next in thread: Robert Baldyga: "[PATCH 3/3] mfd: max8997: move regmap handling to function drivers"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]