| Message ID | 20240828-hid-const-fixup-2-v1-13-663b9210eb69@weissschuh.net (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | HID: constify fixed up report descriptors | expand |
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@weissschuh.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
On 2024-09-05 16:51:48+0000, Benjamin Tissoires wrote: > As you can see in the b4 reply, I've now applied all of the patches but > this one. Please see below. Thanks! > 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@weissschuh.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. Ack. > > > > > 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? The code looked like it wanted to avoid multiple return sites. I tried to preserve that style, but indeed it looks wonky. I'll resend it with the changes.
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; 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; *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; }
Now that the HID core can handle const report descriptors, constify them where possible. Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> --- drivers/hid/hid-lg.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-)