* Re: [PATCH] HID: Add HID transport driver documentation
From: Benjamin Tissoires @ 2014-01-31 23:18 UTC (permalink / raw)
To: David Herrmann; +Cc: Frank Praznik, open list:HID CORE LAYER, Jiri Kosina
In-Reply-To: <CANq1E4TAomD4mKnPYKHBTShV4dynh2dJVQ-bWwAiLJH8g1XFFw@mail.gmail.com>
On Wed, Jan 29, 2014 at 10:35 AM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Wed, Jan 29, 2014 at 4:28 PM, Frank Praznik <frank.praznik@oh.rr.com> wrote:
>> Add David Herrmann's documentation for the new low-level HID transport driver
>> functions.
>>
>> Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
>
> If you copy code, you really should keep the signed-off-by chain. A
> signed-off-by in kernel context means that you either wrote the code
> or have permission to copy it. See here:
> http://developercertificate.org/ (which is a public copy of the
> kernel's signed-off-by practice).
> If you copy code unchanged, it's common practice to even keep the
> "Author" field via "git commit --author", but that's optional.
>
> Anyhow, patch is good, thanks for picking it up!
>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
>
> Putting Benjamin on CC as he reviewed the patch last time and might
> have some more comments (or his final reviewed-by).
>
Thanks David. I am globally happy with it, but I have a little remark:
> Thanks!
> David
>
>> ---
>>
>> Sorry, I forgot to include this in the original patch set.
>>
>> Documentation/hid/hid-transport.txt | 324 ++++++++++++++++++++++++++++++++++++
>> 1 file changed, 324 insertions(+)
>> create mode 100644 Documentation/hid/hid-transport.txt
>>
>> diff --git a/Documentation/hid/hid-transport.txt b/Documentation/hid/hid-transport.txt
>> new file mode 100644
>> index 0000000..14b1c18
>> --- /dev/null
>> +++ b/Documentation/hid/hid-transport.txt
>> @@ -0,0 +1,324 @@
>> + HID I/O Transport Drivers
>> + ===========================
>> +
>> +The HID subsystem is independent of the underlying transport driver. Initially,
>> +only USB was supported, but other specifications adopted the HID design and
>> +provided new transport drivers. The kernel includes at least support for USB,
>> +Bluetooth, I2C and user-space I/O drivers.
>> +
>> +1) HID Bus
>> +==========
>> +
>> +The HID subsystem is designed as a bus. Any I/O subsystem may provide HID
>> +devices and register them with the HID bus. HID core then loads generic device
>> +drivers on top of it. The transport drivers are responsible of raw data
>> +transport and device setup/management. HID core is responsible of
>> +report-parsing, report interpretation and the user-space API. Device specifics
>> +and quirks are handled by all layers depending on the quirk.
>> +
>> + +-----------+ +-----------+ +-----------+ +-----------+
>> + | Device #1 | | Device #i | | Device #j | | Device #k |
>> + +-----------+ +-----------+ +-----------+ +-----------+
>> + \\ // \\ //
>> + +------------+ +------------+
>> + | I/O Driver | | I/O Driver |
>> + +------------+ +------------+
>> + || ||
>> + +------------------+ +------------------+
>> + | Transport Driver | | Transport Driver |
>> + +------------------+ +------------------+
>> + \___ ___/
>> + \ /
>> + +----------------+
>> + | HID Core |
>> + +----------------+
>> + / | | \
>> + / | | \
>> + ____________/ | | \_________________
>> + / | | \
>> + / | | \
>> + +----------------+ +-----------+ +------------------+ +------------------+
>> + | Generic Driver | | MT Driver | | Custom Driver #1 | | Custom Driver #2 |
>> + +----------------+ +-----------+ +------------------+ +------------------+
>> +
>> +Example Drivers:
>> + I/O: USB, I2C, Bluetooth-l2cap
>> + Transport: USB-HID, I2C-HID, BT-HIDP
>> +
>> +Everything below "HID Core" is simplified in this graph as it is only of
>> +interest to HID device drivers. Transport drivers do not need to know the
>> +specifics.
>> +
>> +1.1) Device Setup
>> +-----------------
>> +
>> +I/O drivers normally provide hotplug detection or device enumeration APIs to the
>> +transport drivers. Transport drivers use this to find any suitable HID device.
>> +They allocate HID device objects and register them with HID core. Transport
>> +drivers are not required to register themselves with HID core. HID core is never
>> +aware of which transport drivers are available and is not interested in it. It
>> +is only interested in devices.
>> +
>> +Transport drivers attach a constant "struct hid_ll_driver" object with each
>> +device. Once a device is registered with HID core, the callbacks provided via
>> +this struct are used by HID core to communicate with the device.
>> +
>> +Transport drivers are responsible of detecting device failures and unplugging.
>> +HID core will operate a device as long as it is registered regardless of any
>> +device failures. Once transport drivers detect unplug or failure events, they
>> +must unregister the device from HID core and HID core will stop using the
>> +provided callbacks.
>> +
>> +1.2) Transport Driver Requirements
>> +----------------------------------
>> +
>> +The terms "asynchronous" and "synchronous" in this document describe the
>> +transmission behavior regarding acknowledgements. An asynchronous channel must
>> +not perform any synchronous operations like waiting for acknowledgements or
>> +verifications. Generally, HID calls operating on asynchronous channels must be
>> +running in atomic-context just fine.
>> +On the other hand, synchronous channels can be implemented by the transport
>> +driver in whatever way they like. They might just be the same as asynchronous
>> +channels, but they can also provide acknowledgement reports, automatic
>> +retransmission on failure, etc. in a blocking manner. If such functionality is
>> +required on asynchronous channels, a transport-driver must implement that via
>> +its own worker threads.
>> +
>> +HID core requires transport drivers to follow a given design. A Transport
>> +driver must provide two bi-directional I/O channels to each HID device. These
>> +channels must not necessarily be bi-directional in the hardware itself. A
>> +transport driver might just provide 4 uni-directional channels. Or it might
>> +multiplex all four on a single physical channel. However, in this document we
>> +will describe them as two bi-directional channels as they have several
>> +properties in common.
>> +
>> + - Interrupt Channel (intr): The intr channel is used for asynchronous data
>> + reports. No management commands or data acknowledgements are sent on this
>> + channel. Any unrequested incoming or outgoing data report must be sent on
>> + this channel and is never acknowledged by the remote side. Devices usually
>> + send their input events on this channel. Outgoing events are normally
>> + not send via intr, except if high throughput is required.
>> + - Control Channel (ctrl): The ctrl channel is used for synchronous requests and
>> + device management. Unrequested data input events must not be sent on this
>> + channel and are normally ignored. Instead, devices only send management
>> + events or answers to host requests on this channel.
>> + The control-channel is used for direct blocking queries to the device
>> + independent of any events on the intr-channel.
>> + Outgoing reports are usually sent on the ctrl channel via synchronous
>> + SET_REPORT requests.
>> +
>> +Communication between devices and HID core is mostly done via HID reports. A
>> +report can be of one of three types:
>> +
>> + - INPUT Report: Input reports provide data from device to host. This
>> + data may include button events, axis events, battery status or more. This
>> + data is generated by the device and sent to the host with or without
>> + requiring explicit requests. Devices can choose to send data continuously or
>> + only on change.
>> + - OUTPUT Report: Output reports change device states. They are sent from host
>> + to device and may include LED requests, rumble requests or more. Output
>> + reports are never sent from device to host, but a host can retrieve their
>> + current state.
>> + Hosts may choose to send output reports either continuously or only on
>> + change.
>> + - FEATURE Report: Feature reports are used for specific static device features
>> + and never reported spontaneously. A host can read and/or write them to access
>> + data like battery-state or device-settings.
>> + Feature reports are never sent without requests. A host must explicitly set
>> + or retrieve a feature report. This also means, feature reports are never sent
>> + on the intr channel as this channel is asynchronous.
>> +
>> +INPUT and OUTPUT reports can be sent as pure data reports on the intr channel.
>> +For INPUT reports this is the usual operational mode. But for OUTPUT reports,
>> +this is rarely done as OUTPUT reports are normally quite scarce. But devices are
>> +free to make excessive use of asynchronous OUTPUT reports (for instance, custom
>> +HID audio speakers make great use of it).
>> +
>> +Plain reports must not be sent on the ctrl channel, though. Instead, the ctrl
>> +channel provides synchronous GET/SET_REPORT requests. Plain reports are only
>> +allowed on the intr channel and are the only means of data there.
>> +
>> + - GET_REPORT: A GET_REPORT request has a report ID as payload and is sent
>> + from host to device. The device must answer with a data report for the
>> + requested report ID on the ctrl channel as a synchronous acknowledgement.
>> + Only one GET_REPORT request can be pending for each device. This restriction
>> + is enforced by HID core as several transport drivers don't allow multiple
>> + simultaneous GET_REPORT requests.
>> + Note that data reports which are sent as answer to a GET_REPORT request are
>> + not handled as generic device events. That is, if a device does not operate
>> + in continuous data reporting mode, an answer to GET_REPORT does not replace
>> + the raw data report on the intr channel on state change.
>> + GET_REPORT is only used by custom HID device drivers to query device state.
>> + Normally, HID core caches any device state so this request is not necessary
>> + on devices that follow the HID specs except during device initialization to
>> + retrieve the current state.
>> + GET_REPORT requests can be sent for any of the 3 report types and shall
>> + return the current report state of the device. However, OUTPUT reports as
>> + payload may be blocked by the underlying transport driver if the
>> + specification does not allow them.
>> + - SET_REPORT: A SET_REPORT request has a report ID plus data as payload. It is
>> + sent from host to device and a device must update it's current report state
>> + according to the given data. Any of the 3 report types can be used. However,
>> + INPUT reports as payload might be blocked by the underlying transport driver
>> + if the specification does not allow them.
>> + A device must answer with a synchronous acknowledgement. However, HID core
>> + does not require transport drivers to forward this acknowledgement to HID
>> + core.
>> + Same as for GET_REPORT, only one SET_REPORT can be pending at a time. This
>> + restriction is enforced by HID core as some transport drivers do not support
>> + multiple synchronous SET_REPORT requests.
>> +
>> +Other ctrl-channel requests are supported by USB-HID but are not available
>> +(or deprecated) in most other transport level specifications:
>> +
>> + - GET/SET_IDLE: Only used by USB-HID and I2C-HID.
>> + - GET/SET_PROTOCOL: Not used by HID core.
>> + - RESET: Used by I2C-HID, not hooked up in HID core.
>> + - SET_POWER: Used by I2C-HID, not hooked up in HID core.
>> +
>> +2) HID API
>> +==========
>> +
>> +2.1) Initialization
>> +-------------------
>> +
>> +Transport drivers normally use the following procedure to register a new device
>> +with HID core:
>> +
>> + struct hid_device *hid;
>> + int ret;
>> +
>> + hid = hid_allocate_device();
>> + if (IS_ERR(hid)) {
>> + ret = PTR_ERR(hid);
>> + goto err_<...>;
>> + }
>> +
>> + strlcpy(hid->name, <device-name-src>, 127);
>> + strlcpy(hid->phys, <device-phys-src>, 63);
>> + strlcpy(hid->uniq, <device-uniq-src>, 63);
>> +
>> + hid->ll_driver = &custom_ll_driver;
>> + hid->bus = <device-bus>;
>> + hid->vendor = <device-vendor>;
>> + hid->product = <device-product>;
>> + hid->version = <device-version>;
>> + hid->country = <device-country>;
>> + hid->dev.parent = <pointer-to-parent-device>;
>> + hid->driver_data = <transport-driver-data-field>;
>> +
>> + ret = hid_add_device(hid);
>> + if (ret)
>> + goto err_<...>;
>> +
>> +Once hid_add_device() is entered, HID core might use the callbacks provided in
>> +"custom_ll_driver". Note that fields like "country" can be ignored by underlying
>> +transport-drivers if not supported.
>> +
>> +To unregister a device, use:
>> +
>> + hid_destroy_device(hid);
>> +
>> +Once hid_destroy_device() returns, HID core will no longer make use of any
>> +driver callbacks.
>> +
>> +2.2) hid_ll_driver operations
>> +-----------------------------
>> +
>> +The available HID callbacks are:
>> + - int (*start) (struct hid_device *hdev)
>> + Called from HID device drivers once they want to use the device. Transport
>> + drivers can choose to setup their device in this callback. However, normally
>> + devices are already set up before transport drivers register them to HID core
>> + so this is mostly only used by USB-HID.
>> +
>> + - void (*stop) (struct hid_device *hdev)
>> + Called from HID device drivers once they are done with a device. Transport
>> + drivers can free any buffers and deinitialize the device. But note that
>> + ->start() might be called again if another HID device driver is loaded on the
>> + device.
>> + Transport drivers are free to ignore it and deinitialize devices after they
>> + destroyed them via hid_destroy_device().
>> +
>> + - int (*open) (struct hid_device *hdev)
>> + Called from HID device drivers once they are interested in data reports.
>> + Usually, while user-space didn't open any input API/etc., device drivers are
>> + not interested in device data and transport drivers can put devices asleep.
>> + However, once ->open() is called, transport drivers must be ready for I/O.
>> + ->open() calls are nested for each client that opens the HID device.
>> +
>> + - void (*close) (struct hid_device *hdev)
>> + Called from HID device drivers after ->open() was called but they are no
>> + longer interested in device reports. (Usually if user-space closed any input
>> + devices of the driver).
>> + Transport drivers can put devices asleep and terminate any I/O of all
>> + ->open() calls have been followed by a ->close() call. However, ->start() may
>> + be called again if the device driver is interested in input reports again.
>> +
>> + - int (*parse) (struct hid_device *hdev)
>> + Called once during device setup after ->start() has been called. Transport
>> + drivers must read the HID report-descriptor from the device and tell HID core
>> + about it via hid_parse_report().
>> +
>> + - int (*power) (struct hid_device *hdev, int level)
>> + Called by HID core to give PM hints to transport drivers. Usually this is
>> + analogical to the ->open() and ->close() hints and redundant.
>> +
>> + - void (*request) (struct hid_device *hdev, struct hid_report *report,
>> + int reqtype)
>> + Send an HID request on the ctrl channel. "report" contains the report that
>> + should be sent and "reqtype" the request type. Request-type can be
>> + HID_REQ_SET_REPORT or HID_REQ_GET_REPORT.
>> + This callback is optional. If not provided, HID core will assemble a raw
>> + report following the HID specs and send it via the ->raw_request() callback.
This is not true currently. Aren't we missing a commit in the original series?
But I would love seeing this come true.
Cheers,
Benjamin
>> + The transport driver is free to implement this asynchronously.
>> +
>> + - int (*wait) (struct hid_device *hdev)
>> + Used by HID core before calling ->request() again. A transport driver can use
>> + it to wait for any pending requests to complete if only one request is
>> + allowed at a time.
>> +
>> + - int (*raw_request) (struct hid_device *hdev, unsigned char reportnum,
>> + __u8 *buf, size_t count, unsigned char rtype,
>> + int reqtype)
>> + Same as ->request() but provides the report as raw buffer. This request shall
>> + be synchronous. A transport driver must not use ->wait() to complete such
>> + requests.
>> +
>> + - int (*output_report) (struct hid_device *hdev, __u8 *buf, size_t len)
>> + Send raw output report via intr channel. Used by some HID device drivers
>> + which require high throughput for outgoing requests on the intr channel. This
>> + must not cause SET_REPORT calls! This must be implemented as asynchronous
>> + output report on the intr channel!
>> +
>> + - int (*hidinput_input_event) (struct input_dev *idev, unsigned int type,
>> + unsigned int code, int value)
>> + Obsolete callback used by logitech converters. It is called when userspace
>> + writes input events to the input device (eg., EV_LED). A driver can use this
>> + callback to convert it into an output report and send it to the device. If
>> + this callback is not provided, HID core will use ->request() or
>> + ->raw_request() respectively.
>> +
>> + - int (*idle) (struct hid_device *hdev, int report, int idle, int reqtype)
>> + Perform SET/GET_IDLE request. Only used by USB-HID, do not implement!
>> +
>> +2.3) Data Path
>> +--------------
>> +
>> +Transport drivers are responsible of reading data from I/O devices. They must
>> +handle any I/O-related state-tracking themselves. HID core does not implement
>> +protocol handshakes or other management commands which can be required by the
>> +given HID transport specification.
>> +
>> +Every raw data packet read from a device must be fed into HID core via
>> +hid_input_report(). You must specify the channel-type (intr or ctrl) and report
>> +type (input/output/feature). Under normal conditions, only input reports are
>> +provided via this API.
>> +
>> +Responses to GET_REPORT requests via ->request() must also be provided via this
>> +API. Responses to ->raw_request() are synchronous and must be intercepted by the
>> +transport driver and not passed to hid_input_report().
>> +Acknowledgements to SET_REPORT requests are not of interest to HID core.
>> +
>> +----------------------------------------------------
>> +Written 2013, David Herrmann <dh.herrmann@gmail.com>
>> --
>> 1.8.3.2
>>
^ permalink raw reply
* Re: Sony Vaio Duo 11: getting middle mouse button to work
From: Stephan Mueller @ 2014-01-31 21:02 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Mattia Dongili, platform-driver-x86, Jiri Kosina, linux-input
In-Reply-To: <13914184.mVhhUrzN60@myon.chronox.de>
Am Freitag, 31. Januar 2014, 05:31:53 schrieb Stephan Mueller:
Hi,
The middle mouse button works in Linux, but very differently than you may
expect: I still do not see any events when only pressing the middle mouse
button.
Out of luck, I pressed the middle button and moved the mouse. And there you
see, the mouse returns button 4 and 5.That means that pressing the middle
mouse button seems to morph the mouse into an implicit wheel.
Thus, when I press the middle button and move my mouse, X11 applies the zoom
in and zoom out operations.
I think I can remap the button 4 / 5 to button 2 for the middle mouse. But
then, the button 2 would only work when hitting the middle mouse button and
move the mouse pointer.
This is not what I want. Do you see any other way of turning the middle mouse
button into a real useful button?
Ciao
Stephan
--
| Cui bono? |
^ permalink raw reply
* Re: [PATCH 0/6] HID: Add a stable method for retrieving the client MAC address of a HID device
From: Benjamin Tissoires @ 2014-01-31 20:45 UTC (permalink / raw)
To: Frank Praznik; +Cc: linux-input, Jiri Kosina, David Herrmann
In-Reply-To: <52EC01DF.6050309@oh.rr.com>
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
^ permalink raw reply
* Re: [PATCH 0/6] HID: Add a stable method for retrieving the client MAC address of a HID device
From: Frank Praznik @ 2014-01-31 20:04 UTC (permalink / raw)
To: Benjamin Tissoires, Frank Praznik
Cc: linux-input, Jiri Kosina, David Herrmann
In-Reply-To: <CAN+gG=EVcVmQ8rr2YtoapfftSHaDeYnSu5rCnuZjL=S-v1xU4Q@mail.gmail.com>
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.
^ permalink raw reply
* [PATCH v1] HID: hid-sensor-hub: Processing for duplicate physical ids
From: Srinivas Pandruvada @ 2014-01-31 20:04 UTC (permalink / raw)
To: jkosina; +Cc: jic23, linux-input, Srinivas Pandruvada, Archana Patni
In HID sensor hub, HID physical ids are used to represent different
sensors. For example physical id of 0x73 in usage page = 0x20, represents
an accelerometer. The HID sensor hub driver uses this physical ids to
create platform devices using MFD. There is 1:1 correspondence between
an phy id and a client driver.
But in some cases these physical ids are reused. There is a phy id 0xe1,
which specifies a custom sensor, which can exist multiple times to represent
various custom sensors. In this case there can be multiple instances
of client MFD drivers, processing specific custom sensor. In this
case when client driver looks for report id or a field index, it
should still get the report id specific to its own type. This is
also true for reports, they should be directed towards correct instance.
This change introduce a way to parse and tie physical devices to their
correct instance.
Summary of changes:
- To get physical ids, use collections. If a collection of type=physical
exist then use usage id as in the name of platform device name
- As part of the platform data, we assign a hdsev instance, which has
start and end of collection indexes. Using these indexes attributes
can be tied to correct MFD client instances
- When a report is received, call callback with correct hsdev instance.
In this way using its private data stored as part of its registry, it
can distinguish different sensors even when they have same physical and
logical ids.
This patch is co-authored with Archana Patni<archna.patni@intel.com>.
Reported-by: Archana Patni <archana.patni@intel.com>
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Archana Patni <archana.patni@intel.com>
---
drivers/hid/hid-sensor-hub.c | 188 ++++++++++++++++++++++-------------------
include/linux/hid-sensor-hub.h | 6 +-
2 files changed, 106 insertions(+), 88 deletions(-)
diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index ad2b869..5ab93cb 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -56,9 +56,9 @@ struct sensor_hub_pending {
* @dyn_callback_lock: spin lock to protect callback list
* @hid_sensor_hub_client_devs: Stores all MFD cells for a hub instance.
* @hid_sensor_client_cnt: Number of MFD cells, (no of sensors attached).
+ * @ref_cnt: Number of MFD clients have opened this device
*/
struct sensor_hub_data {
- struct hid_sensor_hub_device *hsdev;
struct mutex mutex;
spinlock_t lock;
struct sensor_hub_pending pending;
@@ -67,6 +67,7 @@ struct sensor_hub_data {
struct mfd_cell *hid_sensor_hub_client_devs;
int hid_sensor_client_cnt;
unsigned long quirks;
+ int ref_cnt;
};
/**
@@ -79,6 +80,7 @@ struct sensor_hub_data {
struct hid_sensor_hub_callbacks_list {
struct list_head list;
u32 usage_id;
+ struct hid_sensor_hub_device *hsdev;
struct hid_sensor_hub_callbacks *usage_callback;
void *priv;
};
@@ -97,20 +99,18 @@ static struct hid_report *sensor_hub_report(int id, struct hid_device *hdev,
return NULL;
}
-static int sensor_hub_get_physical_device_count(
- struct hid_report_enum *report_enum)
+static int sensor_hub_get_physical_device_count(struct hid_device *hdev)
{
- struct hid_report *report;
- struct hid_field *field;
- int cnt = 0;
+ int i;
+ int count = 0;
- list_for_each_entry(report, &report_enum->report_list, list) {
- field = report->field[0];
- if (report->maxfield && field && field->physical)
- cnt++;
+ for (i = 0; i < hdev->maxcollection; ++i) {
+ struct hid_collection *collection = &hdev->collection[i];
+ if (collection->type == HID_COLLECTION_PHYSICAL)
+ ++count;
}
- return cnt;
+ return count;
}
static void sensor_hub_fill_attr_info(
@@ -128,15 +128,23 @@ static void sensor_hub_fill_attr_info(
static struct hid_sensor_hub_callbacks *sensor_hub_get_callback(
struct hid_device *hdev,
- u32 usage_id, void **priv)
+ u32 usage_id,
+ int collection_index,
+ struct hid_sensor_hub_device **hsdev,
+ void **priv)
{
struct hid_sensor_hub_callbacks_list *callback;
struct sensor_hub_data *pdata = hid_get_drvdata(hdev);
spin_lock(&pdata->dyn_callback_lock);
list_for_each_entry(callback, &pdata->dyn_callback_list, list)
- if (callback->usage_id == usage_id) {
+ if (callback->usage_id == usage_id &&
+ (collection_index >=
+ callback->hsdev->start_collection_index) &&
+ (collection_index <
+ callback->hsdev->end_collection_index)) {
*priv = callback->priv;
+ *hsdev = callback->hsdev;
spin_unlock(&pdata->dyn_callback_lock);
return callback->usage_callback;
}
@@ -154,7 +162,8 @@ int sensor_hub_register_callback(struct hid_sensor_hub_device *hsdev,
spin_lock(&pdata->dyn_callback_lock);
list_for_each_entry(callback, &pdata->dyn_callback_list, list)
- if (callback->usage_id == usage_id) {
+ if (callback->usage_id == usage_id &&
+ callback->hsdev == hsdev) {
spin_unlock(&pdata->dyn_callback_lock);
return -EINVAL;
}
@@ -163,6 +172,7 @@ int sensor_hub_register_callback(struct hid_sensor_hub_device *hsdev,
spin_unlock(&pdata->dyn_callback_lock);
return -ENOMEM;
}
+ callback->hsdev = hsdev;
callback->usage_callback = usage_callback;
callback->usage_id = usage_id;
callback->priv = NULL;
@@ -181,7 +191,8 @@ int sensor_hub_remove_callback(struct hid_sensor_hub_device *hsdev,
spin_lock(&pdata->dyn_callback_lock);
list_for_each_entry(callback, &pdata->dyn_callback_list, list)
- if (callback->usage_id == usage_id) {
+ if (callback->usage_id == usage_id &&
+ callback->hsdev == hsdev) {
list_del(&callback->list);
kfree(callback);
break;
@@ -320,8 +331,7 @@ int sensor_hub_input_get_attribute_info(struct hid_sensor_hub_device *hsdev,
struct hid_sensor_hub_attribute_info *info)
{
int ret = -1;
- int i, j;
- int collection_index = -1;
+ int i;
struct hid_report *report;
struct hid_field *field;
struct hid_report_enum *report_enum;
@@ -335,44 +345,31 @@ int sensor_hub_input_get_attribute_info(struct hid_sensor_hub_device *hsdev,
info->units = -1;
info->unit_expo = -1;
- for (i = 0; i < hdev->maxcollection; ++i) {
- struct hid_collection *collection = &hdev->collection[i];
- if (usage_id == collection->usage) {
- collection_index = i;
- break;
- }
- }
- if (collection_index == -1)
- goto err_ret;
-
report_enum = &hdev->report_enum[type];
list_for_each_entry(report, &report_enum->report_list, list) {
for (i = 0; i < report->maxfield; ++i) {
field = report->field[i];
- if (field->physical == usage_id &&
- field->logical == attr_usage_id) {
- sensor_hub_fill_attr_info(info, i, report->id,
- field);
- ret = 0;
- } else {
- for (j = 0; j < field->maxusage; ++j) {
- if (field->usage[j].hid ==
- attr_usage_id &&
- field->usage[j].collection_index ==
- collection_index) {
- sensor_hub_fill_attr_info(info,
- i, report->id, field);
- ret = 0;
- break;
- }
+ if (field->maxusage) {
+ if (field->physical == usage_id &&
+ (field->logical == attr_usage_id ||
+ field->usage[0].hid ==
+ attr_usage_id) &&
+ (field->usage[0].collection_index >=
+ hsdev->start_collection_index) &&
+ (field->usage[0].collection_index <
+ hsdev->end_collection_index)) {
+
+ sensor_hub_fill_attr_info(info, i,
+ report->id,
+ field);
+ ret = 0;
+ break;
}
}
- if (ret == 0)
- break;
}
+
}
-err_ret:
return ret;
}
EXPORT_SYMBOL_GPL(sensor_hub_input_get_attribute_info);
@@ -388,7 +385,7 @@ static int sensor_hub_suspend(struct hid_device *hdev, pm_message_t message)
list_for_each_entry(callback, &pdata->dyn_callback_list, list) {
if (callback->usage_callback->suspend)
callback->usage_callback->suspend(
- pdata->hsdev, callback->priv);
+ callback->hsdev, callback->priv);
}
spin_unlock(&pdata->dyn_callback_lock);
@@ -405,7 +402,7 @@ static int sensor_hub_resume(struct hid_device *hdev)
list_for_each_entry(callback, &pdata->dyn_callback_list, list) {
if (callback->usage_callback->resume)
callback->usage_callback->resume(
- pdata->hsdev, callback->priv);
+ callback->hsdev, callback->priv);
}
spin_unlock(&pdata->dyn_callback_lock);
@@ -432,6 +429,7 @@ static int sensor_hub_raw_event(struct hid_device *hdev,
struct hid_sensor_hub_callbacks *callback = NULL;
struct hid_collection *collection = NULL;
void *priv = NULL;
+ struct hid_sensor_hub_device *hsdev = NULL;
hid_dbg(hdev, "sensor_hub_raw_event report id:0x%x size:%d type:%d\n",
report->id, size, report->type);
@@ -466,23 +464,26 @@ static int sensor_hub_raw_event(struct hid_device *hdev,
report->field[i]->usage->collection_index];
hid_dbg(hdev, "collection->usage %x\n",
collection->usage);
- callback = sensor_hub_get_callback(pdata->hsdev->hdev,
- report->field[i]->physical,
- &priv);
+
+ callback = sensor_hub_get_callback(hdev,
+ report->field[i]->physical,
+ report->field[i]->usage[0].collection_index,
+ &hsdev, &priv);
+
if (callback && callback->capture_sample) {
if (report->field[i]->logical)
- callback->capture_sample(pdata->hsdev,
+ callback->capture_sample(hsdev,
report->field[i]->logical, sz, ptr,
callback->pdev);
else
- callback->capture_sample(pdata->hsdev,
+ callback->capture_sample(hsdev,
report->field[i]->usage->hid, sz, ptr,
callback->pdev);
}
ptr += sz;
}
if (callback && collection && callback->send_event)
- callback->send_event(pdata->hsdev, collection->usage,
+ callback->send_event(hsdev, collection->usage,
callback->pdev);
spin_unlock_irqrestore(&pdata->lock, flags);
@@ -495,7 +496,7 @@ int sensor_hub_device_open(struct hid_sensor_hub_device *hsdev)
struct sensor_hub_data *data = hid_get_drvdata(hsdev->hdev);
mutex_lock(&data->mutex);
- if (!hsdev->ref_cnt) {
+ if (!data->ref_cnt) {
ret = hid_hw_open(hsdev->hdev);
if (ret) {
hid_err(hsdev->hdev, "failed to open hid device\n");
@@ -503,7 +504,7 @@ int sensor_hub_device_open(struct hid_sensor_hub_device *hsdev)
return ret;
}
}
- hsdev->ref_cnt++;
+ data->ref_cnt++;
mutex_unlock(&data->mutex);
return ret;
@@ -515,8 +516,8 @@ void sensor_hub_device_close(struct hid_sensor_hub_device *hsdev)
struct sensor_hub_data *data = hid_get_drvdata(hsdev->hdev);
mutex_lock(&data->mutex);
- hsdev->ref_cnt--;
- if (!hsdev->ref_cnt)
+ data->ref_cnt--;
+ if (!data->ref_cnt)
hid_hw_close(hsdev->hdev);
mutex_unlock(&data->mutex);
}
@@ -563,26 +564,19 @@ static int sensor_hub_probe(struct hid_device *hdev,
struct sensor_hub_data *sd;
int i;
char *name;
- struct hid_report *report;
- struct hid_report_enum *report_enum;
- struct hid_field *field;
int dev_cnt;
+ struct hid_sensor_hub_device *hsdev;
+ struct hid_sensor_hub_device *last_hsdev = NULL;
sd = devm_kzalloc(&hdev->dev, sizeof(*sd), GFP_KERNEL);
if (!sd) {
hid_err(hdev, "cannot allocate Sensor data\n");
return -ENOMEM;
}
- sd->hsdev = devm_kzalloc(&hdev->dev, sizeof(*sd->hsdev), GFP_KERNEL);
- if (!sd->hsdev) {
- hid_err(hdev, "cannot allocate hid_sensor_hub_device\n");
- return -ENOMEM;
- }
+
hid_set_drvdata(hdev, sd);
sd->quirks = id->driver_data;
- sd->hsdev->hdev = hdev;
- sd->hsdev->vendor_id = hdev->vendor;
- sd->hsdev->product_id = hdev->product;
+
spin_lock_init(&sd->lock);
spin_lock_init(&sd->dyn_callback_lock);
mutex_init(&sd->mutex);
@@ -600,9 +594,8 @@ static int sensor_hub_probe(struct hid_device *hdev,
}
INIT_LIST_HEAD(&sd->dyn_callback_list);
sd->hid_sensor_client_cnt = 0;
- report_enum = &hdev->report_enum[HID_INPUT_REPORT];
- dev_cnt = sensor_hub_get_physical_device_count(report_enum);
+ dev_cnt = sensor_hub_get_physical_device_count(hdev);
if (dev_cnt > HID_MAX_PHY_DEVICES) {
hid_err(hdev, "Invalid Physical device count\n");
ret = -EINVAL;
@@ -616,42 +609,63 @@ static int sensor_hub_probe(struct hid_device *hdev,
ret = -ENOMEM;
goto err_stop_hw;
}
- list_for_each_entry(report, &report_enum->report_list, list) {
- hid_dbg(hdev, "Report id:%x\n", report->id);
- field = report->field[0];
- if (report->maxfield && field &&
- field->physical) {
+
+ for (i = 0; i < hdev->maxcollection; ++i) {
+ struct hid_collection *collection = &hdev->collection[i];
+
+ if (collection->type == HID_COLLECTION_PHYSICAL) {
+
+ hsdev = kzalloc(sizeof(*hsdev), GFP_KERNEL);
+ if (!hsdev) {
+ hid_err(hdev, "cannot allocate hid_sensor_hub_device\n");
+ ret = -ENOMEM;
+ goto err_no_mem;
+ }
+ hsdev->hdev = hdev;
+ hsdev->vendor_id = hdev->vendor;
+ hsdev->product_id = hdev->product;
+ hsdev->start_collection_index = i;
+ if (last_hsdev)
+ last_hsdev->end_collection_index = i;
+ last_hsdev = hsdev;
name = kasprintf(GFP_KERNEL, "HID-SENSOR-%x",
- field->physical);
+ collection->usage);
if (name == NULL) {
hid_err(hdev, "Failed MFD device name\n");
ret = -ENOMEM;
- goto err_free_names;
+ goto err_no_mem;
}
sd->hid_sensor_hub_client_devs[
- sd->hid_sensor_client_cnt].id = PLATFORM_DEVID_AUTO;
+ sd->hid_sensor_client_cnt].id =
+ PLATFORM_DEVID_AUTO;
sd->hid_sensor_hub_client_devs[
sd->hid_sensor_client_cnt].name = name;
sd->hid_sensor_hub_client_devs[
sd->hid_sensor_client_cnt].platform_data =
- sd->hsdev;
+ hsdev;
sd->hid_sensor_hub_client_devs[
sd->hid_sensor_client_cnt].pdata_size =
- sizeof(*sd->hsdev);
- hid_dbg(hdev, "Adding %s:%p\n", name, sd);
+ sizeof(*hsdev);
+ hid_dbg(hdev, "Adding %s:%d\n", name,
+ hsdev->start_collection_index);
sd->hid_sensor_client_cnt++;
}
}
+ if (last_hsdev)
+ last_hsdev->end_collection_index = i;
+
ret = mfd_add_devices(&hdev->dev, 0, sd->hid_sensor_hub_client_devs,
sd->hid_sensor_client_cnt, NULL, 0, NULL);
if (ret < 0)
- goto err_free_names;
+ goto err_no_mem;
return ret;
-err_free_names:
- for (i = 0; i < sd->hid_sensor_client_cnt ; ++i)
+err_no_mem:
+ for (i = 0; i < sd->hid_sensor_client_cnt; ++i) {
kfree(sd->hid_sensor_hub_client_devs[i].name);
+ kfree(sd->hid_sensor_hub_client_devs[i].platform_data);
+ }
kfree(sd->hid_sensor_hub_client_devs);
err_stop_hw:
hid_hw_stop(hdev);
@@ -673,8 +687,10 @@ static void sensor_hub_remove(struct hid_device *hdev)
complete(&data->pending.ready);
spin_unlock_irqrestore(&data->lock, flags);
mfd_remove_devices(&hdev->dev);
- for (i = 0; i < data->hid_sensor_client_cnt ; ++i)
+ for (i = 0; i < data->hid_sensor_client_cnt; ++i) {
kfree(data->hid_sensor_hub_client_devs[i].name);
+ kfree(data->hid_sensor_hub_client_devs[i].platform_data);
+ }
kfree(data->hid_sensor_hub_client_devs);
hid_set_drvdata(hdev, NULL);
mutex_destroy(&data->mutex);
diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h
index 205eba0..b70cfd7 100644
--- a/include/linux/hid-sensor-hub.h
+++ b/include/linux/hid-sensor-hub.h
@@ -51,13 +51,15 @@ struct hid_sensor_hub_attribute_info {
* @hdev: Stores the hid instance.
* @vendor_id: Vendor id of hub device.
* @product_id: Product id of hub device.
- * @ref_cnt: Number of MFD clients have opened this device
+ * @start_collection_index: Starting index for a phy type collection
+ * @end_collection_index: Last index for a phy type collection
*/
struct hid_sensor_hub_device {
struct hid_device *hdev;
u32 vendor_id;
u32 product_id;
- int ref_cnt;
+ int start_collection_index;
+ int end_collection_index;
};
/**
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCH 4/6] HID: Add the HIDIOCGRAWCLIENTADDR ioctl to hidraw
From: Benjamin Tissoires @ 2014-01-31 19:19 UTC (permalink / raw)
To: Frank Praznik; +Cc: linux-input, Jiri Kosina, David Herrmann
In-Reply-To: <1391189573-2591-5-git-send-email-frank.praznik@oh.rr.com>
On Fri, Jan 31, 2014 at 12:32 PM, Frank Praznik <frank.praznik@oh.rr.com> wrote:
> Add a new HIDIOCGRAWCLIENTADDR ioctl to hidraw for retrieving the client
> address.
>
> Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
> ---
> drivers/hid/hidraw.c | 19 +++++++++++++++++++
> include/uapi/linux/hidraw.h | 1 +
> 2 files changed, 20 insertions(+)
>
> diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
> index cb0137b..07c328b 100644
> --- a/drivers/hid/hidraw.c
> +++ b/drivers/hid/hidraw.c
> @@ -447,6 +447,25 @@ static long hidraw_ioctl(struct file *file, unsigned int cmd,
> -EFAULT : len;
> break;
> }
> +
> + if (_IOC_NR(cmd) == _IOC_NR(HIDIOCGRAWCLIENTADDR(0))) {
Maybe we are just lacking an ioctl to retrieve hid->uniq...
Cheers,
Benjamin
> + char addr_str[32];
> + int len;
> + ret = snprintf(addr_str, sizeof(addr_str),
> + "%pMR", hid->client_addr);
> + if (ret < 0)
> + break;
> + else if (ret > sizeof(addr_str)-1)
> + len = sizeof(addr_str);
> + else
> + len = ret + 1;
> +
> + if (len > _IOC_SIZE(cmd))
> + len = _IOC_SIZE(cmd);
> + ret = copy_to_user(user_arg, addr_str, len) ?
> + -EFAULT : len;
> + break;
> + }
> }
>
> ret = -ENOTTY;
> diff --git a/include/uapi/linux/hidraw.h b/include/uapi/linux/hidraw.h
> index f5b7329..5b6f4fa 100644
> --- a/include/uapi/linux/hidraw.h
> +++ b/include/uapi/linux/hidraw.h
> @@ -38,6 +38,7 @@ struct hidraw_devinfo {
> /* The first byte of SFEATURE and GFEATURE is the report number */
> #define HIDIOCSFEATURE(len) _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x06, len)
> #define HIDIOCGFEATURE(len) _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x07, len)
> +#define HIDIOCGRAWCLIENTADDR(len) _IOC(_IOC_READ, 'H', 0x08, len)
>
> #define HIDRAW_FIRST_MINOR 0
> #define HIDRAW_MAX_DEVICES 64
> --
> 1.8.5.3
>
> --
> 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 6/6] HID: Update the hidraw sample with the usage of HIDIOCGRAWCLIENTADDR
From: Benjamin Tissoires @ 2014-01-31 19:18 UTC (permalink / raw)
To: Frank Praznik; +Cc: linux-input, Jiri Kosina, David Herrmann
In-Reply-To: <1391189573-2591-7-git-send-email-frank.praznik@oh.rr.com>
On Fri, Jan 31, 2014 at 12:32 PM, Frank Praznik <frank.praznik@oh.rr.com> wrote:
> Add the HIDIOCGRAWCLIENTADDR ioctl to the hidraw sample program.
>
I would personally squash this one with the previous one, but this is
more a matter of taste IMO.
> Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
> ---
> samples/hidraw/hid-example.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/samples/hidraw/hid-example.c b/samples/hidraw/hid-example.c
> index 512a7e5..dbf072d 100644
> --- a/samples/hidraw/hid-example.c
> +++ b/samples/hidraw/hid-example.c
> @@ -23,6 +23,10 @@
> #define HIDIOCSFEATURE(len) _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x06, len)
> #define HIDIOCGFEATURE(len) _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x07, len)
> #endif
> +#ifndef HIDIOCGRAWCLIENTADDR
> +#warning Please have your distro update the userspace kernel headers
> +#define HIDIOCGRAWCLIENTADDR(len) _IOC(_IOC_READ, 'H', 0x08, len)
> +#endif
>
> /* Unix */
> #include <sys/ioctl.h>
> @@ -93,6 +97,13 @@ int main(int argc, char **argv)
> else
> printf("Raw Phys: %s\n", buf);
>
> + /* Get Client Address */
> + res = ioctl(fd, HIDIOCGRAWCLIENTADDR(256), buf);
> + if (res < 0)
> + perror("HIDIOCGRAWCLIENTADDR");
> + else
> + printf("Raw Client Address: %s\n", buf);
> +
> /* Get Raw Info */
> res = ioctl(fd, HIDIOCGRAWINFO, &info);
> if (res < 0) {
> --
> 1.8.5.3
>
> --
> 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 5/6] HID: Add the HIDIOCGRAWCLIENTADDR ioctl to the hidraw documentation
From: Benjamin Tissoires @ 2014-01-31 19:17 UTC (permalink / raw)
To: Frank Praznik; +Cc: linux-input, Jiri Kosina, David Herrmann
In-Reply-To: <1391189573-2591-6-git-send-email-frank.praznik@oh.rr.com>
On Fri, Jan 31, 2014 at 12:32 PM, Frank Praznik <frank.praznik@oh.rr.com> wrote:
> Add the HIDIOCGRAWCLIENTADDR ioctl to the hidraw documentation.
This one should be squashed with the previous one (same reason in 1/6)
>
> Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
> ---
> Documentation/hid/hidraw.txt | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/hid/hidraw.txt b/Documentation/hid/hidraw.txt
> index 029e6cb..737fd1f 100644
> --- a/Documentation/hid/hidraw.txt
> +++ b/Documentation/hid/hidraw.txt
> @@ -93,6 +93,11 @@ For USB devices, the string contains the physical path to the device (the
> USB controller, hubs, ports, etc). For Bluetooth devices, the string
> contains the hardware (MAC) address of the device.
>
> +HIDIOCGRAWCLIENTADDR(len): Get Client Address
> +This ioctl returns a string representing the client address of the device.
> +For Bluetooth devices, the string contains the hardware (MAC) address of the
> +device. For USB devices, the returned string is always 00:00:00:00:00:00.
> +
> HIDIOCSFEATURE(len): Send a Feature Report
> This ioctl will send a feature report to the device. Per the HID
> specification, feature reports are always sent using the control endpoint.
> --
> 1.8.5.3
>
> --
> 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 3/6] HID: Add support for setting client_addr in uhid.
From: Benjamin Tissoires @ 2014-01-31 19:16 UTC (permalink / raw)
To: Frank Praznik; +Cc: linux-input, Jiri Kosina, David Herrmann
In-Reply-To: <1391189573-2591-4-git-send-email-frank.praznik@oh.rr.com>
On Fri, Jan 31, 2014 at 12:32 PM, Frank Praznik <frank.praznik@oh.rr.com> wrote:
> Add support for setting the client address of a device in uhid.
>
> Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
> ---
> drivers/hid/uhid.c | 5 +++++
> include/uapi/linux/uhid.h | 1 +
> 2 files changed, 6 insertions(+)
>
> diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
> index cedc6da..cfd2b93 100644
> --- a/drivers/hid/uhid.c
> +++ b/drivers/hid/uhid.c
> @@ -259,6 +259,7 @@ struct uhid_create_req_compat {
> __u8 name[128];
> __u8 phys[64];
> __u8 uniq[64];
> + __u8 client_addr[6];
>
> compat_uptr_t rd_data;
> __u16 rd_size;
> @@ -308,6 +309,8 @@ static int uhid_event_from_user(const char __user *buffer, size_t len,
> sizeof(compat->phys));
> memcpy(event->u.create.uniq, compat->uniq,
> sizeof(compat->uniq));
> + memcpy(event->u.create.client_addr, compat->client_addr,
> + sizeof(compat->client_addr));
>
> event->u.create.rd_data = compat_ptr(compat->rd_data);
> event->u.create.rd_size = compat->rd_size;
> @@ -375,6 +378,8 @@ static int uhid_dev_create(struct uhid_device *uhid,
> hid->phys[63] = 0;
> strncpy(hid->uniq, ev->u.create.uniq, 63);
> hid->uniq[63] = 0;
> + memcpy(hid->client_addr, ev->u.create.client_addr,
> + sizeof(hid->client_addr));
>
> hid->ll_driver = &uhid_hid_driver;
> hid->hid_get_raw_report = uhid_hid_get_raw;
> diff --git a/include/uapi/linux/uhid.h b/include/uapi/linux/uhid.h
> index 414b74b..5e48acc 100644
> --- a/include/uapi/linux/uhid.h
> +++ b/include/uapi/linux/uhid.h
> @@ -40,6 +40,7 @@ struct uhid_create_req {
> __u8 name[128];
> __u8 phys[64];
> __u8 uniq[64];
> + __u8 client_addr[6];
This is definitively wrong. By that I mean adding a field in the
middle of a struct in a user-space API without handling the case where
a user-space program will use the old API.
You are breaking the API and the ABI here, and you will lead to kernel
crash if the program using this has not been recompiled.
You can add fields in a struct, don't misinterpret me, but you have to
be sure that you will support both old and new API/ABI.
And yes, there are some users (programs) of uhid which would be broken by this.
Cheers,
Benjamin
> __u8 __user *rd_data;
> __u16 rd_size;
>
> --
> 1.8.5.3
>
> --
> 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 1/6] HID: Add the client_addr member to the hid_device struct
From: Benjamin Tissoires @ 2014-01-31 19:12 UTC (permalink / raw)
To: Frank Praznik; +Cc: linux-input, Jiri Kosina, David Herrmann
In-Reply-To: <1391189573-2591-2-git-send-email-frank.praznik@oh.rr.com>
On Fri, Jan 31, 2014 at 12:32 PM, Frank Praznik <frank.praznik@oh.rr.com> wrote:
> Add the client_addr member to the hid_device struct for storing the client
> address of a connected device.
>
> Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
> ---
> include/linux/hid.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index 31b9d29..283fc6e 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -495,6 +495,7 @@ struct hid_device { /* device report descriptor */
> char name[128]; /* Device name */
> char phys[64]; /* Device physical location */
> char uniq[64]; /* Device unique identifier (serial #) */
> + __u8 client_addr[6]; /* Device client address (if wireless) */
It is good to make small patches, but this one should definitively be
squashed with 2/6.
You should be able to say why you add a field in a struct by looking
at the code in the patch.
Cheers,
Benjamin
>
> void *driver_data;
>
> --
> 1.8.5.3
>
> --
> 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 0/6] HID: Add a stable method for retrieving the client MAC address of a HID device
From: Benjamin Tissoires @ 2014-01-31 19:10 UTC (permalink / raw)
To: Frank Praznik; +Cc: linux-input, Jiri Kosina, David Herrmann
In-Reply-To: <1391189573-2591-1-git-send-email-frank.praznik@oh.rr.com>
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.
I also have few general comments on the patches, I'll comment in them.
Cheers,
Benjamin
>
> The first patch adds a new client_addr member to the hid_device struct. This
> member, when populated, will be guaranteed to contain the client hardware
> address of a connected wireless device.
>
> The second patch modifies HIDP to populate the client_addr member of the
> hid_device struct with the client MAC address of the connected wireless device.
>
> The third patch adds support for setting the client_addr value in a uhid
> device.
>
> The final patches add support for retrieving the client_addr value as a string
> in the hidraw interface via a new ioctl called HIDIOCGRAWCLIENTADDR. The
> hidraw documentation and example program are also updated.
> --
> 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
* [PATCH 6/6] HID: Update the hidraw sample with the usage of HIDIOCGRAWCLIENTADDR
From: Frank Praznik @ 2014-01-31 17:32 UTC (permalink / raw)
To: linux-input; +Cc: jkosina, dh.herrmann, Frank Praznik
In-Reply-To: <1391189573-2591-1-git-send-email-frank.praznik@oh.rr.com>
Add the HIDIOCGRAWCLIENTADDR ioctl to the hidraw sample program.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
samples/hidraw/hid-example.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/samples/hidraw/hid-example.c b/samples/hidraw/hid-example.c
index 512a7e5..dbf072d 100644
--- a/samples/hidraw/hid-example.c
+++ b/samples/hidraw/hid-example.c
@@ -23,6 +23,10 @@
#define HIDIOCSFEATURE(len) _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x06, len)
#define HIDIOCGFEATURE(len) _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x07, len)
#endif
+#ifndef HIDIOCGRAWCLIENTADDR
+#warning Please have your distro update the userspace kernel headers
+#define HIDIOCGRAWCLIENTADDR(len) _IOC(_IOC_READ, 'H', 0x08, len)
+#endif
/* Unix */
#include <sys/ioctl.h>
@@ -93,6 +97,13 @@ int main(int argc, char **argv)
else
printf("Raw Phys: %s\n", buf);
+ /* Get Client Address */
+ res = ioctl(fd, HIDIOCGRAWCLIENTADDR(256), buf);
+ if (res < 0)
+ perror("HIDIOCGRAWCLIENTADDR");
+ else
+ printf("Raw Client Address: %s\n", buf);
+
/* Get Raw Info */
res = ioctl(fd, HIDIOCGRAWINFO, &info);
if (res < 0) {
--
1.8.5.3
^ permalink raw reply related
* [PATCH 4/6] HID: Add the HIDIOCGRAWCLIENTADDR ioctl to hidraw
From: Frank Praznik @ 2014-01-31 17:32 UTC (permalink / raw)
To: linux-input; +Cc: jkosina, dh.herrmann, Frank Praznik
In-Reply-To: <1391189573-2591-1-git-send-email-frank.praznik@oh.rr.com>
Add a new HIDIOCGRAWCLIENTADDR ioctl to hidraw for retrieving the client
address.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
drivers/hid/hidraw.c | 19 +++++++++++++++++++
include/uapi/linux/hidraw.h | 1 +
2 files changed, 20 insertions(+)
diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
index cb0137b..07c328b 100644
--- a/drivers/hid/hidraw.c
+++ b/drivers/hid/hidraw.c
@@ -447,6 +447,25 @@ static long hidraw_ioctl(struct file *file, unsigned int cmd,
-EFAULT : len;
break;
}
+
+ if (_IOC_NR(cmd) == _IOC_NR(HIDIOCGRAWCLIENTADDR(0))) {
+ char addr_str[32];
+ int len;
+ ret = snprintf(addr_str, sizeof(addr_str),
+ "%pMR", hid->client_addr);
+ if (ret < 0)
+ break;
+ else if (ret > sizeof(addr_str)-1)
+ len = sizeof(addr_str);
+ else
+ len = ret + 1;
+
+ if (len > _IOC_SIZE(cmd))
+ len = _IOC_SIZE(cmd);
+ ret = copy_to_user(user_arg, addr_str, len) ?
+ -EFAULT : len;
+ break;
+ }
}
ret = -ENOTTY;
diff --git a/include/uapi/linux/hidraw.h b/include/uapi/linux/hidraw.h
index f5b7329..5b6f4fa 100644
--- a/include/uapi/linux/hidraw.h
+++ b/include/uapi/linux/hidraw.h
@@ -38,6 +38,7 @@ struct hidraw_devinfo {
/* The first byte of SFEATURE and GFEATURE is the report number */
#define HIDIOCSFEATURE(len) _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x06, len)
#define HIDIOCGFEATURE(len) _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x07, len)
+#define HIDIOCGRAWCLIENTADDR(len) _IOC(_IOC_READ, 'H', 0x08, len)
#define HIDRAW_FIRST_MINOR 0
#define HIDRAW_MAX_DEVICES 64
--
1.8.5.3
^ permalink raw reply related
* [PATCH 5/6] HID: Add the HIDIOCGRAWCLIENTADDR ioctl to the hidraw documentation
From: Frank Praznik @ 2014-01-31 17:32 UTC (permalink / raw)
To: linux-input; +Cc: jkosina, dh.herrmann, Frank Praznik
In-Reply-To: <1391189573-2591-1-git-send-email-frank.praznik@oh.rr.com>
Add the HIDIOCGRAWCLIENTADDR ioctl to the hidraw documentation.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
Documentation/hid/hidraw.txt | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/hid/hidraw.txt b/Documentation/hid/hidraw.txt
index 029e6cb..737fd1f 100644
--- a/Documentation/hid/hidraw.txt
+++ b/Documentation/hid/hidraw.txt
@@ -93,6 +93,11 @@ For USB devices, the string contains the physical path to the device (the
USB controller, hubs, ports, etc). For Bluetooth devices, the string
contains the hardware (MAC) address of the device.
+HIDIOCGRAWCLIENTADDR(len): Get Client Address
+This ioctl returns a string representing the client address of the device.
+For Bluetooth devices, the string contains the hardware (MAC) address of the
+device. For USB devices, the returned string is always 00:00:00:00:00:00.
+
HIDIOCSFEATURE(len): Send a Feature Report
This ioctl will send a feature report to the device. Per the HID
specification, feature reports are always sent using the control endpoint.
--
1.8.5.3
^ permalink raw reply related
* [PATCH 3/6] HID: Add support for setting client_addr in uhid.
From: Frank Praznik @ 2014-01-31 17:32 UTC (permalink / raw)
To: linux-input; +Cc: jkosina, dh.herrmann, Frank Praznik
In-Reply-To: <1391189573-2591-1-git-send-email-frank.praznik@oh.rr.com>
Add support for setting the client address of a device in uhid.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
drivers/hid/uhid.c | 5 +++++
include/uapi/linux/uhid.h | 1 +
2 files changed, 6 insertions(+)
diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index cedc6da..cfd2b93 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -259,6 +259,7 @@ struct uhid_create_req_compat {
__u8 name[128];
__u8 phys[64];
__u8 uniq[64];
+ __u8 client_addr[6];
compat_uptr_t rd_data;
__u16 rd_size;
@@ -308,6 +309,8 @@ static int uhid_event_from_user(const char __user *buffer, size_t len,
sizeof(compat->phys));
memcpy(event->u.create.uniq, compat->uniq,
sizeof(compat->uniq));
+ memcpy(event->u.create.client_addr, compat->client_addr,
+ sizeof(compat->client_addr));
event->u.create.rd_data = compat_ptr(compat->rd_data);
event->u.create.rd_size = compat->rd_size;
@@ -375,6 +378,8 @@ static int uhid_dev_create(struct uhid_device *uhid,
hid->phys[63] = 0;
strncpy(hid->uniq, ev->u.create.uniq, 63);
hid->uniq[63] = 0;
+ memcpy(hid->client_addr, ev->u.create.client_addr,
+ sizeof(hid->client_addr));
hid->ll_driver = &uhid_hid_driver;
hid->hid_get_raw_report = uhid_hid_get_raw;
diff --git a/include/uapi/linux/uhid.h b/include/uapi/linux/uhid.h
index 414b74b..5e48acc 100644
--- a/include/uapi/linux/uhid.h
+++ b/include/uapi/linux/uhid.h
@@ -40,6 +40,7 @@ struct uhid_create_req {
__u8 name[128];
__u8 phys[64];
__u8 uniq[64];
+ __u8 client_addr[6];
__u8 __user *rd_data;
__u16 rd_size;
--
1.8.5.3
^ permalink raw reply related
* [PATCH 2/6] HID: hidp: Store the device MAC in client_addr.
From: Frank Praznik @ 2014-01-31 17:32 UTC (permalink / raw)
To: linux-input; +Cc: jkosina, dh.herrmann, Frank Praznik
In-Reply-To: <1391189573-2591-1-git-send-email-frank.praznik@oh.rr.com>
Have hidp store the client device MAC address in client_addr
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
net/bluetooth/hidp/core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 292e619..f256f33 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -772,6 +772,9 @@ static int hidp_setup_hid(struct hidp_session *session,
snprintf(hid->uniq, sizeof(hid->uniq), "%pMR",
&l2cap_pi(session->ctrl_sock->sk)->chan->dst);
+ memcpy(hid->client_addr, &l2cap_pi(session->ctrl_sock->sk)->chan->dst,
+ sizeof(hid->client_addr));
+
hid->dev.parent = &session->conn->hcon->dev;
hid->ll_driver = &hidp_hid_driver;
--
1.8.5.3
^ permalink raw reply related
* [PATCH 1/6] HID: Add the client_addr member to the hid_device struct
From: Frank Praznik @ 2014-01-31 17:32 UTC (permalink / raw)
To: linux-input; +Cc: jkosina, dh.herrmann, Frank Praznik
In-Reply-To: <1391189573-2591-1-git-send-email-frank.praznik@oh.rr.com>
Add the client_addr member to the hid_device struct for storing the client
address of a connected device.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
include/linux/hid.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 31b9d29..283fc6e 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -495,6 +495,7 @@ struct hid_device { /* device report descriptor */
char name[128]; /* Device name */
char phys[64]; /* Device physical location */
char uniq[64]; /* Device unique identifier (serial #) */
+ __u8 client_addr[6]; /* Device client address (if wireless) */
void *driver_data;
--
1.8.5.3
^ permalink raw reply related
* [PATCH 0/6] HID: Add a stable method for retrieving the client MAC address of a HID device
From: Frank Praznik @ 2014-01-31 17:32 UTC (permalink / raw)
To: linux-input; +Cc: jkosina, dh.herrmann, Frank Praznik
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.
The first patch adds a new client_addr member to the hid_device struct. This
member, when populated, will be guaranteed to contain the client hardware
address of a connected wireless device.
The second patch modifies HIDP to populate the client_addr member of the
hid_device struct with the client MAC address of the connected wireless device.
The third patch adds support for setting the client_addr value in a uhid
device.
The final patches add support for retrieving the client_addr value as a string
in the hidraw interface via a new ioctl called HIDIOCGRAWCLIENTADDR. The
hidraw documentation and example program are also updated.
^ permalink raw reply
* Re: [PATCH RFC] Input: Add Microchip AR1021 i2c touchscreen
From: Dmitry Torokhov @ 2014-01-31 17:16 UTC (permalink / raw)
To: Christian Gmeiner; +Cc: linux-input
In-Reply-To: <20140131171521.GA20872@core.coreip.homeip.net>
On Fri, Jan 31, 2014 at 09:15:21AM -0800, Dmitry Torokhov wrote:
> Hi Chrisitian,
>
> On Fri, Jan 31, 2014 at 12:40:19PM +0100, Christian Gmeiner wrote:
> > >> --- /dev/null
> > >> +++ b/drivers/input/touchscreen/ar1021_i2c.c
> > >> @@ -0,0 +1,201 @@
> > >> +/*
> > >> + * Microchip AR1021 driver for I2C
> > >> + *
> > >> + * Author: Christian Gmeiner <christian.gmeiner@gmail.com>
> > >> + *
> > >> + * License: GPL as published by the FSF.
>
> By the way, you probably do not want GPL v1 to apply... Maybe say GPL v2
> or GPL v2 and later (depending on your preference and the license of the
> code you used as a base)?
>
> > >> +
> > >> +static int ar1021_i2c_resume(struct device *dev)
> > >> +{
> > >> + struct i2c_client *client = to_i2c_client(dev);
> > >> +
> > >> + enable_irq(client->irq);
> > >
> > > You do not want to enable IRQ if there are no users (nobody opened
> > > device).
> > >
> >
> > Okay.. but then I also do not need the disable_irq(..) call in
> > ar1021_i2c_suspend
> > and can totally remove the PM stuff - or?
>
> No, I think you still need the PM methods, you just need to check if
> device is opened (take dev->mutex, check dev->users) and decide if you
> need to enable/disable IRQ or not.
Hmm, on the other hand enable/disable does the counting for you so maybe
you should leave it all as it was.
--
Dmitry
^ permalink raw reply
* Re: [PATCH RFC] Input: Add Microchip AR1021 i2c touchscreen
From: Dmitry Torokhov @ 2014-01-31 17:15 UTC (permalink / raw)
To: Christian Gmeiner; +Cc: linux-input
In-Reply-To: <CAH9NwWfb1zVrgeKB7+fPyS80yC6BuokvmA4TE0tr7BAs_jwQSA@mail.gmail.com>
Hi Chrisitian,
On Fri, Jan 31, 2014 at 12:40:19PM +0100, Christian Gmeiner wrote:
> >> --- /dev/null
> >> +++ b/drivers/input/touchscreen/ar1021_i2c.c
> >> @@ -0,0 +1,201 @@
> >> +/*
> >> + * Microchip AR1021 driver for I2C
> >> + *
> >> + * Author: Christian Gmeiner <christian.gmeiner@gmail.com>
> >> + *
> >> + * License: GPL as published by the FSF.
By the way, you probably do not want GPL v1 to apply... Maybe say GPL v2
or GPL v2 and later (depending on your preference and the license of the
code you used as a base)?
> >> +
> >> +static int ar1021_i2c_resume(struct device *dev)
> >> +{
> >> + struct i2c_client *client = to_i2c_client(dev);
> >> +
> >> + enable_irq(client->irq);
> >
> > You do not want to enable IRQ if there are no users (nobody opened
> > device).
> >
>
> Okay.. but then I also do not need the disable_irq(..) call in
> ar1021_i2c_suspend
> and can totally remove the PM stuff - or?
No, I think you still need the PM methods, you just need to check if
device is opened (take dev->mutex, check dev->users) and decide if you
need to enable/disable IRQ or not.
--
Dmitry
^ permalink raw reply
* Re: Sony Vaio Duo 11: getting middle mouse button to work
From: Benjamin Tissoires @ 2014-01-31 16:20 UTC (permalink / raw)
To: Stephan Mueller
Cc: Mattia Dongili, platform-driver-x86, Jiri Kosina, linux-input
In-Reply-To: <13914184.mVhhUrzN60@myon.chronox.de>
On Thu, Jan 30, 2014 at 11:31 PM, Stephan Mueller <smueller@chronox.de> wrote:
> Am Donnerstag, 30. Januar 2014, 22:50:25 schrieb Benjamin Tissoires:
>
> Hi Benjamin,
>
>> > Can you please help me how to do that? I install Windows in a KVM with
>> > the mouse device as a USB-passthrough device -- shall I detach the Linux
>> > USB driver?
>>
>> No, you don't have to detach the Linux USB driver (it's done for you
>> by KVM IIRC).
>
> Correct, libvirtd is the one that detaches the driver when I start the VM.
>>
>> > Now, shall I use hid-recorder on that device or rather usbmon while
>> > Windows is booting?
>>
>> usbmon is the tool you need. hid-recorder will not work because the
>> hid subsystem will be disconnected as the device is used by Windows.
>>
>> You can even use wireshark now. It gives you a nice interface and
>> interpretation of the USB packets :)
>
> Please see attached for the USB sniffing on the USB bus with the Crucialtec
> touch pad when Windows starts up and shuts down. To drive the device, Windows
> uses the Crucialtec driver according to its device settings display.
>
> The attached log contains the log information for the mouse (device 001:005)
> and the USB bridge (device 001:002). I cut out the listing for the touch pad
> which is also attached to that bridge.
>
> The strange thing, however, is the following: the mouse works in Windows when
> I do not sniff on the USB bus using usbmon. But as soon as I start sniffing,
> it does not work any more.
>
> Is that log helpful?
Not really :(
I would have expected to see something like that:
XXXXXXXXXXXXXXXX XXXXXXXXXX S Co:1:XXX:0 s 21 09 0202 0002 0004 4 = XXXXXXXX
XXXXXXXXXXXXXXXX XXXXXXXXXX C Co:1:XXX:0 0 4 >
(SET_REPORT on the report ID 02 with a data length of 4)
The capture seems to confuse the windows driver and it is not
initialized correctly.
Long shot: maybe if you start the capture before attaching the device
to the Windows VM (or launching it) it will be in a better shape.
I would say that as long as the device do not work under the VM with
the capture, we are really screwed.
Cheers,
Benjamin
^ permalink raw reply
* [PATCH 1/7] Input: xpad: use proper endpoint type
From: Greg Kroah-Hartman @ 2014-01-31 13:03 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, Greg Kroah-Hartman, Pierre-Loup A. Griffais
In-Reply-To: <1391173414-6199-1-git-send-email-gregkh@linuxfoundation.org>
The xpad wireless endpoint is not a bulk endpoint on my devices, but
rather an interrupt one, so the USB core complains when it is submitted.
I'm guessing that the author really did mean that this should be an
interrupt urb, but as there are a zillion different xpad devices out
there, let's cover out bases and handle both bulk and interrupt
endpoints just as easily.
Signed-off-by: "Pierre-Loup A. Griffais" <pgriffais@valvesoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/input/joystick/xpad.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 603fe0dd3682..517829f6a58b 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -1003,9 +1003,19 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
}
ep_irq_in = &intf->cur_altsetting->endpoint[1].desc;
- usb_fill_bulk_urb(xpad->bulk_out, udev,
- usb_sndbulkpipe(udev, ep_irq_in->bEndpointAddress),
- xpad->bdata, XPAD_PKT_LEN, xpad_bulk_out, xpad);
+ if (usb_endpoint_is_bulk_out(ep_irq_in)) {
+ usb_fill_bulk_urb(xpad->bulk_out, udev,
+ usb_sndbulkpipe(udev,
+ ep_irq_in->bEndpointAddress),
+ xpad->bdata, XPAD_PKT_LEN,
+ xpad_bulk_out, xpad);
+ } else {
+ usb_fill_int_urb(xpad->bulk_out, udev,
+ usb_sndintpipe(udev,
+ ep_irq_in->bEndpointAddress),
+ xpad->bdata, XPAD_PKT_LEN,
+ xpad_bulk_out, xpad, 0);
+ }
/*
* Submit the int URB immediately rather than waiting for open
--
1.8.5.3
^ permalink raw reply related
* [PATCH 7/7] Input: xpad: properly name the LED class devices
From: Greg Kroah-Hartman @ 2014-01-31 13:03 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, Greg Kroah-Hartman, Pierre-Loup A. Griffais
In-Reply-To: <1391173414-6199-1-git-send-email-gregkh@linuxfoundation.org>
Don't just increment the LED device number, but use the joystick id so
that you have a chance to associate the LED device to the correct xpad
device by the name, instead of having to use the sysfs tree, which
really doesn't work.
Cc: "Pierre-Loup A. Griffais" <pgriffais@valvesoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/input/joystick/xpad.c | 40 +++++++++++++++++-----------------------
1 file changed, 17 insertions(+), 23 deletions(-)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index d342d41a7a0d..ae156de46a12 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -781,8 +781,6 @@ static void xpad_led_set(struct led_classdev *led_cdev,
static int xpad_led_probe(struct usb_xpad *xpad)
{
- static atomic_t led_seq = ATOMIC_INIT(0);
- long led_no;
struct xpad_led *led;
struct led_classdev *led_cdev;
int error;
@@ -794,9 +792,7 @@ static int xpad_led_probe(struct usb_xpad *xpad)
if (!led)
return -ENOMEM;
- led_no = (long)atomic_inc_return(&led_seq) - 1;
-
- snprintf(led->name, sizeof(led->name), "xpad%ld", led_no);
+ snprintf(led->name, sizeof(led->name), "xpad%d", xpad->joydev_id);
led->xpad = xpad;
led_cdev = &led->led_cdev;
@@ -944,16 +940,17 @@ static int xpad_init_input(struct usb_xpad *xpad)
}
error = xpad_init_ff(xpad);
- if (error)
- goto fail_init_ff;
-
- error = xpad_led_probe(xpad);
- if (error)
- goto fail_init_led;
+ if (error) {
+ input_free_device(input_dev);
+ return error;
+ }
error = input_register_device(xpad->dev);
- if (error)
- goto fail_input_register;
+ if (error) {
+ input_ff_destroy(input_dev);
+ input_free_device(input_dev);
+ return error;
+ }
joydev_dev = device_find_child(&xpad->dev->dev, NULL,
xpad_find_joydev);
@@ -966,17 +963,14 @@ static int xpad_init_input(struct usb_xpad *xpad)
xpad_send_led_command(xpad, (xpad->joydev_id % 4) + 2);
}
- return 0;
-
-fail_input_register:
- xpad_led_disconnect(xpad);
-
-fail_init_led:
- input_ff_destroy(input_dev);
+ error = xpad_led_probe(xpad);
+ if (error) {
+ input_ff_destroy(input_dev);
+ input_unregister_device(input_dev);
+ return error;
+ }
-fail_init_ff:
- input_free_device(input_dev);
- return error;
+ return 0;
}
static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id)
--
1.8.5.3
^ permalink raw reply related
* [PATCH 6/7] Input: xpad: handle "present" and "gone" correctly
From: Greg Kroah-Hartman @ 2014-01-31 13:03 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, Pierre-Loup A. Griffais, Greg Kroah-Hartman
In-Reply-To: <1391173414-6199-1-git-send-email-gregkh@linuxfoundation.org>
From: "Pierre-Loup A. Griffais" <pgriffais@valvesoftware.com>
Handle the "a new device is present" message properly by dynamically
creating the input device at this point in time. This requires a
workqueue as we are in interrupt context when we learn about this.
Also properly disconnect any devices that we are told are removed.
Signed-off-by: "Pierre-Loup A. Griffais" <pgriffais@valvesoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/input/joystick/xpad.c | 50 ++++++++++++++++++++++++++++++++++---------
1 file changed, 40 insertions(+), 10 deletions(-)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 7a07b95790d7..d342d41a7a0d 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -293,8 +293,11 @@ struct usb_xpad {
int xtype; /* type of xbox device */
int joydev_id; /* the minor of the device */
const char *name; /* name of the device */
+ struct work_struct work; /* to init/remove device from callback */
};
+static int xpad_init_input(struct usb_xpad *xpad);
+
/*
* xpad_process_packet
*
@@ -437,6 +440,22 @@ static void xpad360_process_packet(struct usb_xpad *xpad,
input_sync(dev);
}
+static void presence_work_function(struct work_struct *work)
+{
+ struct usb_xpad *xpad = container_of(work, struct usb_xpad, work);
+ int error;
+
+ if (xpad->pad_present) {
+ error = xpad_init_input(xpad);
+ if (error) {
+ /* complain only, not much else we can do here */
+ dev_err(&xpad->dev->dev, "unable to init device\n");
+ }
+ } else {
+ input_unregister_device(xpad->dev);
+ }
+}
+
/*
* xpad360w_process_packet
*
@@ -451,16 +470,22 @@ static void xpad360_process_packet(struct usb_xpad *xpad,
* 01.1 - Pad state (Bytes 4+) valid
*
*/
-
static void xpad360w_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char *data)
{
/* Presence change */
if (data[0] & 0x08) {
if (data[1] & 0x80) {
- xpad->pad_present = 1;
- usb_submit_urb(xpad->bulk_out, GFP_ATOMIC);
- } else
- xpad->pad_present = 0;
+ if (!xpad->pad_present) {
+ xpad->pad_present = 1;
+ usb_submit_urb(xpad->bulk_out, GFP_ATOMIC);
+ schedule_work(&xpad->work);
+ }
+ } else {
+ if (xpad->pad_present) {
+ xpad->pad_present = 0;
+ schedule_work(&xpad->work);
+ }
+ }
}
/* Valid pad data */
@@ -507,10 +532,13 @@ static void xpad_irq_in(struct urb *urb)
}
exit:
- retval = usb_submit_urb(urb, GFP_ATOMIC);
- if (retval)
- dev_err(dev, "%s - usb_submit_urb failed with result %d\n",
- __func__, retval);
+ if (xpad->pad_present) {
+ retval = usb_submit_urb(urb, GFP_ATOMIC);
+ if (retval)
+ dev_err(dev,
+ "%s - usb_submit_urb failed with result %d\n",
+ __func__, retval);
+ }
}
static void xpad_bulk_out(struct urb *urb)
@@ -991,6 +1019,7 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
xpad->mapping = xpad_device[i].mapping;
xpad->xtype = xpad_device[i].xtype;
xpad->name = xpad_device[i].name;
+ INIT_WORK(&xpad->work, presence_work_function);
if (xpad->xtype == XTYPE_UNKNOWN) {
if (intf->cur_altsetting->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC) {
@@ -1136,7 +1165,8 @@ static void xpad_disconnect(struct usb_interface *intf)
struct usb_xpad *xpad = usb_get_intfdata (intf);
xpad_led_disconnect(xpad);
- input_unregister_device(xpad->dev);
+ if (xpad->pad_present)
+ input_unregister_device(xpad->dev);
xpad_deinit_output(xpad);
if (xpad->xtype == XTYPE_XBOX360W) {
--
1.8.5.3
^ permalink raw reply related
* [PATCH 5/7] Input: xpad: disconnect all Wireless controllers at init
From: Greg Kroah-Hartman @ 2014-01-31 13:03 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, Pierre-Loup A. Griffais, Greg Kroah-Hartman
In-Reply-To: <1391173414-6199-1-git-send-email-gregkh@linuxfoundation.org>
From: "Pierre-Loup A. Griffais" <pgriffais@valvesoftware.com>
We initializing the driver/device, we really don't know how many
controllers are connected. So send a "disconnect all" command to the
base-station, and let the user pair the controllers in the order in
which they want them assigned.
Note, this means we now do not "preallocate" all 4 devices when a single
wireless base station is seen, but require the device to be properly
connected to the base station before that can happen. The allocation of
the device happens in the next patch, not here, so in a way, this patch
breaks all wireless devices...
Because we want to talk to the device, we have to set up the output urbs
no matter if we have CONFIG_JOYSTICK_XPAD_FF or
CONFIG_JOYSTICK_XPAD_LEDS enabled not, so take out the code that allows
those variables and code to be disabled (it's so small it should never
matter...)
Signed-off-by: "Pierre-Loup A. Griffais" <pgriffais@valvesoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/input/joystick/xpad.c | 46 ++++++++++++++++++++++++++++++++-----------
1 file changed, 34 insertions(+), 12 deletions(-)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 7997ae89a877..7a07b95790d7 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -278,12 +278,10 @@ struct usb_xpad {
struct urb *bulk_out;
unsigned char *bdata;
-#if defined(CONFIG_JOYSTICK_XPAD_FF) || defined(CONFIG_JOYSTICK_XPAD_LEDS)
struct urb *irq_out; /* urb for interrupt out report */
unsigned char *odata; /* output data */
dma_addr_t odata_dma;
struct mutex odata_mutex;
-#endif
#if defined(CONFIG_JOYSTICK_XPAD_LEDS)
struct xpad_led *led;
@@ -537,7 +535,6 @@ static void xpad_bulk_out(struct urb *urb)
}
}
-#if defined(CONFIG_JOYSTICK_XPAD_FF) || defined(CONFIG_JOYSTICK_XPAD_LEDS)
static void xpad_irq_out(struct urb *urb)
{
struct usb_xpad *xpad = urb->context;
@@ -623,11 +620,6 @@ static void xpad_deinit_output(struct usb_xpad *xpad)
xpad->odata, xpad->odata_dma);
}
}
-#else
-static int xpad_init_output(struct usb_interface *intf, struct usb_xpad *xpad) { return 0; }
-static void xpad_deinit_output(struct usb_xpad *xpad) {}
-static void xpad_stop_output(struct usb_xpad *xpad) {}
-#endif
#ifdef CONFIG_JOYSTICK_XPAD_FF
static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect)
@@ -1091,11 +1083,41 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
usb_kill_urb(xpad->irq_in);
goto fail9;
}
+
+ /*
+ * We don't know how to check the controller state at driver
+ * load, so just disconnect them all, requiring the user to
+ * repair the device in the order they want them used. Good
+ * point is that we don't connect devices in "random" order,
+ * but the extra step seems a bit harsh as other operating
+ * systems don't require this at the moment.
+ *
+ * Power-off packet information came from an OS-X
+ * reverse-engineered driver located at:
+ * http://tattiebogle.net/index.php/ProjectRoot/Xbox360Controller/OsxDriver#toc1
+ */
+ mutex_lock(&xpad->odata_mutex);
+ xpad->odata[0] = 0x00;
+ xpad->odata[1] = 0x00;
+ xpad->odata[2] = 0x08;
+ xpad->odata[3] = 0xC0;
+ xpad->odata[4] = 0x00;
+ xpad->odata[5] = 0x00;
+ xpad->odata[6] = 0x00;
+ xpad->odata[7] = 0x00;
+ xpad->odata[8] = 0x00;
+ xpad->odata[9] = 0x00;
+ xpad->odata[10] = 0x00;
+ xpad->odata[11] = 0x00;
+ xpad->irq_out->transfer_buffer_length = 12;
+ usb_submit_urb(xpad->irq_out, GFP_KERNEL);
+ mutex_unlock(&xpad->odata_mutex);
+ } else {
+ xpad->pad_present = 1;
+ error = xpad_init_input(xpad);
+ if (error)
+ goto fail9;
}
- xpad->pad_present = 1;
- error = xpad_init_input(xpad);
- if (error)
- goto fail9;
return 0;
--
1.8.5.3
^ permalink raw reply related
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