Re: [PATCH 13/14] HID: lg: constify fixed up report descriptor

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



As you can see in the b4 reply, I've now applied all of the patches but
this one. Please see below.

On Aug 28 2024, Thomas Weißschuh wrote:
> Now that the HID core can handle const report descriptors,
> constify them where possible.
> 
> Signed-off-by: Thomas Weißschuh <linux@xxxxxxxxxxxxxx>
> ---
>  drivers/hid/hid-lg.c | 31 +++++++++++++++++--------------
>  1 file changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c
> index a9be918e2b5c..c1feeb1dd077 100644
> --- a/drivers/hid/hid-lg.c
> +++ b/drivers/hid/hid-lg.c
> @@ -58,7 +58,7 @@
>   * These descriptors remove the combined Y axis and instead report
>   * separate throttle (Y) and brake (RZ).
>   */
> -static __u8 df_rdesc_fixed[] = {
> +static const __u8 df_rdesc_fixed[] = {
>  0x05, 0x01,         /*  Usage Page (Desktop),                   */
>  0x09, 0x04,         /*  Usage (Joystick),                       */
>  0xA1, 0x01,         /*  Collection (Application),               */
> @@ -124,7 +124,7 @@ static __u8 df_rdesc_fixed[] = {
>  0xC0                /*  End Collection                          */
>  };
>  
> -static __u8 dfp_rdesc_fixed[] = {
> +static const __u8 dfp_rdesc_fixed[] = {
>  0x05, 0x01,         /*  Usage Page (Desktop),                   */
>  0x09, 0x04,         /*  Usage (Joystick),                       */
>  0xA1, 0x01,         /*  Collection (Application),               */
> @@ -172,7 +172,7 @@ static __u8 dfp_rdesc_fixed[] = {
>  0xC0                /*  End Collection                          */
>  };
>  
> -static __u8 fv_rdesc_fixed[] = {
> +static const __u8 fv_rdesc_fixed[] = {
>  0x05, 0x01,         /*  Usage Page (Desktop),                   */
>  0x09, 0x04,         /*  Usage (Joystick),                       */
>  0xA1, 0x01,         /*  Collection (Application),               */
> @@ -239,7 +239,7 @@ static __u8 fv_rdesc_fixed[] = {
>  0xC0                /*  End Collection                          */
>  };
>  
> -static __u8 momo_rdesc_fixed[] = {
> +static const __u8 momo_rdesc_fixed[] = {
>  0x05, 0x01,         /*  Usage Page (Desktop),               */
>  0x09, 0x04,         /*  Usage (Joystick),                   */
>  0xA1, 0x01,         /*  Collection (Application),           */
> @@ -285,7 +285,7 @@ static __u8 momo_rdesc_fixed[] = {
>  0xC0                /*  End Collection                      */
>  };
>  
> -static __u8 momo2_rdesc_fixed[] = {
> +static const __u8 momo2_rdesc_fixed[] = {
>  0x05, 0x01,         /*  Usage Page (Desktop),               */
>  0x09, 0x04,         /*  Usage (Joystick),                   */
>  0xA1, 0x01,         /*  Collection (Application),           */
> @@ -333,7 +333,7 @@ static __u8 momo2_rdesc_fixed[] = {
>  0xC0                /*  End Collection                      */
>  };
>  
> -static __u8 ffg_rdesc_fixed[] = {
> +static const __u8 ffg_rdesc_fixed[] = {
>  0x05, 0x01,         /*  Usage Page (Desktop),               */
>  0x09, 0x04,         /*  Usage (Joystik),                    */
>  0xA1, 0x01,         /*  Collection (Application),           */
> @@ -379,7 +379,7 @@ static __u8 ffg_rdesc_fixed[] = {
>  0xC0                /*  End Collection                      */
>  };
>  
> -static __u8 fg_rdesc_fixed[] = {
> +static const __u8 fg_rdesc_fixed[] = {
>  0x05, 0x01,         /*  Usage Page (Desktop),               */
>  0x09, 0x04,         /*  Usage (Joystik),                    */
>  0xA1, 0x01,         /*  Collection (Application),           */
> @@ -431,6 +431,7 @@ static const __u8 *lg_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  		unsigned int *rsize)
>  {
>  	struct lg_drv_data *drv_data = hid_get_drvdata(hdev);
> +	const __u8 *ret = NULL;

Not really happy about this, usually "ret" is an int, and this makes
things slightly harder to read.

>  
>  	if ((drv_data->quirks & LG_RDESC) && *rsize >= 91 && rdesc[83] == 0x26 &&
>  			rdesc[84] == 0x8c && rdesc[85] == 0x02) {
> @@ -453,7 +454,7 @@ static const __u8 *lg_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  		if (*rsize == FG_RDESC_ORIG_SIZE) {
>  			hid_info(hdev,
>  				"fixing up Logitech Wingman Formula GP report descriptor\n");
> -			rdesc = fg_rdesc_fixed;
> +			ret = fg_rdesc_fixed;

can't you just return fg_rdesc_fixed after setting *rsize, like you did
in the other patches?

(same for all of the other branches)

>  			*rsize = sizeof(fg_rdesc_fixed);
>  		} else {
>  			hid_info(hdev,
> @@ -466,7 +467,7 @@ static const __u8 *lg_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  		if (*rsize == FFG_RDESC_ORIG_SIZE) {
>  			hid_info(hdev,
>  				"fixing up Logitech Wingman Formula Force GP report descriptor\n");
> -			rdesc = ffg_rdesc_fixed;
> +			ret = ffg_rdesc_fixed;
>  			*rsize = sizeof(ffg_rdesc_fixed);
>  		}
>  		break;
> @@ -476,7 +477,7 @@ static const __u8 *lg_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  		if (*rsize == DF_RDESC_ORIG_SIZE) {
>  			hid_info(hdev,
>  				"fixing up Logitech Driving Force report descriptor\n");
> -			rdesc = df_rdesc_fixed;
> +			ret = df_rdesc_fixed;
>  			*rsize = sizeof(df_rdesc_fixed);
>  		}
>  		break;
> @@ -485,7 +486,7 @@ static const __u8 *lg_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  		if (*rsize == MOMO_RDESC_ORIG_SIZE) {
>  			hid_info(hdev,
>  				"fixing up Logitech Momo Force (Red) report descriptor\n");
> -			rdesc = momo_rdesc_fixed;
> +			ret = momo_rdesc_fixed;
>  			*rsize = sizeof(momo_rdesc_fixed);
>  		}
>  		break;
> @@ -494,7 +495,7 @@ static const __u8 *lg_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  		if (*rsize == MOMO2_RDESC_ORIG_SIZE) {
>  			hid_info(hdev,
>  				"fixing up Logitech Momo Racing Force (Black) report descriptor\n");
> -			rdesc = momo2_rdesc_fixed;
> +			ret = momo2_rdesc_fixed;
>  			*rsize = sizeof(momo2_rdesc_fixed);
>  		}
>  		break;
> @@ -503,7 +504,7 @@ static const __u8 *lg_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  		if (*rsize == FV_RDESC_ORIG_SIZE) {
>  			hid_info(hdev,
>  				"fixing up Logitech Formula Vibration report descriptor\n");
> -			rdesc = fv_rdesc_fixed;
> +			ret = fv_rdesc_fixed;
>  			*rsize = sizeof(fv_rdesc_fixed);
>  		}
>  		break;
> @@ -512,7 +513,7 @@ static const __u8 *lg_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  		if (*rsize == DFP_RDESC_ORIG_SIZE) {
>  			hid_info(hdev,
>  				"fixing up Logitech Driving Force Pro report descriptor\n");
> -			rdesc = dfp_rdesc_fixed;
> +			ret = dfp_rdesc_fixed;
>  			*rsize = sizeof(dfp_rdesc_fixed);
>  		}
>  		break;
> @@ -529,6 +530,8 @@ static const __u8 *lg_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  		break;
>  	}
>  
> +	if (ret)
> +		return ret;
>  	return rdesc;
>  }
>  
> 
> -- 
> 2.46.0
> 

Cheers,
Benjamin




[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux