From: Bryan Wu <bryan.wu@canonical.com> To: Dan Carpenter <dan.carpenter@oracle.com> Cc: Richard Purdie <rpurdie@rpsys.net>, linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org Subject: Re: [patch -resend] leds-lp5523: BUG() in error handling in probe() Date: Wed, 27 Jun 2012 18:49:10 +0800 [thread overview] Message-ID: <CAK5ve-J9CgB=uz-09=POPoygWvNs7k_R3tbZVAzi0VLKTpvOqA@mail.gmail.com> (raw) In-Reply-To: <20120627091055.GW31212@elgon.mountain> On Wed, Jun 27, 2012 at 5:10 PM, Dan Carpenter <dan.carpenter@oracle.com> 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? -Bryan > > Reported-by: Matt Renzelmann <mjr@cs.wisc.edu> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > 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 <bryan.wu@canonical.com> Kernel Developer +86.186-168-78255 Mobile Canonical Ltd. www.canonical.com Ubuntu - Linux for human beings | www.ubuntu.com
WARNING: multiple messages have this Message-ID (diff)
From: Bryan Wu <bryan.wu@canonical.com> To: Dan Carpenter <dan.carpenter@oracle.com> Cc: Richard Purdie <rpurdie@rpsys.net>, linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org Subject: Re: [patch -resend] leds-lp5523: BUG() in error handling in probe() Date: Wed, 27 Jun 2012 10:49:10 +0000 [thread overview] Message-ID: <CAK5ve-J9CgB=uz-09=POPoygWvNs7k_R3tbZVAzi0VLKTpvOqA@mail.gmail.com> (raw) In-Reply-To: <20120627091055.GW31212@elgon.mountain> On Wed, Jun 27, 2012 at 5:10 PM, Dan Carpenter <dan.carpenter@oracle.com> 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? -Bryan > > Reported-by: Matt Renzelmann <mjr@cs.wisc.edu> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > 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 <bryan.wu@canonical.com> Kernel Developer +86.186-168-78255 Mobile Canonical Ltd. www.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
next prev parent reply other threads:[~2012-06-27 10:49 UTC|newest] Thread overview: 90+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <CAA9_cmeNagC1sF54BAHa1sTzL3sMD3eKoftHQHCM5q9vKq5Dyg@mail.gmail.com> 2012-06-27 8:58 ` [Ksummit-2012-discuss] [ATTEND] Your upstream maintainer just isn't that into you Dan Carpenter 2012-06-27 8:59 ` [patch -resend] [SCSI] bfa: off by one in bfa_ioc_mbox_isr() Dan Carpenter 2012-06-27 8:59 ` Dan Carpenter 2012-06-27 17:44 ` Krishna Gudipati 2012-06-27 17:44 ` Krishna Gudipati 2012-06-27 8:59 ` [patch -resend] [SCSI] bfa: dereferencing freed memory in bfad_im_probe() Dan Carpenter 2012-06-27 8:59 ` Dan Carpenter 2012-06-27 17:45 ` Krishna Gudipati 2012-06-27 17:45 ` Krishna Gudipati 2012-06-27 9:00 ` [patch -resend] [SCSI] megaraid: remove a spurious IRQ enable Dan Carpenter 2012-06-27 9:00 ` Dan Carpenter 2012-06-27 22:36 ` adam radford 2012-06-27 22:36 ` adam radford 2012-06-27 9:00 ` [patch 1/2 -resend] SCSI: advansys: handle errors from scsi_dma_map() Dan Carpenter 2012-06-27 9:00 ` Dan Carpenter 2012-06-27 10:01 ` walter harms 2012-06-27 10:01 ` walter harms 2012-06-27 10:15 ` Dan Carpenter 2012-06-27 10:15 ` Dan Carpenter 2012-06-27 9:01 ` [patch 2/2 -resend] SCSI: advansys: use a subsystem error code Dan Carpenter 2012-06-27 9:01 ` Dan Carpenter 2012-06-27 9:01 ` [patch -resend] 9p: fix min_t() casting in p9pdu_vwritef() Dan Carpenter 2012-06-27 9:01 ` Dan Carpenter 2012-06-27 10:19 ` walter harms 2012-06-27 10:19 ` walter harms 2012-06-27 10:36 ` Dan Carpenter 2012-06-27 10:36 ` Dan Carpenter 2012-06-27 10:56 ` walter harms 2012-06-27 22:26 ` David Miller 2012-06-27 22:26 ` David Miller 2012-06-27 9:02 ` [patch -resend] spi/spidev: handle integer wrap in spidev_message() Dan Carpenter 2012-06-27 9:02 ` Dan Carpenter 2012-06-27 9:02 ` [patch -resend] mmc: ushc: fix an endianness conversion in ushc_request() Dan Carpenter 2012-06-27 9:02 ` Dan Carpenter 2012-06-27 9:03 ` [patch -resend] sgi-xp: nested calls to spin_lock_irqsave() Dan Carpenter 2012-06-27 9:03 ` Dan Carpenter 2012-06-27 9:04 ` [patch 1/3 -resend] [SCSI] pmcraid: remove unneeded check Dan Carpenter 2012-06-27 9:04 ` Dan Carpenter 2012-06-27 9:04 ` [patch 2/3 -resend] [SCSI] pmcraid: cpu_to_le32() => cpu_to_le64() Dan Carpenter 2012-06-27 9:04 ` Dan Carpenter 2012-06-27 9:04 ` [patch 3/3 -resend] [SCSI] pmcraid: find_first_zero_bit() takes bits not bytes Dan Carpenter 2012-06-27 9:04 ` Dan Carpenter 2012-06-27 9:05 ` [patch -resend] [SCSI] isci: add a couple __iomem annotations Dan Carpenter 2012-06-27 9:05 ` Dan Carpenter 2012-06-27 20:58 ` Dan Williams 2012-06-27 20:58 ` Dan Williams 2012-06-27 9:05 ` [SCSI] bfa: Implement LUN Masking feature using the SCSI Slave Callouts Dan Carpenter 2012-06-27 9:06 ` [patch -resend] NVMe: handle allocation failure in nvme_map_user_pages() Dan Carpenter 2012-06-27 9:06 ` Dan Carpenter 2012-06-27 9:06 ` [patch -resend] [media] az6007: precedence bug in az6007_i2c_xfer() Dan Carpenter 2012-06-27 9:06 ` Dan Carpenter 2012-06-27 13:11 ` Mauro Carvalho Chehab 2012-06-27 13:11 ` Mauro Carvalho Chehab 2012-06-28 19:33 ` Dan Carpenter 2012-06-28 19:33 ` Dan Carpenter 2012-06-27 9:07 ` [patch v3 -resend] edac i5000, i5400: fix pointer math in i5000_get_mc_regs() Dan Carpenter 2012-06-27 9:07 ` Dan Carpenter 2012-06-27 12:15 ` Mauro Carvalho Chehab 2012-06-27 12:15 ` Mauro Carvalho Chehab 2012-06-27 9:08 ` [patch -resend] [SCSI] megaraid: cleanup type issue in mega_build_cmd() Dan Carpenter 2012-06-27 9:08 ` Dan Carpenter 2012-06-27 22:36 ` adam radford 2012-06-27 22:36 ` adam radford 2012-06-27 9:08 ` [patch 1/2 -resend] dma-debug: debugfs_create_bool() takes a u32 pointer Dan Carpenter 2012-06-27 9:08 ` Dan Carpenter 2012-06-27 11:09 ` Neil Horman 2012-06-27 11:09 ` Neil Horman 2012-07-02 10:15 ` Joerg Roedel 2012-07-02 10:15 ` Joerg Roedel 2012-06-27 9:09 ` [patch 2/2 -resend] iommu/amd: fix type bug in flush code Dan Carpenter 2012-06-27 9:09 ` Dan Carpenter 2012-06-27 9:09 ` Dan Carpenter 2012-06-27 9:10 ` [patch -resend] isci: make function declaration match implementation Dan Carpenter 2012-06-27 9:10 ` Dan Carpenter 2012-06-27 9:10 ` [patch -resend] drm/i915/bios: cleanup return type of intel_parse_bios() Dan Carpenter 2012-06-27 9:10 ` Dan Carpenter 2012-06-27 9:10 ` [patch -resend] leds-lp5523: BUG() in error handling in probe() Dan Carpenter 2012-06-27 9:10 ` Dan Carpenter 2012-06-27 10:49 ` Bryan Wu [this message] 2012-06-27 10:49 ` Bryan Wu 2012-06-27 10:55 ` Dan Carpenter 2012-06-27 10:55 ` Dan Carpenter 2012-06-28 19:39 ` Matt Renzelmann 2012-06-28 19:39 ` Matt Renzelmann 2012-06-27 9:11 ` [patch -resend] Input: ff-memless - fix a couple min_t() casts Dan Carpenter 2012-06-27 9:11 ` Dan Carpenter 2012-07-08 1:18 ` Dmitry Torokhov 2012-07-08 1:18 ` Dmitry Torokhov 2012-06-27 9:11 ` [patch -resend] [patch] tlb_uv: remove some dead code in parse_tunables_write() Dan Carpenter 2012-06-27 9:11 ` Dan Carpenter
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='CAK5ve-J9CgB=uz-09=POPoygWvNs7k_R3tbZVAzi0VLKTpvOqA@mail.gmail.com' \ --to=bryan.wu@canonical.com \ --cc=dan.carpenter@oracle.com \ --cc=kernel-janitors@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-leds@vger.kernel.org \ --cc=rpurdie@rpsys.net \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.