[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <66uyx73trgi4m5iybuee5ka4vbulh433o4jpqc4uu4teaflaex@av7xsfwjypy5>
Date: Thu, 5 Sep 2024 16:51:48 +0200
From: Benjamin Tissoires <bentiss@...nel.org>
To: Thomas Weißschuh <linux@...ssschuh.net>
Cc: Jiri Kosina <jikos@...nel.org>,
Marcus Folkesson <marcus.folkesson@...il.com>, linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 13/14] HID: lg: constify fixed up report descriptor
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@...ssschuh.net>
> ---
> 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
Powered by blists - more mailing lists