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@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.
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.
