On Mon, Oct 3, 2016 at 7:12 AM, Benjamin Tissoires
<benjamin.tissoires@xxxxxxxxxx> wrote:
> On Sep 30 2016 or thereabouts, roderick@xxxxxxxxxx wrote:
>> From: Roderick Colenbrander <roderick.colenbrander@xxxxxxxx>
>>
>> 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@xxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
