From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757253Ab2F0K4Q (ORCPT ); Wed, 27 Jun 2012 06:56:16 -0400 Received: from rcsinet15.oracle.com ([148.87.113.117]:17113 "EHLO rcsinet15.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756740Ab2F0K4O (ORCPT ); Wed, 27 Jun 2012 06:56:14 -0400 Date: Wed, 27 Jun 2012 13:55:47 +0300 From: Dan Carpenter To: Bryan Wu Cc: Richard Purdie , linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org, Matt Renzelmann Subject: Re: [patch -resend] leds-lp5523: BUG() in error handling in probe() Message-ID: <20120627105547.GD3007@mwanda> References: <20120627085800.GA3007@mwanda> <20120627091055.GW31212@elgon.mountain> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-Source-IP: ucsinet22.oracle.com [156.151.31.94] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jun 27, 2012 at 06:49:10PM +0800, Bryan Wu wrote: > On Wed, Jun 27, 2012 at 5:10 PM, Dan Carpenter wrote: > > Inside the error handling in lp5523_init_led(), there is a place that > > calls to led_classdev_unregister().  When we unregister the LED drivers, > > it tries to set the brightness to OFF.  In this driver setting the > > brightness is done through a work queue and the work queue hasn't been > > initialized yet. > > > > The result is that we trigger a WARN_ON() in the __queue_work(). > > > > The fix is to move the INIT_WORK() in front of the call to > > lp5523_init_led(). > > > > Thanks for resending this, I applied this in my for-next branch. > > > Matt Renzelmann found this using a bug finding tool. > > Just be curious, what's kind of the tool here? > Sorry I should have CC'd Matt on this. I think it's something he is working on at the University of Wisconsin. That's all I know. regards, dan carpenter > -Bryan > > > > > Reported-by: Matt Renzelmann > > Signed-off-by: Dan Carpenter > > --- > > I don't have this hardware, so I can't test it.  I originally sent this > > on Fri, 13 Apr 2012, and that was before Bryan Wu took on the LED > > subsystem.  Also when I sent it, the WARN_ON() in __queue_work() was > > a BUG_ON() so I've updated the commit message. > > > > diff --git a/drivers/leds/leds-lp5523.c b/drivers/leds/leds-lp5523.c > > index 857a3e1..e8a2712 100644 > > --- a/drivers/leds/leds-lp5523.c > > +++ b/drivers/leds/leds-lp5523.c > > @@ -943,6 +943,9 @@ static int __devinit lp5523_probe(struct i2c_client *client, > >                if (pdata->led_config[i].led_current == 0) > >                        continue; > > > > +               INIT_WORK(&chip->leds[led].brightness_work, > > +                       lp5523_led_brightness_work); > > + > >                ret = lp5523_init_led(&chip->leds[led], &client->dev, i, pdata); > >                if (ret) { > >                        dev_err(&client->dev, "error initializing leds\n"); > > @@ -956,9 +959,6 @@ static int __devinit lp5523_probe(struct i2c_client *client, > >                          LP5523_REG_LED_CURRENT_BASE + chip->leds[led].chan_nr, > >                          chip->leds[led].led_current); > > > > -               INIT_WORK(&(chip->leds[led].brightness_work), > > -                       lp5523_led_brightness_work); > > - > >                led++; > >        } > > > > > > -- > Bryan Wu > Kernel Developer    +86.186-168-78255 Mobile > Canonical Ltd.      www.canonical.com > Ubuntu - Linux for human beings | www.ubuntu.com From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dan Carpenter Date: Wed, 27 Jun 2012 10:55:47 +0000 Subject: Re: [patch -resend] leds-lp5523: BUG() in error handling in probe() Message-Id: <20120627105547.GD3007@mwanda> List-Id: References: <20120627085800.GA3007@mwanda> <20120627091055.GW31212@elgon.mountain> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable To: Bryan Wu Cc: Richard Purdie , linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org, Matt Renzelmann On Wed, Jun 27, 2012 at 06:49:10PM +0800, Bryan Wu wrote: > On Wed, Jun 27, 2012 at 5:10 PM, Dan Carpenter = wrote: > > Inside the error handling in lp5523_init_led(), there is a place that > > calls to led_classdev_unregister(). =A0When we unregister the LED drive= rs, > > it tries to set the brightness to OFF. =A0In this driver setting the > > brightness is done through a work queue and the work queue hasn't been > > initialized yet. > > > > The result is that we trigger a WARN_ON() in the __queue_work(). > > > > The fix is to move the INIT_WORK() in front of the call to > > lp5523_init_led(). > > >=20 > Thanks for resending this, I applied this in my for-next branch. >=20 > > Matt Renzelmann found this using a bug finding tool. >=20 > Just be curious, what's kind of the tool here? >=20 Sorry I should have CC'd Matt on this. I think it's something he is working on at the University of Wisconsin. That's all I know. regards, dan carpenter > -Bryan >=20 > > > > Reported-by: Matt Renzelmann > > Signed-off-by: Dan Carpenter > > --- > > I don't have this hardware, so I can't test it. =A0I originally sent th= is > > on Fri, 13 Apr 2012, and that was before Bryan Wu took on the LED > > subsystem. =A0Also when I sent it, the WARN_ON() in __queue_work() was > > a BUG_ON() so I've updated the commit message. > > > > diff --git a/drivers/leds/leds-lp5523.c b/drivers/leds/leds-lp5523.c > > index 857a3e1..e8a2712 100644 > > --- a/drivers/leds/leds-lp5523.c > > +++ b/drivers/leds/leds-lp5523.c > > @@ -943,6 +943,9 @@ static int __devinit lp5523_probe(struct i2c_client= *client, > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (pdata->led_config[i].led_current =3D= 0) > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0continue; > > > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 INIT_WORK(&chip->leds[led].brightness_wor= k, > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 lp5523_led_brightness_wor= k); > > + > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0ret =3D lp5523_init_led(&chip->leds[led]= , &client->dev, i, pdata); > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (ret) { > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0dev_err(&client->dev, "e= rror initializing leds\n"); > > @@ -956,9 +959,6 @@ static int __devinit lp5523_probe(struct i2c_client= *client, > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0LP5523_REG_LED_CURRE= NT_BASE + chip->leds[led].chan_nr, > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0chip->leds[led].led_= current); > > > > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 INIT_WORK(&(chip->leds[led].brightness_wo= rk), > > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 lp5523_led_brightness_wor= k); > > - > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0led++; > > =A0 =A0 =A0 =A0} > > >=20 >=20 >=20 > --=20 > Bryan Wu > Kernel Developer =A0 =A0+86.186-168-78255 Mobile > Canonical Ltd. =A0 =A0 =A0www.canonical.com > Ubuntu - Linux for human beings | www.ubuntu.com -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" = in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html