From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Samuel Ortiz <sameo@linux.intel.com>
Cc: linux-kernel@vger.kernel.org,
Lars-Peter Clausen <lars@metafoo.de>,
Balaji Rao <balajirrao@openmoko.org>
Subject: [PATCH 4/5] mfd: pcf50633 - fix error handling during probe
Date: Sun, 15 Nov 2009 22:08:59 -0800 [thread overview]
Message-ID: <20091116060859.20162.49497.stgit@localhost.localdomain> (raw)
In-Reply-To: <20091116060611.20162.81714.stgit@localhost.localdomain>
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 {
next prev parent reply other threads:[~2009-11-16 6:09 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-11-16 6:08 [PATCH 0/5] pcf50633 - error handling & use threaded IRQs - v2 Dmitry Torokhov
2009-11-16 6:08 ` [PATCH 1/5] mfd: pcf50633 - make 'is_suspended' a bool Dmitry Torokhov
2009-11-16 6:08 ` [PATCH 2/5] mfd: pcf50633 - use threaded interrupts Dmitry Torokhov
2009-11-16 6:08 ` [PATCH 3/5] mfd: pcf50633 - move creating of regulator devices out of line Dmitry Torokhov
2009-11-16 6:08 ` Dmitry Torokhov [this message]
2009-11-16 6:09 ` [PATCH 5/5] mfd: pcf50633 - whitespace and formatting fixes Dmitry Torokhov
2009-11-20 9:45 ` [PATCH 0/5] pcf50633 - error handling & use threaded IRQs - v2 Samuel Ortiz
2010-05-07 21:38 ` Lars-Peter Clausen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20091116060859.20162.49497.stgit@localhost.localdomain \
--to=dmitry.torokhov@gmail.com \
--cc=balajirrao@openmoko.org \
--cc=lars@metafoo.de \
--cc=linux-kernel@vger.kernel.org \
--cc=sameo@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox