Commit d4e7ad03e84b2301be4f9a39ef2778126699ff0c

Authored by Samu Onkalo
Committed by Linus Torvalds
1 parent fbac0812de

leds: lp5521: fix circular locking

Driver contained possibility for circular locking.

One lock is held by sysfs-core and another one by the driver itself.  This
happened when the driver created or removed sysfs entries dynamically.
There is no real need to do those operations.  Now all the sysfs entries
are created at probe and removed at removal.  Engine load sysfs entries
are now visible all the time.  However, access to the entries fails if the
engine is disabled or running.

Signed-off-by: Samu Onkalo <samu.p.onkalo@nokia.com>
Cc: Arun Murthy <arun.murthy@stericsson.com>
Reviewed-by: Ilkka Koskinen <ilkka.koskinen@nokia.com>
Cc: Richard Purdie <rpurdie@rpsys.net>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Showing 1 changed file with 8 additions and 44 deletions Side-by-side Diff

drivers/leds/leds-lp5521.c
... ... @@ -98,7 +98,6 @@
98 98 #define LP5521_EXT_CLK_USED 0x08
99 99  
100 100 struct lp5521_engine {
101   - const struct attribute_group *attributes;
102 101 int id;
103 102 u8 mode;
104 103 u8 prog_page;
105 104  
106 105  
107 106  
... ... @@ -225,25 +224,22 @@
225 224 curr);
226 225 }
227 226  
228   -static void lp5521_init_engine(struct lp5521_chip *chip,
229   - const struct attribute_group *attr_group)
  227 +static void lp5521_init_engine(struct lp5521_chip *chip)
230 228 {
231 229 int i;
232 230 for (i = 0; i < ARRAY_SIZE(chip->engines); i++) {
233 231 chip->engines[i].id = i + 1;
234 232 chip->engines[i].engine_mask = LP5521_ENG_MASK_BASE >> (i * 2);
235 233 chip->engines[i].prog_page = i;
236   - chip->engines[i].attributes = &attr_group[i];
237 234 }
238 235 }
239 236  
240   -static int lp5521_configure(struct i2c_client *client,
241   - const struct attribute_group *attr_group)
  237 +static int lp5521_configure(struct i2c_client *client)
242 238 {
243 239 struct lp5521_chip *chip = i2c_get_clientdata(client);
244 240 int ret;
245 241  
246   - lp5521_init_engine(chip, attr_group);
  242 + lp5521_init_engine(chip);
247 243  
248 244 /* Set all PWMs to direct control mode */
249 245 ret = lp5521_write(client, LP5521_REG_OP_MODE, 0x3F);
... ... @@ -329,9 +325,6 @@
329 325 /* Set engine mode and create appropriate sysfs attributes, if required. */
330 326 static int lp5521_set_mode(struct lp5521_engine *engine, u8 mode)
331 327 {
332   - struct lp5521_chip *chip = engine_to_lp5521(engine);
333   - struct i2c_client *client = chip->client;
334   - struct device *dev = &client->dev;
335 328 int ret = 0;
336 329  
337 330 /* if in that mode already do nothing, except for run */
338 331  
... ... @@ -343,18 +336,10 @@
343 336 } else if (mode == LP5521_CMD_LOAD) {
344 337 lp5521_set_engine_mode(engine, LP5521_CMD_DISABLED);
345 338 lp5521_set_engine_mode(engine, LP5521_CMD_LOAD);
346   -
347   - ret = sysfs_create_group(&dev->kobj, engine->attributes);
348   - if (ret)
349   - return ret;
350 339 } else if (mode == LP5521_CMD_DISABLED) {
351 340 lp5521_set_engine_mode(engine, LP5521_CMD_DISABLED);
352 341 }
353 342  
354   - /* remove load attribute from sysfs if not in load mode */
355   - if (engine->mode == LP5521_CMD_LOAD && mode != LP5521_CMD_LOAD)
356   - sysfs_remove_group(&dev->kobj, engine->attributes);
357   -
358 343 engine->mode = mode;
359 344  
360 345 return ret;
... ... @@ -389,7 +374,10 @@
389 374 goto fail;
390 375  
391 376 mutex_lock(&chip->lock);
392   - ret = lp5521_load_program(engine, pattern);
  377 + if (engine->mode == LP5521_CMD_LOAD)
  378 + ret = lp5521_load_program(engine, pattern);
  379 + else
  380 + ret = -EINVAL;
393 381 mutex_unlock(&chip->lock);
394 382  
395 383 if (ret) {
396 384  
397 385  
... ... @@ -576,20 +564,8 @@
576 564 &dev_attr_engine2_mode.attr,
577 565 &dev_attr_engine3_mode.attr,
578 566 &dev_attr_selftest.attr,
579   - NULL
580   -};
581   -
582   -static struct attribute *lp5521_engine1_attributes[] = {
583 567 &dev_attr_engine1_load.attr,
584   - NULL
585   -};
586   -
587   -static struct attribute *lp5521_engine2_attributes[] = {
588 568 &dev_attr_engine2_load.attr,
589   - NULL
590   -};
591   -
592   -static struct attribute *lp5521_engine3_attributes[] = {
593 569 &dev_attr_engine3_load.attr,
594 570 NULL
595 571 };
... ... @@ -598,12 +574,6 @@
598 574 .attrs = lp5521_attributes,
599 575 };
600 576  
601   -static const struct attribute_group lp5521_engine_group[] = {
602   - {.attrs = lp5521_engine1_attributes },
603   - {.attrs = lp5521_engine2_attributes },
604   - {.attrs = lp5521_engine3_attributes },
605   -};
606   -
607 577 static int lp5521_register_sysfs(struct i2c_client *client)
608 578 {
609 579 struct device *dev = &client->dev;
... ... @@ -618,12 +588,6 @@
618 588  
619 589 sysfs_remove_group(&dev->kobj, &lp5521_group);
620 590  
621   - for (i = 0; i < ARRAY_SIZE(chip->engines); i++) {
622   - if (chip->engines[i].mode == LP5521_CMD_LOAD)
623   - sysfs_remove_group(&dev->kobj,
624   - chip->engines[i].attributes);
625   - }
626   -
627 591 for (i = 0; i < chip->num_leds; i++)
628 592 sysfs_remove_group(&chip->leds[i].cdev.dev->kobj,
629 593 &lp5521_led_attribute_group);
... ... @@ -725,7 +689,7 @@
725 689  
726 690 dev_info(&client->dev, "%s programmable led chip found\n", id->name);
727 691  
728   - ret = lp5521_configure(client, lp5521_engine_group);
  692 + ret = lp5521_configure(client);
729 693 if (ret < 0) {
730 694 dev_err(&client->dev, "error configuring chip\n");
731 695 goto fail2;