From: Antonio Ospite on 17 May 2010 05:20 On Mon, 17 May 2010 09:34:02 +0800 Axel Lin <axel.lin(a)gmail.com> wrote: > In current implementation, lp3944_probe return 0 even if lp3944_configure fail. > Therefore, led_classdev_unregister will be executed twice ( in error handling of lp3944_configure and lp3944_remove ). > This patch properly handles lp3944_configure fail in lp3944_probe. > Hi Axel, thanks for the fix, I agree it's needed. There are some minor comments inlined below. Plus, when possible, I prefer commit messages to be wrapped to 72/80 characters per line. Please, consider this if you are sending a v2. Ah, may I ask where you are using this driver? Just curious. > Signed-off-by: Axel Lin <axel.lin(a)gmail.com> > --- > drivers/leds/leds-lp3944.c | 10 +++++++++- > 1 files changed, 9 insertions(+), 1 deletions(-) > > diff --git a/drivers/leds/leds-lp3944.c b/drivers/leds/leds-lp3944.c > index 8d5ecce..03a24d0 100644 > --- a/drivers/leds/leds-lp3944.c > +++ b/drivers/leds/leds-lp3944.c > @@ -379,6 +379,7 @@ static int __devinit lp3944_probe(struct i2c_client *client, > { > struct lp3944_platform_data *lp3944_pdata = client->dev.platform_data; > struct lp3944_data *data; > + int err; > > if (lp3944_pdata == NULL) { > dev_err(&client->dev, "no platform data\n"); > @@ -403,8 +404,15 @@ static int __devinit lp3944_probe(struct i2c_client *client, > > dev_info(&client->dev, "lp3944 enabled\n"); > > - lp3944_configure(client, data, lp3944_pdata); > + err = lp3944_configure(client, data, lp3944_pdata); > + if (err < 0) > + goto err_configure; > + The dev_info(&client->dev, "lp3944 enabled\n"); could now go right here, before the return 0, so we don't report the driver as enabled even in the case its probe fails. > return 0; > + > +err_configure: > + kfree(data); add i2c_set_clientdata(client, NULL) here just like in lp3944_remove() > + return err; > } > > static int __devexit lp3944_remove(struct i2c_client *client) > -- > 1.5.4.3 > Regards, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail?
|
Pages: 1 Prev: [PATCH][v2 0/3] Convert KVM to use FPU API Next: x86: Export FPU API for KVM use |