Re: [PATCH] HID: Intel-ish-hid: Ishtp: Fix sensor reads after ACPI S3 suspend

Re: [PATCH] HID: Intel-ish-hid: Ishtp: Fix sensor reads after ACPI S3 suspend

[srinivas pandruvada]

On Fri, 2024-02-09 at 14:52 +0800, Even Xu wrote:
> After legacy suspend/resume via ACPI S3, sensor read operation fails
> with timeout. Also, it will cause delay in resume operation as there
> will be retries on failure.
> 
> This is caused by commit f645a90e8ff7 ("HID: intel-ish-hid:
> ishtp-hid-client: use helper functions for connection"), which used
> helper functions to simplify connect, reset and disconnect process.
> Also avoid freeing and allocating client buffers again during
> reconnect
> process.
> 
> But there is a case, when ISH firmware resets after ACPI S3 suspend,
> ishtp bus driver frees client buffers. Since there is no realloc
> again
> during reconnect, there are no client buffers available to send
> connection
> requests to the firmware. Without successful connection to the
> firmware,
> subsequent sensor reads will timeout.
> 
> To address this issue, ishtp bus driver does not free client buffers
> on
> warm reset after S3 resume. Simply add the buffers from the read list
> to free list of buffers.
> 
> Fixes: f645a90e8ff7 ("HID: intel-ish-hid: ishtp-hid-client: use
> helper functions for connection")
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218442
> Signed-off-by: Even Xu <even.xu@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

Hi Jiri,

This regression is introduced with 6.8-rc1, so need a pull request for
this rc cycle.

Thanks,
Srinivas

> ---
>  drivers/hid/intel-ish-hid/ishtp/bus.c    | 2 ++
>  drivers/hid/intel-ish-hid/ishtp/client.c | 4 +++-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c
> b/drivers/hid/intel-ish-hid/ishtp/bus.c
> index aa6cb033bb06..03d5601ce807 100644
> --- a/drivers/hid/intel-ish-hid/ishtp/bus.c
> +++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
> @@ -722,6 +722,8 @@ void ishtp_bus_remove_all_clients(struct
> ishtp_device *ishtp_dev,
>         spin_lock_irqsave(&ishtp_dev->cl_list_lock, flags);
>         list_for_each_entry(cl, &ishtp_dev->cl_list, link) {
>                 cl->state = ISHTP_CL_DISCONNECTED;
> +               if (warm_reset && cl->device->reference_count)
> +                       continue;
>  
>                 /*
>                  * Wake any pending process. The waiter would check
> dev->state
> diff --git a/drivers/hid/intel-ish-hid/ishtp/client.c
> b/drivers/hid/intel-ish-hid/ishtp/client.c
> index 82c907f01bd3..8a7f2f6a4f86 100644
> --- a/drivers/hid/intel-ish-hid/ishtp/client.c
> +++ b/drivers/hid/intel-ish-hid/ishtp/client.c
> @@ -49,7 +49,9 @@ static void ishtp_read_list_flush(struct ishtp_cl
> *cl)
>         list_for_each_entry_safe(rb, next, &cl->dev->read_list.list,
> list)
>                 if (rb->cl && ishtp_cl_cmp_id(cl, rb->cl)) {
>                         list_del(&rb->list);
> -                       ishtp_io_rb_free(rb);
> +                       spin_lock(&cl->free_list_spinlock);
> +                       list_add_tail(&rb->list, &cl-
> >free_rb_list.list);
> +                       spin_unlock(&cl->free_list_spinlock);
>                 }
>         spin_unlock_irqrestore(&cl->dev->read_list_spinlock, flags);
>  }

Re: [PATCH] HID: Intel-ish-hid: Ishtp: Fix sensor reads after ACPI S3 suspend

[Jiri Kosina]

On Fri, 9 Feb 2024, srinivas pandruvada wrote:

> > Fixes: f645a90e8ff7 ("HID: intel-ish-hid: ishtp-hid-client: use
> > helper functions for connection")
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218442
> > Signed-off-by: Even Xu <even.xu@intel.com>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> 
> Hi Jiri,
> 
> This regression is introduced with 6.8-rc1, so need a pull request for
> this rc cycle.

Right; now queued in hid.git#for-6.8/upstream-fixes. Thanks,

-- 
Jiri Kosina
SUSE Labs

RE: [PATCH] HID: Intel-ish-hid: Ishtp: Fix sensor reads after ACPI S3 suspend

[Xu, Even]

Thanks Jiri and Srinivas!

Best Regards,
Even Xu

-----Original Message-----
From: Jiri Kosina <jikos@kernel.org> 
Sent: Tuesday, February 13, 2024 6:32 PM
To: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Xu, Even <even.xu@intel.com>; benjamin.tissoires@redhat.com; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] HID: Intel-ish-hid: Ishtp: Fix sensor reads after ACPI S3 suspend

On Fri, 9 Feb 2024, srinivas pandruvada wrote:

> > Fixes: f645a90e8ff7 ("HID: intel-ish-hid: ishtp-hid-client: use 
> > helper functions for connection")
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218442
> > Signed-off-by: Even Xu <even.xu@intel.com>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> 
> Hi Jiri,
> 
> This regression is introduced with 6.8-rc1, so need a pull request for 
> this rc cycle.

Right; now queued in hid.git#for-6.8/upstream-fixes. Thanks,

--
Jiri Kosina
SUSE Labs