* Re: [PATCH v2 3/6] HID: sony: Add Bluetooth output report formatting
From: David Herrmann @ 2014-02-03 16:29 UTC (permalink / raw)
To: Frank Praznik; +Cc: open list:HID CORE LAYER, Jiri Kosina
In-Reply-To: <1391102648-19381-4-git-send-email-frank.praznik@oh.rr.com>
Hi
On Thu, Jan 30, 2014 at 6:24 PM, Frank Praznik <frank.praznik@oh.rr.com> wrote:
> Add formatting and reporting for Dualshock 4 output reports on Bluetooth.
>
> Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
>
> ---
> drivers/hid/hid-sony.c | 29 +++++++++++++++++++++++------
> 1 file changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index 52162a9..4d12b4e 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -1268,11 +1268,18 @@ static void dualshock4_state_worker(struct work_struct *work)
> struct hid_device *hdev = sc->hdev;
> int offset;
>
> - __u8 buf[32] = { 0 };
> + __u8 buf[78] = { 0 };
>
> - buf[0] = 0x05;
> - buf[1] = 0x03;
> - offset = 4;
> + if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) {
> + buf[0] = 0x05;
> + buf[1] = 0x03;
> + offset = 4;
> + } else {
> + buf[0] = 0x11;
> + buf[1] = 0xB0;
> + buf[3] = 0x0F;
> + offset = 6;
> + }
>
> #ifdef CONFIG_SONY_FF
> buf[offset++] = sc->right;
> @@ -1285,7 +1292,11 @@ static void dualshock4_state_worker(struct work_struct *work)
> buf[offset++] = sc->led_state[1];
> buf[offset++] = sc->led_state[2];
>
> - hdev->ll_driver->output_report(hdev, buf, sizeof(buf));
> + if (sc->quirks & DUALSHOCK4_CONTROLLER_USB)
> + hdev->ll_driver->output_report(hdev, buf, 32);
> + else
> + hdev->ll_driver->raw_request(hdev, 0x11, buf, 78,
> + HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
Ok, now I get it. The USB stuff takes output-reports and BT uses
SET_REPORT. That's exactly the inverse of wiimotes.
> }
>
> #ifdef CONFIG_SONY_FF
> @@ -1482,10 +1493,16 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
> else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
> ret = sixaxis_set_operational_bt(hdev);
> else if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) {
> - if (hdev->ll_driver->output_report == NULL) {
> + if ((sc->quirks & DUALSHOCK4_CONTROLLER_USB) &&
> + hdev->ll_driver->output_report == NULL) {
> hid_err(hdev, "NULL output_report handler\n");
> ret = -EINVAL;
> goto err_stop;
> + } else if ((sc->quirks & DUALSHOCK4_CONTROLLER_BT) &&
> + hdev->ll_driver->raw_request == NULL) {
> + hid_err(hdev, "NULL raw_request handler\n");
> + ret = -EINVAL;
> + goto err_stop;
If you rebase your patches on Benjamins series, you can use
hid_hw_output_report() and hid_hw_raw_request() which already to the
NULL check. So you can drop this stuff here.
Thanks
David
> }
>
> /* The Dualshock 4 touchpad supports 2 touches and has a
> --
> 1.8.5.3
>
^ permalink raw reply
* Re: [PATCH v2 1/6] HID: sony: Use low-level transport driver functions
From: David Herrmann @ 2014-02-03 16:26 UTC (permalink / raw)
To: Frank Praznik; +Cc: open list:HID CORE LAYER, Jiri Kosina
In-Reply-To: <1391102648-19381-2-git-send-email-frank.praznik@oh.rr.com>
Hi
On Thu, Jan 30, 2014 at 6:24 PM, Frank Praznik <frank.praznik@oh.rr.com> wrote:
> From: Franz Frank Praznik <frank.praznik@oh.rr.com>
>
> Change the dualshock4_state_worker function to use the low-level output_report
> function.
>
> Remove sony_set_output_report since it is no longer used.
>
> Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
> ---
> drivers/hid/hid-sony.c | 58 ++++++++++++++++----------------------------------
> 1 file changed, 18 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index 2bd3f13..7a6d7c9 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -502,7 +502,6 @@ struct sony_sc {
> spinlock_t lock;
> struct hid_device *hdev;
> struct led_classdev *leds[MAX_LEDS];
> - struct hid_report *output_report;
> unsigned long quirks;
> struct work_struct state_worker;
> struct power_supply battery;
> @@ -1053,21 +1052,26 @@ static void dualshock4_state_worker(struct work_struct *work)
> {
> struct sony_sc *sc = container_of(work, struct sony_sc, state_worker);
> struct hid_device *hdev = sc->hdev;
> - struct hid_report *report = sc->output_report;
> - __s32 *value = report->field[0]->value;
> + int offset;
> +
> + __u8 buf[32] = { 0 };
>
> - value[0] = 0x03;
> + buf[0] = 0x05;
> + buf[1] = 0x03;
> + offset = 4;
>
> #ifdef CONFIG_SONY_FF
> - value[3] = sc->right;
> - value[4] = sc->left;
> + buf[offset++] = sc->right;
> + buf[offset++] = sc->left;
> +#else
> + offset += 2;
> #endif
>
> - value[5] = sc->led_state[0];
> - value[6] = sc->led_state[1];
> - value[7] = sc->led_state[2];
> + buf[offset++] = sc->led_state[0];
> + buf[offset++] = sc->led_state[1];
> + buf[offset++] = sc->led_state[2];
>
> - hid_hw_request(hdev, report, HID_REQ_SET_REPORT);
> + hdev->ll_driver->output_report(hdev, buf, sizeof(buf));
Ugh? You change SET_REPORT to output_report() here, which is not
supposed to be identical. Are you sure that works? A correct HID
device should accept both, but we're talking about gamepads here..
Thanks
David
> }
>
> #ifdef CONFIG_SONY_FF
> @@ -1200,33 +1204,6 @@ static void sony_battery_remove(struct sony_sc *sc)
> sc->battery.name = NULL;
> }
>
> -static int sony_set_output_report(struct sony_sc *sc, int req_id, int req_size)
> -{
> - struct list_head *head, *list;
> - struct hid_report *report;
> - struct hid_device *hdev = sc->hdev;
> -
> - list = &hdev->report_enum[HID_OUTPUT_REPORT].report_list;
> -
> - list_for_each(head, list) {
> - report = list_entry(head, struct hid_report, list);
> -
> - if (report->id == req_id) {
> - if (report->size < req_size) {
> - hid_err(hdev, "Output report 0x%02x (%i bits) is smaller than requested size (%i bits)\n",
> - req_id, report->size, req_size);
> - return -EINVAL;
> - }
> - sc->output_report = report;
> - return 0;
> - }
> - }
> -
> - hid_err(hdev, "Unable to locate output report 0x%02x\n", req_id);
> -
> - return -EINVAL;
> -}
> -
> static int sony_register_touchpad(struct sony_sc *sc, int touch_count,
> int w, int h)
> {
> @@ -1291,10 +1268,11 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
> else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
> ret = sixaxis_set_operational_bt(hdev);
> else if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) {
> - /* Report 5 (31 bytes) is used to send data to the controller via USB */
> - ret = sony_set_output_report(sc, 0x05, 248);
> - if (ret < 0)
> + if (hdev->ll_driver->output_report == NULL) {
> + hid_err(hdev, "NULL output_report handler\n");
> + ret = -EINVAL;
> goto err_stop;
> + }
>
> /* The Dualshock 4 touchpad supports 2 touches and has a
> * resolution of 1920x940.
> --
> 1.8.5.3
>
^ permalink raw reply
* Re: [PATCH 0/6] HID: Add a stable method for retrieving the client MAC address of a HID device
From: David Herrmann @ 2014-02-03 16:24 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: Frank Praznik, Frank Praznik, linux-input, Jiri Kosina
In-Reply-To: <CAN+gG=H6A93ozP=Q52GXnEjARC1tX4s6k=Oi6MoHK7Mh6kBPiw@mail.gmail.com>
Hi
On Sat, Feb 1, 2014 at 5:28 PM, Benjamin Tissoires
<benjamin.tissoires@gmail.com> wrote:
> On Sat, Feb 1, 2014 at 11:06 AM, Frank Praznik <frank.praznik@gmail.com> wrote:
>> On 1/31/2014 15:45, Benjamin Tissoires wrote:
>>>
>>> On Fri, Jan 31, 2014 at 3:04 PM, Frank Praznik <frank.praznik@oh.rr.com>
>>> wrote:
>>>>
>>>> On 1/31/2014 14:10, Benjamin Tissoires wrote:
>>>>>
>>>>> Hi Frank,
>>>>>
>>>>> just a quick review:
>>>>>
>>>>> On Fri, Jan 31, 2014 at 12:32 PM, Frank Praznik
>>>>> <frank.praznik@oh.rr.com>
>>>>> wrote:
>>>>>>
>>>>>> Currently there is no reliable way for a device module or hidraw
>>>>>> application to
>>>>>> retrieve the client MAC address of the associated wireless device.
>>>>>> This
>>>>>> series
>>>>>> of patches adds a stable way of retrieving this information.
>>>>>
>>>>> Well, if I look at the uevent of a Bluetooth mouse I have:
>>>>>
>>>>> $ cat
>>>>>
>>>>> /sys/devices/pci0000\:00/0000\:00\:14.0/usb3/3-2/3-2\:1.0/bluetooth/hci0/hci0\:43/0005\:046D\:B00D.001F/uevent
>>>>> DRIVER=hid-generic
>>>>> HID_ID=0005:0000046D:0000B00D
>>>>> HID_NAME=Ultrathin Touch Mouse
>>>>> HID_PHYS=00:10:60:ea:df:ae
>>>>> HID_UNIQ=00:1f:20:96:33:47
>>>>> MODALIAS=hid:b0005g0001v0000046Dp0000B00D
>>>>>
>>>>> I would say that HID_UNIQ is the client MAC address set by hidp, no?
>>>>> So you don't need to duplicate the info by adding a new field in
>>>>> hid_device.
>>>>>
>>>> In a patch I recently submitted I was using the UNIQ field for retrieving
>>>> a
>>>> Bluetooth device MAC address and was warned against it because "UNIQ is a
>>>> way to provide unique identifiers for devices, but it's not guaranteed to
>>>> stay the same". HIDP happens to store the MAC in the UNIQ data, but
>>>> there
>>>> is no guarantee that it will always be there. With these patches you can
>>>> be
>>>> completely sure that the data in client_addr is the client device MAC
>>>> address.
>>>
>>> ok. But still, adding a transport-specific information to hid_device
>>> is IMO a bad idea. We fought to make HID agnostic of the transport
>>> layer enough.
>>>
>>> David, could you elaborate why you think that UNIQ may change in the
>>> future regarding BT?
>>> If the BT MAC address is the same principle of an ethernet MAC
>>> address, UNIQ seems to fit perfectly well.
>>>
>>> Otherwise, you may be able to retrieve the MAC address by using the
>>> parent of the current device.
>>>
>>> Cheers,
>>> Benjamin
>>
>> Are you referring to using the hid_device::dev.parent pointer? I know that
>
> Yes
>
>> hidp sets it to point to the hci_conn struct from which src address for the
>> Bluetooth connection can be obtained, but making assumptions about an opaque
>> pointer like that seems dangerous. Is the parent pointer guaranteed to
>> point to the hci_conn struct as long as the bus type is Bluetooth?
>
> And nope. If you use uhid, then the parent will not be a hci_conn.
>
> With enough guards, you should be able to use it, but it's not ideal I agree.
> I really want to have David's opinion regarding the UNIQ field. IMO,
> this seems to be the most transport-agnostic way of doing it.
UNIQ is definitely wrong. It is used to assign a run-time *unique*
value to the connection. So ideally it should be different if a device
is reconnected. The PHYS field is actually used to identify a physical
device. So please, if we're doing this, then we should do it via PHYS.
I'm fine with hard-coding PHYS as bt-address for BT-HID, but please
add a comment to hidp_setup_hid() in net/bluetooth/hidp/core.c before
doing the snprintf() there.
The reason why I disliked hard-coding this behavior is that if a
single BT-device is connected twice to us, we usually want two
different PHYS entries for both depending on which service this
connection provides. However, this seems like an unlikely and
overengineered problem so lets not do that. Furthermore, while BT-HID
would allow such setups with some hacks, it isn't supported in a clean
way. So yeah, I'm actually fine with using PHYS for that.
Thanks
David
^ permalink raw reply
* Re: [PATCH 04/11] HID: logitech-dj: remove hidinput_input_event
From: Benjamin Tissoires @ 2014-02-03 16:21 UTC (permalink / raw)
To: David Herrmann
Cc: Benjamin Tissoires, Jiri Kosina, Frank Praznik,
open list:HID CORE LAYER, linux-kernel
In-Reply-To: <CANq1E4Rn=rijG8Xv97okj2k4iv=ZG6wa30xQrxgjhutQovD+dg@mail.gmail.com>
On Mon, Feb 3, 2014 at 11:06 AM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
>> hid-logitech-dj uses its own ->hidinput_input_event() instead of
>> the generic binding in hid-input.
>> Moving the handling of LEDs towards logi_dj_output_hidraw_report()
>> allows two things:
>> - remove hidinput_input_event in struct hid_device
>> - hidraw user space programs can also set the LEDs
>>
>> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>> ---
>> drivers/hid/hid-logitech-dj.c | 99 +++++++++++++++----------------------------
>> 1 file changed, 35 insertions(+), 64 deletions(-)
>>
>> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
>> index f45279c..61d2bbf 100644
>> --- a/drivers/hid/hid-logitech-dj.c
>> +++ b/drivers/hid/hid-logitech-dj.c
>> @@ -44,14 +44,6 @@ static const char kbd_descriptor[] = {
>> 0x19, 0xE0, /* USAGE_MINIMUM (Left Control) */
>> 0x29, 0xE7, /* USAGE_MAXIMUM (Right GUI) */
>> 0x81, 0x02, /* INPUT (Data,Var,Abs) */
>> - 0x95, 0x05, /* REPORT COUNT (5) */
>> - 0x05, 0x08, /* USAGE PAGE (LED page) */
>> - 0x19, 0x01, /* USAGE MINIMUM (1) */
>> - 0x29, 0x05, /* USAGE MAXIMUM (5) */
>> - 0x91, 0x02, /* OUTPUT (Data, Variable, Absolute) */
>> - 0x95, 0x01, /* REPORT COUNT (1) */
>> - 0x75, 0x03, /* REPORT SIZE (3) */
>> - 0x91, 0x01, /* OUTPUT (Constant) */
>> 0x95, 0x06, /* REPORT_COUNT (6) */
>> 0x75, 0x08, /* REPORT_SIZE (8) */
>> 0x15, 0x00, /* LOGICAL_MINIMUM (0) */
>> @@ -60,6 +52,18 @@ static const char kbd_descriptor[] = {
>> 0x19, 0x00, /* USAGE_MINIMUM (no event) */
>> 0x2A, 0xFF, 0x00, /* USAGE_MAXIMUM (reserved) */
>> 0x81, 0x00, /* INPUT (Data,Ary,Abs) */
>> + 0x85, 0x0e, /* REPORT_ID (14) */
>> + 0x05, 0x08, /* USAGE PAGE (LED page) */
>> + 0x95, 0x05, /* REPORT COUNT (5) */
>> + 0x75, 0x01, /* REPORT SIZE (1) */
>> + 0x15, 0x00, /* LOGICAL_MINIMUM (0) */
>> + 0x25, 0x01, /* LOGICAL_MAXIMUM (1) */
>> + 0x19, 0x01, /* USAGE MINIMUM (1) */
>> + 0x29, 0x05, /* USAGE MAXIMUM (5) */
>> + 0x91, 0x02, /* OUTPUT (Data, Variable, Absolute) */
>> + 0x95, 0x01, /* REPORT COUNT (1) */
>> + 0x75, 0x03, /* REPORT SIZE (3) */
>> + 0x91, 0x01, /* OUTPUT (Constant) */
>> 0xC0
>> };
>>
>> @@ -544,10 +548,30 @@ static int logi_dj_output_hidraw_report(struct hid_device *hid, u8 * buf,
>> size_t count,
>> unsigned char report_type)
>> {
>> - /* Called by hid raw to send data */
>> - dbg_hid("%s\n", __func__);
>> + struct dj_device *djdev = hid->driver_data;
>> + struct dj_receiver_dev *djrcv_dev = djdev->dj_receiver_dev;
>> + u8 *out_buf;
>> + int ret;
>>
>> - return 0;
>> + if (buf[0] != REPORT_TYPE_LEDS)
>> + return -EINVAL;
>> +
>> + out_buf = kzalloc(DJREPORT_SHORT_LENGTH, GFP_ATOMIC);
>> + if (!out_buf)
>> + return -ENOMEM;
>> +
>> + if (count < DJREPORT_SHORT_LENGTH - 2)
>> + count = DJREPORT_SHORT_LENGTH - 2;
>> +
>> + out_buf[0] = REPORT_ID_DJ_SHORT;
>> + out_buf[1] = djdev->device_index;
>> + memcpy(out_buf + 2, buf, count);
>> +
>> + ret = djrcv_dev->hdev->hid_output_raw_report(djrcv_dev->hdev, out_buf,
>> + DJREPORT_SHORT_LENGTH, report_type);
>
> Strictly speaking the code below uses HID_REQ_SET_REPORT and you
> replace it with hid_output_raw_report() here (which at least for BTHID
> is not equivalent). Anyhow, HID-core uses hid_output_raw_report(),
> too, so this should be fine.
Yes, you are right. This fires back on me yesterday when I tried to
remove the hid_output_raw_report() calls.
It occurs it works because usbhid_out_raw_report() uses set_report
when there is no urbout, which is the case with logitech receivers.
So I guess an ideal solution would be to implement a hid_hw_request call.
However, the only concern I have with hid_hw_request is that it does
not allow hidraw to play with the LEDs given that hidraw does not have
an ioctl for SET_REPORT (or GET_REPORT). I already seen this issue
regarding i2c_hid at some point, but it was "solved" in i2c_hid by
using the same ugly trick that we have in USB-hid.
Cheers,
Benjamin
>
> Thanks
> David
>
>> +
>> + kfree(out_buf);
>> + return ret;
>> }
>>
>> static void rdcat(char *rdesc, unsigned int *rsize, const char *data, unsigned int size)
>> @@ -613,58 +637,6 @@ static int logi_dj_ll_parse(struct hid_device *hid)
>> return retval;
>> }
>>
>> -static int logi_dj_ll_input_event(struct input_dev *dev, unsigned int type,
>> - unsigned int code, int value)
>> -{
>> - /* Sent by the input layer to handle leds and Force Feedback */
>> - struct hid_device *dj_hiddev = input_get_drvdata(dev);
>> - struct dj_device *dj_dev = dj_hiddev->driver_data;
>> -
>> - struct dj_receiver_dev *djrcv_dev =
>> - dev_get_drvdata(dj_hiddev->dev.parent);
>> - struct hid_device *dj_rcv_hiddev = djrcv_dev->hdev;
>> - struct hid_report_enum *output_report_enum;
>> -
>> - struct hid_field *field;
>> - struct hid_report *report;
>> - unsigned char *data;
>> - int offset;
>> -
>> - dbg_hid("%s: %s, type:%d | code:%d | value:%d\n",
>> - __func__, dev->phys, type, code, value);
>> -
>> - if (type != EV_LED)
>> - return -1;
>> -
>> - offset = hidinput_find_field(dj_hiddev, type, code, &field);
>> -
>> - if (offset == -1) {
>> - dev_warn(&dev->dev, "event field not found\n");
>> - return -1;
>> - }
>> - hid_set_field(field, offset, value);
>> -
>> - data = hid_alloc_report_buf(field->report, GFP_ATOMIC);
>> - if (!data) {
>> - dev_warn(&dev->dev, "failed to allocate report buf memory\n");
>> - return -1;
>> - }
>> -
>> - hid_output_report(field->report, &data[0]);
>> -
>> - output_report_enum = &dj_rcv_hiddev->report_enum[HID_OUTPUT_REPORT];
>> - report = output_report_enum->report_id_hash[REPORT_ID_DJ_SHORT];
>> - hid_set_field(report->field[0], 0, dj_dev->device_index);
>> - hid_set_field(report->field[0], 1, REPORT_TYPE_LEDS);
>> - hid_set_field(report->field[0], 2, data[1]);
>> -
>> - hid_hw_request(dj_rcv_hiddev, report, HID_REQ_SET_REPORT);
>> -
>> - kfree(data);
>> -
>> - return 0;
>> -}
>> -
>> static int logi_dj_ll_start(struct hid_device *hid)
>> {
>> dbg_hid("%s\n", __func__);
>> @@ -683,7 +655,6 @@ static struct hid_ll_driver logi_dj_ll_driver = {
>> .stop = logi_dj_ll_stop,
>> .open = logi_dj_ll_open,
>> .close = logi_dj_ll_close,
>> - .hidinput_input_event = logi_dj_ll_input_event,
>> };
>>
>>
>> --
>> 1.8.3.1
>>
^ permalink raw reply
* Re: [PATCH 10/11] HID: introduce helper to access hid_output_raw_report()
From: David Herrmann @ 2014-02-03 16:14 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Benjamin Tissoires, Jiri Kosina, Frank Praznik,
open list:HID CORE LAYER, linux-kernel
In-Reply-To: <1391316630-29541-11-git-send-email-benjamin.tissoires@redhat.com>
Hi
On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> Add a helper to access hdev->hid_output_raw_report().
>
> To convert the drivers, use the following snippets:
>
> for i in drivers/hid/*.c
> do
> sed -i.bak "s/[^ \t]*->hid_output_raw_report(/hid_output_raw_report(/g" $i
> done
>
> Then manually fix for checkpatch.pl
Looks all good. I guess you didn't use the hid_hw_* prefix as it's not
a hdrv but hdev callback? But we hopefully can remove this soon,
anyway. So:
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Thanks
David
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
> drivers/hid/hid-input.c | 2 +-
> drivers/hid/hid-lg.c | 6 ++++--
> drivers/hid/hid-logitech-dj.c | 2 +-
> drivers/hid/hid-magicmouse.c | 2 +-
> drivers/hid/hid-sony.c | 5 +++--
> drivers/hid/hid-thingm.c | 4 ++--
> drivers/hid/hid-wacom.c | 16 +++++++---------
> drivers/hid/hid-wiimote-core.c | 2 +-
> drivers/hid/hidraw.c | 2 +-
> include/linux/hid.h | 16 ++++++++++++++++
> 10 files changed, 37 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 1de5997..78293c3 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -1184,7 +1184,7 @@ static void hidinput_led_worker(struct work_struct *work)
>
> hid_output_report(report, buf);
> /* synchronous output report */
> - hid->hid_output_raw_report(hid, buf, len, HID_OUTPUT_REPORT);
> + hid_output_raw_report(hid, buf, len, HID_OUTPUT_REPORT);
> kfree(buf);
> }
>
> diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c
> index 9fe9d4a..76ed7e5 100644
> --- a/drivers/hid/hid-lg.c
> +++ b/drivers/hid/hid-lg.c
> @@ -692,7 +692,8 @@ static int lg_probe(struct hid_device *hdev, const struct hid_device_id *id)
> if (hdev->product == USB_DEVICE_ID_LOGITECH_WII_WHEEL) {
> unsigned char buf[] = { 0x00, 0xAF, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };
>
> - ret = hdev->hid_output_raw_report(hdev, buf, sizeof(buf), HID_FEATURE_REPORT);
> + ret = hid_output_raw_report(hdev, buf, sizeof(buf),
> + HID_FEATURE_REPORT);
>
> if (ret >= 0) {
> /* insert a little delay of 10 jiffies ~ 40ms */
> @@ -704,7 +705,8 @@ static int lg_probe(struct hid_device *hdev, const struct hid_device_id *id)
> buf[1] = 0xB2;
> get_random_bytes(&buf[2], 2);
>
> - ret = hdev->hid_output_raw_report(hdev, buf, sizeof(buf), HID_FEATURE_REPORT);
> + ret = hid_output_raw_report(hdev, buf, sizeof(buf),
> + HID_FEATURE_REPORT);
> }
> }
>
> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
> index 61d2bbf..9347625 100644
> --- a/drivers/hid/hid-logitech-dj.c
> +++ b/drivers/hid/hid-logitech-dj.c
> @@ -567,7 +567,7 @@ static int logi_dj_output_hidraw_report(struct hid_device *hid, u8 * buf,
> out_buf[1] = djdev->device_index;
> memcpy(out_buf + 2, buf, count);
>
> - ret = djrcv_dev->hdev->hid_output_raw_report(djrcv_dev->hdev, out_buf,
> + ret = hid_output_raw_report(djrcv_dev->hdev, out_buf,
> DJREPORT_SHORT_LENGTH, report_type);
>
> kfree(out_buf);
> diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
> index 3b43d1c..cb5db3a 100644
> --- a/drivers/hid/hid-magicmouse.c
> +++ b/drivers/hid/hid-magicmouse.c
> @@ -538,7 +538,7 @@ static int magicmouse_probe(struct hid_device *hdev,
> * but there seems to be no other way of switching the mode.
> * Thus the super-ugly hacky success check below.
> */
> - ret = hdev->hid_output_raw_report(hdev, feature, sizeof(feature),
> + ret = hid_output_raw_report(hdev, feature, sizeof(feature),
> HID_FEATURE_REPORT);
> if (ret != -EIO && ret != sizeof(feature)) {
> hid_err(hdev, "unable to request touch data (%d)\n", ret);
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index 3930acb..8494b8c 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -720,7 +720,8 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev)
> static int sixaxis_set_operational_bt(struct hid_device *hdev)
> {
> unsigned char buf[] = { 0xf4, 0x42, 0x03, 0x00, 0x00 };
> - return hdev->hid_output_raw_report(hdev, buf, sizeof(buf), HID_FEATURE_REPORT);
> + return hid_output_raw_report(hdev, buf, sizeof(buf),
> + HID_FEATURE_REPORT);
> }
>
> static void buzz_set_leds(struct hid_device *hdev, const __u8 *leds)
> @@ -942,7 +943,7 @@ static void sixaxis_state_worker(struct work_struct *work)
> buf[10] |= sc->led_state[2] << 3;
> buf[10] |= sc->led_state[3] << 4;
>
> - sc->hdev->hid_output_raw_report(sc->hdev, buf, sizeof(buf),
> + hid_output_raw_report(sc->hdev, buf, sizeof(buf),
> HID_OUTPUT_REPORT);
> }
>
> diff --git a/drivers/hid/hid-thingm.c b/drivers/hid/hid-thingm.c
> index 99342cf..7dd3197 100644
> --- a/drivers/hid/hid-thingm.c
> +++ b/drivers/hid/hid-thingm.c
> @@ -48,8 +48,8 @@ static int blink1_send_command(struct blink1_data *data,
> buf[0], buf[1], buf[2], buf[3], buf[4],
> buf[5], buf[6], buf[7], buf[8]);
>
> - ret = data->hdev->hid_output_raw_report(data->hdev, buf,
> - BLINK1_CMD_SIZE, HID_FEATURE_REPORT);
> + ret = hid_output_raw_report(data->hdev, buf, BLINK1_CMD_SIZE,
> + HID_FEATURE_REPORT);
>
> return ret < 0 ? ret : 0;
> }
> diff --git a/drivers/hid/hid-wacom.c b/drivers/hid/hid-wacom.c
> index 60c75dc..c720db9 100644
> --- a/drivers/hid/hid-wacom.c
> +++ b/drivers/hid/hid-wacom.c
> @@ -128,8 +128,7 @@ static void wacom_set_image(struct hid_device *hdev, const char *image,
>
> rep_data[0] = WAC_CMD_ICON_START_STOP;
> rep_data[1] = 0;
> - ret = hdev->hid_output_raw_report(hdev, rep_data, 2,
> - HID_FEATURE_REPORT);
> + ret = hid_output_raw_report(hdev, rep_data, 2, HID_FEATURE_REPORT);
> if (ret < 0)
> goto err;
>
> @@ -143,15 +142,14 @@ static void wacom_set_image(struct hid_device *hdev, const char *image,
> rep_data[j + 3] = p[(i << 6) + j];
>
> rep_data[2] = i;
> - ret = hdev->hid_output_raw_report(hdev, rep_data, 67,
> + ret = hid_output_raw_report(hdev, rep_data, 67,
> HID_FEATURE_REPORT);
> }
>
> rep_data[0] = WAC_CMD_ICON_START_STOP;
> rep_data[1] = 0;
>
> - ret = hdev->hid_output_raw_report(hdev, rep_data, 2,
> - HID_FEATURE_REPORT);
> + ret = hid_output_raw_report(hdev, rep_data, 2, HID_FEATURE_REPORT);
>
> err:
> return;
> @@ -183,7 +181,7 @@ static void wacom_leds_set_brightness(struct led_classdev *led_dev,
> buf[3] = value;
> /* use fixed brightness for OLEDs */
> buf[4] = 0x08;
> - hdev->hid_output_raw_report(hdev, buf, 9, HID_FEATURE_REPORT);
> + hid_output_raw_report(hdev, buf, 9, HID_FEATURE_REPORT);
> kfree(buf);
> }
>
> @@ -339,7 +337,7 @@ static void wacom_set_features(struct hid_device *hdev, u8 speed)
> rep_data[0] = 0x03 ; rep_data[1] = 0x00;
> limit = 3;
> do {
> - ret = hdev->hid_output_raw_report(hdev, rep_data, 2,
> + ret = hid_output_raw_report(hdev, rep_data, 2,
> HID_FEATURE_REPORT);
> } while (ret < 0 && limit-- > 0);
>
> @@ -352,7 +350,7 @@ static void wacom_set_features(struct hid_device *hdev, u8 speed)
> rep_data[1] = 0x00;
> limit = 3;
> do {
> - ret = hdev->hid_output_raw_report(hdev,
> + ret = hid_output_raw_report(hdev,
> rep_data, 2, HID_FEATURE_REPORT);
> } while (ret < 0 && limit-- > 0);
>
> @@ -378,7 +376,7 @@ static void wacom_set_features(struct hid_device *hdev, u8 speed)
> rep_data[0] = 0x03;
> rep_data[1] = wdata->features;
>
> - ret = hdev->hid_output_raw_report(hdev, rep_data, 2,
> + ret = hid_output_raw_report(hdev, rep_data, 2,
> HID_FEATURE_REPORT);
> if (ret >= 0)
> wdata->high_speed = speed;
> diff --git a/drivers/hid/hid-wiimote-core.c b/drivers/hid/hid-wiimote-core.c
> index abb20db..d7dc6c5b 100644
> --- a/drivers/hid/hid-wiimote-core.c
> +++ b/drivers/hid/hid-wiimote-core.c
> @@ -35,7 +35,7 @@ static int wiimote_hid_send(struct hid_device *hdev, __u8 *buffer,
> if (!buf)
> return -ENOMEM;
>
> - ret = hdev->hid_output_raw_report(hdev, buf, count, HID_OUTPUT_REPORT);
> + ret = hid_output_raw_report(hdev, buf, count, HID_OUTPUT_REPORT);
>
> kfree(buf);
> return ret;
> diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
> index 4b2dc95..f8708c9 100644
> --- a/drivers/hid/hidraw.c
> +++ b/drivers/hid/hidraw.c
> @@ -153,7 +153,7 @@ static ssize_t hidraw_send_report(struct file *file, const char __user *buffer,
> goto out_free;
> }
>
> - ret = dev->hid_output_raw_report(dev, buf, count, report_type);
> + ret = hid_output_raw_report(dev, buf, count, report_type);
> out_free:
> kfree(buf);
> out:
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index c56681a..a837ede 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -1012,6 +1012,22 @@ static inline int hid_hw_output_report(struct hid_device *hdev, __u8 *buf,
> }
>
> /**
> + * hid_output_raw_report - send an output or a feature report to the device
> + *
> + * @hdev: hid device
> + * @buf: raw data to transfer
> + * @len: length of buf
> + * @report_type: HID_FEATURE_REPORT or HID_OUTPUT_REPORT
> + *
> + * @return: count of data transfered, negative if error
> + */
> +static inline int hid_output_raw_report(struct hid_device *hdev, __u8 *buf,
> + size_t len, unsigned char report_type)
> +{
> + return hdev->hid_output_raw_report(hdev, buf, len, report_type);
> +}
> +
> +/**
> * hid_hw_idle - send idle request to device
> *
> * @hdev: hid device
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [PATCH 09/11] HID: remove hid_get_raw_report in struct hid_device
From: David Herrmann @ 2014-02-03 16:12 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Benjamin Tissoires, Jiri Kosina, Frank Praznik,
open list:HID CORE LAYER, linux-kernel
In-Reply-To: <1391316630-29541-10-git-send-email-benjamin.tissoires@redhat.com>
Hi
On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> dev->hid_get_raw_report(X) and hid_hw_raw_request(X, HID_REQ_GET_REPORT)
> are strictly equivalent. Switch the hid subsystem to the hid_hw notation
> and remove the field .hid_get_raw_report in struct hid_device.
Finally we can remove that beast:
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Thanks
David
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
> drivers/hid/hid-input.c | 6 +++---
> drivers/hid/hid-sony.c | 3 ++-
> drivers/hid/hidraw.c | 7 ++++---
> drivers/hid/i2c-hid/i2c-hid.c | 1 -
> drivers/hid/uhid.c | 1 -
> drivers/hid/usbhid/hid-core.c | 1 -
> include/linux/hid.h | 3 ---
> net/bluetooth/hidp/core.c | 1 -
> 8 files changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 594722d..1de5997 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -350,9 +350,9 @@ static int hidinput_get_battery_property(struct power_supply *psy,
> ret = -ENOMEM;
> break;
> }
> - ret = dev->hid_get_raw_report(dev, dev->battery_report_id,
> - buf, 2,
> - dev->battery_report_type);
> + ret = hid_hw_raw_request(dev, dev->battery_report_id, buf, 2,
> + dev->battery_report_type,
> + HID_REQ_GET_REPORT);
>
> if (ret != 2) {
> ret = -ENODATA;
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index 1235405..3930acb 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -706,7 +706,8 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev)
> if (!buf)
> return -ENOMEM;
>
> - ret = hdev->hid_get_raw_report(hdev, 0xf2, buf, 17, HID_FEATURE_REPORT);
> + ret = hid_hw_raw_request(hdev, 0xf2, buf, 17, HID_FEATURE_REPORT,
> + HID_REQ_GET_REPORT);
>
> if (ret < 0)
> hid_err(hdev, "can't set operational mode\n");
> diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
> index cb0137b..4b2dc95 100644
> --- a/drivers/hid/hidraw.c
> +++ b/drivers/hid/hidraw.c
> @@ -189,7 +189,7 @@ static ssize_t hidraw_get_report(struct file *file, char __user *buffer, size_t
>
> dev = hidraw_table[minor]->hid;
>
> - if (!dev->hid_get_raw_report) {
> + if (!dev->ll_driver->raw_request) {
> ret = -ENODEV;
> goto out;
> }
> @@ -216,14 +216,15 @@ static ssize_t hidraw_get_report(struct file *file, char __user *buffer, size_t
>
> /*
> * Read the first byte from the user. This is the report number,
> - * which is passed to dev->hid_get_raw_report().
> + * which is passed to hid_hw_raw_request().
> */
> if (copy_from_user(&report_number, buffer, 1)) {
> ret = -EFAULT;
> goto out_free;
> }
>
> - ret = dev->hid_get_raw_report(dev, report_number, buf, count, report_type);
> + ret = hid_hw_raw_request(dev, report_number, buf, count, report_type,
> + HID_REQ_GET_REPORT);
>
> if (ret < 0)
> goto out_free;
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index 5099f1f..fe3b392 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -1029,7 +1029,6 @@ static int i2c_hid_probe(struct i2c_client *client,
>
> hid->driver_data = client;
> hid->ll_driver = &i2c_hid_ll_driver;
> - hid->hid_get_raw_report = i2c_hid_get_raw_report;
> hid->hid_output_raw_report = i2c_hid_output_raw_report;
> hid->dev.parent = &client->dev;
> ACPI_COMPANION_SET(&hid->dev, ACPI_COMPANION(&client->dev));
> diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
> index 438c9f1..7358346 100644
> --- a/drivers/hid/uhid.c
> +++ b/drivers/hid/uhid.c
> @@ -421,7 +421,6 @@ static int uhid_dev_create(struct uhid_device *uhid,
> hid->uniq[63] = 0;
>
> hid->ll_driver = &uhid_hid_driver;
> - hid->hid_get_raw_report = uhid_hid_get_raw;
> hid->hid_output_raw_report = uhid_hid_output_raw;
> hid->bus = ev->u.create.bus;
> hid->vendor = ev->u.create.vendor;
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index 406497b..b9a770f 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -1289,7 +1289,6 @@ static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *
>
> usb_set_intfdata(intf, hid);
> hid->ll_driver = &usb_hid_driver;
> - hid->hid_get_raw_report = usbhid_get_raw_report;
> hid->hid_output_raw_report = usbhid_output_raw_report;
> hid->ff_init = hid_pidff_init;
> #ifdef CONFIG_USB_HIDDEV
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index 38c307b..c56681a 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -508,9 +508,6 @@ struct hid_device { /* device report descriptor */
> struct hid_usage *, __s32);
> void (*hiddev_report_event) (struct hid_device *, struct hid_report *);
>
> - /* handler for raw input (Get_Report) data, used by hidraw */
> - int (*hid_get_raw_report) (struct hid_device *, unsigned char, __u8 *, size_t, unsigned char);
> -
> /* handler for raw output data, used by hidraw */
> int (*hid_output_raw_report) (struct hid_device *, __u8 *, size_t, unsigned char);
>
> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> index 02670b3..77c4bad 100644
> --- a/net/bluetooth/hidp/core.c
> +++ b/net/bluetooth/hidp/core.c
> @@ -773,7 +773,6 @@ static int hidp_setup_hid(struct hidp_session *session,
> hid->dev.parent = &session->conn->hcon->dev;
> hid->ll_driver = &hidp_hid_driver;
>
> - hid->hid_get_raw_report = hidp_get_raw_report;
> hid->hid_output_raw_report = hidp_output_raw_report;
>
> /* True if device is blacklisted in drivers/hid/hid-core.c */
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [PATCH 06/11] HID: remove hidinput_input_event handler
From: David Herrmann @ 2014-02-03 16:10 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Benjamin Tissoires, Jiri Kosina, Frank Praznik,
open list:HID CORE LAYER, linux-kernel
In-Reply-To: <1391316630-29541-7-git-send-email-benjamin.tissoires@redhat.com>
Hi
On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> All the different transport drivers use now the generic event handling
> in hid-input. We can remove the handler definitively now.
\o/
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Thanks
David
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
> drivers/hid/hid-input.c | 4 +---
> include/linux/hid.h | 4 ----
> 2 files changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index a713e62..594722d 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -1263,9 +1263,7 @@ static struct hid_input *hidinput_allocate(struct hid_device *hid)
> }
>
> input_set_drvdata(input_dev, hid);
> - if (hid->ll_driver->hidinput_input_event)
> - input_dev->event = hid->ll_driver->hidinput_input_event;
> - else if (hid->ll_driver->request || hid->hid_output_raw_report)
> + if (hid->ll_driver->request || hid->hid_output_raw_report)
> input_dev->event = hidinput_input_event;
> input_dev->open = hidinput_open;
> input_dev->close = hidinput_close;
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index dddcad0..38c307b 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -675,7 +675,6 @@ struct hid_driver {
> * @stop: called on remove
> * @open: called by input layer on open
> * @close: called by input layer on close
> - * @hidinput_input_event: event input event (e.g. ff or leds)
> * @parse: this method is called only once to parse the device data,
> * shouldn't allocate anything to not leak memory
> * @request: send report request to device (e.g. feature report)
> @@ -693,9 +692,6 @@ struct hid_ll_driver {
>
> int (*power)(struct hid_device *hdev, int level);
>
> - int (*hidinput_input_event) (struct input_dev *idev, unsigned int type,
> - unsigned int code, int value);
> -
> int (*parse)(struct hid_device *hdev);
>
> void (*request)(struct hid_device *hdev,
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [PATCH 08/11] HID: usbhid: remove duplicated code
From: David Herrmann @ 2014-02-03 16:09 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Benjamin Tissoires, Jiri Kosina, Frank Praznik,
open list:HID CORE LAYER, linux-kernel
In-Reply-To: <1391316630-29541-9-git-send-email-benjamin.tissoires@redhat.com>
Hi
On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> Well, no use to keep twice the same code.
Yepp, I actually copied the code from that, so this is fine.
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Thanks
David
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
> drivers/hid/usbhid/hid-core.c | 64 ++++++++-----------------------------------
> 1 file changed, 11 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index f8ca312..406497b 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -915,59 +915,6 @@ static int usbhid_set_raw_report(struct hid_device *hid, unsigned int reportnum,
> return ret;
> }
>
> -
> -static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf, size_t count,
> - unsigned char report_type)
> -{
> - struct usbhid_device *usbhid = hid->driver_data;
> - struct usb_device *dev = hid_to_usb_dev(hid);
> - struct usb_interface *intf = usbhid->intf;
> - struct usb_host_interface *interface = intf->cur_altsetting;
> - int ret;
> -
> - if (usbhid->urbout && report_type != HID_FEATURE_REPORT) {
> - int actual_length;
> - int skipped_report_id = 0;
> -
> - if (buf[0] == 0x0) {
> - /* Don't send the Report ID */
> - buf++;
> - count--;
> - skipped_report_id = 1;
> - }
> - ret = usb_interrupt_msg(dev, usbhid->urbout->pipe,
> - buf, count, &actual_length,
> - USB_CTRL_SET_TIMEOUT);
> - /* return the number of bytes transferred */
> - if (ret == 0) {
> - ret = actual_length;
> - /* count also the report id */
> - if (skipped_report_id)
> - ret++;
> - }
> - } else {
> - int skipped_report_id = 0;
> - int report_id = buf[0];
> - if (buf[0] == 0x0) {
> - /* Don't send the Report ID */
> - buf++;
> - count--;
> - skipped_report_id = 1;
> - }
> - ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
> - HID_REQ_SET_REPORT,
> - USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
> - ((report_type + 1) << 8) | report_id,
> - interface->desc.bInterfaceNumber, buf, count,
> - USB_CTRL_SET_TIMEOUT);
> - /* count also the report id, if this was a numbered report. */
> - if (ret > 0 && skipped_report_id)
> - ret++;
> - }
> -
> - return ret;
> -}
> -
> static int usbhid_output_report(struct hid_device *hid, __u8 *buf, size_t count)
> {
> struct usbhid_device *usbhid = hid->driver_data;
> @@ -998,6 +945,17 @@ static int usbhid_output_report(struct hid_device *hid, __u8 *buf, size_t count)
> return ret;
> }
>
> +static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf,
> + size_t count, unsigned char report_type)
> +{
> + struct usbhid_device *usbhid = hid->driver_data;
> +
> + if (usbhid->urbout && report_type != HID_FEATURE_REPORT)
> + return usbhid_output_report(hid, buf, count);
> +
> + return usbhid_set_raw_report(hid, buf[0], buf, count, report_type);
> +}
> +
> static void usbhid_restart_queues(struct usbhid_device *usbhid)
> {
> if (usbhid->urbout && !test_bit(HID_OUT_RUNNING, &usbhid->iofl))
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [PATCH 04/11] HID: logitech-dj: remove hidinput_input_event
From: David Herrmann @ 2014-02-03 16:06 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Benjamin Tissoires, Jiri Kosina, Frank Praznik,
open list:HID CORE LAYER, linux-kernel
In-Reply-To: <1391316630-29541-5-git-send-email-benjamin.tissoires@redhat.com>
Hi
On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> hid-logitech-dj uses its own ->hidinput_input_event() instead of
> the generic binding in hid-input.
> Moving the handling of LEDs towards logi_dj_output_hidraw_report()
> allows two things:
> - remove hidinput_input_event in struct hid_device
> - hidraw user space programs can also set the LEDs
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
> drivers/hid/hid-logitech-dj.c | 99 +++++++++++++++----------------------------
> 1 file changed, 35 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
> index f45279c..61d2bbf 100644
> --- a/drivers/hid/hid-logitech-dj.c
> +++ b/drivers/hid/hid-logitech-dj.c
> @@ -44,14 +44,6 @@ static const char kbd_descriptor[] = {
> 0x19, 0xE0, /* USAGE_MINIMUM (Left Control) */
> 0x29, 0xE7, /* USAGE_MAXIMUM (Right GUI) */
> 0x81, 0x02, /* INPUT (Data,Var,Abs) */
> - 0x95, 0x05, /* REPORT COUNT (5) */
> - 0x05, 0x08, /* USAGE PAGE (LED page) */
> - 0x19, 0x01, /* USAGE MINIMUM (1) */
> - 0x29, 0x05, /* USAGE MAXIMUM (5) */
> - 0x91, 0x02, /* OUTPUT (Data, Variable, Absolute) */
> - 0x95, 0x01, /* REPORT COUNT (1) */
> - 0x75, 0x03, /* REPORT SIZE (3) */
> - 0x91, 0x01, /* OUTPUT (Constant) */
> 0x95, 0x06, /* REPORT_COUNT (6) */
> 0x75, 0x08, /* REPORT_SIZE (8) */
> 0x15, 0x00, /* LOGICAL_MINIMUM (0) */
> @@ -60,6 +52,18 @@ static const char kbd_descriptor[] = {
> 0x19, 0x00, /* USAGE_MINIMUM (no event) */
> 0x2A, 0xFF, 0x00, /* USAGE_MAXIMUM (reserved) */
> 0x81, 0x00, /* INPUT (Data,Ary,Abs) */
> + 0x85, 0x0e, /* REPORT_ID (14) */
> + 0x05, 0x08, /* USAGE PAGE (LED page) */
> + 0x95, 0x05, /* REPORT COUNT (5) */
> + 0x75, 0x01, /* REPORT SIZE (1) */
> + 0x15, 0x00, /* LOGICAL_MINIMUM (0) */
> + 0x25, 0x01, /* LOGICAL_MAXIMUM (1) */
> + 0x19, 0x01, /* USAGE MINIMUM (1) */
> + 0x29, 0x05, /* USAGE MAXIMUM (5) */
> + 0x91, 0x02, /* OUTPUT (Data, Variable, Absolute) */
> + 0x95, 0x01, /* REPORT COUNT (1) */
> + 0x75, 0x03, /* REPORT SIZE (3) */
> + 0x91, 0x01, /* OUTPUT (Constant) */
> 0xC0
> };
>
> @@ -544,10 +548,30 @@ static int logi_dj_output_hidraw_report(struct hid_device *hid, u8 * buf,
> size_t count,
> unsigned char report_type)
> {
> - /* Called by hid raw to send data */
> - dbg_hid("%s\n", __func__);
> + struct dj_device *djdev = hid->driver_data;
> + struct dj_receiver_dev *djrcv_dev = djdev->dj_receiver_dev;
> + u8 *out_buf;
> + int ret;
>
> - return 0;
> + if (buf[0] != REPORT_TYPE_LEDS)
> + return -EINVAL;
> +
> + out_buf = kzalloc(DJREPORT_SHORT_LENGTH, GFP_ATOMIC);
> + if (!out_buf)
> + return -ENOMEM;
> +
> + if (count < DJREPORT_SHORT_LENGTH - 2)
> + count = DJREPORT_SHORT_LENGTH - 2;
> +
> + out_buf[0] = REPORT_ID_DJ_SHORT;
> + out_buf[1] = djdev->device_index;
> + memcpy(out_buf + 2, buf, count);
> +
> + ret = djrcv_dev->hdev->hid_output_raw_report(djrcv_dev->hdev, out_buf,
> + DJREPORT_SHORT_LENGTH, report_type);
Strictly speaking the code below uses HID_REQ_SET_REPORT and you
replace it with hid_output_raw_report() here (which at least for BTHID
is not equivalent). Anyhow, HID-core uses hid_output_raw_report(),
too, so this should be fine.
Thanks
David
> +
> + kfree(out_buf);
> + return ret;
> }
>
> static void rdcat(char *rdesc, unsigned int *rsize, const char *data, unsigned int size)
> @@ -613,58 +637,6 @@ static int logi_dj_ll_parse(struct hid_device *hid)
> return retval;
> }
>
> -static int logi_dj_ll_input_event(struct input_dev *dev, unsigned int type,
> - unsigned int code, int value)
> -{
> - /* Sent by the input layer to handle leds and Force Feedback */
> - struct hid_device *dj_hiddev = input_get_drvdata(dev);
> - struct dj_device *dj_dev = dj_hiddev->driver_data;
> -
> - struct dj_receiver_dev *djrcv_dev =
> - dev_get_drvdata(dj_hiddev->dev.parent);
> - struct hid_device *dj_rcv_hiddev = djrcv_dev->hdev;
> - struct hid_report_enum *output_report_enum;
> -
> - struct hid_field *field;
> - struct hid_report *report;
> - unsigned char *data;
> - int offset;
> -
> - dbg_hid("%s: %s, type:%d | code:%d | value:%d\n",
> - __func__, dev->phys, type, code, value);
> -
> - if (type != EV_LED)
> - return -1;
> -
> - offset = hidinput_find_field(dj_hiddev, type, code, &field);
> -
> - if (offset == -1) {
> - dev_warn(&dev->dev, "event field not found\n");
> - return -1;
> - }
> - hid_set_field(field, offset, value);
> -
> - data = hid_alloc_report_buf(field->report, GFP_ATOMIC);
> - if (!data) {
> - dev_warn(&dev->dev, "failed to allocate report buf memory\n");
> - return -1;
> - }
> -
> - hid_output_report(field->report, &data[0]);
> -
> - output_report_enum = &dj_rcv_hiddev->report_enum[HID_OUTPUT_REPORT];
> - report = output_report_enum->report_id_hash[REPORT_ID_DJ_SHORT];
> - hid_set_field(report->field[0], 0, dj_dev->device_index);
> - hid_set_field(report->field[0], 1, REPORT_TYPE_LEDS);
> - hid_set_field(report->field[0], 2, data[1]);
> -
> - hid_hw_request(dj_rcv_hiddev, report, HID_REQ_SET_REPORT);
> -
> - kfree(data);
> -
> - return 0;
> -}
> -
> static int logi_dj_ll_start(struct hid_device *hid)
> {
> dbg_hid("%s\n", __func__);
> @@ -683,7 +655,6 @@ static struct hid_ll_driver logi_dj_ll_driver = {
> .stop = logi_dj_ll_stop,
> .open = logi_dj_ll_open,
> .close = logi_dj_ll_close,
> - .hidinput_input_event = logi_dj_ll_input_event,
> };
>
>
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [PATCH 02/11] HID: i2c-hid: implement ll_driver transport-layer callbacks
From: David Herrmann @ 2014-02-03 16:04 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Benjamin Tissoires, Jiri Kosina, Frank Praznik,
open list:HID CORE LAYER, linux-kernel
In-Reply-To: <1391316630-29541-3-git-send-email-benjamin.tissoires@redhat.com>
Hi
On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> Add output_report and raw_request to i2c-hid.
> Hopefully, we will manage to have the same transport level between
> all the transport drivers.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
> drivers/hid/i2c-hid/i2c-hid.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index ce68a12..5099f1f 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -574,6 +574,28 @@ static int i2c_hid_output_raw_report(struct hid_device *hid, __u8 *buf,
> return ret;
> }
>
> +static int i2c_hid_output_report(struct hid_device *hid, __u8 *buf,
> + size_t count)
> +{
> + return i2c_hid_output_raw_report(hid, buf, count, HID_OUTPUT_REPORT);
> +}
> +
> +static int i2c_hid_raw_request(struct hid_device *hid, unsigned char reportnum,
> + __u8 *buf, size_t len, unsigned char rtype,
> + int reqtype)
> +{
> + switch (reqtype) {
> + case HID_REQ_GET_REPORT:
> + return i2c_hid_get_raw_report(hid, reportnum, buf, len, rtype);
> + case HID_REQ_SET_REPORT:
> + if (buf[0] != reportnum)
> + return -EINVAL;
> + return i2c_hid_output_raw_report(hid, buf, len, rtype);
I just skimmed the I2C-HID specs and it defines three methods for
input/output reports:
1) Section 6.2:
raw async output-reports can be sent by writing the data at any time
to wOutputRegister.
This should be used as method for hid->output_report().
2) Section 7.1:
SET_REPORT can be issued by writing the right OPCODE + report-ID into
wCommandRegister and the data into wDataRegister.
This should be used as method for hid->raw_request() + HID_REQ_SET_REPORT.
3) Section 7.1:
GET_REPORT can be issued by writing the right OPCODE + report-ID into
wCommandRegister and then waiting for the device to write the data
into wDataRegister.
This should be used for hid->raw_request() + HID_REQ_GET_REPORT
The GET_REPORT implementation looks fine to me, but the
i2c_hid_set_report() seems to support both 1) and 2) depending on the
passed type and capabilities:
- it uses 2) for FEATURE_REPORT reports or if the max OUTPUT_LENGTH is 0
- it uses 1) otherwise
I'm not sure whether the i2c-hid-spec mandates this behavior, so I am
not saying it's wrong. I just wanna understand what we do here. So if
we use hid->output_report() with HID_FEATURE_REPORT, the current code
turns this into a SET_REPORT. Likewise, an hid->raw_request() with
HID_REQ_SET_REPORT but with HID_OUTPUT_REPORT turns into an
output-report.
I'd rather expect this behavior:
hid->output_report() should always do this:
args[index++] = outputRegister & 0xFF;
args[index++] = outputRegister >> 8;
hidcmd = &hid_no_cmd;
while hid->raw_request() should always do this:
args[index++] = dataRegister & 0xFF;
args[index++] = dataRegister >> 8;
The special case for maxOutputLength==0 seems fine to me, but the
"reportType == 0x03" looks weird.
Thanks
David
> + default:
> + return -EIO;
> + }
> +}
> +
> static void i2c_hid_request(struct hid_device *hid, struct hid_report *rep,
> int reqtype)
> {
> @@ -761,6 +783,8 @@ static struct hid_ll_driver i2c_hid_ll_driver = {
> .close = i2c_hid_close,
> .power = i2c_hid_power,
> .request = i2c_hid_request,
> + .output_report = i2c_hid_output_report,
> + .raw_request = i2c_hid_raw_request,
> };
>
> static int i2c_hid_init_irq(struct i2c_client *client)
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [PATCH 03/11] HID: add inliners for ll_driver transport-layer callbacks
From: David Herrmann @ 2014-02-03 15:29 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Benjamin Tissoires, Jiri Kosina, Frank Praznik,
open list:HID CORE LAYER, linux-kernel
In-Reply-To: <1391316630-29541-4-git-send-email-benjamin.tissoires@redhat.com>
Hi
On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> Those callbacks are not mandatory, so it's better to add inliners
> to use them safely.
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Thanks
David
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
> include/linux/hid.h | 45 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 45 insertions(+)
>
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index 003cc8e..dddcad0 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -680,6 +680,8 @@ struct hid_driver {
> * shouldn't allocate anything to not leak memory
> * @request: send report request to device (e.g. feature report)
> * @wait: wait for buffered io to complete (send/recv reports)
> + * @raw_request: send raw report request to device (e.g. feature report)
> + * @output_report: send output report to device
> * @idle: send idle request to device
> */
> struct hid_ll_driver {
> @@ -974,6 +976,49 @@ static inline void hid_hw_request(struct hid_device *hdev,
> }
>
> /**
> + * hid_hw_raw_request - send report request to device
> + *
> + * @hdev: hid device
> + * @reportnum: report ID
> + * @buf: in/out data to transfer
> + * @len: length of buf
> + * @rtype: HID report type
> + * @reqtype: HID_REQ_GET_REPORT or HID_REQ_SET_REPORT
> + *
> + * @return: count of data transfered, negative if error
> + *
> + * Same behavior as hid_hw_request, but with raw buffers instead.
> + */
> +static inline int hid_hw_raw_request(struct hid_device *hdev,
> + unsigned char reportnum, __u8 *buf,
> + size_t len, unsigned char rtype, int reqtype)
> +{
> + if (hdev->ll_driver->raw_request)
> + return hdev->ll_driver->raw_request(hdev, reportnum, buf, len,
> + rtype, reqtype);
> +
> + return -ENOSYS;
> +}
> +
> +/**
> + * hid_hw_output_report - send output report to device
> + *
> + * @hdev: hid device
> + * @buf: raw data to transfer
> + * @len: length of buf
> + *
> + * @return: count of data transfered, negative if error
> + */
> +static inline int hid_hw_output_report(struct hid_device *hdev, __u8 *buf,
> + size_t len)
> +{
> + if (hdev->ll_driver->output_report)
> + return hdev->ll_driver->output_report(hdev, buf, len);
> +
> + return -ENOSYS;
> +}
> +
> +/**
> * hid_hw_idle - send idle request to device
> *
> * @hdev: hid device
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [PATCH 05/11] HID: HIDp: remove hidp_hidinput_event
From: Benjamin Tissoires @ 2014-02-03 15:28 UTC (permalink / raw)
To: David Herrmann
Cc: Benjamin Tissoires, Jiri Kosina, Frank Praznik,
open list:HID CORE LAYER, linux-kernel
In-Reply-To: <CANq1E4S+RLDpDd+zxgBRKsEw0-29un9mzS5jYBcoeJgQf5Upjg@mail.gmail.com>
On Mon, Feb 3, 2014 at 10:10 AM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
>> hidp uses its own ->hidinput_input_event() instead of the generic binding
>> in hid-input.
>> Moving the handling of LEDs towards hidp_hidinput_event() allows two things:
>> - remove hidinput_input_event definitively from struct hid_device
>> - hidraw user space programs can also set the LEDs
>
> This one looks good. hid-input uses output_raw_report(), which is on
> INTR for BT-HID, which is equivalent to what hidp_send_report() does.
> So:
>
> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Thanks
>
> Btw., you might have to keep hidp_send_report() if you drop earlier
> patches (haven't exactly looked at the order), but feel free to keep
> my r-b anyway.
No need to keep it, patches 1 to 4 do not touch HIDp, so no one else
use hidp_send_report().
Cheers,
Benjamin
>
> Thanks
> David
>
>>
>> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>> ---
>> net/bluetooth/hidp/core.c | 46 ----------------------------------------------
>> 1 file changed, 46 deletions(-)
>>
>> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
>> index b062cee..469e61b 100644
>> --- a/net/bluetooth/hidp/core.c
>> +++ b/net/bluetooth/hidp/core.c
>> @@ -223,51 +223,6 @@ static void hidp_input_report(struct hidp_session *session, struct sk_buff *skb)
>> input_sync(dev);
>> }
>>
>> -static int hidp_send_report(struct hidp_session *session, struct hid_report *report)
>> -{
>> - unsigned char hdr;
>> - u8 *buf;
>> - int rsize, ret;
>> -
>> - buf = hid_alloc_report_buf(report, GFP_ATOMIC);
>> - if (!buf)
>> - return -EIO;
>> -
>> - hid_output_report(report, buf);
>> - hdr = HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT;
>> -
>> - rsize = ((report->size - 1) >> 3) + 1 + (report->id > 0);
>> - ret = hidp_send_intr_message(session, hdr, buf, rsize);
>> -
>> - kfree(buf);
>> - return ret;
>> -}
>> -
>> -static int hidp_hidinput_event(struct input_dev *dev, unsigned int type,
>> - unsigned int code, int value)
>> -{
>> - struct hid_device *hid = input_get_drvdata(dev);
>> - struct hidp_session *session = hid->driver_data;
>> - struct hid_field *field;
>> - int offset;
>> -
>> - BT_DBG("session %p type %d code %d value %d",
>> - session, type, code, value);
>> -
>> - if (type != EV_LED)
>> - return -1;
>> -
>> - offset = hidinput_find_field(hid, type, code, &field);
>> - if (offset == -1) {
>> - hid_warn(dev, "event field not found\n");
>> - return -1;
>> - }
>> -
>> - hid_set_field(field, offset, value);
>> -
>> - return hidp_send_report(session, field->report);
>> -}
>> -
>> static int hidp_get_raw_report(struct hid_device *hid,
>> unsigned char report_number,
>> unsigned char *data, size_t count,
>> @@ -817,7 +772,6 @@ static struct hid_ll_driver hidp_hid_driver = {
>> .close = hidp_close,
>> .raw_request = hidp_raw_request,
>> .output_report = hidp_output_report,
>> - .hidinput_input_event = hidp_hidinput_event,
>> };
>>
>> /* This function sets up the hid device. It does not add it
>> --
>> 1.8.3.1
>>
^ permalink raw reply
* Re: [PATCH 01/11] HID: uHID: implement .raw_request
From: David Herrmann @ 2014-02-03 15:26 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Benjamin Tissoires, Jiri Kosina, Frank Praznik,
open list:HID CORE LAYER, linux-kernel
In-Reply-To: <1391316630-29541-2-git-send-email-benjamin.tissoires@redhat.com>
Hi
On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> It was missing, so adding it.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
> drivers/hid/uhid.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
> index f5a2b19..438c9f1 100644
> --- a/drivers/hid/uhid.c
> +++ b/drivers/hid/uhid.c
> @@ -270,6 +270,22 @@ static int uhid_hid_output_report(struct hid_device *hid, __u8 *buf,
> return count;
> }
>
> +static int uhid_raw_request(struct hid_device *hid, unsigned char reportnum,
> + __u8 *buf, size_t len, unsigned char rtype,
> + int reqtype)
> +{
> + switch (reqtype) {
> + case HID_REQ_GET_REPORT:
> + return uhid_hid_get_raw(hid, reportnum, buf, len, rtype);
> + case HID_REQ_SET_REPORT:
> + if (buf[0] != reportnum)
> + return -EINVAL;
> + return uhid_hid_output_raw(hid, buf, len, rtype);
But that one looks wrong. UHID does not have any SET_REPORT query in
the protocol, yet. You turn a SET_REPORT into an OUTPUT_REPORT here.
So if user-space gets the UHID_OUTPUT event, it doesn't know whether
to send a SET_REPORT@ctrl to the device, or an async
OUTPUT_REPORT@intr. This at least matters for low-energy BT in bluez,
which uses uhid.
So we'd have to add an UHID_SET_REPORT event. Note that currently
UHID_FEATURE is the equivalent of UHID_GET_REPORT, but just horribly
named..
Thanks
David
> + default:
> + return -EIO;
> + }
> +}
> +
> static struct hid_ll_driver uhid_hid_driver = {
> .start = uhid_hid_start,
> .stop = uhid_hid_stop,
> @@ -277,6 +293,7 @@ static struct hid_ll_driver uhid_hid_driver = {
> .close = uhid_hid_close,
> .parse = uhid_hid_parse,
> .output_report = uhid_hid_output_report,
> + .raw_request = uhid_raw_request,
> };
>
> #ifdef CONFIG_COMPAT
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [PATCH 07/11] HID: HIDp: remove duplicated coded
From: David Herrmann @ 2014-02-03 15:19 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Benjamin Tissoires, Jiri Kosina, Frank Praznik,
open list:HID CORE LAYER, linux-kernel
In-Reply-To: <CAN+gG=G3njr09E3Q_6kd1SSp5k-Wcn1+aeHee5-Q6tWwFtuA1w@mail.gmail.com>
Hi
On Mon, Feb 3, 2014 at 4:11 PM, Benjamin Tissoires
<benjamin.tissoires@gmail.com> wrote:
> On Mon, Feb 3, 2014 at 10:02 AM, David Herrmann <dh.herrmann@gmail.com> wrote:
>> Hi
>>
>> On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
>> <benjamin.tissoires@redhat.com> wrote:
>>> - Move hidp_output_report() above
>>> - Removed duplicated code in hidp_output_raw_report()
>>>
>>> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>>> ---
>>> net/bluetooth/hidp/core.c | 68 ++++++++---------------------------------------
>>> 1 file changed, 11 insertions(+), 57 deletions(-)
>>>
>>> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
>>> index 469e61b..02670b3 100644
>>> --- a/net/bluetooth/hidp/core.c
>>> +++ b/net/bluetooth/hidp/core.c
>>> @@ -373,62 +373,25 @@ err:
>>> return ret;
>>> }
>>>
>>> -static int hidp_output_raw_report(struct hid_device *hid, unsigned char *data, size_t count,
>>> - unsigned char report_type)
>>> +static int hidp_output_report(struct hid_device *hid, __u8 *data, size_t count)
>>> {
>>> struct hidp_session *session = hid->driver_data;
>>> - int ret;
>>>
>>> + return hidp_send_intr_message(session,
>>> + HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT,
>>> + data, count);
>>> +}
>>> +
>>> +static int hidp_output_raw_report(struct hid_device *hid, unsigned char *data,
>>> + size_t count, unsigned char report_type)
>>> +{
>>> if (report_type == HID_OUTPUT_REPORT) {
>>> - report_type = HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT;
>>> - return hidp_send_intr_message(session, report_type,
>>> - data, count);
>>> + return hidp_output_report(hid, data, count);
>>
>> NAK, that does not work with BT. hidp_output_raw_report() can do both,
>> a synchronous SET_REPORT and an async OUTPUT_REPORT. It's currently
>> screwed up, as for RTYPE_OUTPUT it uses the intr-channel, for
>> RTYPE_FEATURE the ctrl-channel.
>
> David, please, re-review it carefully, and see that there is _no_
> change in the code path with this refactoring:
> hidp_output_report() calls exactly one hidp_send_intr_message() with
> the same arguments.
>
> And for the rest, you will see that the code path is still the same also.
Sorry, my bad. I thought you changed hidp_output_raw_report() to call
hidp_output_report() unconditionally. You're indeed right, the
code-paths are the same and they now highlight the very horrible hack
hid_output_raw_report() has been in the past for HIDP.
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Thanks
David
>>
>> This function is exactly the reason why we added raw_report() and
>> output_report(). So I'd like to keep it as it and just remove it once
>> we have no more users of hidp_output_raw_report().
>
> I worked on that yesterday, so I can make this happening soon (not
> today though).
>
> Cheers,
> Benjamin
>
>>
>> Thanks
>> David
>>
>>> } else if (report_type != HID_FEATURE_REPORT) {
>>> return -EINVAL;
>>> }
>>>
>>> - if (mutex_lock_interruptible(&session->report_mutex))
>>> - return -ERESTARTSYS;
>>> -
>>> - /* Set up our wait, and send the report request to the device. */
>>> - set_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags);
>>> - report_type = HIDP_TRANS_SET_REPORT | HIDP_DATA_RTYPE_FEATURE;
>>> - ret = hidp_send_ctrl_message(session, report_type, data, count);
>>> - if (ret)
>>> - goto err;
>>> -
>>> - /* Wait for the ACK from the device. */
>>> - while (test_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags) &&
>>> - !atomic_read(&session->terminate)) {
>>> - int res;
>>> -
>>> - res = wait_event_interruptible_timeout(session->report_queue,
>>> - !test_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags)
>>> - || atomic_read(&session->terminate),
>>> - 10*HZ);
>>> - if (res == 0) {
>>> - /* timeout */
>>> - ret = -EIO;
>>> - goto err;
>>> - }
>>> - if (res < 0) {
>>> - /* signal */
>>> - ret = -ERESTARTSYS;
>>> - goto err;
>>> - }
>>> - }
>>> -
>>> - if (!session->output_report_success) {
>>> - ret = -EIO;
>>> - goto err;
>>> - }
>>> -
>>> - ret = count;
>>> -
>>> -err:
>>> - clear_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags);
>>> - mutex_unlock(&session->report_mutex);
>>> - return ret;
>>> + return hidp_set_raw_report(hid, data[0], data, count, report_type);
>>> }
>>>
>>> static int hidp_raw_request(struct hid_device *hid, unsigned char reportnum,
>>> @@ -445,15 +408,6 @@ static int hidp_raw_request(struct hid_device *hid, unsigned char reportnum,
>>> }
>>> }
>>>
>>> -static int hidp_output_report(struct hid_device *hid, __u8 *data, size_t count)
>>> -{
>>> - struct hidp_session *session = hid->driver_data;
>>> -
>>> - return hidp_send_intr_message(session,
>>> - HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT,
>>> - data, count);
>>> -}
>>> -
>>> static void hidp_idle_timeout(unsigned long arg)
>>> {
>>> struct hidp_session *session = (struct hidp_session *) arg;
>>> --
>>> 1.8.3.1
>>>
^ permalink raw reply
* Re: [PATCH 07/11] HID: HIDp: remove duplicated coded
From: Benjamin Tissoires @ 2014-02-03 15:11 UTC (permalink / raw)
To: David Herrmann
Cc: Benjamin Tissoires, Jiri Kosina, Frank Praznik,
open list:HID CORE LAYER, linux-kernel
In-Reply-To: <CANq1E4S6jCc4Kd2EKoG0SgSc68TrBBSfnKChGV6Tn3fGKGRnSQ@mail.gmail.com>
On Mon, Feb 3, 2014 at 10:02 AM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
>> - Move hidp_output_report() above
>> - Removed duplicated code in hidp_output_raw_report()
>>
>> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>> ---
>> net/bluetooth/hidp/core.c | 68 ++++++++---------------------------------------
>> 1 file changed, 11 insertions(+), 57 deletions(-)
>>
>> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
>> index 469e61b..02670b3 100644
>> --- a/net/bluetooth/hidp/core.c
>> +++ b/net/bluetooth/hidp/core.c
>> @@ -373,62 +373,25 @@ err:
>> return ret;
>> }
>>
>> -static int hidp_output_raw_report(struct hid_device *hid, unsigned char *data, size_t count,
>> - unsigned char report_type)
>> +static int hidp_output_report(struct hid_device *hid, __u8 *data, size_t count)
>> {
>> struct hidp_session *session = hid->driver_data;
>> - int ret;
>>
>> + return hidp_send_intr_message(session,
>> + HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT,
>> + data, count);
>> +}
>> +
>> +static int hidp_output_raw_report(struct hid_device *hid, unsigned char *data,
>> + size_t count, unsigned char report_type)
>> +{
>> if (report_type == HID_OUTPUT_REPORT) {
>> - report_type = HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT;
>> - return hidp_send_intr_message(session, report_type,
>> - data, count);
>> + return hidp_output_report(hid, data, count);
>
> NAK, that does not work with BT. hidp_output_raw_report() can do both,
> a synchronous SET_REPORT and an async OUTPUT_REPORT. It's currently
> screwed up, as for RTYPE_OUTPUT it uses the intr-channel, for
> RTYPE_FEATURE the ctrl-channel.
David, please, re-review it carefully, and see that there is _no_
change in the code path with this refactoring:
hidp_output_report() calls exactly one hidp_send_intr_message() with
the same arguments.
And for the rest, you will see that the code path is still the same also.
>
> This function is exactly the reason why we added raw_report() and
> output_report(). So I'd like to keep it as it and just remove it once
> we have no more users of hidp_output_raw_report().
I worked on that yesterday, so I can make this happening soon (not
today though).
Cheers,
Benjamin
>
> Thanks
> David
>
>> } else if (report_type != HID_FEATURE_REPORT) {
>> return -EINVAL;
>> }
>>
>> - if (mutex_lock_interruptible(&session->report_mutex))
>> - return -ERESTARTSYS;
>> -
>> - /* Set up our wait, and send the report request to the device. */
>> - set_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags);
>> - report_type = HIDP_TRANS_SET_REPORT | HIDP_DATA_RTYPE_FEATURE;
>> - ret = hidp_send_ctrl_message(session, report_type, data, count);
>> - if (ret)
>> - goto err;
>> -
>> - /* Wait for the ACK from the device. */
>> - while (test_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags) &&
>> - !atomic_read(&session->terminate)) {
>> - int res;
>> -
>> - res = wait_event_interruptible_timeout(session->report_queue,
>> - !test_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags)
>> - || atomic_read(&session->terminate),
>> - 10*HZ);
>> - if (res == 0) {
>> - /* timeout */
>> - ret = -EIO;
>> - goto err;
>> - }
>> - if (res < 0) {
>> - /* signal */
>> - ret = -ERESTARTSYS;
>> - goto err;
>> - }
>> - }
>> -
>> - if (!session->output_report_success) {
>> - ret = -EIO;
>> - goto err;
>> - }
>> -
>> - ret = count;
>> -
>> -err:
>> - clear_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags);
>> - mutex_unlock(&session->report_mutex);
>> - return ret;
>> + return hidp_set_raw_report(hid, data[0], data, count, report_type);
>> }
>>
>> static int hidp_raw_request(struct hid_device *hid, unsigned char reportnum,
>> @@ -445,15 +408,6 @@ static int hidp_raw_request(struct hid_device *hid, unsigned char reportnum,
>> }
>> }
>>
>> -static int hidp_output_report(struct hid_device *hid, __u8 *data, size_t count)
>> -{
>> - struct hidp_session *session = hid->driver_data;
>> -
>> - return hidp_send_intr_message(session,
>> - HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT,
>> - data, count);
>> -}
>> -
>> static void hidp_idle_timeout(unsigned long arg)
>> {
>> struct hidp_session *session = (struct hidp_session *) arg;
>> --
>> 1.8.3.1
>>
^ permalink raw reply
* Re: [PATCH 05/11] HID: HIDp: remove hidp_hidinput_event
From: David Herrmann @ 2014-02-03 15:10 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Benjamin Tissoires, Jiri Kosina, Frank Praznik,
open list:HID CORE LAYER, linux-kernel
In-Reply-To: <1391316630-29541-6-git-send-email-benjamin.tissoires@redhat.com>
Hi
On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> hidp uses its own ->hidinput_input_event() instead of the generic binding
> in hid-input.
> Moving the handling of LEDs towards hidp_hidinput_event() allows two things:
> - remove hidinput_input_event definitively from struct hid_device
> - hidraw user space programs can also set the LEDs
This one looks good. hid-input uses output_raw_report(), which is on
INTR for BT-HID, which is equivalent to what hidp_send_report() does.
So:
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Btw., you might have to keep hidp_send_report() if you drop earlier
patches (haven't exactly looked at the order), but feel free to keep
my r-b anyway.
Thanks
David
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
> net/bluetooth/hidp/core.c | 46 ----------------------------------------------
> 1 file changed, 46 deletions(-)
>
> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> index b062cee..469e61b 100644
> --- a/net/bluetooth/hidp/core.c
> +++ b/net/bluetooth/hidp/core.c
> @@ -223,51 +223,6 @@ static void hidp_input_report(struct hidp_session *session, struct sk_buff *skb)
> input_sync(dev);
> }
>
> -static int hidp_send_report(struct hidp_session *session, struct hid_report *report)
> -{
> - unsigned char hdr;
> - u8 *buf;
> - int rsize, ret;
> -
> - buf = hid_alloc_report_buf(report, GFP_ATOMIC);
> - if (!buf)
> - return -EIO;
> -
> - hid_output_report(report, buf);
> - hdr = HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT;
> -
> - rsize = ((report->size - 1) >> 3) + 1 + (report->id > 0);
> - ret = hidp_send_intr_message(session, hdr, buf, rsize);
> -
> - kfree(buf);
> - return ret;
> -}
> -
> -static int hidp_hidinput_event(struct input_dev *dev, unsigned int type,
> - unsigned int code, int value)
> -{
> - struct hid_device *hid = input_get_drvdata(dev);
> - struct hidp_session *session = hid->driver_data;
> - struct hid_field *field;
> - int offset;
> -
> - BT_DBG("session %p type %d code %d value %d",
> - session, type, code, value);
> -
> - if (type != EV_LED)
> - return -1;
> -
> - offset = hidinput_find_field(hid, type, code, &field);
> - if (offset == -1) {
> - hid_warn(dev, "event field not found\n");
> - return -1;
> - }
> -
> - hid_set_field(field, offset, value);
> -
> - return hidp_send_report(session, field->report);
> -}
> -
> static int hidp_get_raw_report(struct hid_device *hid,
> unsigned char report_number,
> unsigned char *data, size_t count,
> @@ -817,7 +772,6 @@ static struct hid_ll_driver hidp_hid_driver = {
> .close = hidp_close,
> .raw_request = hidp_raw_request,
> .output_report = hidp_output_report,
> - .hidinput_input_event = hidp_hidinput_event,
> };
>
> /* This function sets up the hid device. It does not add it
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [PATCH 07/11] HID: HIDp: remove duplicated coded
From: David Herrmann @ 2014-02-03 15:02 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Benjamin Tissoires, Jiri Kosina, Frank Praznik,
open list:HID CORE LAYER, linux-kernel
In-Reply-To: <1391316630-29541-8-git-send-email-benjamin.tissoires@redhat.com>
Hi
On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> - Move hidp_output_report() above
> - Removed duplicated code in hidp_output_raw_report()
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
> net/bluetooth/hidp/core.c | 68 ++++++++---------------------------------------
> 1 file changed, 11 insertions(+), 57 deletions(-)
>
> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> index 469e61b..02670b3 100644
> --- a/net/bluetooth/hidp/core.c
> +++ b/net/bluetooth/hidp/core.c
> @@ -373,62 +373,25 @@ err:
> return ret;
> }
>
> -static int hidp_output_raw_report(struct hid_device *hid, unsigned char *data, size_t count,
> - unsigned char report_type)
> +static int hidp_output_report(struct hid_device *hid, __u8 *data, size_t count)
> {
> struct hidp_session *session = hid->driver_data;
> - int ret;
>
> + return hidp_send_intr_message(session,
> + HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT,
> + data, count);
> +}
> +
> +static int hidp_output_raw_report(struct hid_device *hid, unsigned char *data,
> + size_t count, unsigned char report_type)
> +{
> if (report_type == HID_OUTPUT_REPORT) {
> - report_type = HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT;
> - return hidp_send_intr_message(session, report_type,
> - data, count);
> + return hidp_output_report(hid, data, count);
NAK, that does not work with BT. hidp_output_raw_report() can do both,
a synchronous SET_REPORT and an async OUTPUT_REPORT. It's currently
screwed up, as for RTYPE_OUTPUT it uses the intr-channel, for
RTYPE_FEATURE the ctrl-channel.
This function is exactly the reason why we added raw_report() and
output_report(). So I'd like to keep it as it and just remove it once
we have no more users of hidp_output_raw_report().
Thanks
David
> } else if (report_type != HID_FEATURE_REPORT) {
> return -EINVAL;
> }
>
> - if (mutex_lock_interruptible(&session->report_mutex))
> - return -ERESTARTSYS;
> -
> - /* Set up our wait, and send the report request to the device. */
> - set_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags);
> - report_type = HIDP_TRANS_SET_REPORT | HIDP_DATA_RTYPE_FEATURE;
> - ret = hidp_send_ctrl_message(session, report_type, data, count);
> - if (ret)
> - goto err;
> -
> - /* Wait for the ACK from the device. */
> - while (test_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags) &&
> - !atomic_read(&session->terminate)) {
> - int res;
> -
> - res = wait_event_interruptible_timeout(session->report_queue,
> - !test_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags)
> - || atomic_read(&session->terminate),
> - 10*HZ);
> - if (res == 0) {
> - /* timeout */
> - ret = -EIO;
> - goto err;
> - }
> - if (res < 0) {
> - /* signal */
> - ret = -ERESTARTSYS;
> - goto err;
> - }
> - }
> -
> - if (!session->output_report_success) {
> - ret = -EIO;
> - goto err;
> - }
> -
> - ret = count;
> -
> -err:
> - clear_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags);
> - mutex_unlock(&session->report_mutex);
> - return ret;
> + return hidp_set_raw_report(hid, data[0], data, count, report_type);
> }
>
> static int hidp_raw_request(struct hid_device *hid, unsigned char reportnum,
> @@ -445,15 +408,6 @@ static int hidp_raw_request(struct hid_device *hid, unsigned char reportnum,
> }
> }
>
> -static int hidp_output_report(struct hid_device *hid, __u8 *data, size_t count)
> -{
> - struct hidp_session *session = hid->driver_data;
> -
> - return hidp_send_intr_message(session,
> - HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT,
> - data, count);
> -}
> -
> static void hidp_idle_timeout(unsigned long arg)
> {
> struct hidp_session *session = (struct hidp_session *) arg;
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [PATCH] Bluetooth: hidp: make sure input buffers are big enough
From: David Herrmann @ 2014-02-03 11:27 UTC (permalink / raw)
To: Jiri Kosina
Cc: Marcel Holtmann, open list:HID CORE LAYER,
linux-bluetooth@vger.kernel.org development, Gustavo F. Padovan
In-Reply-To: <alpine.LNX.2.00.1402031107310.14066@pobox.suse.cz>
Hi Jiri
On Mon, Feb 3, 2014 at 11:08 AM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Wed, 29 Jan 2014, David Herrmann wrote:
>
>> >> Due to various reasons I will not have access to any testing HW for the
>> >> upcoming 2-3 days, so if you can cook something up in that timeframe, it'd
>> >> be appreciated.
>> >>
>> >> Otherwise I'll be working on it by the end of this week.
>> >
>> > David,
>> >
>> > just got back to this, finally ... did you have time to work on this at
>> > all, or should I just start from scratch?
>>
>> Sorry, no. Fosdem is this weekend and I needed to get my code ready
>> for that. But I'll finally have time again next week.
>
> Okay, thanks. I then guess we should proceed with this bandaid (double
> allocation) for 3.14
It would be really nice if we could get the HIDP patch into 3.14-rc2
and backported to stable. There have been quite a bunch of reports now
and I dislike adding a GFP_ATOMIC allocation in HID core. I am back
home now and will try to make HID core honor the length, but that's
quite invasive and shouldn't be used for stable.
Sorry for the delay,
David
^ permalink raw reply
* Re: [PATCH 2/3 RESEND] mfd: tc3589x: Reform device tree probing
From: Lee Jones @ 2014-02-03 11:02 UTC (permalink / raw)
To: Linus Walleij
Cc: devicetree@vger.kernel.org, Dmitry Torokhov, Linux Input,
Samuel Ortiz, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Mark Rutland
In-Reply-To: <CACRpkdaX7AO3C-N+OMCqtQvFUmHmVRfMhPqGY2LciZQHT7iq+g@mail.gmail.com>
> >> > Patch looks good to me. Is there any reason why we should rush this in
> >> > for v3.14, or is it okay to go to -next?
> >>
> >> No rush, but it's been on review like forever so unless there is
> >> some noise from the DT people at -rc1 I'd be very happy if you
> >> could apply patches 1 & 2 by then.
> >
> > I'm just waiting for their Ack. If I don't have it soon I'll review it
> > myself and any changes will have to come in via subsequent patch
> > submissions.
> >
> > I think it's sensible to head for v3.15 for this set.
>
> So now that v3.14-rc1 is out can we queue this stuff?
Queued.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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
^ permalink raw reply
* Re: [PATCH 2/3 RESEND] mfd: tc3589x: Reform device tree probing
From: Lee Jones @ 2014-02-03 11:01 UTC (permalink / raw)
To: Linus Walleij
Cc: devicetree, Dmitry Torokhov, linux-input, Samuel Ortiz,
linux-kernel, linux-arm-kernel, Mark Rutland
In-Reply-To: <1390481008-23900-1-git-send-email-linus.walleij@linaro.org>
> This changes the following mechanisms in the TC3589x device tree
> probing path:
>
> - Use the .of_match_table in struct device_driver to match the
> device in the device tree.
> - Add matches for the proper compatible strings "toshiba,..."
> and all sub-variants, just as is done for the .id matches.
> - Move over all the allocation of platform data etc to the
> tc3589x_of_probe() function and follow the pattern of passing
> a platform data pointer back, or an error pointer on error,
> as found in the STMPE driver.
> - Match the new (proper) compatible strings for the GPIO and
> keypad MFD cells.
> - Use of_device_is_compatible() rather than just !strcmp()
> to discover which cells to instantiate.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> drivers/mfd/tc3589x.c | 84 ++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 59 insertions(+), 25 deletions(-)
Looks good, applied.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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
^ permalink raw reply
* Re: [PATCH 2/3 RESEND] mfd: tc3589x: Reform device tree probing
From: Linus Walleij @ 2014-02-03 10:32 UTC (permalink / raw)
To: Lee Jones
Cc: devicetree@vger.kernel.org, Dmitry Torokhov, Linux Input,
Samuel Ortiz, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Mark Rutland
In-Reply-To: <20140123151123.GE8586@lee--X1>
On Thu, Jan 23, 2014 at 4:11 PM, Lee Jones <lee.jones@linaro.org> wrote:
>> > Patch looks good to me. Is there any reason why we should rush this in
>> > for v3.14, or is it okay to go to -next?
>>
>> No rush, but it's been on review like forever so unless there is
>> some noise from the DT people at -rc1 I'd be very happy if you
>> could apply patches 1 & 2 by then.
>
> I'm just waiting for their Ack. If I don't have it soon I'll review it
> myself and any changes will have to come in via subsequent patch
> submissions.
>
> I think it's sensible to head for v3.15 for this set.
So now that v3.14-rc1 is out can we queue this stuff?
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 0/4] input: Add support for special keys on ms office kb
From: Jiri Kosina @ 2014-02-03 10:12 UTC (permalink / raw)
To: Hans de Goede; +Cc: linux-input
In-Reply-To: <1391014664-2642-1-git-send-email-hdegoede@redhat.com>
On Wed, 29 Jan 2014, Hans de Goede wrote:
> A member of my local hackerspace has donated a Microsoft Office keyboard to
> me. This keyboard has lots of special keys, some of which don't work
> and a scrollwheel which does not work. This patch-set adds support for the
> scrollwheel and the non working special-keys.
Applied, thanks Hans.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] Bluetooth: hidp: make sure input buffers are big enough
From: Jiri Kosina @ 2014-02-03 10:08 UTC (permalink / raw)
To: David Herrmann
Cc: Marcel Holtmann, open list:HID CORE LAYER,
linux-bluetooth@vger.kernel.org development, Gustavo F. Padovan
In-Reply-To: <CANq1E4TW+GU=oaW0tXb=Tun9HMtr26u-PC3H6BsLot560ANgog@mail.gmail.com>
On Wed, 29 Jan 2014, David Herrmann wrote:
> >> Due to various reasons I will not have access to any testing HW for the
> >> upcoming 2-3 days, so if you can cook something up in that timeframe, it'd
> >> be appreciated.
> >>
> >> Otherwise I'll be working on it by the end of this week.
> >
> > David,
> >
> > just got back to this, finally ... did you have time to work on this at
> > all, or should I just start from scratch?
>
> Sorry, no. Fosdem is this weekend and I needed to get my code ready
> for that. But I'll finally have time again next week.
Okay, thanks. I then guess we should proceed with this bandaid (double
allocation) for 3.14
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: fix buffer allocations
From: Jiri Kosina @ 2014-02-03 10:03 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <1391314990-29046-1-git-send-email-benjamin.tissoires@redhat.com>
On Sat, 1 Feb 2014, Benjamin Tissoires wrote:
> When using hid_output_report(), the buffer should be allocated by hid_alloc_report_buf(),
> not a custom malloc.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Good catch, applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 4/7] Input: xpad: Set the correct LED number
From: Greg Kroah-Hartman @ 2014-02-03 8:22 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, Pierre-Loup A. Griffais
In-Reply-To: <1391173414-6199-5-git-send-email-gregkh@linuxfoundation.org>
On Fri, Jan 31, 2014 at 02:03:31PM +0100, Greg Kroah-Hartman wrote:
> From: "Pierre-Loup A. Griffais" <pgriffais@valvesoftware.com>
>
> The LED number should not just be incremented every time, that doesn't
> work, set the number based on the number it actually is.
>
> Note, the joydev subsystem doesn't allow us to easily find out the minor
> number, so we have to walk all devices in order to find a joystick
> device by looking at the name of the device. Odds are, this isn't the
> best way to do it, but I don't know of any other way at the moment.
>
> Signed-off-by: "Pierre-Loup A. Griffais" <pgriffais@valvesoftware.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> drivers/input/joystick/xpad.c | 26 +++++++++++++++++++++-----
> 1 file changed, 21 insertions(+), 5 deletions(-)
I'll preempt David's NAK of this patch before he gets a chance to catch
up on his email from being at FOSDEM :)
I spent some time talking with David about this, and I think I know a
better way to handle this instead of the "walk the joystick devices"
mess that should work for systems that don't even use joydev (which
"modern" systems shouldn't be using anyway.)
So I'll do a respin of this patch series in a week or so when I catch up
with my other stuff and send it out again...
But, if someone wants to review the other 6 in this series, that would
be great.
thanks,
greg k-h
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox