On 9 January 2013 10:38, Laxman Dewangan <[email protected]> wrote:
> On Wednesday 09 January 2013 09:45 AM, Sachin Kamat wrote:
>>
>> devm_* APIs are device managed and make the exit and clean up code
>> simpler.
>>
>> Signed-off-by: Sachin Kamat <[email protected]>
>> Cc: Joonyoung Shim <[email protected]>
>> ---
>
>
> I think this is not resend patch, it is Patch V2. Also you can specify the
> changes done from last patch to this patch so that it will be easy to
> reviewer.

OK.

>
>
>> +       data = devm_kzalloc(&client->dev, sizeof(struct mms114_data),
>
> Because you are here, better to change the size to sizeof(*data) in place of
> sizeof(struct..)

In what way is this change useful or better?

>
>> +
>
>
>> +       error = devm_request_threaded_irq(&client->dev, client->irq, NULL,
>> +                       mms114_interrupt, IRQF_TRIGGER_FALLING |
>> IRQF_ONESHOT,
>> +                       "mms114", data);
>
>
> Should be change to devname(&client->dev) in place of hardconding to
> "mms114".

OK.

>
>
>>         if (error) {
>>                 dev_err(&client->dev, "Failed to register interrupt\n");
>> -               goto err_io_reg;
>> +               return error;
>>         }
>>         disable_irq(client->irq);
>>         error = input_register_device(data->input_dev);
>>         if (error)
>
>
>> -
>> +               return error;
>>         return 0;
>
>
>
> Seems the code turn as
> if (error)
>    return error;
> return 0.
>
> I think simply saying return error is fine here as you are not printing any
> error.

Right.

>
>>   }
>> @@ -590,7 +567,6 @@ static struct i2c_driver mms114_driver = {
>>                 .of_match_table = of_match_ptr(mms114_dt_match),
>>         },
>>         .probe          = mms114_probe,
>> -       .remove         = mms114_remove,
>>         .id_table       = mms114_id,
>>   };
>>


I will re-send after making necessary changes.

-- 
With warm regards,
Sachin
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to