Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1752339AbcCGInJ (ORCPT <rfc822;w@1wt.eu>);
	Mon, 7 Mar 2016 03:43:09 -0500
Received: from mailout1.w1.samsung.com ([210.118.77.11]:56591 "EHLO
	mailout1.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752242AbcCGInF (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Mon, 7 Mar 2016 03:43:05 -0500
X-AuditID: cbfec7f4-f79026d00000418a-30-56dd3f164b25
Message-id: <56DD3F15.20907@samsung.com>
Date: Mon, 07 Mar 2016 09:43:01 +0100
From: Jacek Anaszewski <j.anaszewski@samsung.com>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130804
 Thunderbird/17.0.8
MIME-version: 1.0
To: Karl Palsson <karlp@tweak.net.au>
Cc: Heiner Kallweit <hkallweit1@gmail.com>,
        Benjamin Tissoires <benjamin.tissoires@redhat.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        "linux-leds@vger.kernel.org" <linux-leds@vger.kernel.org>,
        Linux USB Mailing List <linux-usb@vger.kernel.org>
Subject: Re: [PATCH v7 1/5] leds: core: add generic support for RGB LED's
References: <56D9F973.8090207@gmail.com>
 <20160306205158-549-87411-mailpile@palmtree-beeroclock-net>
In-reply-to: <20160306205158-549-87411-mailpile@palmtree-beeroclock-net>
Content-type: text/plain; charset=ISO-8859-1; format=flowed
Content-transfer-encoding: 7bit
X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrILMWRmVeSWpSXmKPExsVy+t/xq7pi9nfDDPZc5be4PmUzq8Wi9zNY
	LS6sWcdkcXnXHDaLrW/WMVosWtbK7MDmsXPWXXaP9/uusnks21Di8XmTXABLFJdNSmpOZllq
	kb5dAldGz6RDbAUHhSv6Fu5jb2A8z9/FyMkhIWAiMaV5HwuELSZx4d56ti5GLg4hgaWMEmfv
	/WGBcJ4xSqz/u5sdpIpXQENi26fnzCA2i4CqxKw3b9hAbDYBQ4mfL14zgdiiAhESf07vY4Wo
	F5T4Mfke2AYRoPrpi/8zggxlFuhnktjf3ALkcHAIC3hKvF8bAlIjJJAm8X3CabA5nAIeEtfW
	vwKbzyxgLbFy0jZGCFteYvOat8wTGAVmIVkxC0nZLCRlCxiZVzGKppYmFxQnpeca6hUn5haX
	5qXrJefnbmKEBPSXHYyLj1kdYhTgYFTi4b3AdTdMiDWxrLgy9xCjBAezkgivsx1QiDclsbIq
	tSg/vqg0J7X4EKM0B4uSOO/cXe9DhATSE0tSs1NTC1KLYLJMHJxSDYySe6o/sByMtj0bZJYo
	G72xhXWOrWheBV/kIruvzlseHRKvzOD5xtqnuChyAc8Ry9ccXDobXu/6+VVz/5IHN1Vsuv68
	PLSZ+ecSG7aFuusyg+238nJY9ta8PX67YrrTl6L/vBHdTHMy7itNer0sR8/l269vD4Qtv81U
	dpna6P1U5NDZiemFsyOVWIozEg21mIuKEwFbQgAtZAIAAA==
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 2428
Lines: 69

Hi Karl,

On 03/06/2016 09:55 PM, Karl Palsson wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Heiner Kallweit <hkallweit1@gmail.com> wrote:
>> Add generic support for RGB LED's.
>>
>> Basic idea is to use enum led_brightness also for the hue and
>> saturation color components.This allows to implement the color
>> extension w/o changes to struct led_classdev.
>>
>> Select LEDS_RGB to enable building drivers using the RGB
>> extension.
>>
>> Flag LED_SET_HUE_SAT allows to specify that hue / saturation
>> should be overridden even if the provided values are zero.
>>
>> Some examples for writing values to
>> /sys/class/leds/<xx>/brightness: (now also hex notation can be
>> used)
>>
>> 255 -> set full brightness and keep existing color if set 0 ->
>> switch LED off but keep existing color so that it can be
>> restored
>>       if the LED is switched on again later
>> 0x1000000 -> switch LED off and set also hue and saturation to
>> 0 0x00ffff -> set full brightness, full saturation and set hue
>> to 0 (red)
>>
>
>
> I admit I hadn't seen this earlier, and I didn't spend all day
> looking at previous attempts, but what's the motivation for
> putting all this overloaded syntax into the "brightness" file? I
> thought the point was to have a single value in each file, one of
> the references I did find was

With single value per file there would be problems with colour
components setting synchronization.

> http://www.spinics.net/lists/linux-leds/msg02995.html Is there
> some thread where this was decided as advantageous? Surely 0-255
> for _brightness_ is what the brightness entry should do?

You can find a reference to the related discussion in [1].

> I'd like to set the rgb colour of a led (or hsv, that can be it's
> own file too) and separately ramp the brightness up and down. I
> also think it's substantially simpler and easier to use from the
> user's point of view, which is surely the place you want easy
> right?

I'm also not very keen on overloading the brightness attribute
semantics. Nonetheless it seems impossible to add support for
setting three colour components otherwise than through a single
syscall, if we want to make it synchronous and compatible with
LED triggers.

HSV color scheme is also very convenient for adapting monochrome
brightness semantics to the RGB realm.


[1] http://www.spinics.net/lists/linux-leds/msg05477.html

-- 
Best regards,
Jacek Anaszewski