[PATCH 0/5] pcf50633 - error handling & use threaded IRQs - v2

[PATCH 0/5] pcf50633 - error handling & use threaded IRQs - v2

[Dmitry Torokhov]

Hi Samuel,

Hopefully this is split enough to your liking. It should apply to for-next
branch of mfd tree.

Compile-tested only, still don't have the hardwrae.

-- 
Dmitry

[PATCH 1/5] mfd: pcf50633 - make 'is_suspended' a bool

[Dmitry Torokhov]

The field holds boolean data and should be typed as such. Also annotate
check for is_spspended in IRQ handler with 'unlikely'.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/mfd/pcf50633-core.c       |    6 +++---
 include/linux/mfd/pcf50633/core.h |    3 ++-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/pcf50633-core.c b/drivers/mfd/pcf50633-core.c
index 03dcc92..fb44e4d 100644
--- a/drivers/mfd/pcf50633-core.c
+++ b/drivers/mfd/pcf50633-core.c
@@ -412,8 +412,8 @@ static void pcf50633_irq_worker(struct work_struct *work)
 	}
 
 	/* Have we just resumed ? */
-	if (pcf->is_suspended) {
-		pcf->is_suspended = 0;
+	if (unlikely(pcf->is_suspended)) {
+		pcf->is_suspended = false;
 
 		/* Set the resume reason filtering out non resumers */
 		for (i = 0; i < ARRAY_SIZE(pcf_int); i++)
@@ -510,7 +510,7 @@ static int pcf50633_suspend(struct i2c_client *client, pm_message_t state)
 		goto out;
 	}
 
-	pcf->is_suspended = 1;
+	pcf->is_suspended = true;
 
 out:
 	return ret;
diff --git a/include/linux/mfd/pcf50633/core.h b/include/linux/mfd/pcf50633/core.h
index d9034cc..43bb2ac 100644
--- a/include/linux/mfd/pcf50633/core.h
+++ b/include/linux/mfd/pcf50633/core.h
@@ -13,6 +13,7 @@
 #ifndef __LINUX_MFD_PCF50633_CORE_H
 #define __LINUX_MFD_PCF50633_CORE_H
 
+#include <linux/types.h>
 #include <linux/i2c.h>
 #include <linux/workqueue.h>
 #include <linux/regulator/driver.h>
@@ -139,7 +140,7 @@ struct pcf50633 {
 
 	u8 suspend_irq_masks[5];
 	u8 resume_reason[5];
-	int is_suspended;
+	bool is_suspended;
 
 	int onkey1s_held;
 

[PATCH 2/5] mfd: pcf50633 - use threaded interrupts

[Dmitry Torokhov]

Switch to using theraded IRQs instead of implementing custom solution
with regular inettrupt handler and a custom workqueue.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/mfd/pcf50633-core.c       |   57 +++++++------------------------------
 include/linux/mfd/pcf50633/core.h |    3 --
 2 files changed, 11 insertions(+), 49 deletions(-)

diff --git a/drivers/mfd/pcf50633-core.c b/drivers/mfd/pcf50633-core.c
index fb44e4d..ff43f34 100644
--- a/drivers/mfd/pcf50633-core.c
+++ b/drivers/mfd/pcf50633-core.c
@@ -324,13 +324,13 @@ 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);
+	dev_dbg(pcf->dev, "pcf50633_irq\n");
 
 	/* Read the 5 INT regs in one transaction */
 	ret = pcf50633_read_block(pcf, PCF50633_REG_INT1,
@@ -435,20 +435,6 @@ 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;
 }
 
@@ -483,13 +469,9 @@ static int pcf50633_suspend(struct i2c_client *client, pm_message_t state)
 
 	pcf = i2c_get_clientdata(client);
 
-	/* Make sure our interrupt handlers are not called
-	 * henceforth */
+	/* Make sure our interrupt handlers are not called 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),
@@ -530,16 +512,11 @@ 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);
+	pcf50633_irq(pcf->irq, pcf);
+	enable_irq(pcf->irq);
 
 	return 0;
 }
@@ -573,22 +550,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",
@@ -602,12 +570,12 @@ 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 */
@@ -650,8 +618,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);
@@ -665,7 +631,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);
diff --git a/include/linux/mfd/pcf50633/core.h b/include/linux/mfd/pcf50633/core.h
index 43bb2ac..fef4bda 100644
--- a/include/linux/mfd/pcf50633/core.h
+++ b/include/linux/mfd/pcf50633/core.h
@@ -15,7 +15,6 @@
 
 #include <linux/types.h>
 #include <linux/i2c.h>
-#include <linux/workqueue.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/machine.h>
 #include <linux/power_supply.h>
@@ -132,8 +131,6 @@ struct pcf50633 {
 	struct pcf50633_platform_data *pdata;
 	int irq;
 	struct pcf50633_irq irq_handler[PCF50633_NUM_IRQ];
-	struct work_struct irq_work;
-	struct workqueue_struct *work_queue;
 	struct mutex lock;
 
 	u8 mask_regs[5];

[PATCH 3/5] mfd: pcf50633 - move creating of regulator devices out of line

[Dmitry Torokhov]

This cleans up pcf50633_probe a bit and lays ground for the patch adding
error handling to the probe function.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/mfd/pcf50633-core.c |   57 +++++++++++++++++++++++++++++++------------
 1 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/drivers/mfd/pcf50633-core.c b/drivers/mfd/pcf50633-core.c
index ff43f34..474a76c 100644
--- a/drivers/mfd/pcf50633-core.c
+++ b/drivers/mfd/pcf50633-core.c
@@ -438,6 +438,45 @@ out:
 	return IRQ_HANDLED;
 }
 
+static int pcf50633_regulator_dev_register(struct pcf50633 *pcf,
+					   unsigned int num)
+{
+	struct pcf50633_platform_data *pdata = pcf->pdata;
+	struct platform_device *pdev;
+	int error;
+
+	pdev = platform_device_alloc("pcf50633-regltr", num);
+	if (!pdev) {
+		dev_err(pcf->dev,
+			"Not enough memory for regulator device %d\n", num);
+		return -ENOMEM;
+	}
+
+	pdev->dev.parent = pcf->dev;
+
+	error = platform_device_add_data(pdev, &pdata->reg_init_data[num],
+					 sizeof(pdata->reg_init_data[num]));
+	if (error) {
+		dev_err(pcf->dev, "Failed to add platform data for %s: %d\n",
+			dev_name(&pdev->dev), error);
+		goto err_free_dev;
+	}
+
+	error = platform_device_add(pdev);
+	if (error) {
+		dev_err(pcf->dev, "Failed to register %s: %d\n",
+			dev_name(&pdev->dev), error);
+		goto err_free_dev;
+	}
+
+	pcf->regulator_pdev[num] = pdev;
+	return 0;
+
+ err_free_dev:
+	platform_device_put(pdev);
+	return error;
+}
+
 static void
 pcf50633_client_dev_register(struct pcf50633 *pcf, const char *name,
 						struct platform_device **pdev)
@@ -588,22 +627,8 @@ static int __devinit pcf50633_probe(struct i2c_client *client,
 	pcf50633_client_dev_register(pcf, "pcf50633-adc",
 						&pcf->adc_pdev);
 
-	for (i = 0; i < PCF50633_NUM_REGULATORS; i++) {
-		struct platform_device *pdev;
-
-		pdev = platform_device_alloc("pcf50633-regltr", i);
-		if (!pdev) {
-			dev_err(pcf->dev, "Cannot create regulator %d\n", i);
-			continue;
-		}
-
-		pdev->dev.parent = pcf->dev;
-		platform_device_add_data(pdev, &pdata->reg_init_data[i],
-					sizeof(pdata->reg_init_data[i]));
-		pcf->regulator_pdev[i] = pdev;
-
-		platform_device_add(pdev);
-	}
+	for (i = 0; i < PCF50633_NUM_REGULATORS; i++)
+		pcf50633_regulator_dev_register(pcf, i);
 
 	if (enable_irq_wake(client->irq) < 0)
 		dev_err(pcf->dev, "IRQ %u cannot be enabled as wake-up source"

[PATCH 4/5] mfd: pcf50633 - fix error handling during probe

[Dmitry Torokhov]

The pcf50633 is very naive - it assumes that all driver core
operations will succeed and will fail badly if one of them
errors out. Implement proper error handling and make sure we
release any and all resources that have been allocated prior
to failure.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/mfd/pcf50633-adc.c        |    2 -
 drivers/mfd/pcf50633-core.c       |  124 +++++++++++++++++++++++--------------
 drivers/power/pcf50633-charger.c  |    6 +-
 include/linux/mfd/pcf50633/core.h |   12 ++--
 4 files changed, 90 insertions(+), 54 deletions(-)

diff --git a/drivers/mfd/pcf50633-adc.c b/drivers/mfd/pcf50633-adc.c
index 6d2e846..d86e7b0 100644
--- a/drivers/mfd/pcf50633-adc.c
+++ b/drivers/mfd/pcf50633-adc.c
@@ -52,7 +52,7 @@ struct pcf50633_adc {
 
 static inline struct pcf50633_adc *__to_adc(struct pcf50633 *pcf)
 {
-	return platform_get_drvdata(pcf->adc_pdev);
+	return platform_get_drvdata(pcf->pdevs[PCF50633_PDEV_ADC_IDX]);
 }
 
 static void adc_setup(struct pcf50633 *pcf, int channel, int avg)
diff --git a/drivers/mfd/pcf50633-core.c b/drivers/mfd/pcf50633-core.c
index 474a76c..2126563 100644
--- a/drivers/mfd/pcf50633-core.c
+++ b/drivers/mfd/pcf50633-core.c
@@ -469,7 +469,7 @@ static int pcf50633_regulator_dev_register(struct pcf50633 *pcf,
 		goto err_free_dev;
 	}
 
-	pcf->regulator_pdev[num] = pdev;
+	pcf->pdevs[num] = pdev;
 	return 0;
 
  err_free_dev:
@@ -477,26 +477,47 @@ static int pcf50633_regulator_dev_register(struct pcf50633 *pcf,
 	return error;
 }
 
-static void
-pcf50633_client_dev_register(struct pcf50633 *pcf, const char *name,
-						struct platform_device **pdev)
+static int pcf50633_client_dev_register(struct pcf50633 *pcf,
+					const char *name, unsigned int num)
 {
-	int ret;
+	struct platform_device *pdev;
+	int error;
 
-	*pdev = platform_device_alloc(name, -1);
-	if (!*pdev) {
+	pdev = platform_device_alloc(name, -1);
+	if (!pdev) {
 		dev_err(pcf->dev, "Falied to allocate %s\n", name);
-		return;
+		return -ENOMEM;
 	}
 
-	(*pdev)->dev.parent = pcf->dev;
+	pdev->dev.parent = pcf->dev;
 
-	ret = platform_device_add(*pdev);
-	if (ret) {
-		dev_err(pcf->dev, "Failed to register %s: %d\n", name, ret);
-		platform_device_put(*pdev);
-		*pdev = NULL;
+	/*
+	 * We reference these platform devices from sub-drivers so they
+	 * need to be set up before registering device.
+	 */
+	pcf->pdevs[num] = pdev;
+
+	error = platform_device_add(pdev);
+	if (error) {
+		dev_err(pcf->dev, "Failed to register %s: %d\n", name, error);
+		goto err_free_dev;
 	}
+
+	return 0;
+
+ err_free_dev:
+	platform_device_put(pdev);
+	pcf->pdevs[num] = pdev;
+	return error;
+}
+
+static void pcf50633_unregister_sub_devices(struct pcf50633 *pcf)
+{
+	int i;
+
+	for (i = 0; i < PCF50633_NUM_PLATFORM_DEVICES; i++)
+		if (pcf->pdevs[i])
+			platform_device_unregister(pcf->pdevs[i]);
 }
 
 #ifdef CONFIG_PM
@@ -569,7 +590,7 @@ static int __devinit pcf50633_probe(struct i2c_client *client,
 {
 	struct pcf50633 *pcf;
 	struct pcf50633_platform_data *pdata = client->dev.platform_data;
-	int i, ret;
+	int i, error;
 	int version, variant;
 
 	if (!client->irq) {
@@ -585,7 +606,6 @@ static int __devinit pcf50633_probe(struct i2c_client *client,
 
 	mutex_init(&pcf->lock);
 
-	i2c_set_clientdata(client, pcf);
 	pcf->dev = &client->dev;
 	pcf->i2c_client = client;
 	pcf->irq = client->irq;
@@ -594,7 +614,7 @@ static int __devinit pcf50633_probe(struct i2c_client *client,
 	variant = pcf50633_reg_read(pcf, 1);
 	if (version < 0 || variant < 0) {
 		dev_err(pcf->dev, "Unable to probe pcf50633\n");
-		ret = -ENODEV;
+		error = -ENODEV;
 		goto err_free;
 	}
 
@@ -609,62 +629,74 @@ 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_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);
+	error = request_threaded_irq(client->irq, NULL, pcf50633_irq,
+				     IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+				     "pcf50633", pcf);
+	if (error) {
+		dev_err(pcf->dev, "Failed to request IRQ %d\n", error);
 		goto err_free;
 	}
 
 	/* Create sub devices */
-	pcf50633_client_dev_register(pcf, "pcf50633-input",
-						&pcf->input_pdev);
-	pcf50633_client_dev_register(pcf, "pcf50633-rtc",
-						&pcf->rtc_pdev);
-	pcf50633_client_dev_register(pcf, "pcf50633-mbc",
-						&pcf->mbc_pdev);
-	pcf50633_client_dev_register(pcf, "pcf50633-adc",
-						&pcf->adc_pdev);
-
-	for (i = 0; i < PCF50633_NUM_REGULATORS; i++)
-		pcf50633_regulator_dev_register(pcf, i);
+	error = pcf50633_client_dev_register(pcf, "pcf50633-input",
+						PCF50633_PDEV_INPUT_IDX);
+	if (error)
+		goto err_free_pdevs;
+
+	error = pcf50633_client_dev_register(pcf, "pcf50633-rtc",
+						PCF50633_PDEV_RTC_IDX);
+	if (error)
+		goto err_free_pdevs;
+
+	error = pcf50633_client_dev_register(pcf, "pcf50633-mbc",
+						PCF50633_PDEV_MBC_IDX);
+	if (error)
+		goto err_free_pdevs;
+
+	error = pcf50633_client_dev_register(pcf, "pcf50633-adc",
+						PCF50633_PDEV_ADC_IDX);
+	if (error)
+		goto err_free_pdevs;
+
+	for (i = 0; i < PCF50633_NUM_REGULATORS; i++) {
+		error = pcf50633_regulator_dev_register(pcf, i);
+		if (error)
+			goto err_free_pdevs;
+	}
 
 	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)
+	error = sysfs_create_group(&client->dev.kobj, &pcf_attr_group);
+	if (error) {
 		dev_err(pcf->dev, "error creating sysfs entries\n");
+		goto err_free_pdevs;
+	}
 
 	if (pdata->probe_done)
 		pdata->probe_done(pcf);
 
+	i2c_set_clientdata(client, pcf);
 	return 0;
 
+err_free_pdevs:
+	pcf50633_unregister_sub_devices(pcf);
+	free_irq(pcf->irq, pcf);
 err_free:
-	i2c_set_clientdata(client, NULL);
 	kfree(pcf);
-
-	return ret;
+	return error;
 }
 
 static int __devexit pcf50633_remove(struct i2c_client *client)
 {
 	struct pcf50633 *pcf = i2c_get_clientdata(client);
-	int i;
 
 	free_irq(pcf->irq, pcf);
 
-	platform_device_unregister(pcf->input_pdev);
-	platform_device_unregister(pcf->rtc_pdev);
-	platform_device_unregister(pcf->mbc_pdev);
-	platform_device_unregister(pcf->adc_pdev);
-
-	for (i = 0; i < PCF50633_NUM_REGULATORS; i++)
-		platform_device_unregister(pcf->regulator_pdev[i]);
+	pcf50633_unregister_sub_devices(pcf);
 
+	i2c_set_clientdata(client, NULL);
 	kfree(pcf);
 
 	return 0;
diff --git a/drivers/power/pcf50633-charger.c b/drivers/power/pcf50633-charger.c
index 6a84a8e..02e1183 100644
--- a/drivers/power/pcf50633-charger.c
+++ b/drivers/power/pcf50633-charger.c
@@ -42,7 +42,8 @@ struct pcf50633_mbc {
 
 int pcf50633_mbc_usb_curlim_set(struct pcf50633 *pcf, int ma)
 {
-	struct pcf50633_mbc *mbc = platform_get_drvdata(pcf->mbc_pdev);
+	struct pcf50633_mbc *mbc =
+		platform_get_drvdata(pcf->pdevs[PCF50633_PDEV_ADC_IDX]);
 	int ret = 0;
 	u8 bits;
 	int charging_start = 1;
@@ -90,7 +91,8 @@ EXPORT_SYMBOL_GPL(pcf50633_mbc_usb_curlim_set);
 
 int pcf50633_mbc_get_status(struct pcf50633 *pcf)
 {
-	struct pcf50633_mbc *mbc  = platform_get_drvdata(pcf->mbc_pdev);
+	struct pcf50633_mbc *mbc =
+		platform_get_drvdata(pcf->pdevs[PCF50633_PDEV_ADC_IDX]);
 	int status = 0;
 
 	if (mbc->usb_online)
diff --git a/include/linux/mfd/pcf50633/core.h b/include/linux/mfd/pcf50633/core.h
index fef4bda..b8e7eed 100644
--- a/include/linux/mfd/pcf50633/core.h
+++ b/include/linux/mfd/pcf50633/core.h
@@ -124,6 +124,12 @@ enum {
 	PCF50633_NUM_IRQ,
 };
 
+#define PCF50633_PDEV_INPUT_IDX		(PCF50633_NUM_REGULATORS + 0)
+#define PCF50633_PDEV_RTC_IDX		(PCF50633_NUM_REGULATORS + 1)
+#define PCF50633_PDEV_MBC_IDX		(PCF50633_NUM_REGULATORS + 2)
+#define PCF50633_PDEV_ADC_IDX		(PCF50633_NUM_REGULATORS + 3)
+#define PCF50633_NUM_PLATFORM_DEVICES	(PCF50633_NUM_REGULATORS + 4)
+
 struct pcf50633 {
 	struct device *dev;
 	struct i2c_client *i2c_client;
@@ -141,11 +147,7 @@ struct pcf50633 {
 
 	int onkey1s_held;
 
-	struct platform_device *rtc_pdev;
-	struct platform_device *mbc_pdev;
-	struct platform_device *adc_pdev;
-	struct platform_device *input_pdev;
-	struct platform_device *regulator_pdev[PCF50633_NUM_REGULATORS];
+	struct platform_device *pdevs[PCF50633_NUM_PLATFORM_DEVICES];
 };
 
 enum pcf50633_reg_int1 {

[PATCH 5/5] mfd: pcf50633 - whitespace and formatting fixes

[Dmitry Torokhov]

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/mfd/pcf50633-core.c |   29 +++++++++++++++++------------
 1 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/mfd/pcf50633-core.c b/drivers/mfd/pcf50633-core.c
index 2126563..f3203bd 100644
--- a/drivers/mfd/pcf50633-core.c
+++ b/drivers/mfd/pcf50633-core.c
@@ -2,7 +2,7 @@
  *
  * (C) 2006-2008 by Openmoko, Inc.
  * Author: Harald Welte <laforge@openmoko.org>
- * 	   Balaji Rao <balajirrao@openmoko.org>
+ *	   Balaji Rao <balajirrao@openmoko.org>
  * All rights reserved.
  *
  *  This program is free software; you can redistribute  it and/or modify it
@@ -28,7 +28,7 @@
 /* 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_USBPRES		0x01
 #define PCF50633_MBCS1_ADAPTPRES	0x01
 
 static int __pcf50633_read(struct pcf50633 *pcf, u8 reg, int num, u8 *data)
@@ -53,7 +53,6 @@ static int __pcf50633_write(struct pcf50633 *pcf, u8 reg, int num, u8 *data)
 		dev_err(pcf->dev, "Error writing %d regs at %d\n", num, reg);
 
 	return ret;
-
 }
 
 /* Read a block of upto 32 regs  */
@@ -348,8 +347,10 @@ static irqreturn_t pcf50633_irq(int irq, void *data)
 	/* 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 */
+	/*
+	 * 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))
@@ -367,12 +368,14 @@ static irqreturn_t pcf50633_irq(int irq, void *data)
 			pcf_int[0] &= ~(1 << 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]);
+	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. */
+	/*
+	 * 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);
@@ -665,8 +668,10 @@ static int __devinit pcf50633_probe(struct i2c_client *client,
 	}
 
 	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);
+		dev_err(pcf->dev,
+			"IRQ %u cannot be enabled as wake-up source "
+			"in this hardware revision\n",
+			client->irq);
 
 	error = sysfs_create_group(&client->dev.kobj, &pcf_attr_group);
 	if (error) {

Re: [PATCH 0/5] pcf50633 - error handling & use threaded IRQs - v2

[Samuel Ortiz]

Hi Dmitry,

On Sun, Nov 15, 2009 at 10:08:37PM -0800, Dmitry Torokhov wrote:
> Hi Samuel,
> 
> Hopefully this is split enough to your liking. 
Looks much better, thanks.


> It should apply to for-next
> branch of mfd tree.
> 
> Compile-tested only, still don't have the hardwrae.
I applied it to my for-next branch. I'd appreciate if Lars or Balaji could
give it a try and report problems with it.

Cheers,
Samuel.


> -- 
> Dmitry

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

Re: [PATCH 0/5] pcf50633 - error handling & use threaded IRQs - v2

[Lars-Peter Clausen]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Samuel Ortiz wrote:
> Hi Dmitry,
>
> On Sun, Nov 15, 2009 at 10:08:37PM -0800, Dmitry Torokhov wrote:
> ... I applied it to my for-next branch. I'd appreciate if Lars or
> Balaji could give it a try and report problems with it.
>
> Cheers, Samuel.
Hi

I was just wondering what happened to this patchset. I can find it
anywhere in the mfd tree.

- - Lars
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEUEARECAAYFAkvkiEEACgkQBX4mSR26RiM6qwCeKWRF6VuZKzEuLk0RkefQV5Mx
kUIAmIHNoj/41wR/GcADpq4lQZV3jis=
=EjLk
-----END PGP SIGNATURE-----