Re: [PATCH v2] HID: rmi: Scan the report descriptor to determine if the device is suitable for the hid-rmi driver

[Andrew Duggan <aduggan@synaptics.com>]

On 11/24/2014 03:37 PM, Andrew Duggan wrote:
> On composite HID devices there may be multiple HID devices on separate interfaces, but hid-rmi
> should only bind to the touchpad. The previous version simply checked that the interface protocol
> was set to mouse. Unfortuately, it is not always the case that the touchpad has the mouse interface
> protocol set. This patch takes a different approach and scans the report descriptor looking for
> the Generic Desktop Pointer usage and the Vendor Specific Top Level Collection needed by the
> hid-rmi driver to interface with the device.
>
> Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
> ---
> This is a second attempt at the patch I submitted back in October. Instead of looking for specific
> HID reports this patch looks for the Generic Desktop Pointer usage. Having that usage along with
> the vendor specific collection seems to be a distinctive enough to id the Synaptics touchpad in
> the composite USB device.
Any comments on this patch?

Thanks,
Andrew
>   drivers/hid/hid-core.c | 21 ++++++++++++++++-----
>   include/linux/hid.h    |  4 +++-
>   2 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 12b6e67..ba9dc59 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -698,10 +698,20 @@ static void hid_scan_feature_usage(struct hid_parser *parser, u32 usage)
>   static void hid_scan_collection(struct hid_parser *parser, unsigned type)
>   {
>   	struct hid_device *hid = parser->device;
> +	int i;
>   
>   	if (((parser->global.usage_page << 16) == HID_UP_SENSOR) &&
>   	    type == HID_COLLECTION_PHYSICAL)
>   		hid->group = HID_GROUP_SENSOR_HUB;
> +
> +	if ((parser->global.usage_page << 16) == HID_UP_GENDESK)
> +		for (i = 0; i < parser->local.usage_index; i++)
> +			if (parser->local.usage[i] == HID_GD_POINTER)
> +				parser->scan_flags
> +					|= HID_SCAN_FLAG_GENDESK_POINTER;
> +
> +	if ((parser->global.usage_page << 16) == HID_UP_MSVENDOR)
> +		parser->scan_flags |= HID_SCAN_FLAG_VENDOR_SPECIFIC;
>   }
>   
>   static int hid_scan_main(struct hid_parser *parser, struct hid_item *item)
> @@ -783,11 +793,12 @@ static int hid_scan_report(struct hid_device *hid)
>   	* Vendor specific handlings
>   	*/
>   	if ((hid->vendor == USB_VENDOR_ID_SYNAPTICS) &&
> -	    (hid->group == HID_GROUP_GENERIC) &&
> -	    /* only bind to the mouse interface of composite USB devices */
> -	    (hid->bus != BUS_USB || hid->type == HID_TYPE_USBMOUSE))
> -		/* hid-rmi should take care of them, not hid-generic */
> -		hid->group = HID_GROUP_RMI;
> +	    (hid->group == HID_GROUP_GENERIC)) {
> +		if ((parser->scan_flags & HID_SCAN_FLAG_VENDOR_SPECIFIC)
> +		    && (parser->scan_flags & HID_SCAN_FLAG_GENDESK_POINTER))
> +			/* hid-rmi should take care of them, not hid-generic */
> +			hid->group = HID_GROUP_RMI;
> +	}
>   
>   	/*
>   	 * Vendor specific handlings
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index f53c4a9..b019f15 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -547,7 +547,9 @@ static inline void hid_set_drvdata(struct hid_device *hdev, void *data)
>   #define HID_GLOBAL_STACK_SIZE 4
>   #define HID_COLLECTION_STACK_SIZE 4
>   
> -#define HID_SCAN_FLAG_MT_WIN_8			0x00000001
> +#define HID_SCAN_FLAG_MT_WIN_8			BIT(0)
> +#define HID_SCAN_FLAG_VENDOR_SPECIFIC		BIT(1)
> +#define HID_SCAN_FLAG_GENDESK_POINTER		BIT(2)
>   
>   struct hid_parser {
>   	struct hid_global     global;