| Message ID | 1475283026-2256-2-git-send-email-roderick@gaikai.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sep 30 2016 or thereabouts, roderick@gaikai.com wrote: > From: Roderick Colenbrander <roderick.colenbrander@sony.com> > > Put the report type (feature / output) in the report size definitions. > This prevents name collisions later on for other reports, which use > the same report id. > > Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com> I am not against such a patch, but I'd rather see the "later" name collision in the same series. From the look of it, it seems you are just renaming constants for the beauty of it, and this might just complexifies backports and git blames. Please keep this one on hold until the actual collision happens. Jiri might have an other opinion, but I can't find a good reason to take this patch. BTW, glad to see people with a @sony.com email address :) Cheers, Benjamin > --- > drivers/hid/hid-sony.c | 30 +++++++++++++++--------------- > 1 file changed, 15 insertions(+), 15 deletions(-) > > diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c > index 189c100..627a785 100644 > --- a/drivers/hid/hid-sony.c > +++ b/drivers/hid/hid-sony.c > @@ -1018,10 +1018,10 @@ struct motion_output_report_02 { > u8 rumble; > }; > > -#define DS4_REPORT_0x02_SIZE 37 > -#define DS4_REPORT_0x05_SIZE 32 > -#define DS4_REPORT_0x11_SIZE 78 > -#define DS4_REPORT_0x81_SIZE 7 > +#define DS4_FEATURE_REPORT_0x02_SIZE 37 > +#define DS4_FEATURE_REPORT_0x81_SIZE 7 > +#define DS4_OUTPUT_REPORT_0x05_SIZE 32 > +#define DS4_OUTPUT_REPORT_0x11_SIZE 78 > #define SIXAXIS_REPORT_0xF2_SIZE 17 > #define SIXAXIS_REPORT_0xF5_SIZE 8 > #define MOTION_REPORT_0x02_SIZE 49 > @@ -1460,11 +1460,11 @@ static int dualshock4_set_operational_bt(struct hid_device *hdev) > u8 *buf; > int ret; > > - buf = kmalloc(DS4_REPORT_0x02_SIZE, GFP_KERNEL); > + buf = kmalloc(DS4_FEATURE_REPORT_0x02_SIZE, GFP_KERNEL); > if (!buf) > return -ENOMEM; > > - ret = hid_hw_raw_request(hdev, 0x02, buf, DS4_REPORT_0x02_SIZE, > + ret = hid_hw_raw_request(hdev, 0x02, buf, DS4_FEATURE_REPORT_0x02_SIZE, > HID_FEATURE_REPORT, HID_REQ_GET_REPORT); > > kfree(buf); > @@ -1869,12 +1869,12 @@ static void dualshock4_send_output_report(struct sony_sc *sc) > * 0xD0 - 66hz > */ > if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) { > - memset(buf, 0, DS4_REPORT_0x05_SIZE); > + memset(buf, 0, DS4_OUTPUT_REPORT_0x05_SIZE); > buf[0] = 0x05; > buf[1] = 0xFF; > offset = 4; > } else { > - memset(buf, 0, DS4_REPORT_0x11_SIZE); > + memset(buf, 0, DS4_OUTPUT_REPORT_0x11_SIZE); > buf[0] = 0x11; > buf[1] = 0x80; > buf[3] = 0x0F; > @@ -1902,9 +1902,9 @@ static void dualshock4_send_output_report(struct sony_sc *sc) > buf[offset++] = sc->led_delay_off[3]; > > if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) > - hid_hw_output_report(hdev, buf, DS4_REPORT_0x05_SIZE); > + hid_hw_output_report(hdev, buf, DS4_OUTPUT_REPORT_0x05_SIZE); > else > - hid_hw_raw_request(hdev, 0x11, buf, DS4_REPORT_0x11_SIZE, > + hid_hw_raw_request(hdev, 0x11, buf, DS4_OUTPUT_REPORT_0x11_SIZE, > HID_OUTPUT_REPORT, HID_REQ_SET_REPORT); > } > > @@ -1949,10 +1949,10 @@ static int sony_allocate_output_report(struct sony_sc *sc) > kmalloc(sizeof(union sixaxis_output_report_01), > GFP_KERNEL); > else if (sc->quirks & DUALSHOCK4_CONTROLLER_BT) > - sc->output_report_dmabuf = kmalloc(DS4_REPORT_0x11_SIZE, > + sc->output_report_dmabuf = kmalloc(DS4_OUTPUT_REPORT_0x11_SIZE, > GFP_KERNEL); > else if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) > - sc->output_report_dmabuf = kmalloc(DS4_REPORT_0x05_SIZE, > + sc->output_report_dmabuf = kmalloc(DS4_OUTPUT_REPORT_0x05_SIZE, > GFP_KERNEL); > else if (sc->quirks & MOTION_CONTROLLER) > sc->output_report_dmabuf = kmalloc(MOTION_REPORT_0x02_SIZE, > @@ -2197,7 +2197,7 @@ static int sony_check_add(struct sony_sc *sc) > return 0; > } > } else if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) { > - buf = kmalloc(DS4_REPORT_0x81_SIZE, GFP_KERNEL); > + buf = kmalloc(DS4_FEATURE_REPORT_0x81_SIZE, GFP_KERNEL); > if (!buf) > return -ENOMEM; > > @@ -2207,10 +2207,10 @@ static int sony_check_add(struct sony_sc *sc) > * offset 1. > */ > ret = hid_hw_raw_request(sc->hdev, 0x81, buf, > - DS4_REPORT_0x81_SIZE, HID_FEATURE_REPORT, > + DS4_FEATURE_REPORT_0x81_SIZE, HID_FEATURE_REPORT, > HID_REQ_GET_REPORT); > > - if (ret != DS4_REPORT_0x81_SIZE) { > + if (ret != DS4_FEATURE_REPORT_0x81_SIZE) { > hid_err(sc->hdev, "failed to retrieve feature report 0x81 with the DualShock 4 MAC address\n"); > ret = ret < 0 ? ret : -EINVAL; > goto out_free; > -- > 2.7.4 > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 3, 2016 at 7:12 AM, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote: > On Sep 30 2016 or thereabouts, roderick@gaikai.com wrote: >> From: Roderick Colenbrander <roderick.colenbrander@sony.com> >> >> Put the report type (feature / output) in the report size definitions. >> This prevents name collisions later on for other reports, which use >> the same report id. >> >> Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com> > > I am not against such a patch, but I'd rather see the "later" name > collision in the same series. From the look of it, it seems you are just > renaming constants for the beauty of it, and this might just > complexifies backports and git blames. > > Please keep this one on hold until the actual collision happens. > Jiri might have an other opinion, but I can't find a good reason to > take this patch. > > BTW, glad to see people with a @sony.com email address :) > > Cheers, > Benjamin > Thanks for the review. I'm seeing if I can release one of the patches causing a collision. Just double checking now. If I can, I will send this series again with some additional patches. Thanks, Roderick >> --- >> drivers/hid/hid-sony.c | 30 +++++++++++++++--------------- >> 1 file changed, 15 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c >> index 189c100..627a785 100644 >> --- a/drivers/hid/hid-sony.c >> +++ b/drivers/hid/hid-sony.c >> @@ -1018,10 +1018,10 @@ struct motion_output_report_02 { >> u8 rumble; >> }; >> >> -#define DS4_REPORT_0x02_SIZE 37 >> -#define DS4_REPORT_0x05_SIZE 32 >> -#define DS4_REPORT_0x11_SIZE 78 >> -#define DS4_REPORT_0x81_SIZE 7 >> +#define DS4_FEATURE_REPORT_0x02_SIZE 37 >> +#define DS4_FEATURE_REPORT_0x81_SIZE 7 >> +#define DS4_OUTPUT_REPORT_0x05_SIZE 32 >> +#define DS4_OUTPUT_REPORT_0x11_SIZE 78 >> #define SIXAXIS_REPORT_0xF2_SIZE 17 >> #define SIXAXIS_REPORT_0xF5_SIZE 8 >> #define MOTION_REPORT_0x02_SIZE 49 >> @@ -1460,11 +1460,11 @@ static int dualshock4_set_operational_bt(struct hid_device *hdev) >> u8 *buf; >> int ret; >> >> - buf = kmalloc(DS4_REPORT_0x02_SIZE, GFP_KERNEL); >> + buf = kmalloc(DS4_FEATURE_REPORT_0x02_SIZE, GFP_KERNEL); >> if (!buf) >> return -ENOMEM; >> >> - ret = hid_hw_raw_request(hdev, 0x02, buf, DS4_REPORT_0x02_SIZE, >> + ret = hid_hw_raw_request(hdev, 0x02, buf, DS4_FEATURE_REPORT_0x02_SIZE, >> HID_FEATURE_REPORT, HID_REQ_GET_REPORT); >> >> kfree(buf); >> @@ -1869,12 +1869,12 @@ static void dualshock4_send_output_report(struct sony_sc *sc) >> * 0xD0 - 66hz >> */ >> if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) { >> - memset(buf, 0, DS4_REPORT_0x05_SIZE); >> + memset(buf, 0, DS4_OUTPUT_REPORT_0x05_SIZE); >> buf[0] = 0x05; >> buf[1] = 0xFF; >> offset = 4; >> } else { >> - memset(buf, 0, DS4_REPORT_0x11_SIZE); >> + memset(buf, 0, DS4_OUTPUT_REPORT_0x11_SIZE); >> buf[0] = 0x11; >> buf[1] = 0x80; >> buf[3] = 0x0F; >> @@ -1902,9 +1902,9 @@ static void dualshock4_send_output_report(struct sony_sc *sc) >> buf[offset++] = sc->led_delay_off[3]; >> >> if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) >> - hid_hw_output_report(hdev, buf, DS4_REPORT_0x05_SIZE); >> + hid_hw_output_report(hdev, buf, DS4_OUTPUT_REPORT_0x05_SIZE); >> else >> - hid_hw_raw_request(hdev, 0x11, buf, DS4_REPORT_0x11_SIZE, >> + hid_hw_raw_request(hdev, 0x11, buf, DS4_OUTPUT_REPORT_0x11_SIZE, >> HID_OUTPUT_REPORT, HID_REQ_SET_REPORT); >> } >> >> @@ -1949,10 +1949,10 @@ static int sony_allocate_output_report(struct sony_sc *sc) >> kmalloc(sizeof(union sixaxis_output_report_01), >> GFP_KERNEL); >> else if (sc->quirks & DUALSHOCK4_CONTROLLER_BT) >> - sc->output_report_dmabuf = kmalloc(DS4_REPORT_0x11_SIZE, >> + sc->output_report_dmabuf = kmalloc(DS4_OUTPUT_REPORT_0x11_SIZE, >> GFP_KERNEL); >> else if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) >> - sc->output_report_dmabuf = kmalloc(DS4_REPORT_0x05_SIZE, >> + sc->output_report_dmabuf = kmalloc(DS4_OUTPUT_REPORT_0x05_SIZE, >> GFP_KERNEL); >> else if (sc->quirks & MOTION_CONTROLLER) >> sc->output_report_dmabuf = kmalloc(MOTION_REPORT_0x02_SIZE, >> @@ -2197,7 +2197,7 @@ static int sony_check_add(struct sony_sc *sc) >> return 0; >> } >> } else if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) { >> - buf = kmalloc(DS4_REPORT_0x81_SIZE, GFP_KERNEL); >> + buf = kmalloc(DS4_FEATURE_REPORT_0x81_SIZE, GFP_KERNEL); >> if (!buf) >> return -ENOMEM; >> >> @@ -2207,10 +2207,10 @@ static int sony_check_add(struct sony_sc *sc) >> * offset 1. >> */ >> ret = hid_hw_raw_request(sc->hdev, 0x81, buf, >> - DS4_REPORT_0x81_SIZE, HID_FEATURE_REPORT, >> + DS4_FEATURE_REPORT_0x81_SIZE, HID_FEATURE_REPORT, >> HID_REQ_GET_REPORT); >> >> - if (ret != DS4_REPORT_0x81_SIZE) { >> + if (ret != DS4_FEATURE_REPORT_0x81_SIZE) { >> hid_err(sc->hdev, "failed to retrieve feature report 0x81 with the DualShock 4 MAC address\n"); >> ret = ret < 0 ? ret : -EINVAL; >> goto out_free; >> -- >> 2.7.4 >> -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c index 189c100..627a785 100644 --- a/drivers/hid/hid-sony.c +++ b/drivers/hid/hid-sony.c @@ -1018,10 +1018,10 @@ struct motion_output_report_02 { u8 rumble; }; -#define DS4_REPORT_0x02_SIZE 37 -#define DS4_REPORT_0x05_SIZE 32 -#define DS4_REPORT_0x11_SIZE 78 -#define DS4_REPORT_0x81_SIZE 7 +#define DS4_FEATURE_REPORT_0x02_SIZE 37 +#define DS4_FEATURE_REPORT_0x81_SIZE 7 +#define DS4_OUTPUT_REPORT_0x05_SIZE 32 +#define DS4_OUTPUT_REPORT_0x11_SIZE 78 #define SIXAXIS_REPORT_0xF2_SIZE 17 #define SIXAXIS_REPORT_0xF5_SIZE 8 #define MOTION_REPORT_0x02_SIZE 49 @@ -1460,11 +1460,11 @@ static int dualshock4_set_operational_bt(struct hid_device *hdev) u8 *buf; int ret; - buf = kmalloc(DS4_REPORT_0x02_SIZE, GFP_KERNEL); + buf = kmalloc(DS4_FEATURE_REPORT_0x02_SIZE, GFP_KERNEL); if (!buf) return -ENOMEM; - ret = hid_hw_raw_request(hdev, 0x02, buf, DS4_REPORT_0x02_SIZE, + ret = hid_hw_raw_request(hdev, 0x02, buf, DS4_FEATURE_REPORT_0x02_SIZE, HID_FEATURE_REPORT, HID_REQ_GET_REPORT); kfree(buf); @@ -1869,12 +1869,12 @@ static void dualshock4_send_output_report(struct sony_sc *sc) * 0xD0 - 66hz */ if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) { - memset(buf, 0, DS4_REPORT_0x05_SIZE); + memset(buf, 0, DS4_OUTPUT_REPORT_0x05_SIZE); buf[0] = 0x05; buf[1] = 0xFF; offset = 4; } else { - memset(buf, 0, DS4_REPORT_0x11_SIZE); + memset(buf, 0, DS4_OUTPUT_REPORT_0x11_SIZE); buf[0] = 0x11; buf[1] = 0x80; buf[3] = 0x0F; @@ -1902,9 +1902,9 @@ static void dualshock4_send_output_report(struct sony_sc *sc) buf[offset++] = sc->led_delay_off[3]; if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) - hid_hw_output_report(hdev, buf, DS4_REPORT_0x05_SIZE); + hid_hw_output_report(hdev, buf, DS4_OUTPUT_REPORT_0x05_SIZE); else - hid_hw_raw_request(hdev, 0x11, buf, DS4_REPORT_0x11_SIZE, + hid_hw_raw_request(hdev, 0x11, buf, DS4_OUTPUT_REPORT_0x11_SIZE, HID_OUTPUT_REPORT, HID_REQ_SET_REPORT); } @@ -1949,10 +1949,10 @@ static int sony_allocate_output_report(struct sony_sc *sc) kmalloc(sizeof(union sixaxis_output_report_01), GFP_KERNEL); else if (sc->quirks & DUALSHOCK4_CONTROLLER_BT) - sc->output_report_dmabuf = kmalloc(DS4_REPORT_0x11_SIZE, + sc->output_report_dmabuf = kmalloc(DS4_OUTPUT_REPORT_0x11_SIZE, GFP_KERNEL); else if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) - sc->output_report_dmabuf = kmalloc(DS4_REPORT_0x05_SIZE, + sc->output_report_dmabuf = kmalloc(DS4_OUTPUT_REPORT_0x05_SIZE, GFP_KERNEL); else if (sc->quirks & MOTION_CONTROLLER) sc->output_report_dmabuf = kmalloc(MOTION_REPORT_0x02_SIZE, @@ -2197,7 +2197,7 @@ static int sony_check_add(struct sony_sc *sc) return 0; } } else if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) { - buf = kmalloc(DS4_REPORT_0x81_SIZE, GFP_KERNEL); + buf = kmalloc(DS4_FEATURE_REPORT_0x81_SIZE, GFP_KERNEL); if (!buf) return -ENOMEM; @@ -2207,10 +2207,10 @@ static int sony_check_add(struct sony_sc *sc) * offset 1. */ ret = hid_hw_raw_request(sc->hdev, 0x81, buf, - DS4_REPORT_0x81_SIZE, HID_FEATURE_REPORT, + DS4_FEATURE_REPORT_0x81_SIZE, HID_FEATURE_REPORT, HID_REQ_GET_REPORT); - if (ret != DS4_REPORT_0x81_SIZE) { + if (ret != DS4_FEATURE_REPORT_0x81_SIZE) { hid_err(sc->hdev, "failed to retrieve feature report 0x81 with the DualShock 4 MAC address\n"); ret = ret < 0 ? ret : -EINVAL; goto out_free;