[PATCH 0/2] more quirks for hid-sensor-hub

[PATCH 0/2] more quirks for hid-sensor-hub

[Benjamin Tissoires]

Hi Jiri,

here are 2 more quirks for the sensor-hub.

Srinivas, I am wondering if we can not detect those automatically.
Looking at the report_fixup, it looks like the buggy report descriptors have
a logical max of 5 (which would correspond to an enum of 0 to 5). But there
are 6 fields in the enum, so that means the report descriptor is just wrong.

Given that Microsoft's driver requires a min of 1, wouldn't it be too much
assumption to say that we need to fix the report descriptor anyway when we
see a range of 0..5 instead of 1..6?

Cheers,
Benjamin

Benjamin Tissoires (2):
  HID: sensor-hub add quirk for Microsoft Surface 3
  HID: sensor-hub: add quirk for Microchip MM7150

 drivers/hid/hid-sensor-hub.c | 6 ++++++
 1 file changed, 6 insertions(+)

-- 
2.7.4

[PATCH 1/2] HID: sensor-hub add quirk for Microsoft Surface 3

[Benjamin Tissoires]

One more device requiring a quirk :/

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-sensor-hub.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 50aad26..f9440b6 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -798,6 +798,9 @@ static const struct hid_device_id sensor_hub_devices[] = {
 	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, USB_VENDOR_ID_MICROSOFT,
 			USB_DEVICE_ID_MS_TYPE_COVER_2),
 			.driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
+	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, USB_VENDOR_ID_MICROSOFT,
+			0x07bd), /* Microsoft Surface 3 TypeCover */
+			.driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
 	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, USB_VENDOR_ID_STM_0,
 			USB_DEVICE_ID_STM_HID_SENSOR),
 			.driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
-- 
2.7.4

Re: [PATCH 1/2] HID: sensor-hub add quirk for Microsoft Surface 3

[Bastien Nocera]

On Fri, 2016-11-25 at 11:03 +0100, Benjamin Tissoires wrote:
> One more device requiring a quirk :/
> 
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Tested-by: Bastien Nocera <hadess@hadess.net>

> ---
>  drivers/hid/hid-sensor-hub.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-
> hub.c
> index 50aad26..f9440b6 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -798,6 +798,9 @@ static const struct hid_device_id
> sensor_hub_devices[] = {
>  	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> USB_VENDOR_ID_MICROSOFT,
>  			USB_DEVICE_ID_MS_TYPE_COVER_2),
>  			.driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
> +	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> USB_VENDOR_ID_MICROSOFT,
> +			0x07bd), /* Microsoft Surface 3 TypeCover */

The sensor is not in the TypeCover though, it's in the main device.

> +			.driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
>  	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> USB_VENDOR_ID_STM_0,
>  			USB_DEVICE_ID_STM_HID_SENSOR),
>  			.driver_data = HID_SENSOR_HUB_ENUM_QUIRK},

Re: [PATCH 1/2] HID: sensor-hub add quirk for Microsoft Surface 3

[Benjamin Tissoires]

On Nov 25 2016 or thereabouts, Bastien Nocera wrote:
> On Fri, 2016-11-25 at 11:03 +0100, Benjamin Tissoires wrote:
> > One more device requiring a quirk :/
> > 
> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> Tested-by: Bastien Nocera <hadess@hadess.net>
> 
> > ---
> >  drivers/hid/hid-sensor-hub.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-
> > hub.c
> > index 50aad26..f9440b6 100644
> > --- a/drivers/hid/hid-sensor-hub.c
> > +++ b/drivers/hid/hid-sensor-hub.c
> > @@ -798,6 +798,9 @@ static const struct hid_device_id
> > sensor_hub_devices[] = {
> >  	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> > USB_VENDOR_ID_MICROSOFT,
> >  			USB_DEVICE_ID_MS_TYPE_COVER_2),
> >  			.driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
> > +	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> > USB_VENDOR_ID_MICROSOFT,
> > +			0x07bd), /* Microsoft Surface 3 TypeCover */
> 
> The sensor is not in the TypeCover though, it's in the main device.

Oops, yes, sorry. I have been traumatised by the type cover apparently
:)

Benjamin

> 
> > +			.driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
> >  	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> > USB_VENDOR_ID_STM_0,
> >  			USB_DEVICE_ID_STM_HID_SENSOR),
> >  			.driver_data = HID_SENSOR_HUB_ENUM_QUIRK},

[PATCH 2/2] HID: sensor-hub: add quirk for Microchip MM7150

[Benjamin Tissoires]

One more device requiring a quirk :/

Reported-by: Christian-Nils Boda <christian-nils.boda@gadz.org>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-sensor-hub.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index f9440b6..9a1c5db 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -786,6 +786,9 @@ static const struct hid_device_id sensor_hub_devices[] = {
 	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, USB_VENDOR_ID_INTEL_1,
 			USB_DEVICE_ID_INTEL_HID_SENSOR_1),
 			.driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
+	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, USB_VENDOR_ID_MICROCHIP,
+			0x0f01), /* MM7150 */
+			.driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
 	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, USB_VENDOR_ID_STM_0,
 			USB_DEVICE_ID_STM_HID_SENSOR),
 			.driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
-- 
2.7.4

Re: [PATCH 0/2] more quirks for hid-sensor-hub

[Jiri Kosina]

On Fri, 25 Nov 2016, Benjamin Tissoires wrote:

> Hi Jiri,
> 
> here are 2 more quirks for the sensor-hub.

Applied to for-4.10/microsoft-surface-3; in case you figure out a way how 
to auto-detect this behavior, please base such patches on top of this.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 0/2] more quirks for hid-sensor-hub

[Srinivas Pandruvada]

Hi Benjamin,

On Fri, 2016-11-25 at 11:03 +0100, Benjamin Tissoires wrote:
> Hi Jiri,
> 
> here are 2 more quirks for the sensor-hub.
> 
> Srinivas, I am wondering if we can not detect those automatically.
> Looking at the report_fixup, it looks like the buggy report
> descriptors have
> a logical max of 5 (which would correspond to an enum of 0 to 5). But
> there
> are 6 fields in the enum, so that means the report descriptor is just
> wrong.
> 
> Given that Microsoft's driver requires a min of 1, wouldn't it be too
> much
> assumption to say that we need to fix the report descriptor anyway
> when we
> see a range of 0..5 instead of 1..6?

The problem is that they should have also changed logical minimum,
which vendors didn't do as Windows driver will always assume
enumeration base as 1.

As far as I can tell there was only Windows platform where base was 0,
but I am sure that Windows update and BIOS update would have fixed that
by now after 3 years.

I am in opinion to change this to match this with Windows driver, so
that we don't need this quirks. This is a small change in IIO sensor
hub driver.

If nobody has objection, I can submit a trivial change for this.

Thanks,
Srinivas


> 
> Cheers,
> Benjamin
> 
> Benjamin Tissoires (2):
>   HID: sensor-hub add quirk for Microsoft Surface 3
>   HID: sensor-hub: add quirk for Microchip MM7150
> 
>  drivers/hid/hid-sensor-hub.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 

Re: [PATCH 0/2] more quirks for hid-sensor-hub

[Benjamin Tissoires]

On Nov 28 2016 or thereabouts, Srinivas Pandruvada wrote:
> Hi Benjamin,
> 
> On Fri, 2016-11-25 at 11:03 +0100, Benjamin Tissoires wrote:
> > Hi Jiri,
> > 
> > here are 2 more quirks for the sensor-hub.
> > 
> > Srinivas, I am wondering if we can not detect those automatically.
> > Looking at the report_fixup, it looks like the buggy report
> > descriptors have
> > a logical max of 5 (which would correspond to an enum of 0 to 5). But
> > there
> > are 6 fields in the enum, so that means the report descriptor is just
> > wrong.
> > 
> > Given that Microsoft's driver requires a min of 1, wouldn't it be too
> > much
> > assumption to say that we need to fix the report descriptor anyway
> > when we
> > see a range of 0..5 instead of 1..6?
> 
> The problem is that they should have also changed logical minimum,
> which vendors didn't do as Windows driver will always assume
> enumeration base as 1.
> 
> As far as I can tell there was only Windows platform where base was 0,
> but I am sure that Windows update and BIOS update would have fixed that
> by now after 3 years.
> 
> I am in opinion to change this to match this with Windows driver, so
> that we don't need this quirks. This is a small change in IIO sensor
> hub driver.
> 
> If nobody has objection, I can submit a trivial change for this.

I don't have any objections (I must confess I don't have a lot of these
- 1 maybe 2), and I'd rather get rid of the quirk at all.

Cheers,
Benjamin

> 
> Thanks,
> Srinivas
> 
> 
> > 
> > Cheers,
> > Benjamin
> > 
> > Benjamin Tissoires (2):
> >   HID: sensor-hub add quirk for Microsoft Surface 3
> >   HID: sensor-hub: add quirk for Microchip MM7150
> > 
> >  drivers/hid/hid-sensor-hub.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >