| Message ID | 20200608112211.12125-4-andrzej.p@collabora.com (mailing list archive) |
|---|---|
| State | Under Review |
| Headers | show |
| Series | [v4,1/7] Input: add input_device_enabled() | expand |
On Mon, Jun 8, 2020 at 1:22 PM Andrzej Pietrasiewicz <andrzej.p@collabora.com> wrote: > > Inspecting input device's 'users' member should be done under device's > mutex, so add appropriate invocations. > > Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com> This looks like a fix that might be applied independently of the other patches in the series. Do you want me to pick it up? > --- > drivers/acpi/button.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c > index 78cfc70cb320..ff7ab291f678 100644 > --- a/drivers/acpi/button.c > +++ b/drivers/acpi/button.c > @@ -456,13 +456,16 @@ static int acpi_button_resume(struct device *dev) > { > struct acpi_device *device = to_acpi_device(dev); > struct acpi_button *button = acpi_driver_data(device); > + struct input_dev *input = button->input; > > button->suspended = false; > - if (button->type == ACPI_BUTTON_TYPE_LID && button->input->users) { > + mutex_lock(&input->mutex); > + if (button->type == ACPI_BUTTON_TYPE_LID && input->users) { > button->last_state = !!acpi_lid_evaluate_state(device); > button->last_time = ktime_get(); > acpi_lid_initialize_state(device); > } > + mutex_unlock(&input->mutex); > return 0; > } > #endif > -- > 2.17.1 >
On Wed, Jun 24, 2020 at 05:00:09PM +0200, Rafael J. Wysocki wrote: > On Mon, Jun 8, 2020 at 1:22 PM Andrzej Pietrasiewicz > <andrzej.p@collabora.com> wrote: > > > > Inspecting input device's 'users' member should be done under device's > > mutex, so add appropriate invocations. > > > > Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com> > > This looks like a fix that might be applied independently of the other > patches in the series. > > Do you want me to pick it up? If you pick it we'll have to have a dance with this series. Can I apply instead? I do not think this change has any practical effect as nobody attaches/detached input handlers or opening/closing input devices when system goes through device resume phase. > > > --- > > drivers/acpi/button.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c > > index 78cfc70cb320..ff7ab291f678 100644 > > --- a/drivers/acpi/button.c > > +++ b/drivers/acpi/button.c > > @@ -456,13 +456,16 @@ static int acpi_button_resume(struct device *dev) > > { > > struct acpi_device *device = to_acpi_device(dev); > > struct acpi_button *button = acpi_driver_data(device); > > + struct input_dev *input = button->input; > > > > button->suspended = false; > > - if (button->type == ACPI_BUTTON_TYPE_LID && button->input->users) { > > + mutex_lock(&input->mutex); > > + if (button->type == ACPI_BUTTON_TYPE_LID && input->users) { > > button->last_state = !!acpi_lid_evaluate_state(device); > > button->last_time = ktime_get(); > > acpi_lid_initialize_state(device); > > } > > + mutex_unlock(&input->mutex); > > return 0; > > } > > #endif > > -- > > 2.17.1 > > Thanks.
On Thu, Jun 25, 2020 at 7:23 AM Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > > On Wed, Jun 24, 2020 at 05:00:09PM +0200, Rafael J. Wysocki wrote: > > On Mon, Jun 8, 2020 at 1:22 PM Andrzej Pietrasiewicz > > <andrzej.p@collabora.com> wrote: > > > > > > Inspecting input device's 'users' member should be done under device's > > > mutex, so add appropriate invocations. > > > > > > Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com> > > > > This looks like a fix that might be applied independently of the other > > patches in the series. > > > > Do you want me to pick it up? > > If you pick it we'll have to have a dance with this series. Can I apply > instead? Yes, please. Also feel free to add Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> to it. > I do not think this change has any practical effect as nobody > attaches/detached input handlers or opening/closing input devices when > system goes through device resume phase. Indeed. Thanks!
On Thu, Jun 25, 2020 at 12:55:29PM +0200, Rafael J. Wysocki wrote: > On Thu, Jun 25, 2020 at 7:23 AM Dmitry Torokhov > <dmitry.torokhov@gmail.com> wrote: > > > > On Wed, Jun 24, 2020 at 05:00:09PM +0200, Rafael J. Wysocki wrote: > > > On Mon, Jun 8, 2020 at 1:22 PM Andrzej Pietrasiewicz > > > <andrzej.p@collabora.com> wrote: > > > > > > > > Inspecting input device's 'users' member should be done under device's > > > > mutex, so add appropriate invocations. > > > > > > > > Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com> > > > > > > This looks like a fix that might be applied independently of the other > > > patches in the series. > > > > > > Do you want me to pick it up? > > > > If you pick it we'll have to have a dance with this series. Can I apply > > instead? > > Yes, please. > > Also feel free to add > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > to it. Looking at the driver I think the patch and the original use of input->users is not proper. I'll post another patch addressing this shortly. Thanks.
diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c index 78cfc70cb320..ff7ab291f678 100644 --- a/drivers/acpi/button.c +++ b/drivers/acpi/button.c @@ -456,13 +456,16 @@ static int acpi_button_resume(struct device *dev) { struct acpi_device *device = to_acpi_device(dev); struct acpi_button *button = acpi_driver_data(device); + struct input_dev *input = button->input; button->suspended = false; - if (button->type == ACPI_BUTTON_TYPE_LID && button->input->users) { + mutex_lock(&input->mutex); + if (button->type == ACPI_BUTTON_TYPE_LID && input->users) { button->last_state = !!acpi_lid_evaluate_state(device); button->last_time = ktime_get(); acpi_lid_initialize_state(device); } + mutex_unlock(&input->mutex); return 0; } #endif
Inspecting input device's 'users' member should be done under device's mutex, so add appropriate invocations. Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com> --- drivers/acpi/button.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)