rev2 - Driver for BlinkM i2c LED module

[Jan-Simon Möller]

New version attched.

Am Sonntag, Juni 03, 2012, 07:32:08 PM schrieb Jonathan Neuschäfer:
> 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.

Yep, tnx.

> > 	/* 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)

Thanks for spotting this. Fixed.

[...]

> > 	/* 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?
That is in by intention.


Best,
Jan-Simon
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-driver-for-BlinkM-device-to-drivers-leds.patch
Type: text/x-patch
Size: 22767 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20120603/1cc20080/attachment-0001.bin