From: Krzysztof Kozlowski <k.kozlowski@samsung.com>
To: Robert Baldyga <r.baldyga@samsung.com>
Cc: sameo@linux.intel.com, lee.jones@linaro.org,
a.zummo@towertech.it, linux-kernel@vger.kernel.org,
rtc-linux@googlegroups.com, m.szyprowski@samsung.com
Subject: Re: [PATCH 2/2] mfd: max77693: handle IRQs using regmap
Date: Mon, 28 Apr 2014 17:01:18 +0200 [thread overview]
Message-ID: <1398697278.12945.19.camel@AMDC1943> (raw)
In-Reply-To: <1398694103-29320-3-git-send-email-r.baldyga@samsung.com>
On pon, 2014-04-28 at 16:08 +0200, 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@samsung.com>
> ---
> drivers/extcon/extcon-max77693.c | 3 +-
> drivers/mfd/Kconfig | 1 +
> drivers/mfd/Makefile | 2 +-
> drivers/mfd/max77693-irq.c | 337 -----------------------------------
> drivers/mfd/max77693.c | 158 ++++++++++++++--
> include/linux/mfd/max77693-private.h | 47 ++++-
> 6 files changed, 196 insertions(+), 352 deletions(-)
> delete mode 100644 drivers/mfd/max77693-irq.c
(...)
> diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
> index 6336251..8b2633d 100644
> --- a/drivers/mfd/max77693.c
> +++ b/drivers/mfd/max77693.c
> @@ -55,12 +55,95 @@ static const struct regmap_config max77693_regmap_config = {
> .max_register = MAX77693_PMIC_REG_END,
> };
>
> +static const struct regmap_irq max77693_led_irqs[] = {
> + { .mask = LED_IRQ_FLED2_OPEN, },
> + { .mask = LED_IRQ_FLED2_SHORT, },
> + { .mask = LED_IRQ_FLED1_OPEN, },
> + { .mask = LED_IRQ_FLED1_SHORT, },
> + { .mask = LED_IRQ_MAX_FLASH, },
> +};
> +
> +static const struct regmap_irq_chip max77693_led_irq_chip = {
> + .name = "max77693-led",
> + .status_base = MAX77693_LED_REG_FLASH_INT,
> + .mask_base = MAX77693_LED_REG_FLASH_INT_MASK,
> + .mask_invert = false,
> + .num_regs = 1,
> + .irqs = max77693_led_irqs,
> + .num_irqs = ARRAY_SIZE(max77693_led_irqs),
> +};
> +
> +static const struct regmap_irq max77693_topsys_irqs[] = {
> + { .mask = TOPSYS_IRQ_T120C_INT, },
> + { .mask = TOPSYS_IRQ_T140C_INT, },
> + { .mask = TOPSYS_IRQ_LOWSYS_INT, },
> +};
> +
> +static const struct regmap_irq_chip max77693_topsys_irq_chip = {
> + .name = "max77693-topsys",
> + .status_base = MAX77693_PMIC_REG_TOPSYS_INT,
> + .mask_base = MAX77693_PMIC_REG_TOPSYS_INT_MASK,
> + .mask_invert = false,
> + .num_regs = 1,
> + .irqs = max77693_topsys_irqs,
> + .num_irqs = ARRAY_SIZE(max77693_topsys_irqs),
> +};
> +
> +static const struct regmap_irq max77693_charger_irqs[] = {
> + { .mask = CHG_IRQ_BYP_I, },
> + { .mask = CHG_IRQ_THM_I, },
> + { .mask = CHG_IRQ_BAT_I, },
> + { .mask = CHG_IRQ_CHG_I, },
> + { .mask = CHG_IRQ_CHGIN_I, },
> +};
> +
> +static const struct regmap_irq_chip max77693_charger_irq_chip = {
> + .name = "max77693-charger",
> + .status_base = MAX77693_CHG_REG_CHG_INT,
> + .mask_base = MAX77693_CHG_REG_CHG_INT_MASK,
> + .mask_invert = false,
> + .num_regs = 1,
> + .irqs = max77693_charger_irqs,
> + .num_irqs = ARRAY_SIZE(max77693_charger_irqs),
> +};
> +
> static const struct regmap_config max77693_regmap_muic_config = {
> .reg_bits = 8,
> .val_bits = 8,
> .max_register = MAX77693_MUIC_REG_END,
> };
>
> +static const struct regmap_irq max77693_muic_irqs[] = {
> + { .reg_offset = 0, .mask = MUIC_IRQ_INT1_ADC, },
> + { .reg_offset = 0, .mask = MUIC_IRQ_INT1_ADC_LOW, },
> + { .reg_offset = 0, .mask = MUIC_IRQ_INT1_ADC_ERR, },
> + { .reg_offset = 0, .mask = MUIC_IRQ_INT1_ADC1K, },
> +
> + { .reg_offset = 1, .mask = MUIC_IRQ_INT2_CHGTYP, },
> + { .reg_offset = 1, .mask = MUIC_IRQ_INT2_CHGDETREUN, },
> + { .reg_offset = 1, .mask = MUIC_IRQ_INT2_DCDTMR, },
> + { .reg_offset = 1, .mask = MUIC_IRQ_INT2_DXOVP, },
> + { .reg_offset = 1, .mask = MUIC_IRQ_INT2_VBVOLT, },
> + { .reg_offset = 1, .mask = MUIC_IRQ_INT2_VIDRM, },
> +
> + { .reg_offset = 2, .mask = MUIC_IRQ_INT3_EOC, },
> + { .reg_offset = 2, .mask = MUIC_IRQ_INT3_CGMBC, },
> + { .reg_offset = 2, .mask = MUIC_IRQ_INT3_OVP, },
> + { .reg_offset = 2, .mask = MUIC_IRQ_INT3_MBCCHG_ERR, },
> + { .reg_offset = 2, .mask = MUIC_IRQ_INT3_CHG_ENABLED, },
> + { .reg_offset = 2, .mask = MUIC_IRQ_INT3_BAT_DET, },
> +};
> +
> +static const struct regmap_irq_chip max77693_muic_irq_chip = {
> + .name = "max77693-muic",
> + .status_base = MAX77693_MUIC_REG_INT1,
> + .mask_base = MAX77693_MUIC_REG_INTMASK1,
> + .mask_invert = true,
> + .num_regs = 3,
> + .irqs = max77693_muic_irqs,
> + .num_irqs = ARRAY_SIZE(max77693_muic_irqs),
> +};
> +
> static int max77693_i2c_probe(struct i2c_client *i2c,
> const struct i2c_device_id *id)
> {
> @@ -124,9 +207,45 @@ static int max77693_i2c_probe(struct i2c_client *i2c,
> goto err_regmap_muic;
> }
>
> - ret = max77693_irq_init(max77693);
> - if (ret < 0)
> - goto err_irq;
> + ret = regmap_add_irq_chip(max77693->regmap, max77693->irq,
> + IRQF_ONESHOT | IRQF_SHARED |
> + IRQF_TRIGGER_FALLING, 0,
> + &max77693_led_irq_chip,
> + &max77693->irq_data_led);
> + if (ret) {
> + dev_err(max77693->dev, "failed to add irq chip: %d\n", ret);
> + goto err_regmap_muic;
> + }
> +
> + ret = regmap_add_irq_chip(max77693->regmap, max77693->irq,
> + IRQF_ONESHOT | IRQF_SHARED |
> + IRQF_TRIGGER_FALLING, 0,
> + &max77693_topsys_irq_chip,
> + &max77693->irq_data_topsys);
> + if (ret) {
> + dev_err(max77693->dev, "failed to add irq chip: %d\n", ret);
> + goto err_irq_topsys;
> + }
> +
> + ret = regmap_add_irq_chip(max77693->regmap, max77693->irq,
> + IRQF_ONESHOT | IRQF_SHARED |
> + IRQF_TRIGGER_FALLING, 0,
> + &max77693_charger_irq_chip,
> + &max77693->irq_data_charger);
> + if (ret) {
> + dev_err(max77693->dev, "failed to add irq chip: %d\n", ret);
> + goto err_irq_charger;
> + }
> +
> + ret = regmap_add_irq_chip(max77693->regmap, max77693->irq,
> + IRQF_ONESHOT | IRQF_SHARED |
> + IRQF_TRIGGER_FALLING, 0,
> + &max77693_muic_irq_chip,
> + &max77693->irq_data_muic);
> + if (ret) {
> + dev_err(max77693->dev, "failed to add irq chip: %d\n", ret);
> + goto err_irq_muic;
> + }
>
> pm_runtime_set_active(max77693->dev);
>
> @@ -138,8 +257,14 @@ static int max77693_i2c_probe(struct i2c_client *i2c,
> return ret;
>
> err_mfd:
> - max77693_irq_exit(max77693);
> -err_irq:
> + mfd_remove_devices(max77693->dev);
Hmmm... The mfd_add_devices() calls mfd_remove_devices() on failure so
why calling it also here?
> + regmap_del_irq_chip(max77693->irq, max77693->irq_data_muic);
> +err_irq_muic:
> + regmap_del_irq_chip(max77693->irq, max77693->irq_data_charger);
> +err_irq_charger:
> + regmap_del_irq_chip(max77693->irq, max77693->irq_data_topsys);
> +err_irq_topsys:
> + regmap_del_irq_chip(max77693->irq, max77693->irq_data_led);
> err_regmap_muic:
> i2c_unregister_device(max77693->haptic);
> err_i2c_haptic:
> @@ -152,7 +277,12 @@ static int max77693_i2c_remove(struct i2c_client *i2c)
> struct max77693_dev *max77693 = i2c_get_clientdata(i2c);
>
> mfd_remove_devices(max77693->dev);
> - max77693_irq_exit(max77693);
> +
> + regmap_del_irq_chip(max77693->irq, max77693->irq_data_muic);
> + regmap_del_irq_chip(max77693->irq, max77693->irq_data_charger);
> + regmap_del_irq_chip(max77693->irq, max77693->irq_data_topsys);
> + regmap_del_irq_chip(max77693->irq, max77693->irq_data_led);
> +
> i2c_unregister_device(max77693->muic);
> i2c_unregister_device(max77693->haptic);
>
> @@ -170,8 +300,11 @@ static int max77693_suspend(struct device *dev)
> struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
> struct max77693_dev *max77693 = i2c_get_clientdata(i2c);
>
> - if (device_may_wakeup(dev))
> - irq_set_irq_wake(max77693->irq, 1);
> + if (device_may_wakeup(dev)) {
> + enable_irq_wake(max77693->irq);
> + disable_irq(max77693->irq);
> + }
The disable_irq() should be called regardless of wakeup source. Actually
I could reproduce error even when the driver was not a wakeup source.
> +
> return 0;
> }
>
> @@ -180,9 +313,12 @@ static int max77693_resume(struct device *dev)
> struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
> struct max77693_dev *max77693 = i2c_get_clientdata(i2c);
>
> - if (device_may_wakeup(dev))
> - irq_set_irq_wake(max77693->irq, 0);
> - return max77693_irq_resume(max77693);
> + if (device_may_wakeup(dev)) {
> + disable_irq_wake(max77693->irq);
> + enable_irq(max77693->irq);
Same here.
Best regards,
Krzysztof
prev parent reply other threads:[~2014-04-28 15:01 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-28 14:08 [PATCH 0/2] mfd: max77693: improve regmap support Robert Baldyga
2014-04-28 14:08 ` [PATCH 1/2] mfd: max77693: remove unnecessary wrapper functions Robert Baldyga
2014-04-28 14:43 ` Krzysztof Kozlowski
2014-04-28 14:08 ` [PATCH 2/2] mfd: max77693: handle IRQs using regmap Robert Baldyga
2014-04-28 15:01 ` Krzysztof Kozlowski [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1398697278.12945.19.camel@AMDC1943 \
--to=k.kozlowski@samsung.com \
--cc=a.zummo@towertech.it \
--cc=lee.jones@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=r.baldyga@samsung.com \
--cc=rtc-linux@googlegroups.com \
--cc=sameo@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox