[i2c] [RFC] Lifebook apanel driver

Dmitry Torokhov dtor at insightbb.com
Thu Jan 4 07:22:35 CET 2007


Hi Stephen,

Overall looks good, couple of issues:

On Wednesday 03 January 2007 18:23, Stephen Hemminger wrote:
> +       err = input_register_device(input);
> +       if (err)
> +               goto out3;
> +
> +       schedule_delayed_work(&ap->poll_timer, POLL_FREQUENCY);

Please consider implementing open() and close() methods for the
input device and start/cancel the polling work from there. There
is no need to poll hardware if nobody is listening for the events.

> +       return 0;
> +out3:
> +       i2c_detach_client(&ap->client);
> +out2:
> +       kfree(input);

Please use input_free_device() here.

> +out1:
> +       kfree(ap);
> +       return err;
> +}
> +
> +static int apanel_detach_client(struct i2c_client *client)
> +{
> +       struct apanel *ap = i2c_get_clientdata(client);
> +
> +       cancel_rearming_delayed_work(&ap->poll_timer);
> +       input_unregister_device(ap->input);
> +       input_free_device(ap->input);

input_free_device() should not be used once device has been successfully
registered - input core will free the memory once the last reference is
dropped (call to input_unregister_device usually does that).   

-- 
Dmitry



More information about the i2c mailing list