Re: [PATCH v2 2/4] leds: core: add color LED sysfs extension

[Jacek Anaszewski <j.anaszewski@samsung.com>]

On 02/17/2016 01:33 PM, Jacek Anaszewski wrote:
> Hi Heiner,
>
> On 02/16/2016 08:28 PM, Heiner Kallweit wrote:
>> Extend brightness sysfs property handling to deal with monochrome
>> and color mode as well.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>> v2:
>> - split from patch 1
>> ---
>>   drivers/leds/led-class.c | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
>> index aa84e5b..09e0007 100644
>> --- a/drivers/leds/led-class.c
>> +++ b/drivers/leds/led-class.c
>> @@ -32,7 +32,10 @@ static ssize_t brightness_show(struct device *dev,
>>       /* no lock needed for this */
>>       led_update_brightness(led_cdev);
>>
>> -    return sprintf(buf, "%u\n", led_cdev->brightness);
>> +    if (led_cdev->brightness > LED_FULL)
>> +        return sprintf(buf, "%#06x\n", led_cdev->brightness);
>> +    else
>> +        return sprintf(buf, "%u\n", led_cdev->brightness);
>>   }

This change also should remain here.

>>   static ssize_t brightness_store(struct device *dev,
>> @@ -49,11 +52,11 @@ static ssize_t brightness_store(struct device *dev,
>>           goto unlock;
>>       }
>>
>> -    ret = kstrtoul(buf, 10, &state);
>> +    ret = kstrtoul(buf, 0, &state);
>>       if (ret)
>>           goto unlock;
>
> Please leave only this change in the patch, as it extends the scope
> of possible arguments for user space. Remaining changes can be moved
> to 1/4.
>
>
>>
>> -    if (state == LED_OFF)
>> +    if (is_off(state))

Only this should go to 1/4.

>>           led_trigger_remove(led_cdev);
>>       led_set_brightness(led_cdev, state);
>>
>>
>
>


-- 
Best regards,
Jacek Anaszewski