* [PATCH] iio: light: hid-sensor-als: Avoid failure for chromaticity support
@ 2023-12-15 16:01 Srinivas Pandruvada
2023-12-15 16:20 ` Thomas Weißschuh
2023-12-17 14:35 ` Jonathan Cameron
0 siblings, 2 replies; 4+ messages in thread
From: Srinivas Pandruvada @ 2023-12-15 16:01 UTC (permalink / raw)
To: jikos, jic23, lars, Basavaraj.Natikar
Cc: linux-input, linux-iio, linux-kernel, Srinivas Pandruvada,
Thomas Weißschuh, stable
With the commit ee3710f39f9d ("iio: hid-sensor-als: Add light chromaticity
support"), there is an assumption that the every HID ALS descriptor has
support of usage ids for chromaticity support. If they are not present,
probe fails for the driver . This breaks ALS functionality on majority of
platforms.
It is possible that chromaticity usage ids are not present. When not
present, restrict number of IIO channels to not include support for
chromaticity and continue.
Fixes: ee3710f39f9d ("iio: hid-sensor-als: Add light chromaticity support")
Reported-by: Thomas Weißschuh <thomas@t-8ch.de>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218223
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: stable@vger.kernel.org
---
drivers/iio/light/hid-sensor-als.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index f17304b54468..9941b0b927c7 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -303,11 +303,14 @@ static int als_parse_report(struct platform_device *pdev,
struct hid_sensor_hub_device *hsdev,
struct iio_chan_spec *channels,
unsigned usage_id,
- struct als_state *st)
+ struct als_state *st,
+ int *max_channels)
{
int ret;
int i;
+ *max_channels = CHANNEL_SCAN_INDEX_MAX;
+
for (i = 0; i <= CHANNEL_SCAN_INDEX_ILLUM; ++i) {
ret = sensor_hub_input_get_attribute_info(hsdev,
HID_INPUT_REPORT,
@@ -326,8 +329,12 @@ static int als_parse_report(struct platform_device *pdev,
usage_id,
HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE,
&st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP]);
- if (ret < 0)
- return ret;
+ if (ret < 0) {
+ *max_channels = CHANNEL_SCAN_INDEX_ILLUM;
+ ret = 0;
+ goto skip_color_chromaticity;
+ }
+
als_adjust_channel_bit_mask(channels, CHANNEL_SCAN_INDEX_COLOR_TEMP,
st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].size);
@@ -354,6 +361,7 @@ static int als_parse_report(struct platform_device *pdev,
st->als[next_scan_index].report_id);
}
+skip_color_chromaticity:
st->scale_precision = hid_sensor_format_scale(usage_id,
&st->als[CHANNEL_SCAN_INDEX_INTENSITY],
&st->scale_pre_decml, &st->scale_post_decml);
@@ -364,7 +372,7 @@ static int als_parse_report(struct platform_device *pdev,
/* Function to initialize the processing for usage id */
static int hid_als_probe(struct platform_device *pdev)
{
- int ret = 0;
+ int ret = 0, max_channels;
static const char *name = "als";
struct iio_dev *indio_dev;
struct als_state *als_state;
@@ -398,15 +406,15 @@ static int hid_als_probe(struct platform_device *pdev)
ret = als_parse_report(pdev, hsdev,
(struct iio_chan_spec *)indio_dev->channels,
- hsdev->usage,
- als_state);
+ hsdev->usage, als_state, &max_channels);
if (ret) {
dev_err(&pdev->dev, "failed to setup attributes\n");
return ret;
}
- indio_dev->num_channels =
- ARRAY_SIZE(als_channels);
+ /* +1 to include time stamp */
+ indio_dev->num_channels = max_channels + 1;
+
indio_dev->info = &als_info;
indio_dev->name = name;
indio_dev->modes = INDIO_DIRECT_MODE;
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: light: hid-sensor-als: Avoid failure for chromaticity support
2023-12-15 16:01 [PATCH] iio: light: hid-sensor-als: Avoid failure for chromaticity support Srinivas Pandruvada
@ 2023-12-15 16:20 ` Thomas Weißschuh
2023-12-17 14:35 ` Jonathan Cameron
1 sibling, 0 replies; 4+ messages in thread
From: Thomas Weißschuh @ 2023-12-15 16:20 UTC (permalink / raw)
To: Srinivas Pandruvada
Cc: jikos, jic23, lars, Basavaraj.Natikar, linux-input, linux-iio,
linux-kernel, stable
On 2023-12-15 08:01:59-0800, Srinivas Pandruvada wrote:
> With the commit ee3710f39f9d ("iio: hid-sensor-als: Add light chromaticity
> support"), there is an assumption that the every HID ALS descriptor has
> support of usage ids for chromaticity support. If they are not present,
> probe fails for the driver . This breaks ALS functionality on majority of
> platforms.
>
> It is possible that chromaticity usage ids are not present. When not
> present, restrict number of IIO channels to not include support for
> chromaticity and continue.
>
> Fixes: ee3710f39f9d ("iio: hid-sensor-als: Add light chromaticity support")
> Reported-by: Thomas Weißschuh <thomas@t-8ch.de>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218223
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: stable@vger.kernel.org
> ---
> drivers/iio/light/hid-sensor-als.c | 24 ++++++++++++++++--------
> 1 file changed, 16 insertions(+), 8 deletions(-)
Thanks!
Tested-by: Thomas Weißschuh <linux@weissschuh.net> # on Framework 13 AMD
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: light: hid-sensor-als: Avoid failure for chromaticity support
2023-12-15 16:01 [PATCH] iio: light: hid-sensor-als: Avoid failure for chromaticity support Srinivas Pandruvada
2023-12-15 16:20 ` Thomas Weißschuh
@ 2023-12-17 14:35 ` Jonathan Cameron
2023-12-17 16:14 ` srinivas pandruvada
1 sibling, 1 reply; 4+ messages in thread
From: Jonathan Cameron @ 2023-12-17 14:35 UTC (permalink / raw)
To: Srinivas Pandruvada
Cc: jikos, lars, Basavaraj.Natikar, linux-input, linux-iio,
linux-kernel, Thomas Weißschuh, stable
On Fri, 15 Dec 2023 08:01:59 -0800
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
> With the commit ee3710f39f9d ("iio: hid-sensor-als: Add light chromaticity
> support"), there is an assumption that the every HID ALS descriptor has
> support of usage ids for chromaticity support. If they are not present,
> probe fails for the driver . This breaks ALS functionality on majority of
> platforms.
>
> It is possible that chromaticity usage ids are not present. When not
> present, restrict number of IIO channels to not include support for
> chromaticity and continue.
>
> Fixes: ee3710f39f9d ("iio: hid-sensor-als: Add light chromaticity support")
> Reported-by: Thomas Weißschuh <thomas@t-8ch.de>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218223
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: stable@vger.kernel.org
> ---
> drivers/iio/light/hid-sensor-als.c | 24 ++++++++++++++++--------
> 1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
> index f17304b54468..9941b0b927c7 100644
> --- a/drivers/iio/light/hid-sensor-als.c
> +++ b/drivers/iio/light/hid-sensor-als.c
> @@ -303,11 +303,14 @@ static int als_parse_report(struct platform_device *pdev,
> struct hid_sensor_hub_device *hsdev,
> struct iio_chan_spec *channels,
> unsigned usage_id,
> - struct als_state *st)
> + struct als_state *st,
> + int *max_channels)
> {
> int ret;
> int i;
>
> + *max_channels = CHANNEL_SCAN_INDEX_MAX;
> +
> for (i = 0; i <= CHANNEL_SCAN_INDEX_ILLUM; ++i) {
> ret = sensor_hub_input_get_attribute_info(hsdev,
> HID_INPUT_REPORT,
> @@ -326,8 +329,12 @@ static int als_parse_report(struct platform_device *pdev,
> usage_id,
> HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE,
> &st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP]);
> - if (ret < 0)
> - return ret;
> + if (ret < 0) {
> + *max_channels = CHANNEL_SCAN_INDEX_ILLUM;
> + ret = 0;
> + goto skip_color_chromaticity;
> + }
> +
> als_adjust_channel_bit_mask(channels, CHANNEL_SCAN_INDEX_COLOR_TEMP,
> st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].size);
>
> @@ -354,6 +361,7 @@ static int als_parse_report(struct platform_device *pdev,
> st->als[next_scan_index].report_id);
> }
>
> +skip_color_chromaticity:
> st->scale_precision = hid_sensor_format_scale(usage_id,
> &st->als[CHANNEL_SCAN_INDEX_INTENSITY],
> &st->scale_pre_decml, &st->scale_post_decml);
> @@ -364,7 +372,7 @@ static int als_parse_report(struct platform_device *pdev,
> /* Function to initialize the processing for usage id */
> static int hid_als_probe(struct platform_device *pdev)
> {
> - int ret = 0;
> + int ret = 0, max_channels;
> static const char *name = "als";
> struct iio_dev *indio_dev;
> struct als_state *als_state;
> @@ -398,15 +406,15 @@ static int hid_als_probe(struct platform_device *pdev)
>
> ret = als_parse_report(pdev, hsdev,
> (struct iio_chan_spec *)indio_dev->channels,
> - hsdev->usage,
> - als_state);
> + hsdev->usage, als_state, &max_channels);
> if (ret) {
> dev_err(&pdev->dev, "failed to setup attributes\n");
> return ret;
> }
>
> - indio_dev->num_channels =
> - ARRAY_SIZE(als_channels);
> + /* +1 to include time stamp */
> + indio_dev->num_channels = max_channels + 1;
In the current array the timestamp channel isn't the next one, so how does this work?
I think we either have to form the channel array dynamically or pick between
one that does have the colour info and one that doesn't for the original case.
Given timing we may just need to revert the broken patch and revisit this next
cycle.
Jonathan
> +
> indio_dev->info = &als_info;
> indio_dev->name = name;
> indio_dev->modes = INDIO_DIRECT_MODE;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: light: hid-sensor-als: Avoid failure for chromaticity support
2023-12-17 14:35 ` Jonathan Cameron
@ 2023-12-17 16:14 ` srinivas pandruvada
0 siblings, 0 replies; 4+ messages in thread
From: srinivas pandruvada @ 2023-12-17 16:14 UTC (permalink / raw)
To: Jonathan Cameron
Cc: jikos, lars, Basavaraj.Natikar, linux-input, linux-iio,
linux-kernel, Thomas Weißschuh, stable
On Sun, 2023-12-17 at 14:35 +0000, Jonathan Cameron wrote:
> On Fri, 15 Dec 2023 08:01:59 -0800
> Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
>
> > With the commit ee3710f39f9d ("iio: hid-sensor-als: Add light
> > chromaticity
> > support"), there is an assumption that the every HID ALS descriptor
> > has
> > support of usage ids for chromaticity support. If they are not
> > present,
> > probe fails for the driver . This breaks ALS functionality on
> > majority of
> > platforms.
> >
> > It is possible that chromaticity usage ids are not present. When
> > not
> > present, restrict number of IIO channels to not include support for
> > chromaticity and continue.
> >
> > Fixes: ee3710f39f9d ("iio: hid-sensor-als: Add light chromaticity
> > support")
> > Reported-by: Thomas Weißschuh <thomas@t-8ch.de>
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218223
> > Signed-off-by: Srinivas Pandruvada
> > <srinivas.pandruvada@linux.intel.com>
> > Cc: stable@vger.kernel.org
> > ---
> > drivers/iio/light/hid-sensor-als.c | 24 ++++++++++++++++--------
> > 1 file changed, 16 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/iio/light/hid-sensor-als.c
> > b/drivers/iio/light/hid-sensor-als.c
> > index f17304b54468..9941b0b927c7 100644
> > --- a/drivers/iio/light/hid-sensor-als.c
> > +++ b/drivers/iio/light/hid-sensor-als.c
> > @@ -303,11 +303,14 @@ static int als_parse_report(struct
> > platform_device *pdev,
> > struct hid_sensor_hub_device
> > *hsdev,
> > struct iio_chan_spec *channels,
> > unsigned usage_id,
> > - struct als_state *st)
> > + struct als_state *st,
> > + int *max_channels)
> > {
> > int ret;
> > int i;
> >
> > + *max_channels = CHANNEL_SCAN_INDEX_MAX;
> > +
> > for (i = 0; i <= CHANNEL_SCAN_INDEX_ILLUM; ++i) {
> > ret = sensor_hub_input_get_attribute_info(hsdev,
> > HID_INPUT_REPORT,
> > @@ -326,8 +329,12 @@ static int als_parse_report(struct
> > platform_device *pdev,
> > usage_id,
> > HID_USAGE_SENSOR_LIGHT_COLOR_TEMPER
> > ATURE,
> > &st-
> > >als[CHANNEL_SCAN_INDEX_COLOR_TEMP]);
> > - if (ret < 0)
> > - return ret;
> > + if (ret < 0) {
> > + *max_channels = CHANNEL_SCAN_INDEX_ILLUM;
> > + ret = 0;
> > + goto skip_color_chromaticity;
> > + }
> > +
> > als_adjust_channel_bit_mask(channels,
> > CHANNEL_SCAN_INDEX_COLOR_TEMP,
> > st-
> > >als[CHANNEL_SCAN_INDEX_COLOR_TEMP].size);
> >
> > @@ -354,6 +361,7 @@ static int als_parse_report(struct
> > platform_device *pdev,
> > st->als[next_scan_index].report_id);
> > }
> >
> > +skip_color_chromaticity:
> > st->scale_precision = hid_sensor_format_scale(usage_id,
> > &st-
> > >als[CHANNEL_SCAN_INDEX_INTENSITY],
> > &st->scale_pre_decml, &st-
> > >scale_post_decml);
> > @@ -364,7 +372,7 @@ static int als_parse_report(struct
> > platform_device *pdev,
> > /* Function to initialize the processing for usage id */
> > static int hid_als_probe(struct platform_device *pdev)
> > {
> > - int ret = 0;
> > + int ret = 0, max_channels;
> > static const char *name = "als";
> > struct iio_dev *indio_dev;
> > struct als_state *als_state;
> > @@ -398,15 +406,15 @@ static int hid_als_probe(struct
> > platform_device *pdev)
> >
> > ret = als_parse_report(pdev, hsdev,
> > (struct iio_chan_spec *)indio_dev-
> > >channels,
> > - hsdev->usage,
> > - als_state);
> > + hsdev->usage, als_state,
> > &max_channels);
> > if (ret) {
> > dev_err(&pdev->dev, "failed to setup
> > attributes\n");
> > return ret;
> > }
> >
> > - indio_dev->num_channels =
> > - ARRAY_SIZE(als_channels);
> > + /* +1 to include time stamp */
> > + indio_dev->num_channels = max_channels + 1;
>
> In the current array the timestamp channel isn't the next one, so how
> does this work?
>
> I think we either have to form the channel array dynamically or pick
> between
> one that does have the colour info and one that doesn't for the
> original case.
>
You are right, let me resubmit.
> Given timing we may just need to revert the broken patch and revisit
> this next
> cycle.
This is better. I will send a revert.
Thanks,
Srinivas
>
> Jonathan
>
>
> > +
> > indio_dev->info = &als_info;
> > indio_dev->name = name;
> > indio_dev->modes = INDIO_DIRECT_MODE;
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-12-17 16:14 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-15 16:01 [PATCH] iio: light: hid-sensor-als: Avoid failure for chromaticity support Srinivas Pandruvada
2023-12-15 16:20 ` Thomas Weißschuh
2023-12-17 14:35 ` Jonathan Cameron
2023-12-17 16:14 ` srinivas pandruvada
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).