Re: [PATCH] leds: lp3944: Fix sparse warning

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Aug 1, 2013 at 6:50 PM, Jingoo Han <jg1.han@xxxxxxxxxxx> wrote:
> On Thursday, August 01, 2013 5:53 PM, Antonio Ospite wrote:
>> On Thu, 01 Aug 2013 15:01:37 +0900, Jingoo Han <jg1.han@xxxxxxxxxxx> wrote:
>> > Casts (enum lp3944_status) to brightness in order to the following
>> > sparse warning:
>> >
>> > drivers/leds/leds-lp3944.c:292:23: warning: mixing different enum types
>> > drivers/leds/leds-lp3944.c:292:23:     int enum led_brightness  versus
>> > drivers/leds/leds-lp3944.c:292:23:     int enum lp3944_status
>> >
>> > Signed-off-by: Jingoo Han <jg1.han@xxxxxxxxxxx>
>> > ---
>> >  drivers/leds/leds-lp3944.c |    2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/leds/leds-lp3944.c b/drivers/leds/leds-lp3944.c
>> > index 2b53d10..60e7e92 100644
>> > --- a/drivers/leds/leds-lp3944.c
>> > +++ b/drivers/leds/leds-lp3944.c
>> > @@ -289,7 +289,7 @@ static void lp3944_led_set_brightness(struct led_classdev *led_cdev,
>> >     dev_dbg(&led->client->dev, "%s: %s, %d\n",
>> >             __func__, led_cdev->name, brightness);
>> >
>> > -   led->status = brightness;
>> > +   led->status = (enum lp3944_status)brightness;
>>
>> That's happen to be OK because the driver uses:
>>       led->ldev.max_brightness = 1
>>
>> and the led core driver limits the brightness value passed to
>> set_brightness()
>
> OK, I see.
>
>>
>> However it might arise doubts in the reader about the difference of the
>> two enums (do their values match?)
>>
>> Given that the led is only a binary led (max_brightness = 1), what
>> about using:
>>       led->status = !!brightness;
>
> It looks good.
>
>>
>> and/or maybe a brief comment about the max_brightness can fade away any
>> doubts left.
>>
>> There was a thread about that:
>> http://lkml.indiana.edu/hypermail/linux/kernel/1110.0/01558.html
>>
>> I even sent a patch for it, but it has never been merged, maybe I had
>> forgotten to Cc someone:
>> http://lkml.indiana.edu/hypermail/linux/kernel/1110.0/02283.html
>
> Antonio,
> I really appreciate your feedback. :)
> Bryan Wu started to maintain LED subsystem on May, 2012.
> He will look at your patch.
>
>
> Bryan Wu,
> Could you consider the patch that Antonio Ospite submitted 2 years ago?
> It looks good to me.
> (http://lkml.indiana.edu/hypermail/linux/kernel/1110.0/02283.html)
>

Sure, this is better solution. I merged it manually from the lkml
tread into my tree.

Thanks Antonio and Jingoo.
-Bryan
--
To unsubscribe from this list: send the line "unsubscribe linux-leds" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux