Messages in this thread | | | Date | Wed, 5 Oct 2011 22:21:43 +0200 | From | Antonio Ospite <> | Subject | Re: [sparse stuff] bug in lp3944_led_set_brightness() |
| |
On Wed, 5 Oct 2011 12:32:48 +0300 Dan Carpenter <dan.carpenter@oracle.com> wrote:
> Hi Antonio, > > I was going through some Sparse warnings and this one seems pretty > valid. > > drivers/leds/leds-lp3944.c +292 23: > warning: mixing different enum types > > 284 static void lp3944_led_set_brightness(struct led_classdev *led_cdev, > 285 enum led_brightness brightness) > 286 { > 287 struct lp3944_led_data *led = ldev_to_led(led_cdev); > 288 > 289 dev_dbg(&led->client->dev, "%s: %s, %d\n", > 290 __func__, led_cdev->name, brightness); > 291 > 292 led->status = brightness; > ^^^^^^^^^^^^^^^^^^^^^^^^ > 293 schedule_work(&led->work); > 294 } > > > led->status should be values between 0-3. brightness is 0, 127 and > 255. >
Hi Dan, thanks for reporting that.
AFAICS this is not a problem _in_practice_ as lp3944_led_set_brightness () is called by the leds framework with brightness values already limited by max_brightness, so it is always 0 or 1 in our case. The sparse warning is not pretty tho, so I guess we can safely get rid of it anyways with:
led->status = !!brightness;
keeping track of LP3944_LED_STATUS_OFF and LP3944_LED_STATUS_ON only in lp3944_led_set_brightness() is OK, as the handling of DIM (blinking) mode[s] is in lp3944_led_set_blink().
I am following up with a patch in that direction tomorrow, after a test on the actual hardware.
This is the rationale behind the current code if it is of any interest: in the development phase of the driver I was abusing brightness = {2,3} to trigger the hardware blinking function, but then I set the max_brightness to 1 and so this abusing mechanism became inactive and I must have forgotten about it.
Thanks again.
Regards, Antonio
-- Antonio Ospite http://ao2.it
PGP public key ID: 0x4553B001
A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? [unhandled content-type:application/pgp-signature] | |