Re: [PATCH RESEND v6 2/7] mfd: rn5t618: add IRQ support
From: Lee Jones <hidden>
Date: 2020-03-19 16:11:08
Also in:
linux-iio, linux-rtc, lkml
On Fri, 13 Mar 2020, Andreas Kemnade wrote:
quoted hunk ↗ jump to hunk
This adds support for IRQ handling in the RC5T619 which is required for properly implementing subdevices like RTC. For now only definitions for the variant RC5T619 are included. Signed-off-by: Andreas Kemnade <andreas@kemnade.info> --- Changes in v5: rn5t618_irq_init static Changes in v4: merge rn5t618-irq.c into rn5t618.c use macros for IRQ table Changes in v3: alignment cleanup Changes in v2: - no dead code, did some more testing and thinking for that - remove extra empty lines drivers/mfd/Kconfig | 1 + drivers/mfd/rn5t618.c | 88 +++++++++++++++++++++++++++++++++++++ include/linux/mfd/rn5t618.h | 15 +++++++ 3 files changed, 104 insertions(+)diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index 2b203290e7b9..a7067888a41e 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig@@ -1058,6 +1058,7 @@ config MFD_RN5T618 depends on OF select MFD_CORE select REGMAP_I2C + select REGMAP_IRQ help Say yes here to add support for the Ricoh RN5T567, RN5T618, RC5T619 PMIC.diff --git a/drivers/mfd/rn5t618.c b/drivers/mfd/rn5t618.c index ead2e79036a9..849a33d64860 100644 --- a/drivers/mfd/rn5t618.c +++ b/drivers/mfd/rn5t618.c@@ -8,6 +8,8 @@ #include <linux/delay.h> #include <linux/i2c.h> +#include <linux/interrupt.h> +#include <linux/irq.h> #include <linux/mfd/core.h> #include <linux/mfd/rn5t618.h> #include <linux/module.h>@@ -46,9 +48,63 @@ static const struct regmap_config rn5t618_regmap_config = { .cache_type = REGCACHE_RBTREE, }; +static const struct regmap_irq rc5t619_irqs[] = { + REGMAP_IRQ_REG(RN5T618_IRQ_SYS, 0, BIT(0)), + REGMAP_IRQ_REG(RN5T618_IRQ_DCDC, 0, BIT(1)), + REGMAP_IRQ_REG(RN5T618_IRQ_RTC, 0, BIT(2)), + REGMAP_IRQ_REG(RN5T618_IRQ_ADC, 0, BIT(3)), + REGMAP_IRQ_REG(RN5T618_IRQ_GPIO, 0, BIT(4)), + REGMAP_IRQ_REG(RN5T618_IRQ_CHG, 0, BIT(6)), +}; + +static const struct regmap_irq_chip rc5t619_irq_chip = { + .name = "rc5t619", + .irqs = rc5t619_irqs, + .num_irqs = ARRAY_SIZE(rc5t619_irqs), + .num_regs = 1, + .status_base = RN5T618_INTMON, + .mask_base = RN5T618_INTEN, + .mask_invert = true, +}; + static struct rn5t618 *rn5t618_pm_power_off; static struct notifier_block rn5t618_restart_handler; +static int rn5t618_irq_init(struct rn5t618 *rn5t618) +{ + const struct regmap_irq_chip *irq_chip; + int ret; + + if (!rn5t618->irq) + return 0; + + switch (rn5t618->variant) { + case RC5T619: + irq_chip = &rc5t619_irq_chip; + break; + default: + irq_chip = NULL;
If you initialised this at the top, you can rid this line. Don't you want to put a message in here about an unsupported device?
+ break;
+ }
+
+ if (!irq_chip) {
+ dev_err(rn5t618->dev, "Currently no IRQ support for variant %d\n",
+ (int)rn5t618->variant);
+ return -ENOENT;
+ }Ah yes. Put this in 'default' instead.
+ ret = devm_regmap_add_irq_chip(rn5t618->dev, rn5t618->regmap,
+ rn5t618->irq,
+ IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+ 0, irq_chip, &rn5t618->irq_data);
+ if (ret) {
+ dev_err(rn5t618->dev, "Failed to register IRQ chip\n");
+ return ret;Make this a one line if.
+ } + + return 0;
... and return ret.
quoted hunk ↗ jump to hunk
+} + static void rn5t618_trigger_poweroff_sequence(bool repower) { /* disable automatic repower-on */@@ -106,6 +162,8 @@ static int rn5t618_i2c_probe(struct i2c_client *i2c, i2c_set_clientdata(i2c, priv); priv->variant = (long)of_id->data; + priv->irq = i2c->irq; + priv->dev = &i2c->dev; priv->regmap = devm_regmap_init_i2c(i2c, &rn5t618_regmap_config); if (IS_ERR(priv->regmap)) {@@ -138,6 +196,11 @@ static int rn5t618_i2c_probe(struct i2c_client *i2c, return ret; } + if (priv->irq > 0) {
There are already have checks in rn5t618_irq_init(). Just call it regardless.
+ if (rn5t618_irq_init(priv))
If this returns an error, you should return that error from .probe().
quoted hunk ↗ jump to hunk
+ priv->irq = 0; + } + return 0; }@@ -155,15 +218,40 @@ static int rn5t618_i2c_remove(struct i2c_client *i2c) return 0; } +static int __maybe_unused rn5t618_i2c_suspend(struct device *dev) +{ + struct rn5t618 *priv = dev_get_drvdata(dev); + + if (priv->irq) + disable_irq(priv->irq); + + return 0; +} + +static int __maybe_unused rn5t618_i2c_resume(struct device *dev) +{ + struct rn5t618 *priv = dev_get_drvdata(dev); + + if (priv->irq) + enable_irq(priv->irq); + + return 0; +} + static const struct i2c_device_id rn5t618_i2c_id[] = { { } }; MODULE_DEVICE_TABLE(i2c, rn5t618_i2c_id); +static SIMPLE_DEV_PM_OPS(rn5t618_i2c_dev_pm_ops, + rn5t618_i2c_suspend, + rn5t618_i2c_resume); + static struct i2c_driver rn5t618_i2c_driver = { .driver = { .name = "rn5t618", .of_match_table = of_match_ptr(rn5t618_of_match), + .pm = &rn5t618_i2c_dev_pm_ops, }, .probe = rn5t618_i2c_probe, .remove = rn5t618_i2c_remove,diff --git a/include/linux/mfd/rn5t618.h b/include/linux/mfd/rn5t618.h index d62ef48060b5..739571656f2b 100644 --- a/include/linux/mfd/rn5t618.h +++ b/include/linux/mfd/rn5t618.h@@ -242,9 +242,24 @@ enum { RC5T619, }; +/* RN5T618 IRQ definitions */ +enum { + RN5T618_IRQ_SYS = 0, + RN5T618_IRQ_DCDC, + RN5T618_IRQ_RTC, + RN5T618_IRQ_ADC, + RN5T618_IRQ_GPIO, + RN5T618_IRQ_CHG, + RN5T618_NR_IRQS, +}; + struct rn5t618 { struct regmap *regmap; + struct device *dev; long variant; + + int irq; + struct regmap_irq_chip_data *irq_data; }; #endif /* __LINUX_MFD_RN5T618_H */
-- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog