Re: [PATCH] HID: logitech-hidpp: rework one more time the retries attempts

[Greg KH]

On Wed, Jun 21, 2023 at 11:42:30AM +0200, Benjamin Tissoires wrote:
> Make the code looks less like Pascal.
> 
> Extract the internal code inside a helper function, fix the
> initialization of the parameters used in the helper function
> (`hidpp->answer_available` was not reset and `*response` wasn't too),
> and use a `do {...} while();` loop.
> 
> Fixes: 586e8fede795 ("HID: logitech-hidpp: Retry commands when device is busy")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx>
> ---
> as requested by https://lore.kernel.org/all/CAHk-=wiMbF38KCNhPFiargenpSBoecSXTLQACKS2UMyo_Vu2ww@xxxxxxxxxxxxxx/
> This is a rewrite of that particular piece of code.
> ---
>  drivers/hid/hid-logitech-hidpp.c | 102 +++++++++++++++++++++++----------------
>  1 file changed, 61 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index dfe8e09a18de..3d1ffe199f08 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -275,21 +275,20 @@ static int __hidpp_send_report(struct hid_device *hdev,
>  }
>  
>  /*
> - * hidpp_send_message_sync() returns 0 in case of success, and something else
> - * in case of a failure.
> - * - If ' something else' is positive, that means that an error has been raised
> - *   by the protocol itself.
> - * - If ' something else' is negative, that means that we had a classic error
> - *   (-ENOMEM, -EPIPE, etc...)
> + * Effectively send the message to the device, waiting for its answer.
> + *
> + * Must be called with hidpp->send_mutex locked
> + *
> + * Same return protocol than hidpp_send_message_sync():
> + * - success on 0
> + * - negative error means transport error
> + * - positive value means protocol error
>   */
> -static int hidpp_send_message_sync(struct hidpp_device *hidpp,
> +static int __do_hidpp_send_message_sync(struct hidpp_device *hidpp,
>  	struct hidpp_report *message,
>  	struct hidpp_report *response)

__must_hold(&hidpp->send_mutex)  ?