diff mbox

[v3,11/15] mfd: menelaus: Start to use irqdomain

Message ID 1386606085-26838-11-git-send-email-balbi@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Felipe Balbi Dec. 9, 2013, 4:21 p.m. UTC 
Introduce an irq_chip and irq_domain for menelaus driver. Following
patches will convert uses to traditional request_threaded_irq().

While at that, some better error handling had to be added, so we could
free irq descs we allocated.

Tested-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/mfd/menelaus.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 120 insertions(+), 7 deletions(-)

Comments

Lee Jones Dec. 10, 2013, 9:20 a.m. UTC | #1 
> Introduce an irq_chip and irq_domain for menelaus driver. Following
> patches will convert uses to traditional request_threaded_irq().
> 
> While at that, some better error handling had to be added, so we could
> free irq descs we allocated.
> 
> Tested-by: Aaro Koskinen <aaro.koskinen@iki.fi>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
>  drivers/mfd/menelaus.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++---

<snip>

> +	irq_domain_add_legacy(node, MENELAUS_NR_IRQS, irq_base, 0,
> +			&irq_domain_simple_ops, m);

When will this driver become DT compliant?

I think you should use irq_domain_add_simple() to future proof
yourself a little.

> +	m->irq_base = irq_base;
> +
> +	for (i = irq_base; i < irq_base + MENELAUS_NR_IRQS; i++) {
> +		irq_set_chip_data(i, m);
> +		irq_set_chip_and_handler(i, &menelaus_irq_chip,
> +				handle_simple_irq);
> +		irq_set_nested_thread(i, 1);
> +		set_irq_flags(i, IRQF_VALID);

This assumes that this h/w only exists on ARM platforms. Is that true?

> +	}

This is usually completed in an *_irq_map() operation call-back.

I can't see where you reverse this process either (usually *_irq_unmap())

<snip>

> -	mutex_init(&m->lock);
> -

This doesn't belong in this patch.

It should have been removed with the final use of the lock was.

<snip>
Felipe Balbi Dec. 10, 2013, 4:34 p.m. UTC | #2 
On Tue, Dec 10, 2013 at 09:20:50AM +0000, Lee Jones wrote:
> > Introduce an irq_chip and irq_domain for menelaus driver. Following
> > patches will convert uses to traditional request_threaded_irq().
> > 
> > While at that, some better error handling had to be added, so we could
> > free irq descs we allocated.
> > 
> > Tested-by: Aaro Koskinen <aaro.koskinen@iki.fi>
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > ---
> >  drivers/mfd/menelaus.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++---
> 
> <snip>
> 
> > +	irq_domain_add_legacy(node, MENELAUS_NR_IRQS, irq_base, 0,
> > +			&irq_domain_simple_ops, m);
> 
> When will this driver become DT compliant?

when OMAP2 becomes DT-compliant. It still boots with legacy board-file +
platform_data.

> > +	m->irq_base = irq_base;
> > +
> > +	for (i = irq_base; i < irq_base + MENELAUS_NR_IRQS; i++) {
> > +		irq_set_chip_data(i, m);
> > +		irq_set_chip_and_handler(i, &menelaus_irq_chip,
> > +				handle_simple_irq);
> > +		irq_set_nested_thread(i, 1);
> > +		set_irq_flags(i, IRQF_VALID);
> 
> This assumes that this h/w only exists on ARM platforms. Is that true?

it was made *only* for Nokia to use on their ARM-only internet tablets.
Lee Jones Dec. 10, 2013, 6:37 p.m. UTC | #3 
> > > Introduce an irq_chip and irq_domain for menelaus driver. Following
> > > patches will convert uses to traditional request_threaded_irq().
> > > 
> > > While at that, some better error handling had to be added, so we could
> > > free irq descs we allocated.
> > > 
> > > Tested-by: Aaro Koskinen <aaro.koskinen@iki.fi>
> > > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > > ---
> > >  drivers/mfd/menelaus.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++---
> > 
> > <snip>
> > 
> > > +	irq_domain_add_legacy(node, MENELAUS_NR_IRQS, irq_base, 0,
> > > +			&irq_domain_simple_ops, m);
> > 
> > When will this driver become DT compliant?
> 
> when OMAP2 becomes DT-compliant. It still boots with legacy board-file +
> platform_data.

Really? Tony, are there any plans to DT the platform?

Felipe, what did you think about using irq_domain_add_simple()?

> > > +	m->irq_base = irq_base;
> > > +
> > > +	for (i = irq_base; i < irq_base + MENELAUS_NR_IRQS; i++) {
> > > +		irq_set_chip_data(i, m);
> > > +		irq_set_chip_and_handler(i, &menelaus_irq_chip,
> > > +				handle_simple_irq);
> > > +		irq_set_nested_thread(i, 1);
> > > +		set_irq_flags(i, IRQF_VALID);
> > 
> > This assumes that this h/w only exists on ARM platforms. Is that true?
> 
> it was made *only* for Nokia to use on their ARM-only internet tablets.

If the IP is genuinely not reusable, them I'm fine with it.

I see that the Kconfig is setup in the correct way too, so we're good.
Tony Lindgren Dec. 10, 2013, 6:46 p.m. UTC | #4 
* Lee Jones <lee.jones@linaro.org> [131210 10:39]:
> > > > Introduce an irq_chip and irq_domain for menelaus driver. Following
> > > > patches will convert uses to traditional request_threaded_irq().
> > > > 
> > > > While at that, some better error handling had to be added, so we could
> > > > free irq descs we allocated.
> > > > 
> > > > Tested-by: Aaro Koskinen <aaro.koskinen@iki.fi>
> > > > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > > > ---
> > > >  drivers/mfd/menelaus.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++---
> > > 
> > > <snip>
> > > 
> > > > +	irq_domain_add_legacy(node, MENELAUS_NR_IRQS, irq_base, 0,
> > > > +			&irq_domain_simple_ops, m);
> > > 
> > > When will this driver become DT compliant?
> > 
> > when OMAP2 becomes DT-compliant. It still boots with legacy board-file +
> > platform_data.
> 
> Really? Tony, are there any plans to DT the platform?

Yeah and what Felipe is doing here is an important step towards that.
We'll be booting omap2 in DT only mode with v3.14, but we still need
to rely on some pdata quirks at least for the MMC because Menelaus
does not provide the needed regulator phandles for the .dts files for
the driver to use.

Regards,

Tony
Lee Jones Dec. 11, 2013, 9:06 a.m. UTC | #5 
On Tue, 10 Dec 2013, Tony Lindgren wrote:

> * Lee Jones <lee.jones@linaro.org> [131210 10:39]:
> > > > > Introduce an irq_chip and irq_domain for menelaus driver. Following
> > > > > patches will convert uses to traditional request_threaded_irq().
> > > > > 
> > > > > While at that, some better error handling had to be added, so we could
> > > > > free irq descs we allocated.
> > > > > 
> > > > > Tested-by: Aaro Koskinen <aaro.koskinen@iki.fi>
> > > > > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > > > > ---
> > > > >  drivers/mfd/menelaus.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++---
> > > > 
> > > > <snip>
> > > > 
> > > > > +	irq_domain_add_legacy(node, MENELAUS_NR_IRQS, irq_base, 0,
> > > > > +			&irq_domain_simple_ops, m);
> > > > 
> > > > When will this driver become DT compliant?
> > > 
> > > when OMAP2 becomes DT-compliant. It still boots with legacy board-file +
> > > platform_data.
> > 
> > Really? Tony, are there any plans to DT the platform?
> 
> Yeah and what Felipe is doing here is an important step towards that.
> We'll be booting omap2 in DT only mode with v3.14

Okay, sounds good. Thanks for the clarification.

> but we still need
> to rely on some pdata quirks at least for the MMC because Menelaus
> does not provide the needed regulator phandles for the .dts files for
> the driver to use.

I'll take your word for it. :)
diff mbox

Patch

diff --git a/drivers/mfd/menelaus.c b/drivers/mfd/menelaus.c
index aa3c579..e4f33b7 100644
--- a/drivers/mfd/menelaus.c
+++ b/drivers/mfd/menelaus.c
@@ -34,6 +34,7 @@ 
 #include <linux/module.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
+#include <linux/irqdomain.h>
 #include <linux/sched.h>
 #include <linux/mutex.h>
 #include <linux/delay.h>
@@ -47,6 +48,7 @@ 
 #include <asm/gpio.h>
 
 #define DRIVER_NAME			"menelaus"
+#define MENELAUS_NR_IRQS		16
 
 #define MENELAUS_I2C_ADDRESS		0x72
 
@@ -168,11 +170,19 @@  struct menelaus_chip {
 	u8			rtc_control;
 	unsigned		uie:1;
 #endif
+	int			irq_base;
 	unsigned		vcore_hw_mode:1;
 	u8			mask1, mask2;
+	u8			ack1, ack2;
+
 	void			(*handlers[16])(struct menelaus_chip *);
 	void			(*mmc_callback)(void *data, u8 mask);
 	void			*mmc_callback_data;
+
+	unsigned		mask1_pending:1;
+	unsigned		mask2_pending:1;
+	unsigned		ack1_pending:1;
+	unsigned		ack2_pending:1;
 };
 
 static struct menelaus_chip *the_menelaus;
@@ -235,6 +245,83 @@  static int menelaus_ack_irq(struct menelaus_chip *m, int irq)
 		return menelaus_write_reg(m, MENELAUS_INT_ACK1, 1 << irq);
 }
 
+static void menelaus_irq_ack(struct irq_data *data)
+{
+	struct menelaus_chip *m = irq_data_get_irq_chip_data(data);
+	int irq = data->irq - m->irq_base;
+
+	if (irq > 7) {
+		m->ack2 |= BIT(irq);
+		m->ack2_pending = true;
+	} else {
+		m->ack1 |= BIT(irq);
+		m->ack1_pending = true;
+	}
+}
+
+static void menelaus_irq_mask(struct irq_data *data)
+{
+	struct menelaus_chip *m = irq_data_get_irq_chip_data(data);
+	int irq = data->irq - m->irq_base;
+
+	if (irq > 7) {
+		m->mask2 |= BIT(irq);
+		m->mask2_pending = true;
+	} else {
+		m->mask1 |= BIT(irq);
+		m->mask1_pending = true;
+	}
+}
+
+static void menelaus_irq_unmask(struct irq_data *data)
+{
+	struct menelaus_chip *m = irq_data_get_irq_chip_data(data);
+	int irq = data->irq - m->irq_base;
+
+	if (irq > 7) {
+		m->mask2 &= ~BIT(irq);
+		m->mask2_pending = true;
+	} else {
+		m->mask1 &= ~BIT(irq);
+		m->mask1_pending = true;
+	}
+}
+
+static void menelaus_irq_bus_lock(struct irq_data *data)
+{
+	struct menelaus_chip *m = irq_data_get_irq_chip_data(data);
+
+	mutex_lock(&m->lock);
+}
+
+static void menelaus_irq_bus_sync_unlock(struct irq_data *data)
+{
+	struct menelaus_chip *m = irq_data_get_irq_chip_data(data);
+
+	if (m->ack1_pending)
+		menelaus_write_reg(m, MENELAUS_INT_ACK1, m->ack1);
+
+	if (m->ack2_pending)
+		menelaus_write_reg(m, MENELAUS_INT_ACK2, m->ack2);
+
+	if (m->mask1_pending)
+		menelaus_write_reg(m, MENELAUS_INT_MASK1, m->mask1);
+
+	if (m->mask2_pending)
+		menelaus_write_reg(m, MENELAUS_INT_MASK2, m->mask2);
+
+	mutex_unlock(&m->lock);
+}
+
+static struct irq_chip menelaus_irq_chip = {
+	.name		= "menelaus",
+	.irq_ack	= menelaus_irq_ack,
+	.irq_mask	= menelaus_irq_mask,
+	.irq_unmask	= menelaus_irq_unmask,
+	.irq_bus_lock	= menelaus_irq_bus_lock,
+	.irq_bus_sync_unlock = menelaus_irq_bus_sync_unlock,
+};
+
 /* Adds a handler for an interrupt. Does not run in interrupt context */
 static int menelaus_add_irq_work(struct menelaus_chip *m, int irq,
 		void (*handler)(struct menelaus_chip *))
@@ -1186,8 +1273,11 @@  static int menelaus_probe(struct i2c_client *client,
 			  const struct i2c_device_id *id)
 {
 	struct menelaus_chip	*m;
+	struct device_node	*node = client->dev.of_node;
 	int			rev = 0, val;
 	int			err = 0;
+	int			irq_base;
+	int			i;
 	struct menelaus_platform_data *menelaus_pdata =
 					dev_get_platdata(&client->dev);
 
@@ -1205,12 +1295,32 @@  static int menelaus_probe(struct i2c_client *client,
 
 	the_menelaus = m;
 	m->client = client;
+	mutex_init(&m->lock);
+
+	irq_base = irq_alloc_descs(-1, 0, MENELAUS_NR_IRQS, 0);
+	if (irq_base < 0) {
+		dev_err(&client->dev, "failed to allocate irq descs\n");
+		return irq_base;
+	}
+
+	irq_domain_add_legacy(node, MENELAUS_NR_IRQS, irq_base, 0,
+			&irq_domain_simple_ops, m);
+
+	m->irq_base = irq_base;
+
+	for (i = irq_base; i < irq_base + MENELAUS_NR_IRQS; i++) {
+		irq_set_chip_data(i, m);
+		irq_set_chip_and_handler(i, &menelaus_irq_chip,
+				handle_simple_irq);
+		irq_set_nested_thread(i, 1);
+		set_irq_flags(i, IRQF_VALID);
+	}
 
 	/* If a true probe check the device */
 	rev = menelaus_read_reg(m, MENELAUS_REV);
 	if (rev < 0) {
 		pr_err(DRIVER_NAME ": device not found");
-		return -ENODEV;
+		goto fail_free_descs;
 	}
 
 	/* Ack and disable all Menelaus interrupts */
@@ -1230,17 +1340,15 @@  static int menelaus_probe(struct i2c_client *client,
 		if (err) {
 			dev_dbg(&client->dev,  "can't get IRQ %d, err %d\n",
 					client->irq, err);
-			return err;
+			goto fail_free_descs;
 		}
 	}
 
-	mutex_init(&m->lock);
-
 	pr_info("Menelaus rev %d.%d\n", rev >> 4, rev & 0x0f);
 
 	val = menelaus_read_reg(m, MENELAUS_VCORE_CTRL1);
 	if (val < 0)
-		goto fail;
+		goto fail_free_irq;
 	if (val & (1 << 7))
 		m->vcore_hw_mode = 1;
 	else
@@ -1249,14 +1357,18 @@  static int menelaus_probe(struct i2c_client *client,
 	if (menelaus_pdata != NULL && menelaus_pdata->late_init != NULL) {
 		err = menelaus_pdata->late_init(&client->dev);
 		if (err < 0)
-			goto fail;
+			goto fail_free_irq;
 	}
 
 	menelaus_rtc_init(m);
 
 	return 0;
-fail:
+fail_free_irq:
 	free_irq(client->irq, m);
+
+fail_free_descs:
+	irq_free_descs(irq_base, MENELAUS_NR_IRQS);
+
 	return err;
 }
 
@@ -1265,6 +1377,7 @@  static int menelaus_remove(struct i2c_client *client)
 	struct menelaus_chip	*m = i2c_get_clientdata(client);
 
 	free_irq(client->irq, m);
+	irq_free_descs(m->irq_base, MENELAUS_NR_IRQS);
 	the_menelaus = NULL;
 	return 0;
 }