Skip to contentSkip to conversation

leds: lp8788: Fix updating scale configuration bits

PATCH In review Axel Lin 3 participants 5 messages Aug 1, 2012 – Aug 8, 2012 LED

lore
Share & tools
lore

Axel Lin

author
#0 lore raw

PATCH 1/1 leds: lp8788: Fix updating scale configuration bits

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

Signed-off-by Axel Lin <[email protected]>

---
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(-)
drivers/leds/leds-lp8788.c +1 -1
index 53bd136..0ade6eb 100644
@@ -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;
Signature · 1 line
1.7.9.5
#1 lore raw
Quoted 10 lines · Axel Lin

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

Signed-off-by Axel Lin <[email protected]>

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

Quoted 23 lines

Thanks,
Axel

 drivers/leds/leds-lp8788.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/leds/leds-lp8788.c +1 -2
index 53bd136..0ade6eb 100644
@@ -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

Signature · 4 lines
Bryan Wu <[email protected]>
Kernel Developer    +86.186-168-78255 Mobile
Canonical Ltd.      www.canonical.com
Ubuntu - Linux for human beings | www.ubuntu.com
#3 lore raw
Quoted 10 lines · Milo
Quoted 10 lines
Quoted 1 line

We need to do left shift (cfg->num + LP8788_ISINK_SCALE_OFFSET) bits

for

Quoted 4 lines

updating scale configuration.

Signed-off-by Axel Lin <[email protected]>

---

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

Quoted 7 lines
Quoted 1 line

-Bryan

Acked-by Milo(Woogyom) Kim <[email protected]>
Tested-by Milo(Woogyom) Kim <[email protected]>

Thanks,
Milo -

Great, Applied

Thanks,

Signature · 4 lines
Bryan Wu <[email protected]>
Kernel Developer    +86.186-168-78255 Mobile
Canonical Ltd.      www.canonical.com
Ubuntu - Linux for human beings | www.ubuntu.com
#4 lore raw
In reply to Axel Lin
Quoted 4 lines

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

Quoted 8 lines
Signed-off-by Axel Lin <[email protected]>

---
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�+�������+%�����ݶ��w��{.n�+����{��G�����{ay�ʇڙ�,j��f���h��������z_�(�階�ݢj"���m������G������?����&���~��iO��z��v�^�m���� ������?�I�