On Sat, Jun 02, 2012 at 11:29:46AM +0200, Jan-Simon Möller wrote:
> static const struct {
> char cmdchar;
> u8 cmdbyte;
> u8 nr_args;
> u8 nr_ret;
> u8 dir:2;
> } blinkm_cmds[17] = {
> /* cmdnr, cmdchar, cmdbyte, nr_args, nr_ret, dir */
cmdnr isn't there anymore.
> /* We start hardware transfers which are not to be
> * mixed with other commands. Aquire a lock now. */
> if (mutex_lock_interruptible(&data->update_lock) < 0)
> return -EAGAIN;
>
> /* switch cmd - usually write before reads */
> switch (cmd) {
[...]
> default:
> printk(KERN_INFO "BlinkM: unknown command %d\n", cmd);
> return -1;
You need to unlock the mutex in the error case, too.
Also the -1 looks suspicious to me. When interpreted as -errno it would
be -EPERM on most (or even all) architectures. (according to
include/asm-generic/errno-base.h)
> static int blinkm_led_common_set(struct led_classdev *led_cdev,
> enum led_brightness value, int color)
> {
> /* led_brightness is 0, 127 or 255 - we just use it here as-is */
> struct blinkm_led *led = cdev_to_blmled(led_cdev);
> struct blinkm_data *data = i2c_get_clientdata(led->i2c_client);
> struct blinkm_work *bl_work = kzalloc(sizeof(struct blinkm_work),
> GFP_ATOMIC);
>
> switch (color) {
> case RED:
> data->red = (u8)value;
> break;
> case GREEN:
> data->green = (u8)value;
> break;
> case BLUE:
> data->blue = (u8)value;
> break;
> default:
> printk(KERN_INFO "BlinkM: unknown color.\n");
> return -1;
> }
> /* data->red=(u8)value; we know it fits ... 0..255 */
This comment looks a little misplaced now.
> /* red fade to off */
> data->red = 0xff;
> ret = blinkm_transfer_hw(client, BLM_GO_RGB);
> if (ret < 0)
> printk(KERN_INFO
> "Failure in blinkm_remove ignored. Continuing.\n");
>
> /* off */
> data->red = 0x00;
> data->green = 0x00;
> data->blue = 0x00;
> ret = blinkm_transfer_hw(client, BLM_FADE_RGB);
> if (ret < 0)
> printk(KERN_INFO
> "Failure in blinkm_remove ignored. Continuing.\n");
Did you leave this fading in for testing?
Thanks,
Jonathan Neuschäfer
_______________________________________________
Kernelnewbies mailing list
[email protected]
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
