[PATCH 1/3] HID: extend autodetect to handle I2C sensors as well

[PATCH 1/3] HID: extend autodetect to handle I2C sensors as well

[Mika Westerberg]

Since the advent of HID over I2C protocol, it is possible to have sensor
hubs behind I2C bus as well. We can autodetect this in a same way than USB
sensor hubs.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/hid/hid-core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index eb2ee11..fe3a59e 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -729,7 +729,7 @@ static int hid_scan_report(struct hid_device *hid)
 			item.type == HID_ITEM_TYPE_MAIN &&
 			item.tag == HID_MAIN_ITEM_TAG_BEGIN_COLLECTION &&
 			(item_udata(&item) & 0xff) == HID_COLLECTION_PHYSICAL &&
-			hid->bus == BUS_USB)
+			(hid->bus == BUS_USB || hid->bus == BUS_I2C))
 			hid->group = HID_GROUP_SENSOR_HUB;
 	}
 
-- 
1.7.10.4

[PATCH 2/3] HID: sensor-hub: get rid of unused sensor_hub_grabbed_usages[] table

[Mika Westerberg]

This table is not used anywhere in the driver so kill it.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/hid/hid-sensor-hub.c |    5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index c06e933..2643bce9 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -608,11 +608,6 @@ static const struct hid_device_id sensor_hub_devices[] = {
 };
 MODULE_DEVICE_TABLE(hid, sensor_hub_devices);
 
-static const struct hid_usage_id sensor_hub_grabbed_usages[] = {
-	{ HID_ANY_ID, HID_ANY_ID, HID_ANY_ID },
-	{ HID_ANY_ID - 1, HID_ANY_ID - 1, HID_ANY_ID - 1 }
-};
-
 static struct hid_driver sensor_hub_driver = {
 	.name = "hid-sensor-hub",
 	.id_table = sensor_hub_devices,
-- 
1.7.10.4

Re: [PATCH 2/3] HID: sensor-hub: get rid of unused sensor_hub_grabbed_usages[] table

[Benjamin Tissoires]

On Mon, Feb 11, 2013 at 11:31 AM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> This table is not used anywhere in the driver so kill it.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---

Yep, it's rather strange that the compiler didn't complained...
Anyway, Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Cheers,
Benjamin

RE: [PATCH 2/3] HID: sensor-hub: get rid of unused sensor_hub_grabbed_usages[] table

[Pandruvada, Srinivas]

This was added to load sensor hub driver on USB plug in without explicit modprobe. 
But with last few changes to remove vendor and product id, we need to do modprobe this driver.
So removal is OK.

Thanks,
Srinivas


-----Original Message-----
From: Mika Westerberg [mailto:mika.westerberg@linux.intel.com] 
Sent: Monday, February 11, 2013 2:31 AM
To: linux-kernel@vger.kernel.org
Cc: Jiri Kosina; Benjamin Tissoires; linux-input@vger.kernel.org; Pandruvada, Srinivas; Mika Westerberg
Subject: [PATCH 2/3] HID: sensor-hub: get rid of unused sensor_hub_grabbed_usages[] table

This table is not used anywhere in the driver so kill it.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/hid/hid-sensor-hub.c |    5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c index c06e933..2643bce9 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -608,11 +608,6 @@ static const struct hid_device_id sensor_hub_devices[] = {  };  MODULE_DEVICE_TABLE(hid, sensor_hub_devices);
 
-static const struct hid_usage_id sensor_hub_grabbed_usages[] = {
-	{ HID_ANY_ID, HID_ANY_ID, HID_ANY_ID },
-	{ HID_ANY_ID - 1, HID_ANY_ID - 1, HID_ANY_ID - 1 }
-};
-
 static struct hid_driver sensor_hub_driver = {
 	.name = "hid-sensor-hub",
 	.id_table = sensor_hub_devices,
--
1.7.10.4

Re: [PATCH 2/3] HID: sensor-hub: get rid of unused sensor_hub_grabbed_usages[] table

[Benjamin Tissoires]

On Mon, Feb 11, 2013 at 4:53 PM, Pandruvada, Srinivas
<srinivas.pandruvada@intel.com> wrote:
> This was added to load sensor hub driver on USB plug in without explicit modprobe.
> But with last few changes to remove vendor and product id, we need to do modprobe this driver.
> So removal is OK.

I just figure your comment out: the change does not seem to have
anything to do with hotplugging. .usage_table is used to decide which
HID reports to forwards to the .event() callback, and NULL means all
events. So even if it was in use, there are no functional changes.

Thanks to the changes to remove the vendor id and product id, you
shouldn't have to manually modprobe the driver as the hid-core
subsystem will tag the hid device as a sensor one, meaning that udev
will modprobe the driver if it's not already loaded.
So if you still have to manually modprobe your driver (or maybe you
are talking about the mfd driver), there is a bug in the detection of
sensors devices. And this has to be fixed :)

Cheers,
Benjamin

RE: [PATCH 2/3] HID: sensor-hub: get rid of unused sensor_hub_grabbed_usages[] table

[Jiri Kosina]

On Mon, 11 Feb 2013, Pandruvada, Srinivas wrote:

> This was added to load sensor hub driver on USB plug in without explicit modprobe. 
> But with last few changes to remove vendor and product id, we need to do modprobe this driver.
> So removal is OK.

Srinivas,

sorry, I have a hard time parsing your message.

The modprobing should work just fine with HID_GROUP_SENSOR_HUB, this has 
nothing to do with ->usage_table callback.

-- 
Jiri Kosina
SUSE Labs

[PATCH 3/3] HID: sensor-hub: don't limit the driver only to USB bus

[Mika Westerberg]

We now have two transport mediums: USB and I2C, where sensor hubs can
exists. So instead of constraining the driver to only these two we let it
to match any HID bus as long as the group is HID_GROUP_SENSOR_HUB.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/hid/hid-sensor-hub.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 2643bce9..c01f10d 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -603,7 +603,8 @@ static void sensor_hub_remove(struct hid_device *hdev)
 }
 
 static const struct hid_device_id sensor_hub_devices[] = {
-	{ HID_DEVICE(BUS_USB, HID_GROUP_SENSOR_HUB, HID_ANY_ID, HID_ANY_ID) },
+	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, HID_ANY_ID,
+		     HID_ANY_ID) },
 	{ }
 };
 MODULE_DEVICE_TABLE(hid, sensor_hub_devices);
-- 
1.7.10.4

Re: [PATCH 3/3] HID: sensor-hub: don't limit the driver only to USB bus

[Benjamin Tissoires]

On Mon, Feb 11, 2013 at 11:31 AM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> We now have two transport mediums: USB and I2C, where sensor hubs can
> exists. So instead of constraining the driver to only these two we let it
> to match any HID bus as long as the group is HID_GROUP_SENSOR_HUB.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Thanks Mika for this series!

Cheers,
Benjamin

Re: [PATCH 3/3] HID: sensor-hub: don't limit the driver only to USB bus

[Jiri Kosina]

On Mon, 11 Feb 2013, Benjamin Tissoires wrote:

> > We now have two transport mediums: USB and I2C, where sensor hubs can
> > exists. So instead of constraining the driver to only these two we let it
> > to match any HID bus as long as the group is HID_GROUP_SENSOR_HUB.
> >
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Thanks a lot Mika, I have now applied the series for 3.9

-- 
Jiri Kosina
SUSE Labs

RE: [PATCH 3/3] HID: sensor-hub: don't limit the driver only to USB bus

[Pandruvada, Srinivas]

So finally we can use HID sensor hub over I2C.
Did you try with any I2C sensor hub?

Thanks,
Srinivas


-----Original Message-----
From: Mika Westerberg [mailto:mika.westerberg@linux.intel.com] 
Sent: Monday, February 11, 2013 2:31 AM
To: linux-kernel@vger.kernel.org
Cc: Jiri Kosina; Benjamin Tissoires; linux-input@vger.kernel.org; Pandruvada, Srinivas; Mika Westerberg
Subject: [PATCH 3/3] HID: sensor-hub: don't limit the driver only to USB bus

We now have two transport mediums: USB and I2C, where sensor hubs can exists. So instead of constraining the driver to only these two we let it to match any HID bus as long as the group is HID_GROUP_SENSOR_HUB.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/hid/hid-sensor-hub.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c index 2643bce9..c01f10d 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -603,7 +603,8 @@ static void sensor_hub_remove(struct hid_device *hdev)  }
 
 static const struct hid_device_id sensor_hub_devices[] = {
-	{ HID_DEVICE(BUS_USB, HID_GROUP_SENSOR_HUB, HID_ANY_ID, HID_ANY_ID) },
+	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, HID_ANY_ID,
+		     HID_ANY_ID) },
 	{ }
 };
 MODULE_DEVICE_TABLE(hid, sensor_hub_devices);
--
1.7.10.4

Re: [PATCH 3/3] HID: sensor-hub: don't limit the driver only to USB bus

[Mika Westerberg]

On Mon, Feb 11, 2013 at 03:54:05PM +0000, Pandruvada, Srinivas wrote:
> So finally we can use HID sensor hub over I2C.
> Did you try with any I2C sensor hub?

Yes, we have I2C HID sensor hub here and I tested with it. However, I'm not
that familiar how it should be used except that I can see the sensors under
/sys/bus/platform/devices/HID-SENSOR* and there are iio devices under each
platform device.

Re: [PATCH 3/3] HID: sensor-hub: don't limit the driver only to USB bus

[Alexander Holler]

Am 11.02.2013 11:31, schrieb Mika Westerberg:
> We now have two transport mediums: USB and I2C, where sensor hubs can
> exists. So instead of constraining the driver to only these two we let it
> to match any HID bus as long as the group is HID_GROUP_SENSOR_HUB.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>   drivers/hid/hid-sensor-hub.c |    3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
> index 2643bce9..c01f10d 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -603,7 +603,8 @@ static void sensor_hub_remove(struct hid_device *hdev)
>   }
>
>   static const struct hid_device_id sensor_hub_devices[] = {
> -	{ HID_DEVICE(BUS_USB, HID_GROUP_SENSOR_HUB, HID_ANY_ID, HID_ANY_ID) },
> +	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, HID_ANY_ID,
> +		     HID_ANY_ID) },
>   	{ }
>   };
>   MODULE_DEVICE_TABLE(hid, sensor_hub_devices);
>

Hmm, what happens with Bluetooth sensor-hubs? Is the driver now able to 
handle them too?

Regards,

Alexander

Re: [PATCH 3/3] HID: sensor-hub: don't limit the driver only to USB bus

[Mika Westerberg]

On Mon, Feb 18, 2013 at 12:03:04PM +0100, Alexander Holler wrote:
> Am 11.02.2013 11:31, schrieb Mika Westerberg:
> >We now have two transport mediums: USB and I2C, where sensor hubs can
> >exists. So instead of constraining the driver to only these two we let it
> >to match any HID bus as long as the group is HID_GROUP_SENSOR_HUB.
> >
> >Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> >---
> >  drivers/hid/hid-sensor-hub.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
> >index 2643bce9..c01f10d 100644
> >--- a/drivers/hid/hid-sensor-hub.c
> >+++ b/drivers/hid/hid-sensor-hub.c
> >@@ -603,7 +603,8 @@ static void sensor_hub_remove(struct hid_device *hdev)
> >  }
> >
> >  static const struct hid_device_id sensor_hub_devices[] = {
> >-	{ HID_DEVICE(BUS_USB, HID_GROUP_SENSOR_HUB, HID_ANY_ID, HID_ANY_ID) },
> >+	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, HID_ANY_ID,
> >+		     HID_ANY_ID) },
> >  	{ }
> >  };
> >  MODULE_DEVICE_TABLE(hid, sensor_hub_devices);
> >
> 
> Hmm, what happens with Bluetooth sensor-hubs? Is the driver now able
> to handle them too?

It should, yes.

Re: [PATCH 3/3] HID: sensor-hub: don't limit the driver only to USB bus

[Alexander Holler]

Am 18.02.2013 12:12, schrieb Mika Westerberg:
> On Mon, Feb 18, 2013 at 12:03:04PM +0100, Alexander Holler wrote:
>> Am 11.02.2013 11:31, schrieb Mika Westerberg:
>>> We now have two transport mediums: USB and I2C, where sensor hubs can
>>> exists. So instead of constraining the driver to only these two we let it
>>> to match any HID bus as long as the group is HID_GROUP_SENSOR_HUB.
>>>
>>> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>>> ---
>>>   drivers/hid/hid-sensor-hub.c |    3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
>>> index 2643bce9..c01f10d 100644
>>> --- a/drivers/hid/hid-sensor-hub.c
>>> +++ b/drivers/hid/hid-sensor-hub.c
>>> @@ -603,7 +603,8 @@ static void sensor_hub_remove(struct hid_device *hdev)
>>>   }
>>>
>>>   static const struct hid_device_id sensor_hub_devices[] = {
>>> -	{ HID_DEVICE(BUS_USB, HID_GROUP_SENSOR_HUB, HID_ANY_ID, HID_ANY_ID) },
>>> +	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, HID_ANY_ID,
>>> +		     HID_ANY_ID) },
>>>   	{ }
>>>   };
>>>   MODULE_DEVICE_TABLE(hid, sensor_hub_devices);
>>>
>>
>> Hmm, what happens with Bluetooth sensor-hubs? Is the driver now able
>> to handle them too?
>
> It should, yes.

If so, I think patch 1/3 should be modified accordingly.

Regards,

Alexander

Re: [PATCH 3/3] HID: sensor-hub: don't limit the driver only to USB bus

[Mika Westerberg]

On Mon, Feb 18, 2013 at 12:22:58PM +0100, Alexander Holler wrote:
> Am 18.02.2013 12:12, schrieb Mika Westerberg:
> >On Mon, Feb 18, 2013 at 12:03:04PM +0100, Alexander Holler wrote:
> >>Am 11.02.2013 11:31, schrieb Mika Westerberg:
> >>>We now have two transport mediums: USB and I2C, where sensor hubs can
> >>>exists. So instead of constraining the driver to only these two we let it
> >>>to match any HID bus as long as the group is HID_GROUP_SENSOR_HUB.
> >>>
> >>>Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> >>>---
> >>>  drivers/hid/hid-sensor-hub.c |    3 ++-
> >>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>>diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
> >>>index 2643bce9..c01f10d 100644
> >>>--- a/drivers/hid/hid-sensor-hub.c
> >>>+++ b/drivers/hid/hid-sensor-hub.c
> >>>@@ -603,7 +603,8 @@ static void sensor_hub_remove(struct hid_device *hdev)
> >>>  }
> >>>
> >>>  static const struct hid_device_id sensor_hub_devices[] = {
> >>>-	{ HID_DEVICE(BUS_USB, HID_GROUP_SENSOR_HUB, HID_ANY_ID, HID_ANY_ID) },
> >>>+	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, HID_ANY_ID,
> >>>+		     HID_ANY_ID) },
> >>>  	{ }
> >>>  };
> >>>  MODULE_DEVICE_TABLE(hid, sensor_hub_devices);
> >>>
> >>
> >>Hmm, what happens with Bluetooth sensor-hubs? Is the driver now able
> >>to handle them too?
> >
> >It should, yes.
> 
> If so, I think patch 1/3 should be modified accordingly.

Do you know if such devices exists currently? If not, I'm not sure if it
makes sense to do that now.

Re: [PATCH 3/3] HID: sensor-hub: don't limit the driver only to USB bus

[Alexander Holler]

Am 18.02.2013 12:33, schrieb Mika Westerberg:
> On Mon, Feb 18, 2013 at 12:22:58PM +0100, Alexander Holler wrote:
>> Am 18.02.2013 12:12, schrieb Mika Westerberg:
>>> On Mon, Feb 18, 2013 at 12:03:04PM +0100, Alexander Holler wrote:
>>>> Am 11.02.2013 11:31, schrieb Mika Westerberg:
>>>>> We now have two transport mediums: USB and I2C, where sensor hubs can
>>>>> exists. So instead of constraining the driver to only these two we let it
>>>>> to match any HID bus as long as the group is HID_GROUP_SENSOR_HUB.
>>>>>
>>>>> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>>>>> ---
>>>>>   drivers/hid/hid-sensor-hub.c |    3 ++-
>>>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
>>>>> index 2643bce9..c01f10d 100644
>>>>> --- a/drivers/hid/hid-sensor-hub.c
>>>>> +++ b/drivers/hid/hid-sensor-hub.c
>>>>> @@ -603,7 +603,8 @@ static void sensor_hub_remove(struct hid_device *hdev)
>>>>>   }
>>>>>
>>>>>   static const struct hid_device_id sensor_hub_devices[] = {
>>>>> -	{ HID_DEVICE(BUS_USB, HID_GROUP_SENSOR_HUB, HID_ANY_ID, HID_ANY_ID) },
>>>>> +	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, HID_ANY_ID,
>>>>> +		     HID_ANY_ID) },
>>>>>   	{ }
>>>>>   };
>>>>>   MODULE_DEVICE_TABLE(hid, sensor_hub_devices);
>>>>>
>>>>
>>>> Hmm, what happens with Bluetooth sensor-hubs? Is the driver now able
>>>> to handle them too?
>>>
>>> It should, yes.
>>
>> If so, I think patch 1/3 should be modified accordingly.
>
> Do you know if such devices exists currently? If not, I'm not sure if it
> makes sense to do that now.

The CC2541DK-SENSOR from TI looks like one. But I'm not sure as I don't 
have one. Besides that, I think Bluetooth (especially with BT4LE) will 
be by far the most used bus for sensors hubs.

Regards,

Alexander

Re: [PATCH 3/3] HID: sensor-hub: don't limit the driver only to USB bus

[Mika Westerberg]

On Mon, Feb 18, 2013 at 12:37:52PM +0100, Alexander Holler wrote:
> Am 18.02.2013 12:33, schrieb Mika Westerberg:
> >On Mon, Feb 18, 2013 at 12:22:58PM +0100, Alexander Holler wrote:
> >>Am 18.02.2013 12:12, schrieb Mika Westerberg:
> >>>On Mon, Feb 18, 2013 at 12:03:04PM +0100, Alexander Holler wrote:
> >>>>Am 11.02.2013 11:31, schrieb Mika Westerberg:
> >>>>>We now have two transport mediums: USB and I2C, where sensor hubs can
> >>>>>exists. So instead of constraining the driver to only these two we let it
> >>>>>to match any HID bus as long as the group is HID_GROUP_SENSOR_HUB.
> >>>>>
> >>>>>Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> >>>>>---
> >>>>>  drivers/hid/hid-sensor-hub.c |    3 ++-
> >>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>>>
> >>>>>diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
> >>>>>index 2643bce9..c01f10d 100644
> >>>>>--- a/drivers/hid/hid-sensor-hub.c
> >>>>>+++ b/drivers/hid/hid-sensor-hub.c
> >>>>>@@ -603,7 +603,8 @@ static void sensor_hub_remove(struct hid_device *hdev)
> >>>>>  }
> >>>>>
> >>>>>  static const struct hid_device_id sensor_hub_devices[] = {
> >>>>>-	{ HID_DEVICE(BUS_USB, HID_GROUP_SENSOR_HUB, HID_ANY_ID, HID_ANY_ID) },
> >>>>>+	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, HID_ANY_ID,
> >>>>>+		     HID_ANY_ID) },
> >>>>>  	{ }
> >>>>>  };
> >>>>>  MODULE_DEVICE_TABLE(hid, sensor_hub_devices);
> >>>>>
> >>>>
> >>>>Hmm, what happens with Bluetooth sensor-hubs? Is the driver now able
> >>>>to handle them too?
> >>>
> >>>It should, yes.
> >>
> >>If so, I think patch 1/3 should be modified accordingly.
> >
> >Do you know if such devices exists currently? If not, I'm not sure if it
> >makes sense to do that now.
> 
> The CC2541DK-SENSOR from TI looks like one. But I'm not sure as I
> don't have one. Besides that, I think Bluetooth (especially with
> BT4LE) will be by far the most used bus for sensors hubs.

OK, thanks.

In that case I think it's best to remove the explicit bus check from the
condition completely and rely on the fact that page == HID_UP_SENSOR.

Since Jiri already applied this patch, I can make an incremental patch
which removes the explicit bus check, if there are no objections.

Re: [PATCH 3/3] HID: sensor-hub: don't limit the driver only to USB bus

[Alexander Holler]

Am 18.02.2013 12:54, schrieb Mika Westerberg:
> On Mon, Feb 18, 2013 at 12:37:52PM +0100, Alexander Holler wrote:
>> Am 18.02.2013 12:33, schrieb Mika Westerberg:
>>> On Mon, Feb 18, 2013 at 12:22:58PM +0100, Alexander Holler wrote:
>>>> Am 18.02.2013 12:12, schrieb Mika Westerberg:
>>>>> On Mon, Feb 18, 2013 at 12:03:04PM +0100, Alexander Holler wrote:
>>>>>> Am 11.02.2013 11:31, schrieb Mika Westerberg:
>>>>>>> We now have two transport mediums: USB and I2C, where sensor hubs can
>>>>>>> exists. So instead of constraining the driver to only these two we let it
>>>>>>> to match any HID bus as long as the group is HID_GROUP_SENSOR_HUB.
>>>>>>>
>>>>>>> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>>>>>>> ---
>>>>>>>   drivers/hid/hid-sensor-hub.c |    3 ++-
>>>>>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
>>>>>>> index 2643bce9..c01f10d 100644
>>>>>>> --- a/drivers/hid/hid-sensor-hub.c
>>>>>>> +++ b/drivers/hid/hid-sensor-hub.c
>>>>>>> @@ -603,7 +603,8 @@ static void sensor_hub_remove(struct hid_device *hdev)
>>>>>>>   }
>>>>>>>
>>>>>>>   static const struct hid_device_id sensor_hub_devices[] = {
>>>>>>> -	{ HID_DEVICE(BUS_USB, HID_GROUP_SENSOR_HUB, HID_ANY_ID, HID_ANY_ID) },
>>>>>>> +	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, HID_ANY_ID,
>>>>>>> +		     HID_ANY_ID) },
>>>>>>>   	{ }
>>>>>>>   };
>>>>>>>   MODULE_DEVICE_TABLE(hid, sensor_hub_devices);
>>>>>>>
>>>>>>
>>>>>> Hmm, what happens with Bluetooth sensor-hubs? Is the driver now able
>>>>>> to handle them too?
>>>>>
>>>>> It should, yes.
>>>>
>>>> If so, I think patch 1/3 should be modified accordingly.
>>>
>>> Do you know if such devices exists currently? If not, I'm not sure if it
>>> makes sense to do that now.
>>
>> The CC2541DK-SENSOR from TI looks like one. But I'm not sure as I
>> don't have one. Besides that, I think Bluetooth (especially with
>> BT4LE) will be by far the most used bus for sensors hubs.
>
> OK, thanks.
>
> In that case I think it's best to remove the explicit bus check from the
> condition completely and rely on the fact that page == HID_UP_SENSOR.
>
> Since Jiri already applied this patch, I can make an incremental patch
> which removes the explicit bus check, if there are no objections.
>

I just had a deeper look at the spec for that TI device. Looks like the 
default firmware uses GATT and not HID. But nevertheless, if the driver 
does support Bluetooth too, I think enabling it makes sense.

Regards,

Alexander

[PATCH] HID: make sensor autodetection independent of underlying bus

[Mika Westerberg]

Instead of limiting HID sensors to USB and I2C busses we can just make
everything that has usage page of HID_UP_SENSOR to be included in
HID_GROUP_SENSOR_HUB group. This allows the sensor-hub to work over
bluetooth (and other transports) as well.

Reported-by: Alexander Holler <holler@ahsoftware.de>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/hid/hid-core.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index fe3a59e..f89e902 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -728,8 +728,7 @@ static int hid_scan_report(struct hid_device *hid)
 		} else if (page == HID_UP_SENSOR &&
 			item.type == HID_ITEM_TYPE_MAIN &&
 			item.tag == HID_MAIN_ITEM_TAG_BEGIN_COLLECTION &&
-			(item_udata(&item) & 0xff) == HID_COLLECTION_PHYSICAL &&
-			(hid->bus == BUS_USB || hid->bus == BUS_I2C))
+			(item_udata(&item) & 0xff) == HID_COLLECTION_PHYSICAL)
 			hid->group = HID_GROUP_SENSOR_HUB;
 	}
 
-- 
1.7.10.4

Re: [PATCH] HID: make sensor autodetection independent of underlying bus

[Benjamin Tissoires]

On Tue, Feb 19, 2013 at 10:22 AM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> Instead of limiting HID sensors to USB and I2C busses we can just make
> everything that has usage page of HID_UP_SENSOR to be included in
> HID_GROUP_SENSOR_HUB group. This allows the sensor-hub to work over
> bluetooth (and other transports) as well.
>
> Reported-by: Alexander Holler <holler@ahsoftware.de>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Reviewed-By: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Cheers,
Benjamin

> ---
>  drivers/hid/hid-core.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index fe3a59e..f89e902 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -728,8 +728,7 @@ static int hid_scan_report(struct hid_device *hid)
>                 } else if (page == HID_UP_SENSOR &&
>                         item.type == HID_ITEM_TYPE_MAIN &&
>                         item.tag == HID_MAIN_ITEM_TAG_BEGIN_COLLECTION &&
> -                       (item_udata(&item) & 0xff) == HID_COLLECTION_PHYSICAL &&
> -                       (hid->bus == BUS_USB || hid->bus == BUS_I2C))
> +                       (item_udata(&item) & 0xff) == HID_COLLECTION_PHYSICAL)
>                         hid->group = HID_GROUP_SENSOR_HUB;
>         }
>
> --
> 1.7.10.4
>

Re: [PATCH] HID: make sensor autodetection independent of underlying bus

[Jiri Kosina]

On Thu, 21 Feb 2013, Benjamin Tissoires wrote:

> > Instead of limiting HID sensors to USB and I2C busses we can just make
> > everything that has usage page of HID_UP_SENSOR to be included in
> > HID_GROUP_SENSOR_HUB group. This allows the sensor-hub to work over
> > bluetooth (and other transports) as well.
> >
> > Reported-by: Alexander Holler <holler@ahsoftware.de>
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> Reviewed-By: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Applied.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 3/3] HID: sensor-hub: don't limit the driver only to USB bus

[Benjamin Tissoires]

Hi,

Sorry for coming this late in the discussion (I was sick at the
beginning of the week, preventing me to answer mails).

On Mon, Feb 18, 2013 at 1:13 PM, Alexander Holler <holler@ahsoftware.de> wrote:
>>>>>>> Hmm, what happens with Bluetooth sensor-hubs? Is the driver now able
>>>>>>> to handle them too?
>>>>>>
>>>>>>
>>>>>> It should, yes.
>>>>>

It should, but the full features are not enabled. We also need to
patch hidp so that it can handle .request() too. Otherwise, when the
sensor hub will request a "get feature" or a "set feature", then
nothing will be triggered.

However, I don't think it will prevent us from removing the test
against USB/I2C. It's just something we need to work on too.

Cheers,
Benjamin

Re: [PATCH 3/3] HID: sensor-hub: don't limit the driver only to USB bus

[Alexander Holler]

Am 21.02.2013 11:34, schrieb Benjamin Tissoires:
> Hi,
>
> Sorry for coming this late in the discussion (I was sick at the
> beginning of the week, preventing me to answer mails).
>
> On Mon, Feb 18, 2013 at 1:13 PM, Alexander Holler <holler@ahsoftware.de> wrote:
>>>>>>>> Hmm, what happens with Bluetooth sensor-hubs? Is the driver now able
>>>>>>>> to handle them too?
>>>>>>>
>>>>>>>
>>>>>>> It should, yes.
>>>>>>
>
> It should, but the full features are not enabled. We also need to
> patch hidp so that it can handle .request() too. Otherwise, when the
> sensor hub will request a "get feature" or a "set feature", then
> nothing will be triggered.
>
> However, I don't think it will prevent us from removing the test
> against USB/I2C. It's just something we need to work on too.

Thanks. I've just ordered me one of those bt-sensors from TI, maybe I 
will be able to make it HID over GATT aware to play a bit with it. The 
BT spec for HID over GATT doesn't talk about sensors, but if I will find 
a way to program the device without buying more hw, I will have a look 
if it is possible. ;)

Regards,

Alexander

Re: [PATCH 1/3] HID: extend autodetect to handle I2C sensors as well

[Benjamin Tissoires]

On Mon, Feb 11, 2013 at 11:31 AM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> Since the advent of HID over I2C protocol, it is possible to have sensor
> hubs behind I2C bus as well. We can autodetect this in a same way than USB
> sensor hubs.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Cheers,
Benjamin