diff mbox series

PATCH: Simplify hidpp_send_rap_command_sync calls

Message ID 4b871484-b19-95b8-f973-bfebcb5348ab@9300
State Superseded
Headers show
Series PATCH: Simplify hidpp_send_rap_command_sync calls | expand

Commit Message

Andreas Bergmeier Nov. 10, 2022, 2:47 p.m. UTC 
Inside function, report_id might get overwritten.
Only REPORT_ID_HIDPP_SHORT is ever passed in.
So there seems to be no point in passing report_id in the first place.
Just directly evaluate which report_id to use in the function itself.

Signed-off-by: Andreas Bergmeier <abergmeier@gmx.net>

 		report_id = REPORT_ID_HIDPP_LONG;

 	switch (report_id) {
@@ -549,7 +550,6 @@ static int hidpp10_set_register(struct hidpp_device
*hidpp_dev,
 	u8 params[3] = { 0 };

 	ret = hidpp_send_rap_command_sync(hidpp_dev,
-					  REPORT_ID_HIDPP_SHORT,
 					  HIDPP_GET_REGISTER,
 					  register_address,
 					  NULL, 0, &response);
@@ -562,7 +562,6 @@ static int hidpp10_set_register(struct hidpp_device
*hidpp_dev,
 	params[byte] |= value & mask;

 	return hidpp_send_rap_command_sync(hidpp_dev,
-					   REPORT_ID_HIDPP_SHORT,
 					   HIDPP_SET_REGISTER,
 					   register_address,
 					   params, 3, &response);
@@ -658,7 +657,6 @@ static int hidpp10_query_battery_status(struct
hidpp_device *hidpp)
 	int ret, status;

 	ret = hidpp_send_rap_command_sync(hidpp,
-					REPORT_ID_HIDPP_SHORT,
 					HIDPP_GET_REGISTER,
 					HIDPP_REG_BATTERY_STATUS,
 					NULL, 0, &response);
@@ -710,7 +708,6 @@ static int hidpp10_query_battery_mileage(struct
hidpp_device *hidpp)
 	int ret, status;

 	ret = hidpp_send_rap_command_sync(hidpp,
-					REPORT_ID_HIDPP_SHORT,
 					HIDPP_GET_REGISTER,
 					HIDPP_REG_BATTERY_MILEAGE,
 					NULL, 0, &response);
@@ -782,7 +779,6 @@ static char *hidpp_unifying_get_name(struct
hidpp_device *hidpp_dev)
 	int len;

 	ret = hidpp_send_rap_command_sync(hidpp_dev,
-					REPORT_ID_HIDPP_SHORT,
 					HIDPP_GET_LONG_REGISTER,
 					HIDPP_REG_PAIRING_INFORMATION,
 					params, 1, &response);
@@ -816,7 +812,6 @@ static int hidpp_unifying_get_serial(struct
hidpp_device *hidpp, u32 *serial)
 	u8 params[1] = { HIDPP_EXTENDED_PAIRING };

 	ret = hidpp_send_rap_command_sync(hidpp,
-					REPORT_ID_HIDPP_SHORT,
 					HIDPP_GET_LONG_REGISTER,
 					HIDPP_REG_PAIRING_INFORMATION,
 					params, 1, &response);
@@ -900,7 +895,6 @@ static int hidpp_root_get_protocol_version(struct
hidpp_device *hidpp)
 	int ret;

 	ret = hidpp_send_rap_command_sync(hidpp,
-			REPORT_ID_HIDPP_SHORT,
 			HIDPP_PAGE_ROOT_IDX,
 			CMD_ROOT_GET_PROTOCOL_VERSION,
 			ping_data, sizeof(ping_data), &response);
@@ -3180,7 +3174,6 @@ static int m560_send_config_command(struct
hid_device *hdev, bool connected)

 	return hidpp_send_rap_command_sync(
 		hidpp_dev,
-		REPORT_ID_HIDPP_SHORT,
 		M560_SUB_ID,
 		M560_BUTTON_MODE_REGISTER,
 		(u8 *)m560_config_parameter,
@@ -3719,7 +3712,6 @@ static int hidpp_initialize_hires_scroll(struct
hidpp_device *hidpp)
 		struct hidpp_report response;

 		ret = hidpp_send_rap_command_sync(hidpp,
-						  REPORT_ID_HIDPP_SHORT,
 						  HIDPP_GET_REGISTER,

HIDPP_ENABLE_FAST_SCROLL,
 						  NULL, 0, &response);

Comments

Bastien Nocera Nov. 14, 2022, 2:20 p.m. UTC | #1 
On Thu, 2022-11-10 at 15:47 +0100, Andreas Bergmeier wrote:
> Inside function, report_id might get overwritten.
> Only REPORT_ID_HIDPP_SHORT is ever passed in.
> So there seems to be no point in passing report_id in the first
> place.
> Just directly evaluate which report_id to use in the function itself.

Looks good.

Reviewed-by: Bastien Nocera <hadess@hadess.net>
diff mbox series

Patch

diff --git a/drivers/hid/hid-logitech-hidpp.c
b/drivers/hid/hid-logitech-hidpp.c
index 898691a77a58..20ae7f73ef08 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -360,15 +360,16 @@  static int hidpp_send_fap_command_sync(struct
hidpp_device *hidpp,
 }

 static int hidpp_send_rap_command_sync(struct hidpp_device *hidpp_dev,
-	u8 report_id, u8 sub_id, u8 reg_address, u8 *params, int
param_count,
+	u8 sub_id, u8 reg_address, u8 *params, int param_count,
 	struct hidpp_report *response)
 {
 	struct hidpp_report *message;
 	int ret, max_count;
+	u8 report_id;

-	/* Send as long report if short reports are not supported. */
-	if (report_id == REPORT_ID_HIDPP_SHORT &&
-	    !(hidpp_dev->supported_reports &
HIDPP_REPORT_SHORT_SUPPORTED))
+	if (hidpp_dev->supported_reports & HIDPP_REPORT_SHORT_SUPPORTED)
+		report_id = REPORT_ID_HIDPP_SHORT;
+	else