On 9 January 2013 10:38, Laxman Dewangan [off-list ref] wrote:

On Wednesday 09 January 2013 09:45 AM, Sachin Kamat wrote:

quoted

devm_* APIs are device managed and make the exit and clean up code
simpler.

Signed-off-by: Sachin Kamat <redacted>
Cc: Joonyoung Shim <redacted>
---

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.

quoted

+       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?

quoted

+

quoted

+       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.

quoted

        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)

quoted

-
+               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.

quoted

  }

@@ -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