Re: [PATCH v3 2/6] platform/x86/tuxedo: Set HID report ID on success

[Werner Sembach <wse@tuxedocomputers.com>]

Am 26.08.26 um 10:11 schrieb Aaron Erhardt:
> While not strictly necessary due to the synchronous handling of HID
> requests, it is better to set the ID of the HID report before
> returning the buffer. This also better complies with the HID spec and
> avoids problems with userspace tools expecting a report ID.
>
> Signed-off-by: Aaron Erhardt <aer@tuxedocomputers.com>
Reviewed-by: Werner Sembach <wse@tuxedocomputers.com>
> ---
>   drivers/platform/x86/tuxedo/nb04/wmi_ab.c | 26 +++++++++++++++++------
>   1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/platform/x86/tuxedo/nb04/wmi_ab.c b/drivers/platform/x86/tuxedo/nb04/wmi_ab.c
> index 72205de72256..11babc7c7767 100644
> --- a/drivers/platform/x86/tuxedo/nb04/wmi_ab.c
> +++ b/drivers/platform/x86/tuxedo/nb04/wmi_ab.c
> @@ -766,6 +766,8 @@ static int handle_lamp_array_control_report(struct hid_device *hdev __always_unu
>   static int tux_ll_raw_request(struct hid_device *hdev, u8 reportnum, u8 *buf,
>   			      size_t len, unsigned char rtype, int reqtype)
>   {
> +	int ret = -EINVAL;
> +
>   	if (rtype != HID_FEATURE_REPORT)
>   		return -EINVAL;
>   
> @@ -775,13 +777,15 @@ static int tux_ll_raw_request(struct hid_device *hdev, u8 reportnum, u8 *buf,
>   		case LAMP_ARRAY_ATTRIBUTES_REPORT_ID:
>   			if (len != sizeof(struct lamp_array_attributes_report_t))
>   				return -EINVAL;
> -			return handle_lamp_array_attributes_report(hdev,
> +			ret = handle_lamp_array_attributes_report(hdev,
>   				(struct lamp_array_attributes_report_t *)buf);
> +			break;
>   		case LAMP_ATTRIBUTES_RESPONSE_REPORT_ID:
>   			if (len != sizeof(struct lamp_attributes_response_report_t))
>   				return -EINVAL;
> -			return handle_lamp_attributes_response_report(hdev,
> +			ret = handle_lamp_attributes_response_report(hdev,
>   				(struct lamp_attributes_response_report_t *)buf);
> +			break;
>   		}
>   		break;
>   	case HID_REQ_SET_REPORT:
> @@ -789,28 +793,36 @@ static int tux_ll_raw_request(struct hid_device *hdev, u8 reportnum, u8 *buf,
>   		case LAMP_ATTRIBUTES_REQUEST_REPORT_ID:
>   			if (len != sizeof(struct lamp_attributes_request_report_t))
>   				return -EINVAL;
> -			return handle_lamp_attributes_request_report(hdev,
> +			ret = handle_lamp_attributes_request_report(hdev,
>   				(struct lamp_attributes_request_report_t *)buf);
> +			break;
>   		case LAMP_MULTI_UPDATE_REPORT_ID:
>   			if (len != sizeof(struct lamp_multi_update_report_t))
>   				return -EINVAL;
> -			return handle_lamp_multi_update_report(hdev,
> +			ret = handle_lamp_multi_update_report(hdev,
>   				(struct lamp_multi_update_report_t *)buf);
> +			break;
>   		case LAMP_RANGE_UPDATE_REPORT_ID:
>   			if (len != sizeof(struct lamp_range_update_report_t))
>   				return -EINVAL;
> -			return handle_lamp_range_update_report(hdev,
> +			ret = handle_lamp_range_update_report(hdev,
>   				(struct lamp_range_update_report_t *)buf);
> +			break;
>   		case LAMP_ARRAY_CONTROL_REPORT_ID:
>   			if (len != sizeof(struct lamp_array_control_report_t))
>   				return -EINVAL;
> -			return handle_lamp_array_control_report(hdev,
> +			ret = handle_lamp_array_control_report(hdev,
>   				(struct lamp_array_control_report_t *)buf);
> +			break;
>   		}
>   		break;
>   	}
>   
> -	return -EINVAL;
> +	/* Set report number on success */
> +	if (ret > 0)
> +		buf[0] = reportnum;
> +
> +	return ret;
>   }
>   
>   static const struct hid_ll_driver tux_ll_driver = {