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

[Grygorii Strashko <grygorii.strashko@ti.com>]

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