2021-06-08 10:12:53

by Matti Vaittinen

[permalink] [raw]
Subject: [PATCH RESEND v2 3/5] extcon: extcon-max77693.c: Fix potential work-queue cancellation race

The extcon IRQ schedules a work item. IRQ is requested using devm while
WQ is cancelld at remove(). This mixing of devm and manual unwinding has
potential case where the WQ has been emptied (.remove() was ran) but
devm unwinding of IRQ was not yet done. It may be possible the IRQ is
triggered at this point scheduling new work item to the already flushed
queue.

According to the input documentation the input device allocated by
devm_input_allocate_device() does not need to be explicitly unregistered.
Use the new devm_work_autocancel() and remove the remove() to simplify the
code.

Signed-off-by: Matti Vaittinen <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Reviewed-by: Hans de Goede <[email protected]>
---

Please note that the change is compile-tested only. All proper testing is
highly appreciated.
---
drivers/extcon/extcon-max77693.c | 17 +++++------------
1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/extcon/extcon-max77693.c b/drivers/extcon/extcon-max77693.c
index 92af97e00828..1f1d9ab0c5c7 100644
--- a/drivers/extcon/extcon-max77693.c
+++ b/drivers/extcon/extcon-max77693.c
@@ -5,6 +5,7 @@
// Copyright (C) 2012 Samsung Electrnoics
// Chanwoo Choi <[email protected]>

+#include <linux/devm-helpers.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/i2c.h>
@@ -1127,7 +1128,10 @@ static int max77693_muic_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, info);
mutex_init(&info->mutex);

- INIT_WORK(&info->irq_work, max77693_muic_irq_work);
+ ret = devm_work_autocancel(&pdev->dev, &info->irq_work,
+ max77693_muic_irq_work);
+ if (ret)
+ return ret;

/* Support irq domain for MAX77693 MUIC device */
for (i = 0; i < ARRAY_SIZE(muic_irqs); i++) {
@@ -1254,22 +1258,11 @@ static int max77693_muic_probe(struct platform_device *pdev)
return ret;
}

-static int max77693_muic_remove(struct platform_device *pdev)
-{
- struct max77693_muic_info *info = platform_get_drvdata(pdev);
-
- cancel_work_sync(&info->irq_work);
- input_unregister_device(info->dock);
-
- return 0;
-}
-
static struct platform_driver max77693_muic_driver = {
.driver = {
.name = DEV_NAME,
},
.probe = max77693_muic_probe,
- .remove = max77693_muic_remove,
};

module_platform_driver(max77693_muic_driver);
--
2.25.4


--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]


Attachments:
(No filename) (2.73 kB)
signature.asc (499.00 B)
Download all attachments

2021-06-10 09:26:02

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH RESEND v2 3/5] extcon: extcon-max77693.c: Fix potential work-queue cancellation race

On 6/8/21 7:10 PM, Matti Vaittinen wrote:
> The extcon IRQ schedules a work item. IRQ is requested using devm while
> WQ is cancelld at remove(). This mixing of devm and manual unwinding has
> potential case where the WQ has been emptied (.remove() was ran) but
> devm unwinding of IRQ was not yet done. It may be possible the IRQ is
> triggered at this point scheduling new work item to the already flushed
> queue.
>
> According to the input documentation the input device allocated by
> devm_input_allocate_device() does not need to be explicitly unregistered.
> Use the new devm_work_autocancel() and remove the remove() to simplify the
> code.
>
> Signed-off-by: Matti Vaittinen <[email protected]>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
> Reviewed-by: Hans de Goede <[email protected]>
> ---
>
> Please note that the change is compile-tested only. All proper testing is
> highly appreciated.
> ---
> drivers/extcon/extcon-max77693.c | 17 +++++------------
> 1 file changed, 5 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/extcon/extcon-max77693.c b/drivers/extcon/extcon-max77693.c
> index 92af97e00828..1f1d9ab0c5c7 100644
> --- a/drivers/extcon/extcon-max77693.c
> +++ b/drivers/extcon/extcon-max77693.c
> @@ -5,6 +5,7 @@
> // Copyright (C) 2012 Samsung Electrnoics
> // Chanwoo Choi <[email protected]>
>
> +#include <linux/devm-helpers.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/i2c.h>
> @@ -1127,7 +1128,10 @@ static int max77693_muic_probe(struct platform_device *pdev)
> platform_set_drvdata(pdev, info);
> mutex_init(&info->mutex);
>
> - INIT_WORK(&info->irq_work, max77693_muic_irq_work);
> + ret = devm_work_autocancel(&pdev->dev, &info->irq_work,
> + max77693_muic_irq_work);
> + if (ret)
> + return ret;
>
> /* Support irq domain for MAX77693 MUIC device */
> for (i = 0; i < ARRAY_SIZE(muic_irqs); i++) {
> @@ -1254,22 +1258,11 @@ static int max77693_muic_probe(struct platform_device *pdev)
> return ret;
> }
>
> -static int max77693_muic_remove(struct platform_device *pdev)
> -{
> - struct max77693_muic_info *info = platform_get_drvdata(pdev);
> -
> - cancel_work_sync(&info->irq_work);
> - input_unregister_device(info->dock);

I think that you have to keep the input_unregister_device().

> -
> - return 0;
> -}
> -
> static struct platform_driver max77693_muic_driver = {
> .driver = {
> .name = DEV_NAME,
> },
> .probe = max77693_muic_probe,
> - .remove = max77693_muic_remove,
> };
>
> module_platform_driver(max77693_muic_driver);
>

--
Best Regards,
Chanwoo Choi
Samsung Electronics

2021-06-10 09:50:55

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH RESEND v2 3/5] extcon: extcon-max77693.c: Fix potential work-queue cancellation race

Hi,

On 6/10/21 11:43 AM, Chanwoo Choi wrote:
> On 6/8/21 7:10 PM, Matti Vaittinen wrote:
>> The extcon IRQ schedules a work item. IRQ is requested using devm while
>> WQ is cancelld at remove(). This mixing of devm and manual unwinding has
>> potential case where the WQ has been emptied (.remove() was ran) but
>> devm unwinding of IRQ was not yet done. It may be possible the IRQ is
>> triggered at this point scheduling new work item to the already flushed
>> queue.
>>
>> According to the input documentation the input device allocated by
>> devm_input_allocate_device() does not need to be explicitly unregistered.
>> Use the new devm_work_autocancel() and remove the remove() to simplify the
>> code.
>>
>> Signed-off-by: Matti Vaittinen <[email protected]>
>> Reviewed-by: Krzysztof Kozlowski <[email protected]>
>> Reviewed-by: Hans de Goede <[email protected]>
>> ---
>>
>> Please note that the change is compile-tested only. All proper testing is
>> highly appreciated.
>> ---
>> drivers/extcon/extcon-max77693.c | 17 +++++------------
>> 1 file changed, 5 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/extcon/extcon-max77693.c b/drivers/extcon/extcon-max77693.c
>> index 92af97e00828..1f1d9ab0c5c7 100644
>> --- a/drivers/extcon/extcon-max77693.c
>> +++ b/drivers/extcon/extcon-max77693.c
>> @@ -5,6 +5,7 @@
>> // Copyright (C) 2012 Samsung Electrnoics
>> // Chanwoo Choi <[email protected]>
>>
>> +#include <linux/devm-helpers.h>
>> #include <linux/kernel.h>
>> #include <linux/module.h>
>> #include <linux/i2c.h>
>> @@ -1127,7 +1128,10 @@ static int max77693_muic_probe(struct platform_device *pdev)
>> platform_set_drvdata(pdev, info);
>> mutex_init(&info->mutex);
>>
>> - INIT_WORK(&info->irq_work, max77693_muic_irq_work);
>> + ret = devm_work_autocancel(&pdev->dev, &info->irq_work,
>> + max77693_muic_irq_work);
>> + if (ret)
>> + return ret;
>>
>> /* Support irq domain for MAX77693 MUIC device */
>> for (i = 0; i < ARRAY_SIZE(muic_irqs); i++) {
>> @@ -1254,22 +1258,11 @@ static int max77693_muic_probe(struct platform_device *pdev)
>> return ret;
>> }
>>
>> -static int max77693_muic_remove(struct platform_device *pdev)
>> -{
>> - struct max77693_muic_info *info = platform_get_drvdata(pdev);
>> -
>> - cancel_work_sync(&info->irq_work);
>> - input_unregister_device(info->dock);
>
> I think that you have to keep the input_unregister_device().

As mentioned in the commit message, in input_unregister_device
is not necessary for input-devices created with
devm_input_allocate_device():

"According to the input documentation the input device allocated by
devm_input_allocate_device() does not need to be explicitly unregistered."

I have verified that the documentation is correct here, so there is
no need to keep the input_unregister_device() as it was never necessary
to have that there.

Regards,

Hans



>
>> -
>> - return 0;
>> -}
>> -
>> static struct platform_driver max77693_muic_driver = {
>> .driver = {
>> .name = DEV_NAME,
>> },
>> .probe = max77693_muic_probe,
>> - .remove = max77693_muic_remove,
>> };
>>
>> module_platform_driver(max77693_muic_driver);
>>
>

2021-06-10 09:59:17

by Matti Vaittinen

[permalink] [raw]
Subject: Re: [PATCH RESEND v2 3/5] extcon: extcon-max77693.c: Fix potential work-queue cancellation race


On Thu, 2021-06-10 at 18:43 +0900, Chanwoo Choi wrote:
> On 6/8/21 7:10 PM, Matti Vaittinen wrote:
> > The extcon IRQ schedules a work item. IRQ is requested using devm
> > while
> > WQ is cancelld at remove(). This mixing of devm and manual
> > unwinding has
> > potential case where the WQ has been emptied (.remove() was ran)
> > but
> > devm unwinding of IRQ was not yet done. It may be possible the IRQ
> > is
> > triggered at this point scheduling new work item to the already
> > flushed
> > queue.
> >
> > According to the input documentation the input device allocated by
> > devm_input_allocate_device() does not need to be explicitly
> > unregistered.
> > Use the new devm_work_autocancel() and remove the remove() to
> > simplify the
> > code.
> >
> > Signed-off-by: Matti Vaittinen <[email protected]>
> > Reviewed-by: Krzysztof Kozlowski <[email protected]
> > >
> > Reviewed-by: Hans de Goede <[email protected]>
> > ---
> >
> > Please note that the change is compile-tested only. All proper
> > testing is
> > highly appreciated.
> > ---
> > drivers/extcon/extcon-max77693.c | 17 +++++------------
> > 1 file changed, 5 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/extcon/extcon-max77693.c
> > b/drivers/extcon/extcon-max77693.c
> > index 92af97e00828..1f1d9ab0c5c7 100644
> > --- a/drivers/extcon/extcon-max77693.c
> > +++ b/drivers/extcon/extcon-max77693.c
> > @@ -5,6 +5,7 @@
> > // Copyright (C) 2012 Samsung Electrnoics
> > // Chanwoo Choi <[email protected]>
> >
> > +#include <linux/devm-helpers.h>
> > #include <linux/kernel.h>
> > #include <linux/module.h>
> > #include <linux/i2c.h>
> > @@ -1127,7 +1128,10 @@ static int max77693_muic_probe(struct
> > platform_device *pdev)
> > platform_set_drvdata(pdev, info);
> > mutex_init(&info->mutex);
> >
> > - INIT_WORK(&info->irq_work, max77693_muic_irq_work);
> > + ret = devm_work_autocancel(&pdev->dev, &info->irq_work,
> > + max77693_muic_irq_work);
> > + if (ret)
> > + return ret;
> >
> > /* Support irq domain for MAX77693 MUIC device */
> > for (i = 0; i < ARRAY_SIZE(muic_irqs); i++) {
> > @@ -1254,22 +1258,11 @@ static int max77693_muic_probe(struct
> > platform_device *pdev)
> > return ret;
> > }
> >
> > -static int max77693_muic_remove(struct platform_device *pdev)
> > -{
> > - struct max77693_muic_info *info = platform_get_drvdata(pdev);
> > -
> > - cancel_work_sync(&info->irq_work);
> > - input_unregister_device(info->dock);
>
> I think that you have to keep the input_unregister_device().

Are you sure? I can add back the remove() if required - but the
kerneldoc for devm_input_allocate_device() seems to be suggesting that
this would not be needed:

* Managed input devices do not need to be explicitly unregistered or
* freed as it will be done automatically when owner device unbinds
from
* 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,
* should you need them. In most cases however, managed input device
need
* not be explicitly unregistered or freed.

https://elixir.bootlin.com/linux/v5.13-rc5/source/drivers/input/input.c#L1955

I am not going to argue with you though - I am not really familiar with
the input subsystem. I'd appreciate if someone could shed some light on
when the input_unregister_device() can be omitted?

Best Regards
Matti Vaittinen


2021-06-10 22:19:45

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH RESEND v2 3/5] extcon: extcon-max77693.c: Fix potential work-queue cancellation race

On Thu, Jun 10, 2021 at 12:57:40PM +0300, Matti Vaittinen wrote:
>
> On Thu, 2021-06-10 at 18:43 +0900, Chanwoo Choi wrote:
> > On 6/8/21 7:10 PM, Matti Vaittinen wrote:
> > > The extcon IRQ schedules a work item. IRQ is requested using devm
> > > while
> > > WQ is cancelld at remove(). This mixing of devm and manual
> > > unwinding has
> > > potential case where the WQ has been emptied (.remove() was ran)
> > > but
> > > devm unwinding of IRQ was not yet done. It may be possible the IRQ
> > > is
> > > triggered at this point scheduling new work item to the already
> > > flushed
> > > queue.
> > >
> > > According to the input documentation the input device allocated by
> > > devm_input_allocate_device() does not need to be explicitly
> > > unregistered.
> > > Use the new devm_work_autocancel() and remove the remove() to
> > > simplify the
> > > code.
> > >
> > > Signed-off-by: Matti Vaittinen <[email protected]>
> > > Reviewed-by: Krzysztof Kozlowski <[email protected]
> > > >
> > > Reviewed-by: Hans de Goede <[email protected]>
> > > ---
> > >
> > > Please note that the change is compile-tested only. All proper
> > > testing is
> > > highly appreciated.
> > > ---
> > > drivers/extcon/extcon-max77693.c | 17 +++++------------
> > > 1 file changed, 5 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/extcon/extcon-max77693.c
> > > b/drivers/extcon/extcon-max77693.c
> > > index 92af97e00828..1f1d9ab0c5c7 100644
> > > --- a/drivers/extcon/extcon-max77693.c
> > > +++ b/drivers/extcon/extcon-max77693.c
> > > @@ -5,6 +5,7 @@
> > > // Copyright (C) 2012 Samsung Electrnoics
> > > // Chanwoo Choi <[email protected]>
> > >
> > > +#include <linux/devm-helpers.h>
> > > #include <linux/kernel.h>
> > > #include <linux/module.h>
> > > #include <linux/i2c.h>
> > > @@ -1127,7 +1128,10 @@ static int max77693_muic_probe(struct
> > > platform_device *pdev)
> > > platform_set_drvdata(pdev, info);
> > > mutex_init(&info->mutex);
> > >
> > > - INIT_WORK(&info->irq_work, max77693_muic_irq_work);
> > > + ret = devm_work_autocancel(&pdev->dev, &info->irq_work,
> > > + max77693_muic_irq_work);
> > > + if (ret)
> > > + return ret;
> > >
> > > /* Support irq domain for MAX77693 MUIC device */
> > > for (i = 0; i < ARRAY_SIZE(muic_irqs); i++) {
> > > @@ -1254,22 +1258,11 @@ static int max77693_muic_probe(struct
> > > platform_device *pdev)
> > > return ret;
> > > }
> > >
> > > -static int max77693_muic_remove(struct platform_device *pdev)
> > > -{
> > > - struct max77693_muic_info *info = platform_get_drvdata(pdev);
> > > -
> > > - cancel_work_sync(&info->irq_work);
> > > - input_unregister_device(info->dock);
> >
> > I think that you have to keep the input_unregister_device().
>
> Are you sure? I can add back the remove() if required - but the
> kerneldoc for devm_input_allocate_device() seems to be suggesting that
> this would not be needed:
>
> * Managed input devices do not need to be explicitly unregistered or
> * freed as it will be done automatically when owner device unbinds
> from
> * 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,
> * should you need them. In most cases however, managed input device
> need
> * not be explicitly unregistered or freed.
>
> https://elixir.bootlin.com/linux/v5.13-rc5/source/drivers/input/input.c#L1955
>
> I am not going to argue with you though - I am not really familiar with
> the input subsystem. I'd appreciate if someone could shed some light on
> when the input_unregister_device() can be omitted?

That is correct, you do not need to call input_unregister_device() for
input devices allocated with devm_input_allocate_device().

Thanks.

--
Dmitry

2021-06-11 06:59:04

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH RESEND v2 3/5] extcon: extcon-max77693.c: Fix potential work-queue cancellation race

On 6/10/21 6:57 PM, Matti Vaittinen wrote:
>
> On Thu, 2021-06-10 at 18:43 +0900, Chanwoo Choi wrote:
>> On 6/8/21 7:10 PM, Matti Vaittinen wrote:
>>> The extcon IRQ schedules a work item. IRQ is requested using devm
>>> while
>>> WQ is cancelld at remove(). This mixing of devm and manual
>>> unwinding has
>>> potential case where the WQ has been emptied (.remove() was ran)
>>> but
>>> devm unwinding of IRQ was not yet done. It may be possible the IRQ
>>> is
>>> triggered at this point scheduling new work item to the already
>>> flushed
>>> queue.
>>>
>>> According to the input documentation the input device allocated by
>>> devm_input_allocate_device() does not need to be explicitly
>>> unregistered.
>>> Use the new devm_work_autocancel() and remove the remove() to
>>> simplify the
>>> code.
>>>
>>> Signed-off-by: Matti Vaittinen <[email protected]>
>>> Reviewed-by: Krzysztof Kozlowski <[email protected]
>>>>
>>> Reviewed-by: Hans de Goede <[email protected]>
>>> ---
>>>
>>> Please note that the change is compile-tested only. All proper
>>> testing is
>>> highly appreciated.
>>> ---
>>> drivers/extcon/extcon-max77693.c | 17 +++++------------
>>> 1 file changed, 5 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/extcon/extcon-max77693.c
>>> b/drivers/extcon/extcon-max77693.c
>>> index 92af97e00828..1f1d9ab0c5c7 100644
>>> --- a/drivers/extcon/extcon-max77693.c
>>> +++ b/drivers/extcon/extcon-max77693.c
>>> @@ -5,6 +5,7 @@
>>> // Copyright (C) 2012 Samsung Electrnoics
>>> // Chanwoo Choi <[email protected]>
>>>
>>> +#include <linux/devm-helpers.h>
>>> #include <linux/kernel.h>
>>> #include <linux/module.h>
>>> #include <linux/i2c.h>
>>> @@ -1127,7 +1128,10 @@ static int max77693_muic_probe(struct
>>> platform_device *pdev)
>>> platform_set_drvdata(pdev, info);
>>> mutex_init(&info->mutex);
>>>
>>> - INIT_WORK(&info->irq_work, max77693_muic_irq_work);
>>> + ret = devm_work_autocancel(&pdev->dev, &info->irq_work,
>>> + max77693_muic_irq_work);
>>> + if (ret)
>>> + return ret;
>>>
>>> /* Support irq domain for MAX77693 MUIC device */
>>> for (i = 0; i < ARRAY_SIZE(muic_irqs); i++) {
>>> @@ -1254,22 +1258,11 @@ static int max77693_muic_probe(struct
>>> platform_device *pdev)
>>> return ret;
>>> }
>>>
>>> -static int max77693_muic_remove(struct platform_device *pdev)
>>> -{
>>> - struct max77693_muic_info *info = platform_get_drvdata(pdev);
>>> -
>>> - cancel_work_sync(&info->irq_work);
>>> - input_unregister_device(info->dock);
>>
>> I think that you have to keep the input_unregister_device().
>
> Are you sure? I can add back the remove() if required - but the
> kerneldoc for devm_input_allocate_device() seems to be suggesting that
> this would not be needed:
>
> * Managed input devices do not need to be explicitly unregistered or
> * freed as it will be done automatically when owner device unbinds
> from
> * 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,
> * should you need them. In most cases however, managed input device
> need
> * not be explicitly unregistered or freed.
>
> https://protect2.fireeye.com/v1/url?k=ffe6f053-a07dc951-ffe77b1c-0cc47a312ab0-aa86636d08cba7ad&q=1&e=f8aab92a-f090-4b48-91f8-4dffa41042e9&u=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Fv5.13-rc5%2Fsource%2Fdrivers%2Finput%2Finput.c%23L1955
>
> I am not going to argue with you though - I am not really familiar with
> the input subsystem. I'd appreciate if someone could shed some light on
> when the input_unregister_device() can be omitted?

You're right. Thanks for pointing out.

Acked-by: Chanwoo Choi <[email protected]>

--
Best Regards,
Chanwoo Choi
Samsung Electronics

2021-06-11 08:48:48

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH RESEND v2 3/5] extcon: extcon-max77693.c: Fix potential work-queue cancellation race

On 6/10/21 6:49 PM, Hans de Goede wrote:
> Hi,
>
> On 6/10/21 11:43 AM, Chanwoo Choi wrote:
>> On 6/8/21 7:10 PM, Matti Vaittinen wrote:
>>> The extcon IRQ schedules a work item. IRQ is requested using devm while
>>> WQ is cancelld at remove(). This mixing of devm and manual unwinding has
>>> potential case where the WQ has been emptied (.remove() was ran) but
>>> devm unwinding of IRQ was not yet done. It may be possible the IRQ is
>>> triggered at this point scheduling new work item to the already flushed
>>> queue.
>>>
>>> According to the input documentation the input device allocated by
>>> devm_input_allocate_device() does not need to be explicitly unregistered.
>>> Use the new devm_work_autocancel() and remove the remove() to simplify the
>>> code.
>>>
>>> Signed-off-by: Matti Vaittinen <[email protected]>
>>> Reviewed-by: Krzysztof Kozlowski <[email protected]>
>>> Reviewed-by: Hans de Goede <[email protected]>
>>> ---
>>>
>>> Please note that the change is compile-tested only. All proper testing is
>>> highly appreciated.
>>> ---
>>> drivers/extcon/extcon-max77693.c | 17 +++++------------
>>> 1 file changed, 5 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/extcon/extcon-max77693.c b/drivers/extcon/extcon-max77693.c
>>> index 92af97e00828..1f1d9ab0c5c7 100644
>>> --- a/drivers/extcon/extcon-max77693.c
>>> +++ b/drivers/extcon/extcon-max77693.c
>>> @@ -5,6 +5,7 @@
>>> // Copyright (C) 2012 Samsung Electrnoics
>>> // Chanwoo Choi <[email protected]>
>>>
>>> +#include <linux/devm-helpers.h>
>>> #include <linux/kernel.h>
>>> #include <linux/module.h>
>>> #include <linux/i2c.h>
>>> @@ -1127,7 +1128,10 @@ static int max77693_muic_probe(struct platform_device *pdev)
>>> platform_set_drvdata(pdev, info);
>>> mutex_init(&info->mutex);
>>>
>>> - INIT_WORK(&info->irq_work, max77693_muic_irq_work);
>>> + ret = devm_work_autocancel(&pdev->dev, &info->irq_work,
>>> + max77693_muic_irq_work);
>>> + if (ret)
>>> + return ret;
>>>
>>> /* Support irq domain for MAX77693 MUIC device */
>>> for (i = 0; i < ARRAY_SIZE(muic_irqs); i++) {
>>> @@ -1254,22 +1258,11 @@ static int max77693_muic_probe(struct platform_device *pdev)
>>> return ret;
>>> }
>>>
>>> -static int max77693_muic_remove(struct platform_device *pdev)
>>> -{
>>> - struct max77693_muic_info *info = platform_get_drvdata(pdev);
>>> -
>>> - cancel_work_sync(&info->irq_work);
>>> - input_unregister_device(info->dock);
>>
>> I think that you have to keep the input_unregister_device().
>
> As mentioned in the commit message, in input_unregister_device
> is not necessary for input-devices created with
> devm_input_allocate_device():
>
> "According to the input documentation the input device allocated by
> devm_input_allocate_device() does not need to be explicitly unregistered."
>
> I have verified that the documentation is correct here, so there is
> no need to keep the input_unregister_device() as it was never necessary
> to have that there.

You're right. I got it from Matti Vaittinen review.
I replied the ack message.

--
Best Regards,
Chanwoo Choi
Samsung Electronics