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

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

 



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)


Best regards,
Jingoo Han


--
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