Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <a8a22af2-3d11-bda8-5453-768e2174dc08@traphandler.com>
Date:   Thu, 6 Apr 2023 08:54:38 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.0
Subject: Re: [PATCH v8 6/6] leds: Add a multicolor LED driver to group
 monochromatic LEDs
Content-Language: en-US
To:     Andy Shevchenko <andy.shevchenko@gmail.com>
CC:     <lee.jones@linaro.org>, <pavel@ucw.cz>, <robh+dt@kernel.org>,
        <sven.schwermer@disruptive-technologies.com>,
        <krzysztof.kozlowski+dt@linaro.org>, <johan+linaro@kernel.org>,
        <marijn.suijten@somainline.org>, <jacek.anaszewski@gmail.com>,
        <linux-leds@vger.kernel.org>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
References: <20230328161541.599463-1-jjhiblot@traphandler.com>
 <20230328161541.599463-7-jjhiblot@traphandler.com>
 <CAHp75Vca0SKPkyPM_14Zny+Vn0a=hKWRfYW9qzTpN8AVZ_R_YQ@mail.gmail.com>
From:   Jean-Jacques Hiblot <jjhiblot@traphandler.com>
In-Reply-To: <CAHp75Vca0SKPkyPM_14Zny+Vn0a=hKWRfYW9qzTpN8AVZ_R_YQ@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 28/03/2023 19:20, Andy Shevchenko wrote:
> On Tue, Mar 28, 2023 at 7:15 PM Jean-Jacques Hiblot
> <jjhiblot@traphandler.com> wrote:
>>
>> Grouping multiple monochrome LEDs into a multicolor LED device has a few
>> benefits over handling the group in user-space:
>> - The state of the LEDs relative to each other is consistent. In other
>>    words, if 2 threads competes to set the LED to green and red, the
>>    end-result cannot be black or yellow.
>> - The multicolor LED as a whole can be driven through the sysfs LED
>>    interface.
> 
> ...
> 
>> +config LEDS_GROUP_MULTICOLOR
>> +       tristate "LEDs group multi-color support"
>> +       depends on OF || COMPILE_TEST
> 
> Why is OF a dependency?
> Can't we make it a firmware provider agnostic solution from day 1?
That would be possible, unfortunately I have no way of testing the ACPI 
stuff. That's why I prefer sticking to OF.

> 
>> +       help
>> +         This option enables support for monochrome LEDs that are
>> +         grouped into multicolor LEDs.
>> +         This useful in the case where LEDs of different colors are
> 
> This is
> 
>> +         physically grouped in a single multi-color LED and driven
>> +         by a controller that doesn't have multi-color support.
>> +
>> +         To compile this driver as a module, choose M here: the module
>> +         will be called leds-group-multicolor.
> 
> ...
> 
>> +               led_cdev = devm_of_led_get_optional(dev, count);
> 
> I don't see how this is OF specific to this driver. Maybe it needs to
> be patched first, so we will have something non-OF specific?
> 
>> +               if (IS_ERR(led_cdev))
>> +                       return dev_err_probe(dev, PTR_ERR(led_cdev), "Unable to get LED #%d",
>> +                                            count);
> 
> At least we need to have a FIXME or so here.
>