| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-id: <1398697278.12945.19.camel@AMDC1943>
Date: Mon, 28 Apr 2014 17:01:18 +0200
From: Krzysztof Kozlowski <k.kozlowski@...sung.com>
To: Robert Baldyga <r.baldyga@...sung.com>
Cc: sameo@...ux.intel.com, lee.jones@...aro.org, a.zummo@...ertech.it,
linux-kernel@...r.kernel.org, rtc-linux@...glegroups.com,
m.szyprowski@...sung.com
Subject: Re: [PATCH 2/2] mfd: max77693: handle IRQs using regmap
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@...sung.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists