Re: [PATCH 1/1] Input: mms114: Use devm_* APIs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dmitry,

On 8 January 2013 22:55, Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote:
> Hi Sachin,
>
> On Tue, Jan 08, 2013 at 05:08:12PM +0530, Sachin Kamat wrote:
>> Hi Laxman,
>>
>> Thank you for your suggestion.
>>
>> On 8 January 2013 16:38, Laxman Dewangan <ldewangan@xxxxxxxxxx> wrote:
>> > On Tuesday 08 January 2013 03:35 PM, Sachin Kamat wrote:
>> >>
>> >> devm_* APIs are device managed and make the exit and clean up code
>> >> simpler.
>> >>
>> >> Signed-off-by: Sachin Kamat <sachin.kamat@xxxxxxxxxx>
>> >> Cc: Joonyoung Shim <jy0922.shim@xxxxxxxxxxx>
>> >> ---
>> >> Compile tested against linux-next tree (20130108).
>> >> ---
>> >>   drivers/input/touchscreen/mms114.c |   41
>> >> +++++++++++------------------------
>> >>   1 files changed, 13 insertions(+), 28 deletions(-)
>> >
>> >
>> >>   static int mms114_remove(struct i2c_client *client)
>> >>   {
>> >>         struct mms114_data *data = i2c_get_clientdata(client);
>> >>   -     free_irq(client->irq, data);
>> >> -       regulator_put(data->io_reg);
>> >> -       regulator_put(data->core_reg);
>> >>         input_unregister_device(data->input_dev);
>> >
>> >
>> > Because you are using devm_input_allocate_device() for allocating input
>> > device, you need not to have the input_unregister_device() and so you can
>> > get rid of remove() callback at all.
>> > This is what I learnt from Dmitry on similar patch for Tegra KBC.
>>
>> input_free_device() is the one which is not required when you use
>> devm_input_allocate_device().
>> Unregister call, AFAIK, is required since we are registering using
>> input_register_device().
>>
>> Dmitry, please let me know if my understanding is right.
>
> Laxman is correct, neither of the calls is normally needed for managed
> input devices. I am considering applying the patch below clarifying this
> fact.

Thank you for clarifying and providing the explanatory patch. I will
resend my patch with the suggested change.

I believe the patch a57da347954 ("Input: samsung-keypad - switch to
using managed resources") also needs similar treatment. I will update
that as well.

Laxman,
Thank you for bringing this point to my notice.


>
> Thanks.
>
> --
> Dmitry
>
>
> Input: document that unregistering managed devices is not necessary
>
> Apparently some users of managed input devices are confused whether
> input_unregister_device() is needed when working with them. Clarify
> this in the kernel doc for devm_input_allocate_device(): in most cases
> there is no need to call either input_unregister_device() nor
> input_free_device() when working with managed devices.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> ---
>  drivers/input/input.c |   16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index ce01332f..c044699 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -1785,12 +1785,13 @@ static void devm_input_device_release(struct device *dev, void *res)
>   * its driver (or binding fails). Once managed input device is allocated,
>   * it is ready to be set up and registered in the same fashion as regular
>   * input device. There are no special devm_input_device_[un]register()
> - * variants, regular ones work with both managed and unmanaged devices.
> + * variants, regular ones work with both managed and unmanaged devices,
> + * should you need them. In most cases however, managed input device need
> + * not be explicitly unregistered or freed.
>   *
>   * NOTE: the owner device is set up as parent of input device and users
>   * should not override it.
>   */
> -
>  struct input_dev *devm_input_allocate_device(struct device *dev)
>  {
>         struct input_dev *input;
> @@ -2004,6 +2005,17 @@ static void devm_input_device_unregister(struct device *dev, void *res)
>   * Once device has been successfully registered it can be unregistered
>   * with input_unregister_device(); input_free_device() should not be
>   * called in this case.
> + *
> + * Note that this function is also used to register managed input devices
> + * (ones allocated with devm_input_allocate_device()). Such managed input
> + * devices need not be explicitly unregistered or freed, their tear down
> + * is controlled by the devres infrastructure. It is also worth noting
> + * that tear down of managed input devices is internally a 2-step process:
> + * registered managed input device is first unregistered, but stays in
> + * memory and can still handle input_event() calls (although events will
> + * not be delivered anywhere). The freeing of managed input device will
> + * happen later, when devres stack is unwound to the point where device
> + * allocation was made.
>   */
>  int input_register_device(struct input_dev *dev)
>  {



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


[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux