| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 20 Oct 2009 11:23:22 +0200
From: Samuel Ortiz <sameo@...ux.intel.com>
To: Paul Fertser <fercerpav@...il.com>
Cc: linux-kernel@...r.kernel.org,
Nelson Castillo <arhuaco@...aks-unidos.net>,
Lars-Peter Clausen <lars@...afoo.de>
Subject: Re: [PATCH 5/7] mfd: pcf50633: Cleanup pcf50633_probe error
handling
On Tue, Oct 20, 2009 at 03:09:52AM +0400, Paul Fertser wrote:
> Hi Samuel,
>
> Big thanks for your comments, next time i send anything upstream i
> will certainly provide a minimal changelog for every patch :)
>
> On Mon, Oct 19, 2009 at 05:08:13PM +0200, Samuel Ortiz wrote:
> > On Wed, Oct 14, 2009 at 02:12:34AM +0400, Paul Fertser wrote:
> > > if (enable_irq_wake(client->irq) < 0)
> > > - dev_err(pcf->dev, "IRQ %u cannot be enabled as wake-up source"
> > > + dev_info(pcf->dev, "IRQ %u cannot be enabled as wake-up source"
> > > "in this hardware revision", client->irq);
> > 2 things here: This doesnt really belong to this patch, and also I'd prefer to
> > keep that as an error message.
> [...]
> > > ret = sysfs_create_group(&client->dev.kobj, &pcf_attr_group);
> > > if (ret)
> > > - dev_err(pcf->dev, "error creating sysfs entries\n");
> > > + dev_info(pcf->dev, "Failed to create sysfs entries\n");
> > Same here.
>
> Sure. Please find amended version in attachement.
Patch applied, thanks a lot.
Cheers,
Samuel.
> --
> Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software!
> mailto:fercerpav@...il.com
> From 7f982d01515371fcbab0086bf77448f351b09a42 Mon Sep 17 00:00:00 2001
> From: Lars-Peter Clausen <lars@...afoo.de>
> Date: Thu, 8 Oct 2009 00:24:54 +0200
> Subject: [PATCH] mfd: pcf50633: Cleanup pcf50633_probe error handling
>
> Currently the child devices were not freed if the irq could not be requested.
> This patch restructures the function, that in case of an error all previously
> allocated resources are freed.
>
> Signed-off-by: Lars-Peter Clausen <lars@...afoo.de>
> Signed-off-by: Paul Fertser <fercerpav@...il.com>
> ---
> drivers/mfd/pcf50633-core.c | 43 ++++++++++++++++++++++++++-----------------
> 1 files changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/mfd/pcf50633-core.c b/drivers/mfd/pcf50633-core.c
> index 69cdbdc..5aed527 100644
> --- a/drivers/mfd/pcf50633-core.c
> +++ b/drivers/mfd/pcf50633-core.c
> @@ -553,9 +553,14 @@ static int __devinit pcf50633_probe(struct i2c_client *client,
> {
> struct pcf50633 *pcf;
> struct pcf50633_platform_data *pdata = client->dev.platform_data;
> - int i, ret = 0;
> + int i, ret;
> int version, variant;
>
> + if (!client->irq) {
> + dev_err(&client->dev, "Missing IRQ\n");
> + return -ENOENT;
> + }
> +
> pcf = kzalloc(sizeof(*pcf), GFP_KERNEL);
> if (!pcf)
> return -ENOMEM;
> @@ -570,6 +575,12 @@ static int __devinit pcf50633_probe(struct 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);
> @@ -577,7 +588,7 @@ static int __devinit pcf50633_probe(struct i2c_client *client,
> if (version < 0 || variant < 0) {
> dev_err(pcf->dev, "Unable to probe pcf50633\n");
> ret = -ENODEV;
> - goto err;
> + goto err_destroy_workqueue;
> }
>
> dev_info(pcf->dev, "Probed device version %d variant %d\n",
> @@ -591,6 +602,14 @@ 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);
> +
> + if (ret) {
> + dev_err(pcf->dev, "Failed to request IRQ %d\n", ret);
> + goto err_destroy_workqueue;
> + }
> +
> /* Create sub devices */
> pcf50633_client_dev_register(pcf, "pcf50633-input",
> &pcf->input_pdev);
> @@ -606,7 +625,7 @@ static int __devinit pcf50633_probe(struct i2c_client *client,
>
> pdev = platform_device_alloc("pcf50633-regltr", i);
> if (!pdev) {
> - dev_err(pcf->dev, "Cannot create regulator\n");
> + dev_err(pcf->dev, "Cannot create regulator %d\n", i);
> continue;
> }
>
> @@ -618,19 +637,6 @@ static int __devinit pcf50633_probe(struct i2c_client *client,
> platform_device_add(pdev);
> }
>
> - if (client->irq) {
> - ret = request_irq(client->irq, pcf50633_irq,
> - IRQF_TRIGGER_LOW, "pcf50633", pcf);
> -
> - if (ret) {
> - dev_err(pcf->dev, "Failed to request IRQ %d\n", ret);
> - goto err;
> - }
> - } else {
> - dev_err(pcf->dev, "No IRQ configured\n");
> - goto err;
> - }
> -
> 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);
> @@ -644,9 +650,12 @@ static int __devinit pcf50633_probe(struct i2c_client *client,
>
> return 0;
>
> -err:
> +err_destroy_workqueue:
> destroy_workqueue(pcf->work_queue);
> +err_free:
> + i2c_set_clientdata(client, NULL);
> kfree(pcf);
> +
> return ret;
> }
>
> --
> 1.6.0.6
>
--
Intel Open Source Technology Centre
http://oss.intel.com/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists