[PATCH RFT] leds: lp8788: Fix updating scale configuration bits

[PATCH RFT] leds: lp8788: Fix updating scale configuration bits

[Axel Lin]

We need to do left shift (cfg->num + LP8788_ISINK_SCALE_OFFSET) bits for
updating scale configuration.

Signed-off-by: Axel Lin <axel.lin@gmail.com>
---
Hi Milo,
Current code of updating scale configuration bits looks wrong to me
because the mask does not match the val.
I don't have this hardware, can you test this patch?

Thanks,
Axel
 drivers/leds/leds-lp8788.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/leds-lp8788.c b/drivers/leds/leds-lp8788.c
index 53bd136..0ade6eb 100644
--- a/drivers/leds/leds-lp8788.c
+++ b/drivers/leds/leds-lp8788.c
@@ -63,7 +63,7 @@ static int lp8788_led_init_device(struct lp8788_led *led,
 	/* scale configuration */
 	addr = LP8788_ISINK_CTRL;
 	mask = 1 << (cfg->num + LP8788_ISINK_SCALE_OFFSET);
-	val = cfg->scale << cfg->num;
+	val = cfg->scale << (cfg->num + LP8788_ISINK_SCALE_OFFSET);
 	ret = lp8788_update_bits(led->lp, addr, mask, val);
 	if (ret)
 		return ret;
-- 
1.7.9.5

Re: [PATCH RFT] leds: lp8788: Fix updating scale configuration bits

[Bryan Wu]

On Wed, Aug 1, 2012 at 8:40 PM, Axel Lin <axel.lin@gmail.com> wrote:
> We need to do left shift (cfg->num + LP8788_ISINK_SCALE_OFFSET) bits for
> updating scale configuration.
>
> Signed-off-by: Axel Lin <axel.lin@gmail.com>
> ---
> Hi Milo,
> Current code of updating scale configuration bits looks wrong to me
> because the mask does not match the val.
> I don't have this hardware, can you test this patch?
>

Milo, I think this patch from Axel is reasonable. could you please
take a look at this?

-Bryan

> Thanks,
> Axel
>  drivers/leds/leds-lp8788.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/leds/leds-lp8788.c b/drivers/leds/leds-lp8788.c
> index 53bd136..0ade6eb 100644
> --- a/drivers/leds/leds-lp8788.c
> +++ b/drivers/leds/leds-lp8788.c
> @@ -63,7 +63,7 @@ static int lp8788_led_init_device(struct lp8788_led *led,
>         /* scale configuration */
>         addr = LP8788_ISINK_CTRL;
>         mask = 1 << (cfg->num + LP8788_ISINK_SCALE_OFFSET);
> -       val = cfg->scale << cfg->num;
> +       val = cfg->scale << (cfg->num + LP8788_ISINK_SCALE_OFFSET);
>         ret = lp8788_update_bits(led->lp, addr, mask, val);
>         if (ret)
>                 return ret;
> --
> 1.7.9.5
>
>
>



-- 
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

RE: [PATCH RFT] leds: lp8788: Fix updating scale configuration bits

[Kim, Milo]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 657 bytes --]

> We need to do left shift (cfg->num + LP8788_ISINK_SCALE_OFFSET) bits
> for
> updating scale configuration.
> 
> Signed-off-by: Axel Lin <axel.lin@gmail.com>
> ---
> Hi Milo,
> Current code of updating scale configuration bits looks wrong to me
> because the mask does not match the val.
> I don't have this hardware, can you test this patch?
> 
> Thanks,

The scale bits can be never updated without this patch.
This patch should be applied.

Thanks for fixing this bug !

Best Regards,
Milo



ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH RFT] leds: lp8788: Fix updating scale configuration bits

[Kim, Milo]

> > We need to do left shift (cfg->num + LP8788_ISINK_SCALE_OFFSET) bits
> for
> > updating scale configuration.
> >
> > Signed-off-by: Axel Lin <axel.lin@gmail.com>
> > ---
> 
> Milo, I think this patch from Axel is reasonable. could you please
> take a look at this?
> 
> -Bryan

Acked-by: Milo(Woogyom) Kim <milo.kim@ti.com>
Tested-by: Milo(Woogyom) Kim <milo.kim@ti.com>

Thanks,
Milo -

Re: [PATCH RFT] leds: lp8788: Fix updating scale configuration bits

[Bryan Wu]

On Tue, Aug 7, 2012 at 4:09 PM, Kim, Milo <Milo.Kim@ti.com> wrote:
>> > We need to do left shift (cfg->num + LP8788_ISINK_SCALE_OFFSET) bits
>> for
>> > updating scale configuration.
>> >
>> > Signed-off-by: Axel Lin <axel.lin@gmail.com>
>> > ---
>>
>> Milo, I think this patch from Axel is reasonable. could you please
>> take a look at this?
>>
>> -Bryan
>
> Acked-by: Milo(Woogyom) Kim <milo.kim@ti.com>
> Tested-by: Milo(Woogyom) Kim <milo.kim@ti.com>
>
> Thanks,
> Milo -

Great, Applied

Thanks,
-- 
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