lkml.org 
[lkml]   [2010]   [Nov]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] input: joystick: Adding Austria Microsystem AS5011 joystick driver
Hi Anirudh,

Thanks for your code review.

>> +static void as5011_update_axes(struct work_struct *work)
>> +{
>> + struct as5011_platform_data *plat_dat =
>> + container_of(work,
>> + struct as5011_platform_data,
>> + update_axes_work);
>> + signed char x, y;
>> +
>> + x = as5011_i2c_read(plat_dat->i2c_client, AS5011_X_RES_INT);
>> + y = as5011_i2c_read(plat_dat->i2c_client, AS5011_Y_RES_INT);
>
> What if the read call returns an error ?
It's a problem yes. But I don't know how to manage this error in this context.

>> +static int as5011_probe(struct i2c_client *client,
>> + const struct i2c_device_id *id)
>> +{
>
> __devinit as5011_probe()

Ok, done.

>> + if (request_irq(plat_dat->int_irq, as5011_int_interrupt,
>> + 0, plat_dat->int_irq_name, (void *)plat_dat)) {
>
> How about using request_any_context_irq() ?

Hmm, in fact my platform use a hard IRQ and I used it without question.
I will see how to improve it.
>
>> + dev_err(&client->dev, "as5011: Can't allocate int irq %d\n",
>> + plat_dat->int_irq);
>> + retval = -EBUSY;
>> + goto request_int_irq_error;
>> + }
>> +
>> + retval = input_register_device(plat_dat->input_dev);
>
> Its better to register your device at the end, after all the initialization.

Yes it was a mistake. Done.
>
>> + if (retval) {
>> + dev_err(&client->dev, "as5011: Failed to register device\n");
>> + retval = -EINVAL;
>> + goto input_register_device_error;
>> + }
>> +
>> + retval = plat_dat->init_gpio();
>
> Please check if the function pointer is NULL.

Ok, done.

>> + plat_dat->exit_gpio();
>
> Please check if the function pointer is NULL.
done.

>> + plat_dat->exit_gpio();
>
> Same here.
done.


>> + .remove = as5011_remove,
>
> __devexit_p (as5011_remove)
done.

Thanks.
Fabien



\
 
 \ /
  Last update: 2010-11-25 11:35    [from the cache]
©2003-2011 Jasper Spaans