* [PATCH 1/3] leds: lp5521/5523: Fix multiple engine usage bug
@ 2013-11-21 6:13 Milo Kim
2013-12-02 19:37 ` Bryan Wu
0 siblings, 1 reply; 2+ messages in thread
From: Milo Kim @ 2013-11-21 6:13 UTC (permalink / raw)
To: Bryan Wu; +Cc: linux-kernel, linux-leds, Milo Kim, Pali Rohár
Whenever the engine is loaded by the user-application, the operation mode is
reset first. But it has a problem in case of multiple engine used because
previous engine settings are cleared.
The driver should update not whole 8bits but each engine bit by masking.
On the other hands, whole engines should be reset when the driver is unloaded
and on initializing the LP5523 driver.
So, new functions are used for this handling - lp5521/5523_stop_all_engines().
Cc: Pali Rohár <pali.rohar@gmail.com>
Signed-off-by: Milo Kim <milo.kim@ti.com>
---
drivers/leds/leds-lp5521.c | 18 ++++++++++++++++--
drivers/leds/leds-lp5523.c | 20 +++++++++++++++++---
2 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/drivers/leds/leds-lp5521.c b/drivers/leds/leds-lp5521.c
index 0518835..26f89ac 100644
--- a/drivers/leds/leds-lp5521.c
+++ b/drivers/leds/leds-lp5521.c
@@ -152,12 +152,26 @@ static void lp5521_load_engine(struct lp55xx_chip *chip)
lp5521_wait_opmode_done();
}
-static void lp5521_stop_engine(struct lp55xx_chip *chip)
+static void lp5521_stop_all_engines(struct lp55xx_chip *chip)
{
lp55xx_write(chip, LP5521_REG_OP_MODE, 0);
lp5521_wait_opmode_done();
}
+static void lp5521_stop_engine(struct lp55xx_chip *chip)
+{
+ enum lp55xx_engine_index idx = chip->engine_idx;
+ u8 mask[] = {
+ [LP55XX_ENGINE_1] = LP5521_MODE_R_M,
+ [LP55XX_ENGINE_2] = LP5521_MODE_G_M,
+ [LP55XX_ENGINE_3] = LP5521_MODE_B_M,
+ };
+
+ lp55xx_update_bits(chip, LP5521_REG_OP_MODE, mask[idx], 0);
+
+ lp5521_wait_opmode_done();
+}
+
static void lp5521_run_engine(struct lp55xx_chip *chip, bool start)
{
int ret;
@@ -568,7 +582,7 @@ static int lp5521_remove(struct i2c_client *client)
struct lp55xx_led *led = i2c_get_clientdata(client);
struct lp55xx_chip *chip = led->chip;
- lp5521_stop_engine(chip);
+ lp5521_stop_all_engines(chip);
lp55xx_unregister_sysfs(chip);
lp55xx_unregister_leds(led, chip);
lp55xx_deinit_device(chip);
diff --git a/drivers/leds/leds-lp5523.c b/drivers/leds/leds-lp5523.c
index 6b553d9..2ce1723 100644
--- a/drivers/leds/leds-lp5523.c
+++ b/drivers/leds/leds-lp5523.c
@@ -187,12 +187,26 @@ static void lp5523_load_engine_and_select_page(struct lp55xx_chip *chip)
lp55xx_write(chip, LP5523_REG_PROG_PAGE_SEL, page_sel[idx]);
}
-static void lp5523_stop_engine(struct lp55xx_chip *chip)
+static void lp5523_stop_all_engines(struct lp55xx_chip *chip)
{
lp55xx_write(chip, LP5523_REG_OP_MODE, 0);
lp5523_wait_opmode_done();
}
+static void lp5523_stop_engine(struct lp55xx_chip *chip)
+{
+ enum lp55xx_engine_index idx = chip->engine_idx;
+ u8 mask[] = {
+ [LP55XX_ENGINE_1] = LP5523_MODE_ENG1_M,
+ [LP55XX_ENGINE_2] = LP5523_MODE_ENG2_M,
+ [LP55XX_ENGINE_3] = LP5523_MODE_ENG3_M,
+ };
+
+ lp55xx_update_bits(chip, LP5523_REG_OP_MODE, mask[idx], 0);
+
+ lp5523_wait_opmode_done();
+}
+
static void lp5523_turn_off_channels(struct lp55xx_chip *chip)
{
int i;
@@ -303,7 +317,7 @@ static int lp5523_init_program_engine(struct lp55xx_chip *chip)
}
out:
- lp5523_stop_engine(chip);
+ lp5523_stop_all_engines(chip);
return ret;
}
@@ -778,7 +792,7 @@ static int lp5523_remove(struct i2c_client *client)
struct lp55xx_led *led = i2c_get_clientdata(client);
struct lp55xx_chip *chip = led->chip;
- lp5523_stop_engine(chip);
+ lp5523_stop_all_engines(chip);
lp55xx_unregister_sysfs(chip);
lp55xx_unregister_leds(led, chip);
lp55xx_deinit_device(chip);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH 1/3] leds: lp5521/5523: Fix multiple engine usage bug
2013-11-21 6:13 [PATCH 1/3] leds: lp5521/5523: Fix multiple engine usage bug Milo Kim
@ 2013-12-02 19:37 ` Bryan Wu
0 siblings, 0 replies; 2+ messages in thread
From: Bryan Wu @ 2013-12-02 19:37 UTC (permalink / raw)
To: Milo Kim; +Cc: lkml, Linux LED Subsystem, Pali Rohár
On Wed, Nov 20, 2013 at 10:13 PM, Milo Kim <milo.kim@ti.com> wrote:
> Whenever the engine is loaded by the user-application, the operation mode is
> reset first. But it has a problem in case of multiple engine used because
> previous engine settings are cleared.
> The driver should update not whole 8bits but each engine bit by masking.
>
> On the other hands, whole engines should be reset when the driver is unloaded
> and on initializing the LP5523 driver.
> So, new functions are used for this handling - lp5521/5523_stop_all_engines().
>
This one looks good to me, I will merge it.
Thanks,
-Bryan
> Cc: Pali Rohár <pali.rohar@gmail.com>
> Signed-off-by: Milo Kim <milo.kim@ti.com>
> ---
> drivers/leds/leds-lp5521.c | 18 ++++++++++++++++--
> drivers/leds/leds-lp5523.c | 20 +++++++++++++++++---
> 2 files changed, 33 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/leds/leds-lp5521.c b/drivers/leds/leds-lp5521.c
> index 0518835..26f89ac 100644
> --- a/drivers/leds/leds-lp5521.c
> +++ b/drivers/leds/leds-lp5521.c
> @@ -152,12 +152,26 @@ static void lp5521_load_engine(struct lp55xx_chip *chip)
> lp5521_wait_opmode_done();
> }
>
> -static void lp5521_stop_engine(struct lp55xx_chip *chip)
> +static void lp5521_stop_all_engines(struct lp55xx_chip *chip)
> {
> lp55xx_write(chip, LP5521_REG_OP_MODE, 0);
> lp5521_wait_opmode_done();
> }
>
> +static void lp5521_stop_engine(struct lp55xx_chip *chip)
> +{
> + enum lp55xx_engine_index idx = chip->engine_idx;
> + u8 mask[] = {
> + [LP55XX_ENGINE_1] = LP5521_MODE_R_M,
> + [LP55XX_ENGINE_2] = LP5521_MODE_G_M,
> + [LP55XX_ENGINE_3] = LP5521_MODE_B_M,
> + };
> +
> + lp55xx_update_bits(chip, LP5521_REG_OP_MODE, mask[idx], 0);
> +
> + lp5521_wait_opmode_done();
> +}
> +
> static void lp5521_run_engine(struct lp55xx_chip *chip, bool start)
> {
> int ret;
> @@ -568,7 +582,7 @@ static int lp5521_remove(struct i2c_client *client)
> struct lp55xx_led *led = i2c_get_clientdata(client);
> struct lp55xx_chip *chip = led->chip;
>
> - lp5521_stop_engine(chip);
> + lp5521_stop_all_engines(chip);
> lp55xx_unregister_sysfs(chip);
> lp55xx_unregister_leds(led, chip);
> lp55xx_deinit_device(chip);
> diff --git a/drivers/leds/leds-lp5523.c b/drivers/leds/leds-lp5523.c
> index 6b553d9..2ce1723 100644
> --- a/drivers/leds/leds-lp5523.c
> +++ b/drivers/leds/leds-lp5523.c
> @@ -187,12 +187,26 @@ static void lp5523_load_engine_and_select_page(struct lp55xx_chip *chip)
> lp55xx_write(chip, LP5523_REG_PROG_PAGE_SEL, page_sel[idx]);
> }
>
> -static void lp5523_stop_engine(struct lp55xx_chip *chip)
> +static void lp5523_stop_all_engines(struct lp55xx_chip *chip)
> {
> lp55xx_write(chip, LP5523_REG_OP_MODE, 0);
> lp5523_wait_opmode_done();
> }
>
> +static void lp5523_stop_engine(struct lp55xx_chip *chip)
> +{
> + enum lp55xx_engine_index idx = chip->engine_idx;
> + u8 mask[] = {
> + [LP55XX_ENGINE_1] = LP5523_MODE_ENG1_M,
> + [LP55XX_ENGINE_2] = LP5523_MODE_ENG2_M,
> + [LP55XX_ENGINE_3] = LP5523_MODE_ENG3_M,
> + };
> +
> + lp55xx_update_bits(chip, LP5523_REG_OP_MODE, mask[idx], 0);
> +
> + lp5523_wait_opmode_done();
> +}
> +
> static void lp5523_turn_off_channels(struct lp55xx_chip *chip)
> {
> int i;
> @@ -303,7 +317,7 @@ static int lp5523_init_program_engine(struct lp55xx_chip *chip)
> }
>
> out:
> - lp5523_stop_engine(chip);
> + lp5523_stop_all_engines(chip);
> return ret;
> }
>
> @@ -778,7 +792,7 @@ static int lp5523_remove(struct i2c_client *client)
> struct lp55xx_led *led = i2c_get_clientdata(client);
> struct lp55xx_chip *chip = led->chip;
>
> - lp5523_stop_engine(chip);
> + lp5523_stop_all_engines(chip);
> lp55xx_unregister_sysfs(chip);
> lp55xx_unregister_leds(led, chip);
> lp55xx_deinit_device(chip);
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2013-12-02 19:38 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-21 6:13 [PATCH 1/3] leds: lp5521/5523: Fix multiple engine usage bug Milo Kim
2013-12-02 19:37 ` Bryan Wu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox