On Sun, Mar 18, 2012 at 8:13 PM, walter harms <wharms@xxxxxx> wrote: > > > Am 17.03.2012 18:36, schrieb santosh nayak: >> From: Santosh Nayak <santoshprasadnayak@xxxxxxxxx> >> >> Use uninterruptable sleep lock 'mutex_lock()' in place of >> mutex_lock_interruptible() because there is no userspace >> for s2250_probe(). >> >> Return -ENOMEM if kzalloc() fails to allocate and initialize. >> >> Signed-off-by: Santosh Nayak <santoshprasadnayak@xxxxxxxxx> >> --- >> drivers/staging/media/go7007/s2250-board.c | 43 +++++++++++++++------------ >> 1 files changed, 24 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/staging/media/go7007/s2250-board.c b/drivers/staging/media/go7007/s2250-board.c >> index 014d384..1406a37 100644 >> --- a/drivers/staging/media/go7007/s2250-board.c >> +++ b/drivers/staging/media/go7007/s2250-board.c >> @@ -637,27 +637,32 @@ static int s2250_probe(struct i2c_client *client, >> state->audio_input = 0; >> write_reg(client, 0x08, 0x02); /* Line In */ >> >> - if (mutex_lock_interruptible(&usb->i2c_lock) == 0) { >> - data = kzalloc(16, GFP_KERNEL); >> - if (data != NULL) { >> - int rc; >> - rc = go7007_usb_vendor_request(go, 0x41, 0, 0, >> - data, 16, 1); >> - if (rc > 0) { >> - u8 mask; >> - data[0] = 0; >> - mask = 1<<5; >> - data[0] &= ~mask; >> - data[1] |= mask; >> - go7007_usb_vendor_request(go, 0x40, 0, >> - (data[1]<<8) >> - + data[1], >> - data, 16, 0); >> - } >> - kfree(data); >> - } >> + mutex_lock(&usb->i2c_lock); >> + data = kzalloc(16, GFP_KERNEL); >> + if (data == NULL) { >> + i2c_unregister_device(audio); >> + kfree(state); >> mutex_unlock(&usb->i2c_lock); >> + return -ENOMEM; >> + } else { >> + int rc; >> + rc = go7007_usb_vendor_request(go, 0x41, 0, 0, >> + data, 16, 1); >> + if (rc > 0) { >> + u8 mask; >> + data[0] = 0; >> + mask = 1<<5; >> + data[0] &= ~mask; >> + data[1] |= mask; >> + go7007_usb_vendor_request(go, 0x40, 0, >> + (data[1]<<8) >> + + data[1], >> + data, 16, 0); >> + } >> + kfree(data); >> } >> + mutex_unlock(&usb->i2c_lock); >> + >> >> v4l2_info(sd, "initialized successfully\n"); >> return 0; > > hi, > You can drop the else > 1. there is no 3. way > 2. you can save 1 indent level ok. I will do it. > > just one question: the (data[1]<<8)+ data[1] is intended ? always data[1] ? > (i have no clue, it is the original code, it just feels .. strange ) I just added mutex_lock and error handling for kzalloc . Rest of the things were there before. No change done by me. regards Santosh > re, > wh -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html