[07/10] mfd: menelaus: Start to use irqdomain

Commit Message

Felipe Balbi Dec. 17, 2013, 12:08 a.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 <[e-mail]>
Signed-off-by: Felipe Balbi <[e-mail]>
---
 drivers/mfd/menelaus.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 119 insertions(+), 4 deletions(-)

Comments

Lee Jones Dec. 17, 2013, 10:32 a.m. UTC | #1

On Mon, 16 Dec 2013, Felipe Balbi 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 <[e-mail]>
> Signed-off-by: Felipe Balbi <[e-mail]>
> ---
>  drivers/mfd/menelaus.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 119 insertions(+), 4 deletions(-)
> 

<snip>

> +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;

Shoudn't this now be?
  int irq = data->hwirq;

> +	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;

Same for here, and all the others.

<snip>

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

Please use irq_domain_add_simple() here.

> +	m->irq_base = irq_base;

I don't think you need this.

> +	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);
> +	}

You should put this in the .map() operation and the reverse in .unmap().

>  fail:
> +	irq_free_descs(irq_base, MENELAUS_NR_IRQS);

I don't think you need to do this.

>  
>  static int menelaus_remove(struct i2c_client *client)
>  {
> +	struct menelaus_chip	*m = i2c_get_clientdata(client);
> +
> +	irq_free_descs(m->irq_base, MENELAUS_NR_IRQS);

Nor here.

>  	the_menelaus = NULL;

Shouldn't this have been removed in patch 6?

>  	return 0;
>  }
>

Patch

diff --git a/drivers/mfd/menelaus.c b/drivers/mfd/menelaus.c
index ae3209d..838d6bf 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;
 	}
 
 	/* Ack and disable all Menelaus interrupts */
@@ -1230,12 +1340,10 @@  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;
 		}
 	}
 
-	mutex_init(&m->lock);
-
 	pr_info("Menelaus rev %d.%d\n", rev >> 4, rev & 0x0f);
 
 	val = menelaus_read_reg(m, MENELAUS_VCORE_CTRL1);
@@ -1255,13 +1363,20 @@  static int menelaus_probe(struct i2c_client *client,
 	menelaus_rtc_init(m);
 
 	return 0;
+
 fail:
+	irq_free_descs(irq_base, MENELAUS_NR_IRQS);
+
 	return err;
 }
 
 static int menelaus_remove(struct i2c_client *client)
 {
+	struct menelaus_chip	*m = i2c_get_clientdata(client);
+
+	irq_free_descs(m->irq_base, MENELAUS_NR_IRQS);
 	the_menelaus = NULL;
+
 	return 0;
 }