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