Re: [PATCH v3 1/5] mfd: tps65217: Add support for IRQs

Grygorii Strashko
2016-06-16T13:03:40Z

On 06/16/2016 02:41 PM, Marcin Niestroj wrote:
> Add support for handling IRQs: power button, AC and USB power state
> changes. Mask and interrupt bits are shared within one register, which
> prevents us to use regmap_irq implementation. New irq_domain is created in
> order to add interrupt handling for each tps65217's subsystem. IRQ
> resources have been added for charger subsystem to be able to notify about
> AC and USB state changes.
>
> Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
> ---
> drivers/mfd/Kconfig | 1 +
> drivers/mfd/tps65217.c | 194 +++++++++++++++++++++++++++++++++++++++++--
> include/linux/mfd/tps65217.h | 11 +++
> 3 files changed, 198 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 1bcf601..f8c9580 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1200,6 +1200,7 @@ config MFD_TPS65217
> depends on I2C
> select MFD_CORE
> select REGMAP_I2C
> + select IRQ_DOMAIN
> help
> If you say yes here you get support for the TPS65217 series of
> Power Management / White LED chips.
> diff --git a/drivers/mfd/tps65217.c b/drivers/mfd/tps65217.c
> index 049a6fc..7763dbc 100644
> --- a/drivers/mfd/tps65217.c
> +++ b/drivers/mfd/tps65217.c
> @@ -15,22 +15,99 @@
> * GNU General Public License for more details.
> */
>
> -#include <linux/kernel.h>
> #include <linux/device.h>
> -#include <linux/module.h>
> -#include <linux/platform_device.h>
> +#include <linux/err.h>
> #include <linux/init.h>
> +#include <linux/interrupt.h>
> #include <linux/i2c.h>
> -#include <linux/slab.h>
> -#include <linux/regmap.h>
> -#include <linux/err.h>
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
>
> #include <linux/mfd/core.h>
> #include <linux/mfd/tps65217.h>
>
> -static const struct mfd_cell tps65217s[] = {
> +static struct resource charger_resources[] = {
> + DEFINE_RES_IRQ_NAMED(TPS65217_IRQ_AC, "AC"),
> + DEFINE_RES_IRQ_NAMED(TPS65217_IRQ_USB, "USB"),
> +};
> +
> +struct tps65217_irq {
> + int mask;
> + int interrupt;
> +};
> +
> +static const struct tps65217_irq tps65217_irqs[] = {
> + [TPS65217_IRQ_PB] = {
> + .mask = TPS65217_INT_PBM,
> + .interrupt = TPS65217_INT_PBI,
> + },
> + [TPS65217_IRQ_AC] = {
> + .mask = TPS65217_INT_ACM,
> + .interrupt = TPS65217_INT_ACI,
> + },
> + [TPS65217_IRQ_USB] = {
> + .mask = TPS65217_INT_USBM,
> + .interrupt = TPS65217_INT_USBI,
> + },
> +};
> +
> +static void tps65217_irq_lock(struct irq_data *data)
> +{
> + struct tps65217 *tps = irq_data_get_irq_chip_data(data);
> +
> + mutex_lock(&tps->irq_lock);
> +}
> +
> +static void tps65217_irq_sync_unlock(struct irq_data *data)
> +{
> + struct tps65217 *tps = irq_data_get_irq_chip_data(data);
> + int ret;
> +
> + ret = tps65217_reg_write(tps, TPS65217_REG_INT, tps->irq_mask,
> + TPS65217_PROTECT_NONE);
> + if (ret != 0)
> + dev_err(tps->dev, "Failed to sync IRQ masks\n");
> +
> + mutex_unlock(&tps->irq_lock);
> +}
> +
> +static const inline struct tps65217_irq *
> +irq_to_tps65217_irq(struct tps65217 *tps, struct irq_data *data)
> +{
> + return &tps65217_irqs[data->hwirq];
> +}
> +
> +static void tps65217_irq_enable(struct irq_data *data)
> +{
> + struct tps65217 *tps = irq_data_get_irq_chip_data(data);
> + const struct tps65217_irq *irq_data = irq_to_tps65217_irq(tps, data);
> +
> + tps->irq_mask &= ~irq_data->mask;
> +}
> +
> +static void tps65217_irq_disable(struct irq_data *data)
> +{
> + struct tps65217 *tps = irq_data_get_irq_chip_data(data);
> + const struct tps65217_irq *irq_data = irq_to_tps65217_irq(tps, data);
> +
> + tps->irq_mask |= irq_data->mask;
> +}
> +
> +static struct irq_chip tps65217_irq_chip = {
> + .irq_bus_lock = tps65217_irq_lock,
> + .irq_bus_sync_unlock = tps65217_irq_sync_unlock,
> + .irq_enable = tps65217_irq_enable,
> + .irq_disable = tps65217_irq_disable,
> +};
> +
> +static struct mfd_cell tps65217s[] = {
> {
> .name = "tps65217-pmic",
> .of_compatible = "ti,tps65217-pmic",
> @@ -41,10 +118,89 @@ static const struct mfd_cell tps65217s[] = {
> },
> {
> .name = "tps65217-charger",
> + .num_resources = ARRAY_SIZE(charger_resources),
> + .resources = charger_resources,
> .of_compatible = "ti,tps65217-charger",
> },
> };
>
> +static irqreturn_t tps65217_irq_thread(int irq, void *data)
> +{
> + struct tps65217 *tps = data;
> + unsigned int status;
> + bool handled = false;
> + int i;
> + int ret;
> +
> + ret = tps65217_reg_read(tps, TPS65217_REG_INT, &status);
> + if (ret < 0) {
> + dev_err(tps->dev, "Failed to read IRQ status: %d\n",
> + ret);
> + return IRQ_NONE;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(tps65217_irqs); i++) {
> + if (status & tps65217_irqs[i].interrupt) {
> + handle_nested_irq(irq_find_mapping(tps->irq_domain, i));
> + handled = true;
> + }
> + }
> +
> + if (handled)
> + return IRQ_HANDLED;
> +
> + return IRQ_NONE;
> +}
> +
> +static int tps65217_irq_map(struct irq_domain *h, unsigned int virq,
> + irq_hw_number_t hw)
> +{
> + struct tps65217 *tps = h->host_data;
> +
> + irq_set_chip_data(virq, tps);
> + irq_set_chip_and_handler(virq, &tps65217_irq_chip, handle_edge_irq);
> + irq_set_nested_thread(virq, 1);
> + irq_set_noprobe(virq);

irq_set_parent()?

> +
> + return 0;
> +}
> +
> +static const struct irq_domain_ops tps65217_irq_domain_ops = {
> + .map = tps65217_irq_map,
> +};
> +
> +static int tps65217_irq_init(struct tps65217 *tps, int irq)
> +{
> + int ret;
> +
> + mutex_init(&tps->irq_lock);
> +
> + /* Mask all interrupt sources */
> + tps->irq_mask = (TPS65217_INT_RESERVEDM | TPS65217_INT_PBM
> + | TPS65217_INT_ACM | TPS65217_INT_USBM);
> + tps65217_reg_write(tps, TPS65217_REG_INT, tps->irq_mask,
> + TPS65217_PROTECT_NONE);
> +
> + tps->irq_domain = irq_domain_add_linear(tps->dev->of_node,
> + TPS65217_NUM_IRQ, &tps65217_irq_domain_ops, tps);
> + if (!tps->irq_domain) {
> + dev_err(tps->dev, "Could not create IRQ domain\n");
> + return -ENOMEM;
> + }
> +
> + ret = devm_request_threaded_irq(tps->dev, irq, NULL,
> + tps65217_irq_thread,
> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,

Are there any reasons why IRQ trigger type specified here explicitly?

> + "tps65217-irq", tps);
> + if (ret) {
> + dev_err(tps->dev, "Failed to request IRQ %d: %d\n",
> + irq, ret);
> + return ret;
> + }
> +
> + return 0;
> +}

--
regards,
-grygorii