diff mbox series

[1/1] HID: Print specific timeout error in probe

Message ID 20221110204131.1354704-1-abergmeier@gmx.net
State New
Headers
Series [1/1] HID: Print specific timeout error in probe |

Commit Message

Andreas Bergmeier Nov. 10, 2022, 8:41 p.m. UTC 
  When -ETIMEOUT gets reported ensure that the error message mentiones
timeout.

Signed-off-by: Andreas Bergmeier <abergmeier@gmx.net>
---
 drivers/hid/hid-logitech-hidpp.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

--
2.34.1

Comments

Jiri Kosina Nov. 14, 2022, 2:09 p.m. UTC | #1 
On Thu, 10 Nov 2022, Andreas Bergmeier wrote:

> When -ETIMEOUT gets reported ensure that the error message mentiones
> timeout.
> 
> Signed-off-by: Andreas Bergmeier <abergmeier@gmx.net>
> ---
>  drivers/hid/hid-logitech-hidpp.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 20ae7f73ef08..a0c148a8df6c 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -4438,12 +4438,16 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  	if (hidpp->quirks & HIDPP_QUIRK_UNIFYING)
>  		hidpp_unifying_init(hidpp);
> 
> -	connected = hidpp_root_get_protocol_version(hidpp) == 0;
> +	ret = hidpp_root_get_protocol_version(hidpp);
> +	connected = ret == 0;
>  	atomic_set(&hidpp->connected, connected);
>  	if (!(hidpp->quirks & HIDPP_QUIRK_UNIFYING)) {
>  		if (!connected) {
> +			if (ret == -ETIMEDOUT)
> +				hid_err(hdev, "Device connection timed out");
> +			else
> +				hid_err(hdev, "Device not connected");
>  			ret = -ENODEV;
> -			hid_err(hdev, "Device not connected");

Printing as specific error as possible makes sense to me.

I am however wondering whether the better aproach (covering all the cases, 
not just ETIMEDOUT specifically) wouldn't be to convert all the dbg_hid() 
in hidpp_send_message_sync() to be proper error messages instead? That 
would cover also your case, as ETIMEDOUT is handled there as well with 
debugging message.

Thanks,
diff mbox series

Patch

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 20ae7f73ef08..a0c148a8df6c 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4438,12 +4438,16 @@  static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	if (hidpp->quirks & HIDPP_QUIRK_UNIFYING)
 		hidpp_unifying_init(hidpp);

-	connected = hidpp_root_get_protocol_version(hidpp) == 0;
+	ret = hidpp_root_get_protocol_version(hidpp);
+	connected = ret == 0;
 	atomic_set(&hidpp->connected, connected);
 	if (!(hidpp->quirks & HIDPP_QUIRK_UNIFYING)) {
 		if (!connected) {
+			if (ret == -ETIMEDOUT)
+				hid_err(hdev, "Device connection timed out");
+			else
+				hid_err(hdev, "Device not connected");
 			ret = -ENODEV;
-			hid_err(hdev, "Device not connected");
 			goto hid_hw_init_fail;
 		}