All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] MFD: pcf50633: Fixes and updates
@ 2010-05-12  0:10 Lars-Peter Clausen
  2010-05-12  0:10 ` [PATCH 1/4] MFD: pcf50633: Fix bitfield logic in interrupt handler Lars-Peter Clausen
  2010-05-16 22:02 ` [PATCH 0/4] MFD: pcf50633: Fixes and updates Samuel Ortiz
  0 siblings, 2 replies; 6+ messages in thread
From: Lars-Peter Clausen @ 2010-05-12  0:10 UTC (permalink / raw)
  To: sameo; +Cc: fercerpav, linux-kernel, Lars-Peter Clausen

Hi

This patch series contains fixes and updates for the pcf50633 mfd and adc
driver.

Lars-Peter Clausen (4):
  MFD: pcf50633: Fix bitfield logic in interrupt handler
  MFD: pcf50633-adc: Fix potential race pcf50633_adc_sync_read
  MFD: pcf50633: Use threaded irq
  MFD: pcf50633: Move irq related functions to its own file.

 drivers/mfd/Makefile        |    3 +-
 drivers/mfd/pcf50633-adc.c  |   39 ++---
 drivers/mfd/pcf50633-core.c |  344 ++-----------------------------------------
 drivers/mfd/pcf50633-irq.c  |  318 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 346 insertions(+), 358 deletions(-)
 create mode 100644 drivers/mfd/pcf50633-irq.c


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/4] MFD: pcf50633: Fix bitfield logic in interrupt handler
  2010-05-12  0:10 [PATCH 0/4] MFD: pcf50633: Fixes and updates Lars-Peter Clausen
@ 2010-05-12  0:10 ` Lars-Peter Clausen
  2010-05-12  0:10   ` [PATCH 2/4] MFD: pcf50633-adc: Fix potential race pcf50633_adc_sync_read Lars-Peter Clausen
  2010-05-16 22:02 ` [PATCH 0/4] MFD: pcf50633: Fixes and updates Samuel Ortiz
  1 sibling, 1 reply; 6+ messages in thread
From: Lars-Peter Clausen @ 2010-05-12  0:10 UTC (permalink / raw)
  To: sameo; +Cc: fercerpav, linux-kernel, Lars-Peter Clausen

Those constants are alreay bitfields.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/mfd/pcf50633-core.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/pcf50633-core.c b/drivers/mfd/pcf50633-core.c
index d66f389..6f7fb60 100644
--- a/drivers/mfd/pcf50633-core.c
+++ b/drivers/mfd/pcf50633-core.c
@@ -354,18 +354,18 @@ static void pcf50633_irq_worker(struct work_struct *work)
 	if (pcf_int[0] & (PCF50633_INT1_USBINS | PCF50633_INT1_USBREM)) {
 		chgstat = pcf50633_reg_read(pcf, PCF50633_REG_MBCS2);
 		if (chgstat & (0x3 << 4))
-			pcf_int[0] &= ~(1 << PCF50633_INT1_USBREM);
+			pcf_int[0] &= ~PCF50633_INT1_USBREM;
 		else
-			pcf_int[0] &= ~(1 << PCF50633_INT1_USBINS);
+			pcf_int[0] &= ~PCF50633_INT1_USBINS;
 	}
 
 	/* Make sure only one of ADPINS or ADPREM is set */
 	if (pcf_int[0] & (PCF50633_INT1_ADPINS | PCF50633_INT1_ADPREM)) {
 		chgstat = pcf50633_reg_read(pcf, PCF50633_REG_MBCS2);
 		if (chgstat & (0x3 << 4))
-			pcf_int[0] &= ~(1 << PCF50633_INT1_ADPREM);
+			pcf_int[0] &= ~PCF50633_INT1_ADPREM;
 		else
-			pcf_int[0] &= ~(1 << PCF50633_INT1_ADPINS);
+			pcf_int[0] &= ~PCF50633_INT1_ADPINS;
 	}
 
 	dev_dbg(pcf->dev, "INT1=0x%02x INT2=0x%02x INT3=0x%02x "
-- 
1.5.6.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/4] MFD: pcf50633-adc: Fix potential race pcf50633_adc_sync_read
  2010-05-12  0:10 ` [PATCH 1/4] MFD: pcf50633: Fix bitfield logic in interrupt handler Lars-Peter Clausen
@ 2010-05-12  0:10   ` Lars-Peter Clausen
  2010-05-12  0:10     ` [PATCH 3/4] MFD: pcf50633: Use threaded irq Lars-Peter Clausen
  0 siblings, 1 reply; 6+ messages in thread
From: Lars-Peter Clausen @ 2010-05-12  0:10 UTC (permalink / raw)
  To: sameo; +Cc: fercerpav, linux-kernel, Lars-Peter Clausen

Currently it's not guaranteed that request struct is not already freed when
reading from it. Fix this by moving synced request related fields from the
pcf50633_adc_request struct to its own struct and store it on the functions
stack.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/mfd/pcf50633-adc.c |   39 +++++++++++++++------------------------
 1 files changed, 15 insertions(+), 24 deletions(-)

diff --git a/drivers/mfd/pcf50633-adc.c b/drivers/mfd/pcf50633-adc.c
index fe8f922..aed0d2a 100644
--- a/drivers/mfd/pcf50633-adc.c
+++ b/drivers/mfd/pcf50633-adc.c
@@ -30,13 +30,13 @@
 struct pcf50633_adc_request {
 	int mux;
 	int avg;
-	int result;
 	void (*callback)(struct pcf50633 *, void *, int);
 	void *callback_param;
+};
 
-	/* Used in case of sync requests */
+struct pcf50633_adc_sync_request {
+	int result;
 	struct completion completion;
-
 };
 
 #define PCF50633_MAX_ADC_FIFO_DEPTH 8
@@ -109,10 +109,10 @@ adc_enqueue_request(struct pcf50633 *pcf, struct pcf50633_adc_request *req)
 	return 0;
 }
 
-static void
-pcf50633_adc_sync_read_callback(struct pcf50633 *pcf, void *param, int result)
+static void pcf50633_adc_sync_read_callback(struct pcf50633 *pcf, void *param,
+	int result)
 {
-	struct pcf50633_adc_request *req = param;
+	struct pcf50633_adc_sync_request *req = param;
 
 	req->result = result;
 	complete(&req->completion);
@@ -120,28 +120,19 @@ pcf50633_adc_sync_read_callback(struct pcf50633 *pcf, void *param, int result)
 
 int pcf50633_adc_sync_read(struct pcf50633 *pcf, int mux, int avg)
 {
-	struct pcf50633_adc_request *req;
-	int err;
+	struct pcf50633_adc_sync_request req;
+	int ret;
 
-	/* req is freed when the result is ready, in interrupt handler */
-	req = kzalloc(sizeof(*req), GFP_KERNEL);
-	if (!req)
-		return -ENOMEM;
-
-	req->mux = mux;
-	req->avg = avg;
-	req->callback =  pcf50633_adc_sync_read_callback;
-	req->callback_param = req;
+	init_completion(&req.completion);
 
-	init_completion(&req->completion);
-	err = adc_enqueue_request(pcf, req);
-	if (err)
-		return err;
+	ret = pcf50633_adc_async_read(pcf, mux, avg,
+		pcf50633_adc_sync_read_callback, &req);
+	if (ret)
+		return ret;
 
-	wait_for_completion(&req->completion);
+	wait_for_completion(&req.completion);
 
-	/* FIXME by this time req might be already freed */
-	return req->result;
+	return req.result;
 }
 EXPORT_SYMBOL_GPL(pcf50633_adc_sync_read);
 
-- 
1.5.6.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/4] MFD: pcf50633: Use threaded irq
  2010-05-12  0:10   ` [PATCH 2/4] MFD: pcf50633-adc: Fix potential race pcf50633_adc_sync_read Lars-Peter Clausen
@ 2010-05-12  0:10     ` Lars-Peter Clausen
  2010-05-12  0:10       ` [PATCH 4/4] MFD: pcf50633: Move irq related functions to its own file Lars-Peter Clausen
  0 siblings, 1 reply; 6+ messages in thread
From: Lars-Peter Clausen @ 2010-05-12  0:10 UTC (permalink / raw)
  To: sameo; +Cc: fercerpav, linux-kernel, Lars-Peter Clausen

Use threaded oneshot irq handler instead of normal irq handler and a workqueue.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/mfd/Makefile        |    2 +-
 drivers/mfd/pcf50633-core.c |   57 +++++++------------------------------------
 2 files changed, 10 insertions(+), 49 deletions(-)

diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 4c684c1..8593496 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -57,7 +57,7 @@ obj-$(CONFIG_PMIC_DA903X)	+= da903x.o
 max8925-objs			:= max8925-core.o max8925-i2c.o
 obj-$(CONFIG_MFD_MAX8925)	+= max8925.o
 
-obj-$(CONFIG_MFD_PCF50633)	+= pcf50633-core.o
+obj-$(CONFIG_MFD_PCF50633)	+= pcf50633-core.o pcf50633-irq.o
 obj-$(CONFIG_PCF50633_ADC)	+= pcf50633-adc.o
 obj-$(CONFIG_PCF50633_GPIO)	+= pcf50633-gpio.o
 obj-$(CONFIG_ABX500_CORE)	+= abx500-core.o
diff --git a/drivers/mfd/pcf50633-core.c b/drivers/mfd/pcf50633-core.c
index 6f7fb60..fc5cd7f 100644
--- a/drivers/mfd/pcf50633-core.c
+++ b/drivers/mfd/pcf50633-core.c
@@ -325,14 +325,12 @@ static void pcf50633_irq_call_handler(struct pcf50633 *pcf, int irq)
 /* Maximum amount of time ONKEY is held before emergency action is taken */
 #define PCF50633_ONKEY1S_TIMEOUT 8
 
-static void pcf50633_irq_worker(struct work_struct *work)
+static irqreturn_t pcf50633_irq(int irq, void *data)
 {
-	struct pcf50633 *pcf;
+	struct pcf50633 *pcf = data;
 	int ret, i, j;
 	u8 pcf_int[5], chgstat;
 
-	pcf = container_of(work, struct pcf50633, irq_work);
-
 	/* Read the 5 INT regs in one transaction */
 	ret = pcf50633_read_block(pcf, PCF50633_REG_INT1,
 						ARRAY_SIZE(pcf_int), pcf_int);
@@ -436,21 +434,7 @@ static void pcf50633_irq_worker(struct work_struct *work)
 	}
 
 out:
-	put_device(pcf->dev);
-	enable_irq(pcf->irq);
-}
-
-static irqreturn_t pcf50633_irq(int irq, void *data)
-{
-	struct pcf50633 *pcf = data;
-
-	dev_dbg(pcf->dev, "pcf50633_irq\n");
-
-	get_device(pcf->dev);
-	disable_irq_nosync(pcf->irq);
-	queue_work(pcf->work_queue, &pcf->irq_work);
-
-	return IRQ_HANDLED;
+	return IRQ_HANDLED
 }
 
 static void
@@ -488,9 +472,6 @@ static int pcf50633_suspend(struct i2c_client *client, pm_message_t state)
 	 * henceforth */
 	disable_irq(pcf->irq);
 
-	/* Make sure that any running IRQ worker has quit */
-	cancel_work_sync(&pcf->irq_work);
-
 	/* Save the masks */
 	ret = pcf50633_read_block(pcf, PCF50633_REG_INT1M,
 				ARRAY_SIZE(pcf->suspend_irq_masks),
@@ -531,16 +512,7 @@ static int pcf50633_resume(struct i2c_client *client)
 	if (ret < 0)
 		dev_err(pcf->dev, "Error restoring saved suspend masks\n");
 
-	/* Restore regulators' state */
-
-
-	get_device(pcf->dev);
-
-	/*
-	 * Clear any pending interrupts and set resume reason if any.
-	 * This will leave with enable_irq()
-	 */
-	pcf50633_irq_worker(&pcf->irq_work);
+	enable_irq(pcf->irq);
 
 	return 0;
 }
@@ -574,22 +546,13 @@ static int __devinit pcf50633_probe(struct i2c_client *client,
 	pcf->dev = &client->dev;
 	pcf->i2c_client = client;
 	pcf->irq = client->irq;
-	pcf->work_queue = create_singlethread_workqueue("pcf50633");
-
-	if (!pcf->work_queue) {
-		dev_err(&client->dev, "Failed to alloc workqueue\n");
-		ret = -ENOMEM;
-		goto err_free;
-	}
-
-	INIT_WORK(&pcf->irq_work, pcf50633_irq_worker);
 
 	version = pcf50633_reg_read(pcf, 0);
 	variant = pcf50633_reg_read(pcf, 1);
 	if (version < 0 || variant < 0) {
 		dev_err(pcf->dev, "Unable to probe pcf50633\n");
 		ret = -ENODEV;
-		goto err_destroy_workqueue;
+		goto err_free;
 	}
 
 	dev_info(pcf->dev, "Probed device version %d variant %d\n",
@@ -603,12 +566,13 @@ static int __devinit pcf50633_probe(struct i2c_client *client,
 	pcf50633_reg_write(pcf, PCF50633_REG_INT4M, 0x00);
 	pcf50633_reg_write(pcf, PCF50633_REG_INT5M, 0x00);
 
-	ret = request_irq(client->irq, pcf50633_irq,
-					IRQF_TRIGGER_LOW, "pcf50633", pcf);
+	ret = request_threaded_irq(client->irq, NULL, pcf50633_irq,
+					IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+					"pcf50633", pcf);
 
 	if (ret) {
 		dev_err(pcf->dev, "Failed to request IRQ %d\n", ret);
-		goto err_destroy_workqueue;
+		goto err_free;
 	}
 
 	/* Create sub devices */
@@ -651,8 +615,6 @@ static int __devinit pcf50633_probe(struct i2c_client *client,
 
 	return 0;
 
-err_destroy_workqueue:
-	destroy_workqueue(pcf->work_queue);
 err_free:
 	i2c_set_clientdata(client, NULL);
 	kfree(pcf);
@@ -666,7 +628,6 @@ static int __devexit pcf50633_remove(struct i2c_client *client)
 	int i;
 
 	free_irq(pcf->irq, pcf);
-	destroy_workqueue(pcf->work_queue);
 
 	platform_device_unregister(pcf->input_pdev);
 	platform_device_unregister(pcf->rtc_pdev);
-- 
1.5.6.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4/4] MFD: pcf50633: Move irq related functions to its own file.
  2010-05-12  0:10     ` [PATCH 3/4] MFD: pcf50633: Use threaded irq Lars-Peter Clausen
@ 2010-05-12  0:10       ` Lars-Peter Clausen
  0 siblings, 0 replies; 6+ messages in thread
From: Lars-Peter Clausen @ 2010-05-12  0:10 UTC (permalink / raw)
  To: sameo; +Cc: fercerpav, linux-kernel, Lars-Peter Clausen

This reduces code clutter a bit and will ease an migration to genirq.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/mfd/Makefile        |    3 +-
 drivers/mfd/pcf50633-core.c |  303 ++---------------------------------------
 drivers/mfd/pcf50633-irq.c  |  318 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 330 insertions(+), 294 deletions(-)
 create mode 100644 drivers/mfd/pcf50633-irq.c

diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 8593496..ca1517e 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -57,7 +57,8 @@ obj-$(CONFIG_PMIC_DA903X)	+= da903x.o
 max8925-objs			:= max8925-core.o max8925-i2c.o
 obj-$(CONFIG_MFD_MAX8925)	+= max8925.o
 
-obj-$(CONFIG_MFD_PCF50633)	+= pcf50633-core.o pcf50633-irq.o
+pcf50633-objs			:= pcf50633-core.o pcf50633-irq.o
+obj-$(CONFIG_MFD_PCF50633)	+= pcf50633.o
 obj-$(CONFIG_PCF50633_ADC)	+= pcf50633-adc.o
 obj-$(CONFIG_PCF50633_GPIO)	+= pcf50633-gpio.o
 obj-$(CONFIG_ABX500_CORE)	+= abx500-core.o
diff --git a/drivers/mfd/pcf50633-core.c b/drivers/mfd/pcf50633-core.c
index fc5cd7f..e1ee046 100644
--- a/drivers/mfd/pcf50633-core.c
+++ b/drivers/mfd/pcf50633-core.c
@@ -21,16 +21,16 @@
 #include <linux/workqueue.h>
 #include <linux/platform_device.h>
 #include <linux/i2c.h>
-#include <linux/irq.h>
 #include <linux/slab.h>
 
 #include <linux/mfd/pcf50633/core.h>
 
-/* Two MBCS registers used during cold start */
-#define PCF50633_REG_MBCS1		0x4b
-#define PCF50633_REG_MBCS2		0x4c
-#define PCF50633_MBCS1_USBPRES 		0x01
-#define PCF50633_MBCS1_ADAPTPRES	0x01
+int pcf50633_irq_init(struct pcf50633 *pcf, int irq);
+void pcf50633_irq_free(struct pcf50633 *pcf);
+#ifdef CONFIG_PM
+int pcf50633_irq_suspend(struct pcf50633 *pcf);
+int pcf50633_irq_resume(struct pcf50633 *pcf);
+#endif
 
 static int __pcf50633_read(struct pcf50633 *pcf, u8 reg, int num, u8 *data)
 {
@@ -215,228 +215,6 @@ static struct attribute_group pcf_attr_group = {
 	.attrs	= pcf_sysfs_entries,
 };
 
-int pcf50633_register_irq(struct pcf50633 *pcf, int irq,
-			void (*handler) (int, void *), void *data)
-{
-	if (irq < 0 || irq >= PCF50633_NUM_IRQ || !handler)
-		return -EINVAL;
-
-	if (WARN_ON(pcf->irq_handler[irq].handler))
-		return -EBUSY;
-
-	mutex_lock(&pcf->lock);
-	pcf->irq_handler[irq].handler = handler;
-	pcf->irq_handler[irq].data = data;
-	mutex_unlock(&pcf->lock);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(pcf50633_register_irq);
-
-int pcf50633_free_irq(struct pcf50633 *pcf, int irq)
-{
-	if (irq < 0 || irq >= PCF50633_NUM_IRQ)
-		return -EINVAL;
-
-	mutex_lock(&pcf->lock);
-	pcf->irq_handler[irq].handler = NULL;
-	mutex_unlock(&pcf->lock);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(pcf50633_free_irq);
-
-static int __pcf50633_irq_mask_set(struct pcf50633 *pcf, int irq, u8 mask)
-{
-	u8 reg, bits, tmp;
-	int ret = 0, idx;
-
-	idx = irq >> 3;
-	reg =  PCF50633_REG_INT1M + idx;
-	bits = 1 << (irq & 0x07);
-
-	mutex_lock(&pcf->lock);
-
-	if (mask) {
-		ret = __pcf50633_read(pcf, reg, 1, &tmp);
-		if (ret < 0)
-			goto out;
-
-		tmp |= bits;
-
-		ret = __pcf50633_write(pcf, reg, 1, &tmp);
-		if (ret < 0)
-			goto out;
-
-		pcf->mask_regs[idx] &= ~bits;
-		pcf->mask_regs[idx] |= bits;
-	} else {
-		ret = __pcf50633_read(pcf, reg, 1, &tmp);
-		if (ret < 0)
-			goto out;
-
-		tmp &= ~bits;
-
-		ret = __pcf50633_write(pcf, reg, 1, &tmp);
-		if (ret < 0)
-			goto out;
-
-		pcf->mask_regs[idx] &= ~bits;
-	}
-out:
-	mutex_unlock(&pcf->lock);
-
-	return ret;
-}
-
-int pcf50633_irq_mask(struct pcf50633 *pcf, int irq)
-{
-	dev_dbg(pcf->dev, "Masking IRQ %d\n", irq);
-
-	return __pcf50633_irq_mask_set(pcf, irq, 1);
-}
-EXPORT_SYMBOL_GPL(pcf50633_irq_mask);
-
-int pcf50633_irq_unmask(struct pcf50633 *pcf, int irq)
-{
-	dev_dbg(pcf->dev, "Unmasking IRQ %d\n", irq);
-
-	return __pcf50633_irq_mask_set(pcf, irq, 0);
-}
-EXPORT_SYMBOL_GPL(pcf50633_irq_unmask);
-
-int pcf50633_irq_mask_get(struct pcf50633 *pcf, int irq)
-{
-	u8 reg, bits;
-
-	reg =  irq >> 3;
-	bits = 1 << (irq & 0x07);
-
-	return pcf->mask_regs[reg] & bits;
-}
-EXPORT_SYMBOL_GPL(pcf50633_irq_mask_get);
-
-static void pcf50633_irq_call_handler(struct pcf50633 *pcf, int irq)
-{
-	if (pcf->irq_handler[irq].handler)
-		pcf->irq_handler[irq].handler(irq, pcf->irq_handler[irq].data);
-}
-
-/* Maximum amount of time ONKEY is held before emergency action is taken */
-#define PCF50633_ONKEY1S_TIMEOUT 8
-
-static irqreturn_t pcf50633_irq(int irq, void *data)
-{
-	struct pcf50633 *pcf = data;
-	int ret, i, j;
-	u8 pcf_int[5], chgstat;
-
-	/* Read the 5 INT regs in one transaction */
-	ret = pcf50633_read_block(pcf, PCF50633_REG_INT1,
-						ARRAY_SIZE(pcf_int), pcf_int);
-	if (ret != ARRAY_SIZE(pcf_int)) {
-		dev_err(pcf->dev, "Error reading INT registers\n");
-
-		/*
-		 * If this doesn't ACK the interrupt to the chip, we'll be
-		 * called once again as we're level triggered.
-		 */
-		goto out;
-	}
-
-	/* defeat 8s death from lowsys on A5 */
-	pcf50633_reg_write(pcf, PCF50633_REG_OOCSHDWN,  0x04);
-
-	/* We immediately read the usb and adapter status. We thus make sure
-	 * only of USBINS/USBREM IRQ handlers are called */
-	if (pcf_int[0] & (PCF50633_INT1_USBINS | PCF50633_INT1_USBREM)) {
-		chgstat = pcf50633_reg_read(pcf, PCF50633_REG_MBCS2);
-		if (chgstat & (0x3 << 4))
-			pcf_int[0] &= ~PCF50633_INT1_USBREM;
-		else
-			pcf_int[0] &= ~PCF50633_INT1_USBINS;
-	}
-
-	/* Make sure only one of ADPINS or ADPREM is set */
-	if (pcf_int[0] & (PCF50633_INT1_ADPINS | PCF50633_INT1_ADPREM)) {
-		chgstat = pcf50633_reg_read(pcf, PCF50633_REG_MBCS2);
-		if (chgstat & (0x3 << 4))
-			pcf_int[0] &= ~PCF50633_INT1_ADPREM;
-		else
-			pcf_int[0] &= ~PCF50633_INT1_ADPINS;
-	}
-
-	dev_dbg(pcf->dev, "INT1=0x%02x INT2=0x%02x INT3=0x%02x "
-			"INT4=0x%02x INT5=0x%02x\n", pcf_int[0],
-			pcf_int[1], pcf_int[2], pcf_int[3], pcf_int[4]);
-
-	/* Some revisions of the chip don't have a 8s standby mode on
-	 * ONKEY1S press. We try to manually do it in such cases. */
-	if ((pcf_int[0] & PCF50633_INT1_SECOND) && pcf->onkey1s_held) {
-		dev_info(pcf->dev, "ONKEY1S held for %d secs\n",
-							pcf->onkey1s_held);
-		if (pcf->onkey1s_held++ == PCF50633_ONKEY1S_TIMEOUT)
-			if (pcf->pdata->force_shutdown)
-				pcf->pdata->force_shutdown(pcf);
-	}
-
-	if (pcf_int[2] & PCF50633_INT3_ONKEY1S) {
-		dev_info(pcf->dev, "ONKEY1S held\n");
-		pcf->onkey1s_held = 1 ;
-
-		/* Unmask IRQ_SECOND */
-		pcf50633_reg_clear_bits(pcf, PCF50633_REG_INT1M,
-						PCF50633_INT1_SECOND);
-
-		/* Unmask IRQ_ONKEYR */
-		pcf50633_reg_clear_bits(pcf, PCF50633_REG_INT2M,
-						PCF50633_INT2_ONKEYR);
-	}
-
-	if ((pcf_int[1] & PCF50633_INT2_ONKEYR) && pcf->onkey1s_held) {
-		pcf->onkey1s_held = 0;
-
-		/* Mask SECOND and ONKEYR interrupts */
-		if (pcf->mask_regs[0] & PCF50633_INT1_SECOND)
-			pcf50633_reg_set_bit_mask(pcf,
-					PCF50633_REG_INT1M,
-					PCF50633_INT1_SECOND,
-					PCF50633_INT1_SECOND);
-
-		if (pcf->mask_regs[1] & PCF50633_INT2_ONKEYR)
-			pcf50633_reg_set_bit_mask(pcf,
-					PCF50633_REG_INT2M,
-					PCF50633_INT2_ONKEYR,
-					PCF50633_INT2_ONKEYR);
-	}
-
-	/* Have we just resumed ? */
-	if (pcf->is_suspended) {
-		pcf->is_suspended = 0;
-
-		/* Set the resume reason filtering out non resumers */
-		for (i = 0; i < ARRAY_SIZE(pcf_int); i++)
-			pcf->resume_reason[i] = pcf_int[i] &
-						pcf->pdata->resumers[i];
-
-		/* Make sure we don't pass on any ONKEY events to
-		 * userspace now */
-		pcf_int[1] &= ~(PCF50633_INT2_ONKEYR | PCF50633_INT2_ONKEYF);
-	}
-
-	for (i = 0; i < ARRAY_SIZE(pcf_int); i++) {
-		/* Unset masked interrupts */
-		pcf_int[i] &= ~pcf->mask_regs[i];
-
-		for (j = 0; j < 8 ; j++)
-			if (pcf_int[i] & (1 << j))
-				pcf50633_irq_call_handler(pcf, (i * 8) + j);
-	}
-
-out:
-	return IRQ_HANDLED
-}
-
 static void
 pcf50633_client_dev_register(struct pcf50633 *pcf, const char *name,
 						struct platform_device **pdev)
@@ -463,58 +241,17 @@ pcf50633_client_dev_register(struct pcf50633 *pcf, const char *name,
 static int pcf50633_suspend(struct i2c_client *client, pm_message_t state)
 {
 	struct pcf50633 *pcf;
-	int ret = 0, i;
-	u8 res[5];
-
 	pcf = i2c_get_clientdata(client);
 
-	/* Make sure our interrupt handlers are not called
-	 * henceforth */
-	disable_irq(pcf->irq);
-
-	/* Save the masks */
-	ret = pcf50633_read_block(pcf, PCF50633_REG_INT1M,
-				ARRAY_SIZE(pcf->suspend_irq_masks),
-					pcf->suspend_irq_masks);
-	if (ret < 0) {
-		dev_err(pcf->dev, "error saving irq masks\n");
-		goto out;
-	}
-
-	/* Write wakeup irq masks */
-	for (i = 0; i < ARRAY_SIZE(res); i++)
-		res[i] = ~pcf->pdata->resumers[i];
-
-	ret = pcf50633_write_block(pcf, PCF50633_REG_INT1M,
-					ARRAY_SIZE(res), &res[0]);
-	if (ret < 0) {
-		dev_err(pcf->dev, "error writing wakeup irq masks\n");
-		goto out;
-	}
-
-	pcf->is_suspended = 1;
-
-out:
-	return ret;
+	return pcf50633_irq_suspend(pcf);
 }
 
 static int pcf50633_resume(struct i2c_client *client)
 {
 	struct pcf50633 *pcf;
-	int ret;
-
 	pcf = i2c_get_clientdata(client);
 
-	/* Write the saved mask registers */
-	ret = pcf50633_write_block(pcf, PCF50633_REG_INT1M,
-				ARRAY_SIZE(pcf->suspend_irq_masks),
-					pcf->suspend_irq_masks);
-	if (ret < 0)
-		dev_err(pcf->dev, "Error restoring saved suspend masks\n");
-
-	enable_irq(pcf->irq);
-
-	return 0;
+	return pcf50633_irq_resume(pcf);
 }
 #else
 #define pcf50633_suspend NULL
@@ -545,7 +282,6 @@ static int __devinit pcf50633_probe(struct i2c_client *client,
 	i2c_set_clientdata(client, pcf);
 	pcf->dev = &client->dev;
 	pcf->i2c_client = client;
-	pcf->irq = client->irq;
 
 	version = pcf50633_reg_read(pcf, 0);
 	variant = pcf50633_reg_read(pcf, 1);
@@ -558,22 +294,7 @@ static int __devinit pcf50633_probe(struct i2c_client *client,
 	dev_info(pcf->dev, "Probed device version %d variant %d\n",
 							version, variant);
 
-	/* Enable all interrupts except RTC SECOND */
-	pcf->mask_regs[0] = 0x80;
-	pcf50633_reg_write(pcf, PCF50633_REG_INT1M, pcf->mask_regs[0]);
-	pcf50633_reg_write(pcf, PCF50633_REG_INT2M, 0x00);
-	pcf50633_reg_write(pcf, PCF50633_REG_INT3M, 0x00);
-	pcf50633_reg_write(pcf, PCF50633_REG_INT4M, 0x00);
-	pcf50633_reg_write(pcf, PCF50633_REG_INT5M, 0x00);
-
-	ret = request_threaded_irq(client->irq, NULL, pcf50633_irq,
-					IRQF_TRIGGER_LOW | IRQF_ONESHOT,
-					"pcf50633", pcf);
-
-	if (ret) {
-		dev_err(pcf->dev, "Failed to request IRQ %d\n", ret);
-		goto err_free;
-	}
+	pcf50633_irq_init(pcf, client->irq);
 
 	/* Create sub devices */
 	pcf50633_client_dev_register(pcf, "pcf50633-input",
@@ -602,10 +323,6 @@ static int __devinit pcf50633_probe(struct i2c_client *client,
 		platform_device_add(pdev);
 	}
 
-	if (enable_irq_wake(client->irq) < 0)
-		dev_err(pcf->dev, "IRQ %u cannot be enabled as wake-up source"
-			"in this hardware revision", client->irq);
-
 	ret = sysfs_create_group(&client->dev.kobj, &pcf_attr_group);
 	if (ret)
 		dev_err(pcf->dev, "error creating sysfs entries\n");
@@ -627,7 +344,7 @@ static int __devexit pcf50633_remove(struct i2c_client *client)
 	struct pcf50633 *pcf = i2c_get_clientdata(client);
 	int i;
 
-	free_irq(pcf->irq, pcf);
+	pcf50633_irq_free(pcf);
 
 	platform_device_unregister(pcf->input_pdev);
 	platform_device_unregister(pcf->rtc_pdev);
diff --git a/drivers/mfd/pcf50633-irq.c b/drivers/mfd/pcf50633-irq.c
new file mode 100644
index 0000000..1b0192f
--- /dev/null
+++ b/drivers/mfd/pcf50633-irq.c
@@ -0,0 +1,318 @@
+/* NXP PCF50633 Power Management Unit (PMU) driver
+ *
+ * (C) 2006-2008 by Openmoko, Inc.
+ * Author: Harald Welte <laforge@openmoko.org>
+ * 	   Balaji Rao <balajirrao@openmoko.org>
+ * All rights reserved.
+ *
+ *  This program is free software; you can redistribute  it and/or modify it
+ *  under  the terms of  the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the  License, or (at your
+ *  option) any later version.
+ *
+ */
+
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+
+#include <linux/mfd/pcf50633/core.h>
+
+/* Two MBCS registers used during cold start */
+#define PCF50633_REG_MBCS1		0x4b
+#define PCF50633_REG_MBCS2		0x4c
+#define PCF50633_MBCS1_USBPRES 		0x01
+#define PCF50633_MBCS1_ADAPTPRES	0x01
+
+int pcf50633_register_irq(struct pcf50633 *pcf, int irq,
+			void (*handler) (int, void *), void *data)
+{
+	if (irq < 0 || irq >= PCF50633_NUM_IRQ || !handler)
+		return -EINVAL;
+
+	if (WARN_ON(pcf->irq_handler[irq].handler))
+		return -EBUSY;
+
+	mutex_lock(&pcf->lock);
+	pcf->irq_handler[irq].handler = handler;
+	pcf->irq_handler[irq].data = data;
+	mutex_unlock(&pcf->lock);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pcf50633_register_irq);
+
+int pcf50633_free_irq(struct pcf50633 *pcf, int irq)
+{
+	if (irq < 0 || irq >= PCF50633_NUM_IRQ)
+		return -EINVAL;
+
+	mutex_lock(&pcf->lock);
+	pcf->irq_handler[irq].handler = NULL;
+	mutex_unlock(&pcf->lock);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pcf50633_free_irq);
+
+static int __pcf50633_irq_mask_set(struct pcf50633 *pcf, int irq, u8 mask)
+{
+	u8 reg, bit;
+	int ret = 0, idx;
+
+	idx = irq >> 3;
+	reg = PCF50633_REG_INT1M + idx;
+	bit = 1 << (irq & 0x07);
+
+	pcf50633_reg_set_bit_mask(pcf, reg, bit, mask ? bit : 0);
+
+	mutex_lock(&pcf->lock);
+
+	if (mask)
+		pcf->mask_regs[idx] |= bit;
+	else
+		pcf->mask_regs[idx] &= ~bit;
+
+	mutex_unlock(&pcf->lock);
+
+	return ret;
+}
+
+int pcf50633_irq_mask(struct pcf50633 *pcf, int irq)
+{
+	dev_dbg(pcf->dev, "Masking IRQ %d\n", irq);
+
+	return __pcf50633_irq_mask_set(pcf, irq, 1);
+}
+EXPORT_SYMBOL_GPL(pcf50633_irq_mask);
+
+int pcf50633_irq_unmask(struct pcf50633 *pcf, int irq)
+{
+	dev_dbg(pcf->dev, "Unmasking IRQ %d\n", irq);
+
+	return __pcf50633_irq_mask_set(pcf, irq, 0);
+}
+EXPORT_SYMBOL_GPL(pcf50633_irq_unmask);
+
+int pcf50633_irq_mask_get(struct pcf50633 *pcf, int irq)
+{
+	u8 reg, bits;
+
+	reg =  irq >> 3;
+	bits = 1 << (irq & 0x07);
+
+	return pcf->mask_regs[reg] & bits;
+}
+EXPORT_SYMBOL_GPL(pcf50633_irq_mask_get);
+
+static void pcf50633_irq_call_handler(struct pcf50633 *pcf, int irq)
+{
+	if (pcf->irq_handler[irq].handler)
+		pcf->irq_handler[irq].handler(irq, pcf->irq_handler[irq].data);
+}
+
+/* Maximum amount of time ONKEY is held before emergency action is taken */
+#define PCF50633_ONKEY1S_TIMEOUT 8
+
+static irqreturn_t pcf50633_irq(int irq, void *data)
+{
+	struct pcf50633 *pcf = data;
+	int ret, i, j;
+	u8 pcf_int[5], chgstat;
+
+	/* Read the 5 INT regs in one transaction */
+	ret = pcf50633_read_block(pcf, PCF50633_REG_INT1,
+						ARRAY_SIZE(pcf_int), pcf_int);
+	if (ret != ARRAY_SIZE(pcf_int)) {
+		dev_err(pcf->dev, "Error reading INT registers\n");
+
+		/*
+		 * If this doesn't ACK the interrupt to the chip, we'll be
+		 * called once again as we're level triggered.
+		 */
+		goto out;
+	}
+
+	/* defeat 8s death from lowsys on A5 */
+	pcf50633_reg_write(pcf, PCF50633_REG_OOCSHDWN,  0x04);
+
+	/* We immediately read the usb and adapter status. We thus make sure
+	 * only of USBINS/USBREM IRQ handlers are called */
+	if (pcf_int[0] & (PCF50633_INT1_USBINS | PCF50633_INT1_USBREM)) {
+		chgstat = pcf50633_reg_read(pcf, PCF50633_REG_MBCS2);
+		if (chgstat & (0x3 << 4))
+			pcf_int[0] &= ~PCF50633_INT1_USBREM;
+		else
+			pcf_int[0] &= ~PCF50633_INT1_USBINS;
+	}
+
+	/* Make sure only one of ADPINS or ADPREM is set */
+	if (pcf_int[0] & (PCF50633_INT1_ADPINS | PCF50633_INT1_ADPREM)) {
+		chgstat = pcf50633_reg_read(pcf, PCF50633_REG_MBCS2);
+		if (chgstat & (0x3 << 4))
+			pcf_int[0] &= ~PCF50633_INT1_ADPREM;
+		else
+			pcf_int[0] &= ~PCF50633_INT1_ADPINS;
+	}
+
+	dev_dbg(pcf->dev, "INT1=0x%02x INT2=0x%02x INT3=0x%02x "
+			"INT4=0x%02x INT5=0x%02x\n", pcf_int[0],
+			pcf_int[1], pcf_int[2], pcf_int[3], pcf_int[4]);
+
+	/* Some revisions of the chip don't have a 8s standby mode on
+	 * ONKEY1S press. We try to manually do it in such cases. */
+	if ((pcf_int[0] & PCF50633_INT1_SECOND) && pcf->onkey1s_held) {
+		dev_info(pcf->dev, "ONKEY1S held for %d secs\n",
+							pcf->onkey1s_held);
+		if (pcf->onkey1s_held++ == PCF50633_ONKEY1S_TIMEOUT)
+			if (pcf->pdata->force_shutdown)
+				pcf->pdata->force_shutdown(pcf);
+	}
+
+	if (pcf_int[2] & PCF50633_INT3_ONKEY1S) {
+		dev_info(pcf->dev, "ONKEY1S held\n");
+		pcf->onkey1s_held = 1 ;
+
+		/* Unmask IRQ_SECOND */
+		pcf50633_reg_clear_bits(pcf, PCF50633_REG_INT1M,
+						PCF50633_INT1_SECOND);
+
+		/* Unmask IRQ_ONKEYR */
+		pcf50633_reg_clear_bits(pcf, PCF50633_REG_INT2M,
+						PCF50633_INT2_ONKEYR);
+	}
+
+	if ((pcf_int[1] & PCF50633_INT2_ONKEYR) && pcf->onkey1s_held) {
+		pcf->onkey1s_held = 0;
+
+		/* Mask SECOND and ONKEYR interrupts */
+		if (pcf->mask_regs[0] & PCF50633_INT1_SECOND)
+			pcf50633_reg_set_bit_mask(pcf,
+					PCF50633_REG_INT1M,
+					PCF50633_INT1_SECOND,
+					PCF50633_INT1_SECOND);
+
+		if (pcf->mask_regs[1] & PCF50633_INT2_ONKEYR)
+			pcf50633_reg_set_bit_mask(pcf,
+					PCF50633_REG_INT2M,
+					PCF50633_INT2_ONKEYR,
+					PCF50633_INT2_ONKEYR);
+	}
+
+	/* Have we just resumed ? */
+	if (pcf->is_suspended) {
+		pcf->is_suspended = 0;
+
+		/* Set the resume reason filtering out non resumers */
+		for (i = 0; i < ARRAY_SIZE(pcf_int); i++)
+			pcf->resume_reason[i] = pcf_int[i] &
+						pcf->pdata->resumers[i];
+
+		/* Make sure we don't pass on any ONKEY events to
+		 * userspace now */
+		pcf_int[1] &= ~(PCF50633_INT2_ONKEYR | PCF50633_INT2_ONKEYF);
+	}
+
+	for (i = 0; i < ARRAY_SIZE(pcf_int); i++) {
+		/* Unset masked interrupts */
+		pcf_int[i] &= ~pcf->mask_regs[i];
+
+		for (j = 0; j < 8 ; j++)
+			if (pcf_int[i] & (1 << j))
+				pcf50633_irq_call_handler(pcf, (i * 8) + j);
+	}
+
+out:
+	return IRQ_HANDLED;
+}
+
+#ifdef CONFIG_PM
+
+int pcf50633_irq_suspend(struct pcf50633 *pcf)
+{
+	int ret;
+	int i;
+	u8 res[5];
+
+
+	/* Make sure our interrupt handlers are not called
+	 * henceforth */
+	disable_irq(pcf->irq);
+
+	/* Save the masks */
+	ret = pcf50633_read_block(pcf, PCF50633_REG_INT1M,
+				ARRAY_SIZE(pcf->suspend_irq_masks),
+					pcf->suspend_irq_masks);
+	if (ret < 0) {
+		dev_err(pcf->dev, "error saving irq masks\n");
+		goto out;
+	}
+
+	/* Write wakeup irq masks */
+	for (i = 0; i < ARRAY_SIZE(res); i++)
+		res[i] = ~pcf->pdata->resumers[i];
+
+	ret = pcf50633_write_block(pcf, PCF50633_REG_INT1M,
+					ARRAY_SIZE(res), &res[0]);
+	if (ret < 0) {
+		dev_err(pcf->dev, "error writing wakeup irq masks\n");
+		goto out;
+	}
+
+	pcf->is_suspended = 1;
+
+out:
+	return ret;
+}
+
+int pcf50633_irq_resume(struct pcf50633 *pcf)
+{
+	int ret;
+
+	/* Write the saved mask registers */
+	ret = pcf50633_write_block(pcf, PCF50633_REG_INT1M,
+				ARRAY_SIZE(pcf->suspend_irq_masks),
+					pcf->suspend_irq_masks);
+	if (ret < 0)
+		dev_err(pcf->dev, "Error restoring saved suspend masks\n");
+
+	enable_irq(pcf->irq);
+
+	return ret;
+}
+
+#endif
+
+int pcf50633_irq_init(struct pcf50633 *pcf, int irq)
+{
+	int ret;
+
+	pcf->irq = irq;
+
+	/* Enable all interrupts except RTC SECOND */
+	pcf->mask_regs[0] = 0x80;
+	pcf50633_reg_write(pcf, PCF50633_REG_INT1M, pcf->mask_regs[0]);
+	pcf50633_reg_write(pcf, PCF50633_REG_INT2M, 0x00);
+	pcf50633_reg_write(pcf, PCF50633_REG_INT3M, 0x00);
+	pcf50633_reg_write(pcf, PCF50633_REG_INT4M, 0x00);
+	pcf50633_reg_write(pcf, PCF50633_REG_INT5M, 0x00);
+
+	ret = request_threaded_irq(irq, NULL, pcf50633_irq,
+					IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+					"pcf50633", pcf);
+
+	if (ret)
+		dev_err(pcf->dev, "Failed to request IRQ %d\n", ret);
+
+	if (enable_irq_wake(irq) < 0)
+		dev_err(pcf->dev, "IRQ %u cannot be enabled as wake-up source"
+			"in this hardware revision", irq);
+
+	return ret;
+}
+
+void pcf50633_irq_free(struct pcf50633 *pcf)
+{
+	free_irq(pcf->irq, pcf);
+}
-- 
1.5.6.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/4] MFD: pcf50633: Fixes and updates
  2010-05-12  0:10 [PATCH 0/4] MFD: pcf50633: Fixes and updates Lars-Peter Clausen
  2010-05-12  0:10 ` [PATCH 1/4] MFD: pcf50633: Fix bitfield logic in interrupt handler Lars-Peter Clausen
@ 2010-05-16 22:02 ` Samuel Ortiz
  1 sibling, 0 replies; 6+ messages in thread
From: Samuel Ortiz @ 2010-05-16 22:02 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: fercerpav, linux-kernel

Hi Lars-Peter,

On Wed, May 12, 2010 at 02:10:52AM +0200, Lars-Peter Clausen wrote:
> Hi
> 
> This patch series contains fixes and updates for the pcf50633 mfd and adc
> driver.
All 4 patches applied, thanks a lot.

Cheers,
Samuel.


> Lars-Peter Clausen (4):
>   MFD: pcf50633: Fix bitfield logic in interrupt handler
>   MFD: pcf50633-adc: Fix potential race pcf50633_adc_sync_read
>   MFD: pcf50633: Use threaded irq
>   MFD: pcf50633: Move irq related functions to its own file.
> 
>  drivers/mfd/Makefile        |    3 +-
>  drivers/mfd/pcf50633-adc.c  |   39 ++---
>  drivers/mfd/pcf50633-core.c |  344 ++-----------------------------------------
>  drivers/mfd/pcf50633-irq.c  |  318 +++++++++++++++++++++++++++++++++++++++
>  4 files changed, 346 insertions(+), 358 deletions(-)
>  create mode 100644 drivers/mfd/pcf50633-irq.c
> 

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-05-16 22:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-12  0:10 [PATCH 0/4] MFD: pcf50633: Fixes and updates Lars-Peter Clausen
2010-05-12  0:10 ` [PATCH 1/4] MFD: pcf50633: Fix bitfield logic in interrupt handler Lars-Peter Clausen
2010-05-12  0:10   ` [PATCH 2/4] MFD: pcf50633-adc: Fix potential race pcf50633_adc_sync_read Lars-Peter Clausen
2010-05-12  0:10     ` [PATCH 3/4] MFD: pcf50633: Use threaded irq Lars-Peter Clausen
2010-05-12  0:10       ` [PATCH 4/4] MFD: pcf50633: Move irq related functions to its own file Lars-Peter Clausen
2010-05-16 22:02 ` [PATCH 0/4] MFD: pcf50633: Fixes and updates Samuel Ortiz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.