[U-Boot] [PATCH] TWL4030: make LEDs selectable in twl4030_led_init()

[Grazvydas Ignotas]

On Wed, Dec 2, 2009 at 4:55 PM, Tom <Tom.Rix at windriver.com> wrote:
> Grazvydas Ignotas wrote:
>>

<snip>

>> diff --git a/drivers/misc/twl4030_led.c b/drivers/misc/twl4030_led.c
>> index bfdafef..d2cc3c8 100644
>> --- a/drivers/misc/twl4030_led.c
>> +++ b/drivers/misc/twl4030_led.c
>> @@ -39,12 +39,15 @@
>>  #define LEDAPWM                        (0x1 << 4)
>>  #define LEDBPWM                        (0x1 << 5)
>>  -void twl4030_led_init(void)
>> +void twl4030_led_init(unsigned int ledon_mask)
>>  {
>> -       unsigned char byte;
>> +       unsigned char byte = 0;
>>  -       /* enable LED */
>> -       byte = LEDBPWM | LEDAPWM | LEDBON | LEDAON;
>> +       /* enable LEDs */
>> +       if (ledon_mask & TWL4030_LEDON_LEDA)
>> +               byte |= LEDAPWM | LEDAON;
>
> TWL4030_LEDON_LEDA and LEDAON are the same.
> You can move all the #defined twl4030 bits to twl4030.
> So there there should be
>
> #define TWL4030_LED_LEDEN_LEDAON                        (0x1 << 0)
> #define TWL4030_LED_LEDEN_LEDBON                        (0x1 << 1)
> #define TWL4030_LED_LEDEN_LEDAPWM                       (0x1 << 4)
> #define TWL4030_LED_LEDEN_LEDBPWM                       (0x1 << 5)
>
> These should be listed under the LED define
>
> /* LED */
> #define TWL4030_LED_LEDEN                               0xEE

So, do you want twl4030_led_init() argument to take raw LEDEN register
value, or just move TWL4030_LED* defines to twl4030.h? If
twl4030_led_init() will just take register value now, maybe it can be
replaced with twl4030_i2c_write_u8() and drivers/misc/twl4030_led.c
removed, as it just becomes wrapper?

>
>
>> +       if (ledon_mask & TWL4030_LEDON_LEDB)
>> +               byte |= LEDBPWM | LEDBON;
>>          twl4030_i2c_write_u8(TWL4030_CHIP_LED, byte,
>>                             TWL4030_LED_LEDEN);
>> diff --git a/include/twl4030.h b/include/twl4030.h
>> index f260ecb..d0c62c3 100644
>> --- a/include/twl4030.h
>> +++ b/include/twl4030.h
>> @@ -396,6 +396,9 @@ void twl4030_power_mmc_init(void);
>>  /*
>>  * LED
>>  */
>> -void twl4030_led_init(void);
>> +#define TWL4030_LEDON_LEDA     (1 << 0)
>> +#define TWL4030_LEDON_LEDB     (1 << 1)
>> +
>> +void twl4030_led_init(unsigned int ledon_mask);
>>   #endif /* TWL4030_H */
>
>