[PATCH 0/3] mfd: tps6586x: Convert to irq domain

[PATCH 0/3] mfd: tps6586x: Convert to irq domain

[Laxman Dewangan]

This patch series convert the irq implementation to use the
irq domain.
Accordingly, gpio driver and rtc registration is updated.

Laxman Dewangan (3):
  mfd: Convert tps6586x to irq_domain
  mfd: tps6586x: add irq io-resource for rtc sub driver
  mfd: tps6586x: implement gpio_to_irq

 drivers/gpio/gpio-tps6586x.c |    9 ++++
 drivers/mfd/tps6586x.c       |  103 +++++++++++++++++++++++++++++++-----------
 include/linux/mfd/tps6586x.h |    1 +
 3 files changed, 87 insertions(+), 26 deletions(-)

[PATCH 1/3] mfd: Convert tps6586x to irq_domain

[Laxman Dewangan]

Allocate the irq base if it base is not porvided i.e.
in case of device tree invocation of this driver.
Convert the tps6586x driver to irq domain, using a
legacy IRQ mapping if an irq_base is specified in
platform data or dynamically allocated and otherwise
using a linear mapping.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/mfd/tps6586x.c       |   91 ++++++++++++++++++++++++++++++-----------
 include/linux/mfd/tps6586x.h |    1 +
 2 files changed, 67 insertions(+), 25 deletions(-)

diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c
index 4674643..2cdf1e6 100644
--- a/drivers/mfd/tps6586x.c
+++ b/drivers/mfd/tps6586x.c
@@ -17,12 +17,14 @@
 
 #include <linux/interrupt.h>
 #include <linux/irq.h>
+#include <linux/irqdomain.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/slab.h>
 #include <linux/err.h>
 #include <linux/i2c.h>
+#include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/regulator/of_regulator.h>
 #include <linux/regulator/machine.h>
@@ -116,6 +118,7 @@ struct tps6586x {
 	int			irq_base;
 	u32			irq_en;
 	u8			mask_reg[5];
+	struct irq_domain	*irq_domain;
 };
 
 static inline struct tps6586x *dev_to_tps6586x(struct device *dev)
@@ -184,6 +187,14 @@ int tps6586x_update(struct device *dev, int reg, uint8_t val, uint8_t mask)
 }
 EXPORT_SYMBOL_GPL(tps6586x_update);
 
+int tps6586x_irq_get_virq(struct device *dev, int irq)
+{
+	struct tps6586x *tps6586x = dev_to_tps6586x(dev);
+
+	return irq_create_mapping(tps6586x->irq_domain, irq);
+}
+EXPORT_SYMBOL_GPL(tps6586x_irq_get_virq);
+
 static int __remove_subdev(struct device *dev, void *unused)
 {
 	platform_device_unregister(to_platform_device(dev));
@@ -205,7 +216,7 @@ static void tps6586x_irq_lock(struct irq_data *data)
 static void tps6586x_irq_enable(struct irq_data *irq_data)
 {
 	struct tps6586x *tps6586x = irq_data_get_irq_chip_data(irq_data);
-	unsigned int __irq = irq_data->irq - tps6586x->irq_base;
+	unsigned int __irq = irq_data->hwirq;
 	const struct tps6586x_irq_data *data = &tps6586x_irqs[__irq];
 
 	tps6586x->mask_reg[data->mask_reg] &= ~data->mask_mask;
@@ -216,7 +227,7 @@ static void tps6586x_irq_disable(struct irq_data *irq_data)
 {
 	struct tps6586x *tps6586x = irq_data_get_irq_chip_data(irq_data);
 
-	unsigned int __irq = irq_data->irq - tps6586x->irq_base;
+	unsigned int __irq = irq_data->hwirq;
 	const struct tps6586x_irq_data *data = &tps6586x_irqs[__irq];
 
 	tps6586x->mask_reg[data->mask_reg] |= data->mask_mask;
@@ -239,6 +250,39 @@ static void tps6586x_irq_sync_unlock(struct irq_data *data)
 	mutex_unlock(&tps6586x->irq_lock);
 }
 
+static struct irq_chip tps6586x_irq_chip = {
+	.name = "tps6586x",
+	.irq_bus_lock = tps6586x_irq_lock,
+	.irq_bus_sync_unlock = tps6586x_irq_sync_unlock,
+	.irq_disable = tps6586x_irq_disable,
+	.irq_enable = tps6586x_irq_enable,
+};
+
+static int tps6586x_irq_map(struct irq_domain *h, unsigned int virq,
+				irq_hw_number_t hw)
+{
+	struct tps6586x *tps6586x = h->host_data;
+
+	irq_set_chip_data(virq, tps6586x);
+	irq_set_chip_and_handler(virq, &tps6586x_irq_chip, handle_simple_irq);
+	irq_set_nested_thread(virq, 1);
+
+	/* ARM needs us to explicitly flag the IRQ as valid
+	 * and will set them noprobe when we do so. */
+#ifdef CONFIG_ARM
+	set_irq_flags(virq, IRQF_VALID);
+#else
+	irq_set_noprobe(virq);
+#endif
+
+	return 0;
+}
+
+static struct irq_domain_ops tps6586x_domain_ops = {
+	.map    = tps6586x_irq_map,
+	.xlate  = irq_domain_xlate_twocell,
+};
+
 static irqreturn_t tps6586x_irq(int irq, void *data)
 {
 	struct tps6586x *tps6586x = data;
@@ -259,7 +303,8 @@ static irqreturn_t tps6586x_irq(int irq, void *data)
 		int i = __ffs(acks);
 
 		if (tps6586x->irq_en & (1 << i))
-			handle_nested_irq(tps6586x->irq_base + i);
+			handle_nested_irq(
+				irq_find_mapping(tps6586x->irq_domain, i));
 
 		acks &= ~(1 << i);
 	}
@@ -272,11 +317,8 @@ static int __devinit tps6586x_irq_init(struct tps6586x *tps6586x, int irq,
 {
 	int i, ret;
 	u8 tmp[4];
-
-	if (!irq_base) {
-		dev_warn(tps6586x->dev, "No interrupt support on IRQ base\n");
-		return -EINVAL;
-	}
+	int new_irq_base;
+	int irq_num = ARRAY_SIZE(tps6586x_irqs);
 
 	mutex_init(&tps6586x->irq_lock);
 	for (i = 0; i < 5; i++) {
@@ -286,25 +328,24 @@ static int __devinit tps6586x_irq_init(struct tps6586x *tps6586x, int irq,
 
 	tps6586x_reads(tps6586x->dev, TPS6586X_INT_ACK1, sizeof(tmp), tmp);
 
-	tps6586x->irq_base = irq_base;
-
-	tps6586x->irq_chip.name = "tps6586x";
-	tps6586x->irq_chip.irq_enable = tps6586x_irq_enable;
-	tps6586x->irq_chip.irq_disable = tps6586x_irq_disable;
-	tps6586x->irq_chip.irq_bus_lock = tps6586x_irq_lock;
-	tps6586x->irq_chip.irq_bus_sync_unlock = tps6586x_irq_sync_unlock;
-
-	for (i = 0; i < ARRAY_SIZE(tps6586x_irqs); i++) {
-		int __irq = i + tps6586x->irq_base;
-		irq_set_chip_data(__irq, tps6586x);
-		irq_set_chip_and_handler(__irq, &tps6586x->irq_chip,
-					 handle_simple_irq);
-		irq_set_nested_thread(__irq, 1);
-#ifdef CONFIG_ARM
-		set_irq_flags(__irq, IRQF_VALID);
-#endif
+	if  (irq_base > 0) {
+		new_irq_base = irq_alloc_descs(irq_base, 0, irq_num, -1);
+		if (new_irq_base < 0) {
+			dev_err(tps6586x->dev,
+				"Failed to alloc IRQs: %d\n", new_irq_base);
+			return new_irq_base;
+		}
+	} else {
+		new_irq_base = 0;
 	}
 
+	tps6586x->irq_domain = irq_domain_add_simple(tps6586x->dev->of_node,
+				irq_num, new_irq_base, &tps6586x_domain_ops,
+				tps6586x);
+	if (!tps6586x->irq_domain) {
+		dev_err(tps6586x->dev, "Failed to create IRQ domain\n");
+		return -ENOMEM;
+	}
 	ret = request_threaded_irq(irq, NULL, tps6586x_irq, IRQF_ONESHOT,
 				   "tps6586x", tps6586x);
 
diff --git a/include/linux/mfd/tps6586x.h b/include/linux/mfd/tps6586x.h
index 2dd1231..ebd8b08 100644
--- a/include/linux/mfd/tps6586x.h
+++ b/include/linux/mfd/tps6586x.h
@@ -93,5 +93,6 @@ extern int tps6586x_set_bits(struct device *dev, int reg, uint8_t bit_mask);
 extern int tps6586x_clr_bits(struct device *dev, int reg, uint8_t bit_mask);
 extern int tps6586x_update(struct device *dev, int reg, uint8_t val,
 			   uint8_t mask);
+extern int tps6586x_irq_get_virq(struct device *dev, int irq);
 
 #endif /*__LINUX_MFD_TPS6586X_H */
-- 
1.7.1.1

Re: [PATCH 1/3] mfd: Convert tps6586x to irq_domain

[Mark Brown]

On Mon, Oct 08, 2012 at 09:43:23PM +0530, Laxman Dewangan wrote:
> Allocate the irq base if it base is not porvided i.e.
> in case of device tree invocation of this driver.
> Convert the tps6586x driver to irq domain, using a
> legacy IRQ mapping if an irq_base is specified in
> platform data or dynamically allocated and otherwise
> using a linear mapping.

Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

but can you convert to regmap_irq?

Re: [PATCH 1/3] mfd: Convert tps6586x to irq_domain

[Laxman Dewangan]

On Tuesday 09 October 2012 11:36 AM, Mark Brown wrote:
> On Mon, Oct 08, 2012 at 09:43:23PM +0530, Laxman Dewangan wrote:
>> Allocate the irq base if it base is not porvided i.e.
>> in case of device tree invocation of this driver.
>> Convert the tps6586x driver to irq domain, using a
>> legacy IRQ mapping if an irq_base is specified in
>> platform data or dynamically allocated and otherwise
>> using a linear mapping.
> Reviewed-by: Mark Brown<broonie@opensource.wolfsonmicro.com>
>
> but can you convert to regmap_irq?

Yes, the motivation was this when I started this cleanups but found that 
there is 4 interrupt status and 5 interrupt mask register. Probably we 
need to pass the number of interrupt status and interrupt mask register 
to expand the regmap-irq framework to handle this case also. If it is 
fine then I can work towards that.

Re: [PATCH 1/3] mfd: Convert tps6586x to irq_domain

[Mark Brown]

On Tue, Oct 09, 2012 at 11:18:24AM +0530, Laxman Dewangan wrote:
> On Tuesday 09 October 2012 11:36 AM, Mark Brown wrote:

> >but can you convert to regmap_irq?

> Yes, the motivation was this when I started this cleanups but found
> that there is 4 interrupt status and 5 interrupt mask register.
> Probably we need to pass the number of interrupt status and
> interrupt mask register to expand the regmap-irq framework to handle
> this case also. If it is fine then I can work towards that.

Can you explain in more detail - I guess there's a different mapping of
the bits into the registers for mask and status (in which case the
generic code just won't work anyway).

Re: [PATCH 1/3] mfd: Convert tps6586x to irq_domain

[Laxman Dewangan]

On Tuesday 09 October 2012 11:53 AM, Mark Brown wrote:
> On Tue, Oct 09, 2012 at 11:18:24AM +0530, Laxman Dewangan wrote:
>> On Tuesday 09 October 2012 11:36 AM, Mark Brown wrote:
>>> but can you convert to regmap_irq?
>> Yes, the motivation was this when I started this cleanups but found
>> that there is 4 interrupt status and 5 interrupt mask register.
>> Probably we need to pass the number of interrupt status and
>> interrupt mask register to expand the regmap-irq framework to handle
>> this case also. If it is fine then I can work towards that.
> Can you explain in more detail - I guess there's a different mapping of
> the bits into the registers for mask and status (in which case the
> generic code just won't work anyway).
The bit definitions are also different in status and mask register and 
yes, this will be again problem as we do in isr thread
  data->status_buf[i] &= ~data->mask_buf[i];

So given the constraint, generic will not work here.

Re: [PATCH 1/3] mfd: Convert tps6586x to irq_domain

[Mark Brown]

On Tue, Oct 09, 2012 at 11:34:47AM +0530, Laxman Dewangan wrote:

> The bit definitions are also different in status and mask register
> and yes, this will be again problem as we do in isr thread
>  data->status_buf[i] &= ~data->mask_buf[i];

> So given the constraint, generic will not work here.

OK, that's a good reason not to use the generic code.

Re: [PATCH 1/3] mfd: Convert tps6586x to irq_domain

[Laxman Dewangan]

Hi Mark/Samuel,
There is one more change for doing cleanups in regulator parsing where I 
moved the regulator parsing from mfd to regulator driver:
[PATCH] mfd: tps6586x: move regulator dt parsing to regulator driver

Although this series and above change is independent but I like to go 
these in same branch to avoid the merge conflict.

If this is fine to go in same branch theb I can create a new series of 
these 3 changes and above change.
These series is already reviewed by Mark.

Please let me know your opinion.


Thanks,
Laxman

On Tuesday 09 October 2012 12:10 PM, Mark Brown wrote:
> On Tue, Oct 09, 2012 at 11:34:47AM +0530, Laxman Dewangan wrote:
>
>> The bit definitions are also different in status and mask register
>> and yes, this will be again problem as we do in isr thread
>>   data->status_buf[i]&= ~data->mask_buf[i];
>> So given the constraint, generic will not work here.
> OK, that's a good reason not to use the generic code.

Re: [PATCH 1/3] mfd: Convert tps6586x to irq_domain

[Laxman Dewangan]

Hi Mark/Samuel,
There is one more change for doing cleanups in regulator parsing where I 
moved the regulator parsing from mfd to regulator driver:
[PATCH] mfd: tps6586x: move regulator dt parsing to regulator driver

Although this series and above change is independent but I like to go 
these in same branch to avoid the merge conflict.

If this is fine to go in same branch theb I can create a new series of 
these 3 changes and above change.
These series is already reviewed by Mark.

Please let me know your opinion.


Thanks,
Laxman

On Tuesday 09 October 2012 12:10 PM, Mark Brown wrote:
> On Tue, Oct 09, 2012 at 11:34:47AM +0530, Laxman Dewangan wrote:
>
>> The bit definitions are also different in status and mask register
>> and yes, this will be again problem as we do in isr thread
>>   data->status_buf[i]&= ~data->mask_buf[i];
>> So given the constraint, generic will not work here.
> OK, that's a good reason not to use the generic code.

[PATCH 2/3] mfd: tps6586x: add irq io-resource for rtc sub driver

[Laxman Dewangan]

Add IRQ IORESOURCE for rtc sub driver of this device.
The rtc driver can get the irq by calling platform_get_irq().

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/mfd/tps6586x.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c
index 2cdf1e6..c11539a 100644
--- a/drivers/mfd/tps6586x.c
+++ b/drivers/mfd/tps6586x.c
@@ -96,12 +96,22 @@ static const struct tps6586x_irq_data tps6586x_irqs[] = {
 	[TPS6586X_INT_RTC_ALM2] = TPS6586X_IRQ(TPS6586X_INT_MASK4, 1 << 1),
 };
 
+static struct resource tps6586x_rtc_resources[] = {
+	{
+		.start  = TPS6586X_INT_RTC_ALM1,
+		.end	= TPS6586X_INT_RTC_ALM1,
+		.flags	= IORESOURCE_IRQ,
+	},
+};
+
 static struct mfd_cell tps6586x_cell[] = {
 	{
 		.name = "tps6586x-gpio",
 	},
 	{
 		.name = "tps6586x-rtc",
+		.num_resources = ARRAY_SIZE(tps6586x_rtc_resources),
+		.resources = &tps6586x_rtc_resources[0],
 	},
 	{
 		.name = "tps6586x-onkey",
@@ -562,7 +572,7 @@ static int __devinit tps6586x_i2c_probe(struct i2c_client *client,
 
 	ret = mfd_add_devices(tps6586x->dev, -1,
 			      tps6586x_cell, ARRAY_SIZE(tps6586x_cell),
-			      NULL, 0, NULL);
+			      NULL, 0, tps6586x->irq_domain);
 	if (ret < 0) {
 		dev_err(&client->dev, "mfd_add_devices failed: %d\n", ret);
 		goto err_mfd_add;
-- 
1.7.1.1

Re: [PATCH 2/3] mfd: tps6586x: add irq io-resource for rtc sub driver

[Mark Brown]

On Mon, Oct 08, 2012 at 09:43:24PM +0530, Laxman Dewangan wrote:
> Add IRQ IORESOURCE for rtc sub driver of this device.
> The rtc driver can get the irq by calling platform_get_irq().
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>

Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

[PATCH 3/3] mfd: tps6586x: implement gpio_to_irq

[Laxman Dewangan]

The TPS6586x adds the interrupt of this device using
linear mapping on irq domain.
Hence, implement gpio_to_irq to get the irq number
corresponding to TPS6586x GPIOs which is created
dynamically.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/gpio/gpio-tps6586x.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/gpio/gpio-tps6586x.c b/drivers/gpio/gpio-tps6586x.c
index 2526b3b..62e9e1c 100644
--- a/drivers/gpio/gpio-tps6586x.c
+++ b/drivers/gpio/gpio-tps6586x.c
@@ -80,6 +80,14 @@ static int tps6586x_gpio_output(struct gpio_chip *gc, unsigned offset,
 				val, mask);
 }
 
+static int tps6586x_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
+{
+	struct tps6586x_gpio *tps6586x_gpio = to_tps6586x_gpio(gc);
+
+	return tps6586x_irq_get_virq(tps6586x_gpio->parent,
+				TPS6586X_INT_PLDO_0 + offset);
+}
+
 static int __devinit tps6586x_gpio_probe(struct platform_device *pdev)
 {
 	struct tps6586x_platform_data *pdata;
@@ -106,6 +114,7 @@ static int __devinit tps6586x_gpio_probe(struct platform_device *pdev)
 	tps6586x_gpio->gpio_chip.direction_output = tps6586x_gpio_output;
 	tps6586x_gpio->gpio_chip.set	= tps6586x_gpio_set;
 	tps6586x_gpio->gpio_chip.get	= tps6586x_gpio_get;
+	tps6586x_gpio->gpio_chip.to_irq	= tps6586x_gpio_to_irq;
 
 #ifdef CONFIG_OF_GPIO
 	tps6586x_gpio->gpio_chip.of_node = pdev->dev.parent->of_node;
-- 
1.7.1.1

Re: [PATCH 3/3] mfd: tps6586x: implement gpio_to_irq

[Mark Brown]

On Mon, Oct 08, 2012 at 09:43:25PM +0530, Laxman Dewangan wrote:
> The TPS6586x adds the interrupt of this device using
> linear mapping on irq domain.
> Hence, implement gpio_to_irq to get the irq number
> corresponding to TPS6586x GPIOs which is created
> dynamically.

Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Re: [PATCH 3/3] mfd: tps6586x: implement gpio_to_irq

[Linus Walleij]

On Mon, Oct 8, 2012 at 6:13 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:

> The TPS6586x adds the interrupt of this device using
> linear mapping on irq domain.
> Hence, implement gpio_to_irq to get the irq number
> corresponding to TPS6586x GPIOs which is created
> dynamically.
>
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>

Looks good to me:
Acked-by: Linus Walleij <linus.walleij@linaro.org>

I guess you will funnel this through Sam's MFD tree?

Yours,
Linus Walleij