| Message ID | 20231219231503.1506801-1-jekhor@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | HID: sensor-hub: Enable hid core report processing for all devices | expand |
On Wed, 20 Dec 2023 01:15:03 +0200 Yauhen Kharuzhy <jekhor@gmail.com> 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 <jekhor@gmail.com> 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 <jic23@kernel.org>: > > On Wed, 20 Dec 2023 01:15:03 +0200 > Yauhen Kharuzhy <jekhor@gmail.com> 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 <jekhor@gmail.com> > 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; >
On Wed, 2023-12-20 at 17:04 +0200, Yauhen Kharuzhy wrote: > ср, 20 дек. 2023 г. в 16:52, Jonathan Cameron <jic23@kernel.org>: > > > > On Wed, 20 Dec 2023 01:15:03 +0200 > > Yauhen Kharuzhy <jekhor@gmail.com> 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 <jekhor@gmail.com> > > 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 <srinivas.pandruvada@linux.intel.com> wrote: > > On Wed, 2023-12-20 at 17:04 +0200, Yauhen Kharuzhy wrote: > > ср, 20 дек. 2023 г. в 16:52, Jonathan Cameron <jic23@kernel.org>: > > > > > > On Wed, 20 Dec 2023 01:15:03 +0200 > > > Yauhen Kharuzhy <jekhor@gmail.com> 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 <jekhor@gmail.com> > > > 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 > <srinivas.pandruvada@linux.intel.com> wrote: > > > > On Wed, 2023-12-20 at 17:04 +0200, Yauhen Kharuzhy wrote: > > > ср, 20 дек. 2023 г. в 16:52, Jonathan Cameron <jic23@kernel.org>: > > > > > > > > On Wed, 20 Dec 2023 01:15:03 +0200 > > > > Yauhen Kharuzhy <jekhor@gmail.com> 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 <jekhor@gmail.com> > > > > 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 <srinivas.pandruvada@linux.intel.com> 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 <jekhor@gmail.com> 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 <daniel.thompson@linaro.org> 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,
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;
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 <jekhor@gmail.com> --- drivers/hid/hid-sensor-hub.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)