lkml.org 
[lkml]   [2012]   [Jan]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    SubjectRE: [PATCH 7/7] leds-lm3530: enhanced arithmetic operation
    The reason why dividing by two is only 7-bits of brightness value in the LM3530 register.

    Bit7 | bit6 | bit5 | bit4 | bit3 | bit2 | bit1 | bit0
    ------------------------------------------------------
    N/A | LED Brightness Data (Bits[6:0])
    ------------------------------------------------------

    So maximum value is limited to 127.

    And we can think the alternative is '(brt_val & 0x7F)' - taking off the MSB to make it 7bits.
    But the result is unexpected because value 128 and 255 has same brightness.
    Brightness should be changed step by step, so appropriate method is dividing by two.

    For the bit arithmetic and division, is it safe to predict same results ?
    As you told, gcc makes same operation for that.(The objdump shows the same assembly code.)
    But I think that using bit arithmetic is good habit.

    Thanks,
    Milo -

    -----Original Message-----
    From: Lars-Peter Clausen [mailto:lars@metafoo.de]
    Sent: Friday, January 20, 2012 5:32 PM
    To: Kim, Milo
    Cc: Linus Walleij; shreshthakumar.sahu@stericsson.com; linux-kernel@vger.kernel.org; rpurdie@rpsys.net
    Subject: Re: [PATCH 7/7] leds-lm3530: enhanced arithmetic operation

    On 01/20/2012 02:52 AM, Kim, Milo wrote:
    > Use shift operation rather than 'divide-by-2'.

    The compiler will already take of this for you.

    But why is the divide by two necessary anyway? This sort of looks like
    max_brightness is not set properly and the code compensates for it by
    ignoring the lsb.

    >
    > Signed-off-by: Milo(Woogyom) Kim <milo.kim@ti.com>
    >
    > diff --git a/drivers/leds/leds-lm3530.c b/drivers/leds/leds-lm3530.c
    > index 51c1f6c..e0b1ba8 100644
    > --- a/drivers/leds/leds-lm3530.c
    > +++ b/drivers/leds/leds-lm3530.c
    > @@ -249,12 +249,12 @@ static void lm3530_brightness_set(struct led_classdev *led_cdev,
    >
    > /* set the brightness in brightness control register*/
    > err = i2c_smbus_write_byte_data(drvdata->client,
    > - LM3530_BRT_CTRL_REG, brt_val / 2);
    > + LM3530_BRT_CTRL_REG, brt_val >> 1);
    > if (err)
    > dev_err(&drvdata->client->dev,
    > "Unable to set brightness: %d\n", err);
    > else
    > - drvdata->brightness = brt_val / 2;
    > + drvdata->brightness = brt_val >> 1;
    >
    > if (brt_val == 0 && !pdata->is_vin_always_on) {
    > err = regulator_disable(drvdata->regulator);




    \
     
     \ /
      Last update: 2012-01-20 12:23    [W:0.028 / U:29.928 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site