lkml.org 
[lkml]   [2010]   [Dec]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [PATCH 3/3] leds: lp5521: Fix circular locking
    Date
    Hi,

    >From: Samu Onkalo [mailto:samu.p.onkalo@nokia.com]
    >Sent: 17 December, 2010 12:19
    >Subject: [PATCH 3/3] 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>

    Reviewed-by: Ilkka Koskinen <ilkka.koskinen@nokia.com>


    >---
    > drivers/leds/leds-lp5521.c | 52 ++++++-------------------------------
    >------
    > 1 files changed, 8 insertions(+), 44 deletions(-)
    >
    >diff --git a/drivers/leds/leds-lp5521.c b/drivers/leds/leds-lp5521.c
    >index 745ad99..eda99b9 100644
    >--- a/drivers/leds/leds-lp5521.c
    >+++ b/drivers/leds/leds-lp5521.c
    >@@ -98,7 +98,6 @@
    > #define LP5521_EXT_CLK_USED 0x08
    >
    > struct lp5521_engine {
    >- const struct attribute_group *attributes;
    > int id;
    > u8 mode;
    > u8 prog_page;
    >@@ -225,25 +224,22 @@ static int lp5521_set_led_current(struct
    >lp5521_chip *chip, int led, u8 curr)
    > curr);
    > }
    >
    >-static void lp5521_init_engine(struct lp5521_chip *chip,
    >- const struct attribute_group *attr_group)
    >+static void lp5521_init_engine(struct lp5521_chip *chip)
    > {
    > int i;
    > for (i = 0; i < ARRAY_SIZE(chip->engines); i++) {
    > chip->engines[i].id = i + 1;
    > chip->engines[i].engine_mask = LP5521_ENG_MASK_BASE >> (i *
    >2);
    > chip->engines[i].prog_page = i;
    >- chip->engines[i].attributes = &attr_group[i];
    > }
    > }
    >
    >-static int lp5521_configure(struct i2c_client *client,
    >- const struct attribute_group *attr_group)
    >+static int lp5521_configure(struct i2c_client *client)
    > {
    > struct lp5521_chip *chip = i2c_get_clientdata(client);
    > int ret;
    >
    >- lp5521_init_engine(chip, attr_group);
    >+ lp5521_init_engine(chip);
    >
    > /* Set all PWMs to direct control mode */
    > ret = lp5521_write(client, LP5521_REG_OP_MODE, 0x3F);
    >@@ -329,9 +325,6 @@ static int lp5521_detect(struct i2c_client *client)
    > /* Set engine mode and create appropriate sysfs attributes, if
    >required. */
    > static int lp5521_set_mode(struct lp5521_engine *engine, u8 mode)
    > {
    >- struct lp5521_chip *chip = engine_to_lp5521(engine);
    >- struct i2c_client *client = chip->client;
    >- struct device *dev = &client->dev;
    > int ret = 0;
    >
    > /* if in that mode already do nothing, except for run */
    >@@ -343,18 +336,10 @@ static int lp5521_set_mode(struct lp5521_engine
    >*engine, u8 mode)
    > } else if (mode == LP5521_CMD_LOAD) {
    > lp5521_set_engine_mode(engine, LP5521_CMD_DISABLED);
    > lp5521_set_engine_mode(engine, LP5521_CMD_LOAD);
    >-
    >- ret = sysfs_create_group(&dev->kobj, engine->attributes);
    >- if (ret)
    >- return ret;
    > } else if (mode == LP5521_CMD_DISABLED) {
    > lp5521_set_engine_mode(engine, LP5521_CMD_DISABLED);
    > }
    >
    >- /* remove load attribute from sysfs if not in load mode */
    >- if (engine->mode == LP5521_CMD_LOAD && mode != LP5521_CMD_LOAD)
    >- sysfs_remove_group(&dev->kobj, engine->attributes);
    >-
    > engine->mode = mode;
    >
    > return ret;
    >@@ -387,7 +372,10 @@ static int lp5521_do_store_load(struct
    >lp5521_engine *engine,
    > goto fail;
    >
    > mutex_lock(&chip->lock);
    >- ret = lp5521_load_program(engine, pattern);
    >+ if (engine->mode == LP5521_CMD_LOAD)
    >+ ret = lp5521_load_program(engine, pattern);
    >+ else
    >+ ret = -EINVAL;
    > mutex_unlock(&chip->lock);
    >
    > if (ret) {
    >@@ -574,20 +562,8 @@ static struct attribute *lp5521_attributes[] = {
    > &dev_attr_engine2_mode.attr,
    > &dev_attr_engine3_mode.attr,
    > &dev_attr_selftest.attr,
    >- NULL
    >-};
    >-
    >-static struct attribute *lp5521_engine1_attributes[] = {
    > &dev_attr_engine1_load.attr,
    >- NULL
    >-};
    >-
    >-static struct attribute *lp5521_engine2_attributes[] = {
    > &dev_attr_engine2_load.attr,
    >- NULL
    >-};
    >-
    >-static struct attribute *lp5521_engine3_attributes[] = {
    > &dev_attr_engine3_load.attr,
    > NULL
    > };
    >@@ -596,12 +572,6 @@ static const struct attribute_group lp5521_group =
    >{
    > .attrs = lp5521_attributes,
    > };
    >
    >-static const struct attribute_group lp5521_engine_group[] = {
    >- {.attrs = lp5521_engine1_attributes },
    >- {.attrs = lp5521_engine2_attributes },
    >- {.attrs = lp5521_engine3_attributes },
    >-};
    >-
    > static int lp5521_register_sysfs(struct i2c_client *client)
    > {
    > struct device *dev = &client->dev;
    >@@ -616,12 +586,6 @@ static void lp5521_unregister_sysfs(struct
    >i2c_client *client)
    >
    > sysfs_remove_group(&dev->kobj, &lp5521_group);
    >
    >- for (i = 0; i < ARRAY_SIZE(chip->engines); i++) {
    >- if (chip->engines[i].mode == LP5521_CMD_LOAD)
    >- sysfs_remove_group(&dev->kobj,
    >- chip->engines[i].attributes);
    >- }
    >-
    > for (i = 0; i < chip->num_leds; i++)
    > sysfs_remove_group(&chip->leds[i].cdev.dev->kobj,
    > &lp5521_led_attribute_group);
    >@@ -724,7 +688,7 @@ static int lp5521_probe(struct i2c_client *client,
    >
    > dev_info(&client->dev, "%s programmable led chip found\n", id-
    >>name);
    >
    >- ret = lp5521_configure(client, lp5521_engine_group);
    >+ ret = lp5521_configure(client);
    > if (ret < 0) {
    > dev_err(&client->dev, "error configuring chip\n");
    > goto fail2;
    >--
    >1.7.0.4



    \
     
     \ /
      Last update: 2010-12-17 13:49    [W:0.034 / U:90.800 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site