2019-11-13 06:38:17

by Chuhong Yuan

[permalink] [raw]
Subject: [PATCH] Input: synaptics-rmi4 - add missed operations in remove

The driver forgets to deal with work and workqueue in remove like what
is done when probe fails.
Add the missed operations to fix it.

Signed-off-by: Chuhong Yuan <[email protected]>
---
drivers/input/rmi4/rmi_f54.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
index 710b02595486..2c0cde5c775c 100644
--- a/drivers/input/rmi4/rmi_f54.c
+++ b/drivers/input/rmi4/rmi_f54.c
@@ -730,6 +730,9 @@ static void rmi_f54_remove(struct rmi_function *fn)

video_unregister_device(&f54->vdev);
v4l2_device_unregister(&f54->v4l2);
+ cancel_delayed_work_sync(&f54->work);
+ flush_workqueue(f54->workqueue);
+ destroy_workqueue(f54->workqueue);
}

struct rmi_function_handler rmi_f54_handler = {
--
2.23.0


2019-11-13 08:27:00

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] Input: synaptics-rmi4 - add missed operations in remove

On Wed, Nov 13, 2019 at 02:36:56PM +0800, Chuhong Yuan wrote:
> The driver forgets to deal with work and workqueue in remove like what
> is done when probe fails.
> Add the missed operations to fix it.

Is it really possible for the work to still be pending when fully
registered device is properly unregistered? I thought we'd wait for
successful data acquisition in rmi_f54_buffer_queue() before unregister
can complete.

>
> Signed-off-by: Chuhong Yuan <[email protected]>
> ---
> drivers/input/rmi4/rmi_f54.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
> index 710b02595486..2c0cde5c775c 100644
> --- a/drivers/input/rmi4/rmi_f54.c
> +++ b/drivers/input/rmi4/rmi_f54.c
> @@ -730,6 +730,9 @@ static void rmi_f54_remove(struct rmi_function *fn)
>
> video_unregister_device(&f54->vdev);
> v4l2_device_unregister(&f54->v4l2);
> + cancel_delayed_work_sync(&f54->work);
> + flush_workqueue(f54->workqueue);
> + destroy_workqueue(f54->workqueue);
> }
>
> struct rmi_function_handler rmi_f54_handler = {
> --
> 2.23.0
>

Thanks.

--
Dmitry

2019-11-13 08:56:52

by Chuhong Yuan

[permalink] [raw]
Subject: Re: [PATCH] Input: synaptics-rmi4 - add missed operations in remove

On Wed, Nov 13, 2019 at 4:23 PM Dmitry Torokhov
<[email protected]> wrote:
>
> On Wed, Nov 13, 2019 at 02:36:56PM +0800, Chuhong Yuan wrote:
> > The driver forgets to deal with work and workqueue in remove like what
> > is done when probe fails.
> > Add the missed operations to fix it.
>
> Is it really possible for the work to still be pending when fully
> registered device is properly unregistered? I thought we'd wait for
> successful data acquisition in rmi_f54_buffer_queue() before unregister
> can complete.
>

In fact, I am not familiar with the mechanism here...
I have checked other drivers with video_unregister_device and found none of
them deals with work in remove.
Therefore, I think your opinion should be right and we only need to deal with
the workqueue.

Regards,
Chuhong

> >
> > Signed-off-by: Chuhong Yuan <[email protected]>
> > ---
> > drivers/input/rmi4/rmi_f54.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
> > index 710b02595486..2c0cde5c775c 100644
> > --- a/drivers/input/rmi4/rmi_f54.c
> > +++ b/drivers/input/rmi4/rmi_f54.c
> > @@ -730,6 +730,9 @@ static void rmi_f54_remove(struct rmi_function *fn)
> >
> > video_unregister_device(&f54->vdev);
> > v4l2_device_unregister(&f54->v4l2);
> > + cancel_delayed_work_sync(&f54->work);
> > + flush_workqueue(f54->workqueue);
> > + destroy_workqueue(f54->workqueue);
> > }
> >
> > struct rmi_function_handler rmi_f54_handler = {
> > --
> > 2.23.0
> >
>
> Thanks.
>
> --
> Dmitry

2019-11-13 19:51:22

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] Input: synaptics-rmi4 - add missed operations in remove

On Wed, Nov 13, 2019 at 04:52:59PM +0800, Chuhong Yuan wrote:
> On Wed, Nov 13, 2019 at 4:23 PM Dmitry Torokhov
> <[email protected]> wrote:
> >
> > On Wed, Nov 13, 2019 at 02:36:56PM +0800, Chuhong Yuan wrote:
> > > The driver forgets to deal with work and workqueue in remove like what
> > > is done when probe fails.
> > > Add the missed operations to fix it.
> >
> > Is it really possible for the work to still be pending when fully
> > registered device is properly unregistered? I thought we'd wait for
> > successful data acquisition in rmi_f54_buffer_queue() before unregister
> > can complete.
> >
>
> In fact, I am not familiar with the mechanism here...
> I have checked other drivers with video_unregister_device and found none of
> them deals with work in remove.
> Therefore, I think your opinion should be right and we only need to deal with
> the workqueue.

OK, please send the updated patch then.

Thanks!

--
Dmitry