| Message ID | 20191113063656.8713-1-hslester96@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Input: synaptics-rmi4 - add missed operations in remove | expand |
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 <hslester96@gmail.com> > --- > 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.
On Wed, Nov 13, 2019 at 4:23 PM Dmitry Torokhov <dmitry.torokhov@gmail.com> 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 <hslester96@gmail.com> > > --- > > 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
On Wed, Nov 13, 2019 at 04:52:59PM +0800, Chuhong Yuan wrote: > On Wed, Nov 13, 2019 at 4:23 PM Dmitry Torokhov > <dmitry.torokhov@gmail.com> 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!
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 = {
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 <hslester96@gmail.com> --- drivers/input/rmi4/rmi_f54.c | 3 +++ 1 file changed, 3 insertions(+)