Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] leds: tlc591xx: update the maximum brightness
To:     Jacek Anaszewski <jacek.anaszewski@gmail.com>,
        Pavel Machek <pavel@ucw.cz>
CC:     <dmurphy@ti.com>, <linux-leds@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, Andrew Lunn <andrew@lunn.ch>
References: <20190923100250.22326-1-jjhiblot@ti.com>
 <91864098-a6e8-e275-4b07-e4bb15469f78@gmail.com> <20191013114508.GI5653@amd>
 <844845d6-01fe-50c4-94cd-e19ce8a5d060@gmail.com>
From:   Jean-Jacques Hiblot <jjhiblot@ti.com>
Message-ID: <e551e3ea-5b75-9b6e-d898-b4516a090c54@ti.com>
Date:   Mon, 14 Oct 2019 12:37:53 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.9.0
MIME-Version: 1.0
In-Reply-To: <844845d6-01fe-50c4-94cd-e19ce8a5d060@gmail.com>
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 13/10/2019 18:36, Jacek Anaszewski wrote:
> On 10/13/19 1:45 PM, Pavel Machek wrote:
>> Hi!
>>
>>>> @@ -112,11 +113,11 @@ tlc591xx_brightness_set(struct led_classdev *led_cdev,
>>>>   	struct tlc591xx_priv *priv = led->priv;
>>>>   	int err;
>>>>   
>>>> -	switch (brightness) {
>>>> +	switch ((int)brightness) {
>>>>   	case 0:
>> Can we get a rid of the cast here? Do we need to move away from the
>> enum for the brightness?
> I at first also wanted to ask for dropping the cast but first tried
> to do it myself. Then I found out compiler (or sparse, I don't recall
> exactly) complains about TLC591XX_MAX_BRIGHTNESS not being a value of
> enum led_brighteess type. That's the reason for the cast Jean added,
> I presume.

Indeed that cast is to fix the warning.

JJ

>>> Added tag:
>>>
>>> Fixes: e370d010a5fe ("leds: tlc591xx: Driver for the TI 8/16 Channel i2c
>>> LED driver")
>>>
>>> and applied to the for-5.5 branch.
>> Actually, careful with the Fixes tag. -stable people will want to
>> apply it, and it may not be a good idea in this case. Maximum
>> brightness of 256 is pretty unusual, so I'd call this "a bit risky".
> I entirely disagree. Not seeing anything risky in that since
> max_brightness is also initialized to this value. If userspace properly
> uses the ABI, then it will be safe.
>
>> Best regards,
>> 									Pavel
>>