From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932661AbcC3H5v (ORCPT ); Wed, 30 Mar 2016 03:57:51 -0400 Received: from mailout4.w1.samsung.com ([210.118.77.14]:31457 "EHLO mailout4.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758748AbcC3H5n (ORCPT ); Wed, 30 Mar 2016 03:57:43 -0400 X-AuditID: cbfec7f5-f792a6d000001302-25-56fb86f3e664 Message-id: <56FB86F2.4060000@samsung.com> Date: Wed, 30 Mar 2016 09:57:38 +0200 From: Jacek Anaszewski User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130804 Thunderbird/17.0.8 MIME-version: 1.0 To: Heiner Kallweit Cc: Pavel Machek , Greg KH , linux-leds@vger.kernel.org, Benjamin Tissoires , linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org Subject: Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color LED's References: <56D608ED.2090406@gmail.com> <20160329100258.GA24964@amd> <56FAE7A8.2070302@gmail.com> In-reply-to: <56FAE7A8.2070302@gmail.com> Content-type: text/plain; charset=windows-1252; format=flowed Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrNLMWRmVeSWpSXmKPExsVy+t/xa7qf236HGXy+rGVxfcpmVotzC2Yw Wix6P4PV4vKuOWwWW9+sA3KXtTJb3D11lM2B3WPnrLvsHm8fBni833eVzWPF6u/sHp83yQWw RnHZpKTmZJalFunbJXBl7Jp1nL3gnFTFwubFrA2My0W7GDk5JARMJA6+7GSCsMUkLtxbz9bF yMUhJLCUUeLQrZXsEM4zRokn2w6zgVTxCmhJ9D14AtbBIqAq8allCZjNJmAo8fPFazBbVCBC 4s/pfawQ9YISPybfYwGxRYB6J7xeA7aBWeAEo0TbtwNgQ4UF/CXOf2xk7mLkANqWIrGkpwIk zCmgKfGleycjiM0sYCux4P06FghbXmLzmrfMExgFZiFZMQtJ2SwkZQsYmVcxiqaWJhcUJ6Xn GukVJ+YWl+al6yXn525ihAT51x2MS49ZHWIU4GBU4uHdIP07TIg1say4MvcQowQHs5II7+dW oBBvSmJlVWpRfnxRaU5q8SFGaQ4WJXHembvehwgJpCeWpGanphakFsFkmTg4pRoYVzxMCZDJ +Ob75uvFV3uufVjN4/dt7/G9Hmzf2dmLWfctnBCaYP41WLhNuf9iTMXGjOTgdaG68/x/veA4 ud57S/bT6CfndCUSCsx8ny/9vOS5qkROiRDHZMnXm+XsV8h/C45xaE6xuim/eJ9/iki/7YPA gBt91hPCShaz8pom9eerWm97sqhKiaU4I9FQi7moOBEAwF9kXW4CAAA= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Heiner and Pavel, On 03/29/2016 10:38 PM, Heiner Kallweit wrote: > Am 29.03.2016 um 12:02 schrieb Pavel Machek: >> Hi! >> >> First, please Cc me on RGB color support. >> >>> Add generic support for RGB Color 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//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) >> >> Umm, that's rather strange interface -- and three values in single sysfs >> file is actually forbidden. >> >> Plus, it is very much unlike existing interfaces for RGB LEDs, which >> we already have supported in the tree. (At least nokia N900 and Sony >> motion controller already contain supported three-color LEDs). >> >> Now... yes, there's work to be done for the 3-color LEDs. Currently, >> they are treated as three different LEDs. (Which makes some sense, you >> can use "battery charging" trigger for LED, and CPU activity trigger >> for green, for example). It would be good to have some kind of >> grouping, so that userspace can tell "these 3 leds are actually >> combined into one light". >> > At first thanks for the review comments. > Treating the three physical LEDs of a RGB LED as separate LED devices > might have been implemented due to the lack of alternatives. > With one trigger controlling the red LED and another controlling the green > LED we may end up with a yellow light. Not sure whether this is what we want. > > One driver for this extension was the idea of triggers using color > to visualize states etc. > Therefore it's not only about userspace controlling the color. > As a trigger is bound to a led_classdev we need a led_classdev > representing a RGB LED device. > > And ok: If required the sysfs interface can be splitted into separate > attributes for hue, saturation, and (existing) brightness. It would have the same downsides as in case of having r, g and b in separate attributes, i.e. - problems with setting LED colour in a consistent way. This way LED blinking in whatever colour couldn't be supported reliably. It was one of your primary rationale standing behind this design, if I remember correctly. Second - what about triggers? We've had a long discussion about it and this design turned out to be most fitting. It's hard to address these requirements by having the settings in separate attributes, due to synchronization issues, and LED trigger mechanism specificity. There is a question whether we can bend the sysfs "one value per sysfs file" rule down to RGB LEDs needs. Of course other brilliant ideas on how to approach the problem are more than expected. -- Best regards, Jacek Anaszewski