* [PATCH] iio: hid-sensor-als: Don't stop probing at non-supported attribute
@ 2023-12-16 11:42 Yauhen Kharuzhy
2023-12-16 12:45 ` Yauhen Kharuzhy
2023-12-17 14:40 ` Jonathan Cameron
0 siblings, 2 replies; 3+ messages in thread
From: Yauhen Kharuzhy @ 2023-12-16 11:42 UTC (permalink / raw)
To: linux-input, linux-iio
Cc: Srinivas Pandruvada, Jonathan Cameron, linux-kernel, Jiri Kosina,
Basavaraj Natikar, Yauhen Kharuzhy
Some ambient light sensors don't support color temperature and
chromaticity attributes. The driver stops probing if it finds this.
To support sensors without of color temperature and chromaticity
attributes, just skip them at probing if they weren't found.
Tested at Lenovo Yogabook YB1-X91L tablet.
Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com>
---
drivers/iio/light/hid-sensor-als.c | 39 ++++++++++++++++++------------
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index f17304b54468..b711bac3bb2b 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -314,8 +314,11 @@ static int als_parse_report(struct platform_device *pdev,
usage_id,
HID_USAGE_SENSOR_LIGHT_ILLUM,
&st->als[i]);
- if (ret < 0)
+ if (ret < 0) {
+ dev_err(&pdev->dev,
+ "Failed to setup Illuminance attribute\n");
return ret;
+ }
als_adjust_channel_bit_mask(channels, i, st->als[i].size);
dev_dbg(&pdev->dev, "als %x:%x\n", st->als[i].index,
@@ -326,14 +329,16 @@ 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;
- als_adjust_channel_bit_mask(channels, CHANNEL_SCAN_INDEX_COLOR_TEMP,
- st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].size);
+ if (!ret) {
+ dev_info(&pdev->dev, "Color temperature is supported\n");
+ als_adjust_channel_bit_mask(channels,
+ CHANNEL_SCAN_INDEX_COLOR_TEMP,
+ st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].size);
- dev_dbg(&pdev->dev, "als %x:%x\n",
- st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].index,
- st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].report_id);
+ dev_dbg(&pdev->dev, "als %x:%x\n",
+ st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].index,
+ st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].report_id);
+ }
for (i = 0; i < 2; i++) {
int next_scan_index = CHANNEL_SCAN_INDEX_CHROMATICITY_X + i;
@@ -342,23 +347,25 @@ static int als_parse_report(struct platform_device *pdev,
HID_INPUT_REPORT, usage_id,
HID_USAGE_SENSOR_LIGHT_CHROMATICITY_X + i,
&st->als[next_scan_index]);
- if (ret < 0)
- return ret;
-
- als_adjust_channel_bit_mask(channels,
+ if (!ret) {
+ dev_info(&pdev->dev,
+ "Light chromaticity %c is supported\n",
+ i ? 'Y' : 'X');
+ als_adjust_channel_bit_mask(channels,
CHANNEL_SCAN_INDEX_CHROMATICITY_X + i,
st->als[next_scan_index].size);
- dev_dbg(&pdev->dev, "als %x:%x\n",
- st->als[next_scan_index].index,
- st->als[next_scan_index].report_id);
+ dev_dbg(&pdev->dev, "als %x:%x\n",
+ st->als[next_scan_index].index,
+ st->als[next_scan_index].report_id);
+ }
}
st->scale_precision = hid_sensor_format_scale(usage_id,
&st->als[CHANNEL_SCAN_INDEX_INTENSITY],
&st->scale_pre_decml, &st->scale_post_decml);
- return ret;
+ return 0;
}
/* Function to initialize the processing for usage id */
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] iio: hid-sensor-als: Don't stop probing at non-supported attribute
2023-12-16 11:42 [PATCH] iio: hid-sensor-als: Don't stop probing at non-supported attribute Yauhen Kharuzhy
@ 2023-12-16 12:45 ` Yauhen Kharuzhy
2023-12-17 14:40 ` Jonathan Cameron
1 sibling, 0 replies; 3+ messages in thread
From: Yauhen Kharuzhy @ 2023-12-16 12:45 UTC (permalink / raw)
To: linux-input, linux-iio
Cc: Srinivas Pandruvada, Jonathan Cameron, linux-kernel, Jiri Kosina,
Basavaraj Natikar
On Sat, Dec 16, 2023 at 01:42:29PM +0200, Yauhen Kharuzhy wrote:
> Some ambient light sensors don't support color temperature and
> chromaticity attributes. The driver stops probing if it finds this.
>
> To support sensors without of color temperature and chromaticity
> attributes, just skip them at probing if they weren't found.
>
> Tested at Lenovo Yogabook YB1-X91L tablet.
Hi, It seems that Srinivas Pandruvada has posted another patch fixing
the same issue. So, drop my patch in favor of his one.
>
> Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com>
> ---
> drivers/iio/light/hid-sensor-als.c | 39 ++++++++++++++++++------------
> 1 file changed, 23 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
> index f17304b54468..b711bac3bb2b 100644
> --- a/drivers/iio/light/hid-sensor-als.c
> +++ b/drivers/iio/light/hid-sensor-als.c
> @@ -314,8 +314,11 @@ static int als_parse_report(struct platform_device *pdev,
> usage_id,
> HID_USAGE_SENSOR_LIGHT_ILLUM,
> &st->als[i]);
> - if (ret < 0)
> + if (ret < 0) {
> + dev_err(&pdev->dev,
> + "Failed to setup Illuminance attribute\n");
> return ret;
> + }
> als_adjust_channel_bit_mask(channels, i, st->als[i].size);
>
> dev_dbg(&pdev->dev, "als %x:%x\n", st->als[i].index,
> @@ -326,14 +329,16 @@ 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;
> - als_adjust_channel_bit_mask(channels, CHANNEL_SCAN_INDEX_COLOR_TEMP,
> - st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].size);
> + if (!ret) {
> + dev_info(&pdev->dev, "Color temperature is supported\n");
> + als_adjust_channel_bit_mask(channels,
> + CHANNEL_SCAN_INDEX_COLOR_TEMP,
> + st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].size);
>
> - dev_dbg(&pdev->dev, "als %x:%x\n",
> - st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].index,
> - st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].report_id);
> + dev_dbg(&pdev->dev, "als %x:%x\n",
> + st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].index,
> + st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].report_id);
> + }
>
> for (i = 0; i < 2; i++) {
> int next_scan_index = CHANNEL_SCAN_INDEX_CHROMATICITY_X + i;
> @@ -342,23 +347,25 @@ static int als_parse_report(struct platform_device *pdev,
> HID_INPUT_REPORT, usage_id,
> HID_USAGE_SENSOR_LIGHT_CHROMATICITY_X + i,
> &st->als[next_scan_index]);
> - if (ret < 0)
> - return ret;
> -
> - als_adjust_channel_bit_mask(channels,
> + if (!ret) {
> + dev_info(&pdev->dev,
> + "Light chromaticity %c is supported\n",
> + i ? 'Y' : 'X');
> + als_adjust_channel_bit_mask(channels,
> CHANNEL_SCAN_INDEX_CHROMATICITY_X + i,
> st->als[next_scan_index].size);
>
> - dev_dbg(&pdev->dev, "als %x:%x\n",
> - st->als[next_scan_index].index,
> - st->als[next_scan_index].report_id);
> + dev_dbg(&pdev->dev, "als %x:%x\n",
> + st->als[next_scan_index].index,
> + st->als[next_scan_index].report_id);
> + }
> }
>
> st->scale_precision = hid_sensor_format_scale(usage_id,
> &st->als[CHANNEL_SCAN_INDEX_INTENSITY],
> &st->scale_pre_decml, &st->scale_post_decml);
>
> - return ret;
> + return 0;
> }
>
> /* Function to initialize the processing for usage id */
> --
> 2.43.0
>
--
Yauhen Kharuzhy
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] iio: hid-sensor-als: Don't stop probing at non-supported attribute
2023-12-16 11:42 [PATCH] iio: hid-sensor-als: Don't stop probing at non-supported attribute Yauhen Kharuzhy
2023-12-16 12:45 ` Yauhen Kharuzhy
@ 2023-12-17 14:40 ` Jonathan Cameron
1 sibling, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2023-12-17 14:40 UTC (permalink / raw)
To: Yauhen Kharuzhy
Cc: linux-input, linux-iio, Srinivas Pandruvada, linux-kernel,
Jiri Kosina, Basavaraj Natikar
On Sat, 16 Dec 2023 13:42:29 +0200
Yauhen Kharuzhy <jekhor@gmail.com> wrote:
> Some ambient light sensors don't support color temperature and
> chromaticity attributes. The driver stops probing if it finds this.
>
> To support sensors without of color temperature and chromaticity
> attributes, just skip them at probing if they weren't found.
>
> Tested at Lenovo Yogabook YB1-X91L tablet.
>
> Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com>
i reviewed this one as as well as Srinivas' as that had issues that need solving.
This one just seems to half paper over the problem It won't update the channels
etc but the set of channels provided to userspace are still garbage.
So better than before, but not fixing the issue fully.
Jonathan
> ---
> drivers/iio/light/hid-sensor-als.c | 39 ++++++++++++++++++------------
> 1 file changed, 23 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
> index f17304b54468..b711bac3bb2b 100644
> --- a/drivers/iio/light/hid-sensor-als.c
> +++ b/drivers/iio/light/hid-sensor-als.c
> @@ -314,8 +314,11 @@ static int als_parse_report(struct platform_device *pdev,
> usage_id,
> HID_USAGE_SENSOR_LIGHT_ILLUM,
> &st->als[i]);
> - if (ret < 0)
> + if (ret < 0) {
> + dev_err(&pdev->dev,
> + "Failed to setup Illuminance attribute\n");
> return ret;
> + }
Unrelated change. For a fix we should look to keep things minimal.
> als_adjust_channel_bit_mask(channels, i, st->als[i].size);
>
> dev_dbg(&pdev->dev, "als %x:%x\n", st->als[i].index,
> @@ -326,14 +329,16 @@ 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;
> - als_adjust_channel_bit_mask(channels, CHANNEL_SCAN_INDEX_COLOR_TEMP,
> - st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].size);
> + if (!ret) {
> + dev_info(&pdev->dev, "Color temperature is supported\n");
I'd argue we shouldn't print a message on this.
Use the availability of channels after driver is probed to figure this out if
needed.
> + als_adjust_channel_bit_mask(channels,
> + CHANNEL_SCAN_INDEX_COLOR_TEMP,
> + st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].size);
>
> - dev_dbg(&pdev->dev, "als %x:%x\n",
> - st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].index,
> - st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].report_id);
> + dev_dbg(&pdev->dev, "als %x:%x\n",
> + st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].index,
> + st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].report_id);
> + }
>
> for (i = 0; i < 2; i++) {
> int next_scan_index = CHANNEL_SCAN_INDEX_CHROMATICITY_X + i;
> @@ -342,23 +347,25 @@ static int als_parse_report(struct platform_device *pdev,
> HID_INPUT_REPORT, usage_id,
> HID_USAGE_SENSOR_LIGHT_CHROMATICITY_X + i,
> &st->als[next_scan_index]);
> - if (ret < 0)
> - return ret;
> -
> - als_adjust_channel_bit_mask(channels,
> + if (!ret) {
> + dev_info(&pdev->dev,
> + "Light chromaticity %c is supported\n",
> + i ? 'Y' : 'X');
> + als_adjust_channel_bit_mask(channels,
> CHANNEL_SCAN_INDEX_CHROMATICITY_X + i,
> st->als[next_scan_index].size);
>
> - dev_dbg(&pdev->dev, "als %x:%x\n",
> - st->als[next_scan_index].index,
> - st->als[next_scan_index].report_id);
> + dev_dbg(&pdev->dev, "als %x:%x\n",
> + st->als[next_scan_index].index,
> + st->als[next_scan_index].report_id);
> + }
> }
>
> st->scale_precision = hid_sensor_format_scale(usage_id,
> &st->als[CHANNEL_SCAN_INDEX_INTENSITY],
> &st->scale_pre_decml, &st->scale_post_decml);
>
> - return ret;
> + return 0;
> }
>
> /* Function to initialize the processing for usage id */
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-12-17 14:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-16 11:42 [PATCH] iio: hid-sensor-als: Don't stop probing at non-supported attribute Yauhen Kharuzhy
2023-12-16 12:45 ` Yauhen Kharuzhy
2023-12-17 14:40 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox