After the commit 666cf30a589a ("HID: sensor-hub: Allow multi-function
sensor devices") hub devices are claimed by hidraw driver in hid_connect().
This causes stoppping of processing HID reports by hid core due to
optimization.
In such case, the hid-sensor-custom driver cannot match a known custom
sensor in hid_sensor_custom_get_known() because it try to check custom
properties which weren't filled from the report because hid core didn't
parsed it.
As result, custom sensors like hinge angle sensor and LISS sensors
don't work.
Mark the sensor hub devices claimed by some driver to avoid hidraw-related
optimizations.
Signed-off-by: Yauhen Kharuzhy <[email protected]>
---
drivers/hid/hid-sensor-hub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 2eba152e8b90..26e93a331a51 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -632,7 +632,7 @@ static int sensor_hub_probe(struct hid_device *hdev,
}
INIT_LIST_HEAD(&hdev->inputs);
- ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT | HID_CONNECT_DRIVER);
if (ret) {
hid_err(hdev, "hw start failed\n");
return ret;
--
2.43.0
On Wed, 20 Dec 2023 01:15:03 +0200
Yauhen Kharuzhy <[email protected]> wrote:
> After the commit 666cf30a589a ("HID: sensor-hub: Allow multi-function
> sensor devices") hub devices are claimed by hidraw driver in hid_connect().
> This causes stoppping of processing HID reports by hid core due to
> optimization.
>
> In such case, the hid-sensor-custom driver cannot match a known custom
> sensor in hid_sensor_custom_get_known() because it try to check custom
> properties which weren't filled from the report because hid core didn't
> parsed it.
>
> As result, custom sensors like hinge angle sensor and LISS sensors
> don't work.
>
> Mark the sensor hub devices claimed by some driver to avoid hidraw-related
> optimizations.
>
> Signed-off-by: Yauhen Kharuzhy <[email protected]>
Fixes tag?
> ---
> drivers/hid/hid-sensor-hub.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
> index 2eba152e8b90..26e93a331a51 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -632,7 +632,7 @@ static int sensor_hub_probe(struct hid_device *hdev,
> }
> INIT_LIST_HEAD(&hdev->inputs);
>
> - ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT | HID_CONNECT_DRIVER);
> if (ret) {
> hid_err(hdev, "hw start failed\n");
> return ret;
ср, 20 дек. 2023 г. в 16:52, Jonathan Cameron <[email protected]>:
>
> On Wed, 20 Dec 2023 01:15:03 +0200
> Yauhen Kharuzhy <[email protected]> wrote:
>
> > After the commit 666cf30a589a ("HID: sensor-hub: Allow multi-function
> > sensor devices") hub devices are claimed by hidraw driver in hid_connect().
> > This causes stoppping of processing HID reports by hid core due to
> > optimization.
> >
> > In such case, the hid-sensor-custom driver cannot match a known custom
> > sensor in hid_sensor_custom_get_known() because it try to check custom
> > properties which weren't filled from the report because hid core didn't
> > parsed it.
> >
> > As result, custom sensors like hinge angle sensor and LISS sensors
> > don't work.
> >
> > Mark the sensor hub devices claimed by some driver to avoid hidraw-related
> > optimizations.
> >
> > Signed-off-by: Yauhen Kharuzhy <[email protected]>
> Fixes tag?
Fixes: 666cf30a589a ("HID: sensor-hub: Allow multi-function sensor devices")
>
> > ---
> > drivers/hid/hid-sensor-hub.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
> > index 2eba152e8b90..26e93a331a51 100644
> > --- a/drivers/hid/hid-sensor-hub.c
> > +++ b/drivers/hid/hid-sensor-hub.c
> > @@ -632,7 +632,7 @@ static int sensor_hub_probe(struct hid_device *hdev,
> > }
> > INIT_LIST_HEAD(&hdev->inputs);
> >
> > - ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> > + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT | HID_CONNECT_DRIVER);
> > if (ret) {
> > hid_err(hdev, "hw start failed\n");
> > return ret;
>
--
Yauhen Kharuzhy
On Wed, 2023-12-20 at 17:04 +0200, Yauhen Kharuzhy wrote:
> ср, 20 дек. 2023 г. в 16:52, Jonathan Cameron <[email protected]>:
> >
> > On Wed, 20 Dec 2023 01:15:03 +0200
> > Yauhen Kharuzhy <[email protected]> wrote:
> >
> > > After the commit 666cf30a589a ("HID: sensor-hub: Allow multi-
> > > function
> > > sensor devices") hub devices are claimed by hidraw driver in
> > > hid_connect().
> > > This causes stoppping of processing HID reports by hid core due
> > > to
> > > optimization.
> > >
> > > In such case, the hid-sensor-custom driver cannot match a known
> > > custom
> > > sensor in hid_sensor_custom_get_known() because it try to check
> > > custom
> > > properties which weren't filled from the report because hid core
> > > didn't
> > > parsed it.
> > >
> > > As result, custom sensors like hinge angle sensor and LISS
> > > sensors
> > > don't work.
> > >
> > > Mark the sensor hub devices claimed by some driver to avoid
> > > hidraw-related
> > > optimizations.
> > >
> > > Signed-off-by: Yauhen Kharuzhy <[email protected]>
> > Fixes tag?
>
> Fixes: 666cf30a589a ("HID: sensor-hub: Allow multi-function sensor
> devices")
>
This flag causes
hdev->claimed |= HID_CLAIMED_DRIVER;
I don't see the flag is used anywhere after this assignment in hid
core. Only two other drivers are setting this flag. We need Jiri's help
here why this is a special case.
Thanks,
Srinivas
> >
> > > ---
> > > drivers/hid/hid-sensor-hub.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-
> > > sensor-hub.c
> > > index 2eba152e8b90..26e93a331a51 100644
> > > --- a/drivers/hid/hid-sensor-hub.c
> > > +++ b/drivers/hid/hid-sensor-hub.c
> > > @@ -632,7 +632,7 @@ static int sensor_hub_probe(struct hid_device
> > > *hdev,
> > > }
> > > INIT_LIST_HEAD(&hdev->inputs);
> > >
> > > - ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> > > + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT |
> > > HID_CONNECT_DRIVER);
> > > if (ret) {
> > > hid_err(hdev, "hw start failed\n");
> > > return ret;
> >
>
>
On Fri, Dec 22, 2023 at 1:44 PM srinivas pandruvada
<[email protected]> wrote:
>
> On Wed, 2023-12-20 at 17:04 +0200, Yauhen Kharuzhy wrote:
> > ср, 20 дек. 2023 г. в 16:52, Jonathan Cameron <[email protected]>:
> > >
> > > On Wed, 20 Dec 2023 01:15:03 +0200
> > > Yauhen Kharuzhy <[email protected]> wrote:
> > >
> > > > After the commit 666cf30a589a ("HID: sensor-hub: Allow multi-
> > > > function
> > > > sensor devices") hub devices are claimed by hidraw driver in
> > > > hid_connect().
> > > > This causes stoppping of processing HID reports by hid core due
> > > > to
> > > > optimization.
> > > >
> > > > In such case, the hid-sensor-custom driver cannot match a known
> > > > custom
> > > > sensor in hid_sensor_custom_get_known() because it try to check
> > > > custom
> > > > properties which weren't filled from the report because hid core
> > > > didn't
> > > > parsed it.
> > > >
> > > > As result, custom sensors like hinge angle sensor and LISS
> > > > sensors
> > > > don't work.
> > > >
> > > > Mark the sensor hub devices claimed by some driver to avoid
> > > > hidraw-related
> > > > optimizations.
> > > >
> > > > Signed-off-by: Yauhen Kharuzhy <[email protected]>
> > > Fixes tag?
> >
> > Fixes: 666cf30a589a ("HID: sensor-hub: Allow multi-function sensor
> > devices")
> >
> This flag causes
> hdev->claimed |= HID_CLAIMED_DRIVER;
> I don't see the flag is used anywhere after this assignment in hid
> core. Only two other drivers are setting this flag. We need Jiri's help
> here why this is a special case.
It's used in hid_report_raw_event()[0]:
```
if (hid->claimed != HID_CLAIMED_HIDRAW && report->maxfield) {
hid_process_report(hid, report, cdata, interrupt);
hdrv = hid->driver;
if (hdrv && hdrv->report)
hdrv->report(hid, report);
}
```
The whole point of setting HID_CLAIMED_DRIVER is to have hid->claimed
not equal to HID_CLAIMED_HIDRAW, in case we need the hid core
processing.
Cheers,
Benjamin
[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/hid/hid-core.c#n2015
>
> Thanks,
> Srinivas
>
> > >
> > > > ---
> > > > drivers/hid/hid-sensor-hub.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-
> > > > sensor-hub.c
> > > > index 2eba152e8b90..26e93a331a51 100644
> > > > --- a/drivers/hid/hid-sensor-hub.c
> > > > +++ b/drivers/hid/hid-sensor-hub.c
> > > > @@ -632,7 +632,7 @@ static int sensor_hub_probe(struct hid_device
> > > > *hdev,
> > > > }
> > > > INIT_LIST_HEAD(&hdev->inputs);
> > > >
> > > > - ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> > > > + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT |
> > > > HID_CONNECT_DRIVER);
> > > > if (ret) {
> > > > hid_err(hdev, "hw start failed\n");
> > > > return ret;
> > >
> >
> >
>
On Fri, 2023-12-22 at 14:28 +0100, Benjamin Tissoires wrote:
> On Fri, Dec 22, 2023 at 1:44 PM srinivas pandruvada
> <[email protected]> wrote:
> >
> > On Wed, 2023-12-20 at 17:04 +0200, Yauhen Kharuzhy wrote:
> > > ср, 20 дек. 2023 г. в 16:52, Jonathan Cameron <[email protected]>:
> > > >
> > > > On Wed, 20 Dec 2023 01:15:03 +0200
> > > > Yauhen Kharuzhy <[email protected]> wrote:
> > > >
> > > > > After the commit 666cf30a589a ("HID: sensor-hub: Allow multi-
> > > > > function
> > > > > sensor devices") hub devices are claimed by hidraw driver in
> > > > > hid_connect().
> > > > > This causes stoppping of processing HID reports by hid core
> > > > > due
> > > > > to
> > > > > optimization.
> > > > >
> > > > > In such case, the hid-sensor-custom driver cannot match a
> > > > > known
> > > > > custom
> > > > > sensor in hid_sensor_custom_get_known() because it try to
> > > > > check
> > > > > custom
> > > > > properties which weren't filled from the report because hid
> > > > > core
> > > > > didn't
> > > > > parsed it.
> > > > >
> > > > > As result, custom sensors like hinge angle sensor and LISS
> > > > > sensors
> > > > > don't work.
> > > > >
> > > > > Mark the sensor hub devices claimed by some driver to avoid
> > > > > hidraw-related
> > > > > optimizations.
> > > > >
> > > > > Signed-off-by: Yauhen Kharuzhy <[email protected]>
> > > > Fixes tag?
> > >
> > > Fixes: 666cf30a589a ("HID: sensor-hub: Allow multi-function
> > > sensor
> > > devices")
> > >
> > This flag causes
> > hdev->claimed |= HID_CLAIMED_DRIVER;
> > I don't see the flag is used anywhere after this assignment in hid
> > core. Only two other drivers are setting this flag. We need Jiri's
> > help
> > here why this is a special case.
>
> It's used in hid_report_raw_event()[0]:
> ```
> if (hid->claimed != HID_CLAIMED_HIDRAW && report->maxfield) {
> hid_process_report(hid, report, cdata, interrupt);
> hdrv = hid->driver;
> if (hdrv && hdrv->report)
> hdrv->report(hid, report);
> }
> ```
>
> The whole point of setting HID_CLAIMED_DRIVER is to have hid->claimed
> not equal to HID_CLAIMED_HIDRAW, in case we need the hid core
> processing.
Thanks Benjamin for explaining.
Then this change looks fine as sensor hub driver will claim this device
and it needs hid core to process report.
Acked-by: Srinivas Pandruvada <[email protected]>
Thanks,
Srinivas
>
> Cheers,
> Benjamin
>
>
> [0]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/hid/hid-core.c#n2015
>
> >
> > Thanks,
> > Srinivas
> >
> > > >
> > > > > ---
> > > > > drivers/hid/hid-sensor-hub.c | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-
> > > > > sensor-hub.c
> > > > > index 2eba152e8b90..26e93a331a51 100644
> > > > > --- a/drivers/hid/hid-sensor-hub.c
> > > > > +++ b/drivers/hid/hid-sensor-hub.c
> > > > > @@ -632,7 +632,7 @@ static int sensor_hub_probe(struct
> > > > > hid_device
> > > > > *hdev,
> > > > > }
> > > > > INIT_LIST_HEAD(&hdev->inputs);
> > > > >
> > > > > - ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> > > > > + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT |
> > > > > HID_CONNECT_DRIVER);
> > > > > if (ret) {
> > > > > hid_err(hdev, "hw start failed\n");
> > > > > return ret;
> > > >
> > >
> > >
> >
>
On Wed, Dec 20, 2023 at 01:15:03AM +0200, Yauhen Kharuzhy wrote:
> After the commit 666cf30a589a ("HID: sensor-hub: Allow multi-function
> sensor devices") hub devices are claimed by hidraw driver in hid_connect().
> This causes stoppping of processing HID reports by hid core due to
> optimization.
>
> In such case, the hid-sensor-custom driver cannot match a known custom
> sensor in hid_sensor_custom_get_known() because it try to check custom
> properties which weren't filled from the report because hid core didn't
> parsed it.
>
> As result, custom sensors like hinge angle sensor and LISS sensors
> don't work.
>
> Mark the sensor hub devices claimed by some driver to avoid hidraw-related
> optimizations.
>
> Signed-off-by: Yauhen Kharuzhy <[email protected]>
I dusted off the Yoga C630 that provoked me into posting
666cf30a589a ("HID: sensor-hub: Allow multi-function sensor devices")
in the first place. Keyboard is still going strong so isn't not
comprehensive but for whatever it is worth this is:
Tested-by: Daniel Thompson <[email protected]>
Daniel.
On Wed, 20 Dec 2023 01:15:03 +0200, Yauhen Kharuzhy wrote:
> After the commit 666cf30a589a ("HID: sensor-hub: Allow multi-function
> sensor devices") hub devices are claimed by hidraw driver in hid_connect().
> This causes stoppping of processing HID reports by hid core due to
> optimization.
>
> In such case, the hid-sensor-custom driver cannot match a known custom
> sensor in hid_sensor_custom_get_known() because it try to check custom
> properties which weren't filled from the report because hid core didn't
> parsed it.
>
> [...]
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-6.8/sensor-hub), thanks!
[1/1] HID: sensor-hub: Enable hid core report processing for all devices
https://git.kernel.org/hid/hid/c/8e2f79f41a5d
Cheers,
--
Benjamin Tissoires <[email protected]>