2023-12-17 20:07:25

by Srinivas Pandruvada

[permalink] [raw]
Subject: [PATCH 0/2] Fix regression in ALS

Addition of color temperature and chromaticity support breaks ALS sensor
on several platforms. Till we have a good solution, revert these commits
for 6.7 cycle.

Srinivas Pandruvada (2):
Revert "iio: hid-sensor-als: Add light chromaticity support"
Revert "iio: hid-sensor-als: Add light color temperature support"

drivers/iio/light/hid-sensor-als.c | 100 +----------------------------
include/linux/hid-sensor-ids.h | 4 --
2 files changed, 2 insertions(+), 102 deletions(-)

--
2.43.0



2023-12-17 20:07:40

by Srinivas Pandruvada

[permalink] [raw]
Subject: [PATCH 1/2] Revert "iio: hid-sensor-als: Add light chromaticity support"

This reverts commit ee3710f39f9d0ae5137a866138d005fe1ad18132.

This commit assumes that every HID descriptor for ALS sensor has
presence of usage id ID HID_USAGE_SENSOR_LIGHT_CHROMATICITY_X and
HID_USAGE_SENSOR_LIGHT_CHROMATICITY_Y. When the above usage ids are
absent, driver probe fails. This breaks ALS sensor functionality on
many platforms.

Till we have a good solution, revert this commit.

Reported-by: Thomas Weißschuh <[email protected]>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218223
Signed-off-by: Srinivas Pandruvada <[email protected]>
Cc: [email protected]
---
drivers/iio/light/hid-sensor-als.c | 63 ------------------------------
include/linux/hid-sensor-ids.h | 3 --
2 files changed, 66 deletions(-)

diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index f17304b54468..d44b3f30ae4a 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -17,8 +17,6 @@ enum {
CHANNEL_SCAN_INDEX_INTENSITY,
CHANNEL_SCAN_INDEX_ILLUM,
CHANNEL_SCAN_INDEX_COLOR_TEMP,
- CHANNEL_SCAN_INDEX_CHROMATICITY_X,
- CHANNEL_SCAN_INDEX_CHROMATICITY_Y,
CHANNEL_SCAN_INDEX_MAX
};

@@ -78,30 +76,6 @@ static const struct iio_chan_spec als_channels[] = {
BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
.scan_index = CHANNEL_SCAN_INDEX_COLOR_TEMP,
},
- {
- .type = IIO_CHROMATICITY,
- .modified = 1,
- .channel2 = IIO_MOD_X,
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
- .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
- BIT(IIO_CHAN_INFO_SCALE) |
- BIT(IIO_CHAN_INFO_SAMP_FREQ) |
- BIT(IIO_CHAN_INFO_HYSTERESIS) |
- BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
- .scan_index = CHANNEL_SCAN_INDEX_CHROMATICITY_X,
- },
- {
- .type = IIO_CHROMATICITY,
- .modified = 1,
- .channel2 = IIO_MOD_Y,
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
- .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
- BIT(IIO_CHAN_INFO_SCALE) |
- BIT(IIO_CHAN_INFO_SAMP_FREQ) |
- BIT(IIO_CHAN_INFO_HYSTERESIS) |
- BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
- .scan_index = CHANNEL_SCAN_INDEX_CHROMATICITY_Y,
- },
IIO_CHAN_SOFT_TIMESTAMP(CHANNEL_SCAN_INDEX_TIMESTAMP)
};

@@ -145,16 +119,6 @@ static int als_read_raw(struct iio_dev *indio_dev,
min = als_state->als[chan->scan_index].logical_minimum;
address = HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE;
break;
- case CHANNEL_SCAN_INDEX_CHROMATICITY_X:
- report_id = als_state->als[chan->scan_index].report_id;
- min = als_state->als[chan->scan_index].logical_minimum;
- address = HID_USAGE_SENSOR_LIGHT_CHROMATICITY_X;
- break;
- case CHANNEL_SCAN_INDEX_CHROMATICITY_Y:
- report_id = als_state->als[chan->scan_index].report_id;
- min = als_state->als[chan->scan_index].logical_minimum;
- address = HID_USAGE_SENSOR_LIGHT_CHROMATICITY_Y;
- break;
default:
report_id = -1;
break;
@@ -279,14 +243,6 @@ static int als_capture_sample(struct hid_sensor_hub_device *hsdev,
als_state->scan.illum[CHANNEL_SCAN_INDEX_COLOR_TEMP] = sample_data;
ret = 0;
break;
- case HID_USAGE_SENSOR_LIGHT_CHROMATICITY_X:
- als_state->scan.illum[CHANNEL_SCAN_INDEX_CHROMATICITY_X] = sample_data;
- ret = 0;
- break;
- case HID_USAGE_SENSOR_LIGHT_CHROMATICITY_Y:
- als_state->scan.illum[CHANNEL_SCAN_INDEX_CHROMATICITY_Y] = sample_data;
- ret = 0;
- break;
case HID_USAGE_SENSOR_TIME_TIMESTAMP:
als_state->timestamp = hid_sensor_convert_timestamp(&als_state->common_attributes,
*(s64 *)raw_data);
@@ -335,25 +291,6 @@ static int als_parse_report(struct platform_device *pdev,
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;
-
- ret = sensor_hub_input_get_attribute_info(hsdev,
- 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,
- 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);
- }
-
st->scale_precision = hid_sensor_format_scale(usage_id,
&st->als[CHANNEL_SCAN_INDEX_INTENSITY],
&st->scale_pre_decml, &st->scale_post_decml);
diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-sensor-ids.h
index 6730ee900ee1..8af4fb3e0254 100644
--- a/include/linux/hid-sensor-ids.h
+++ b/include/linux/hid-sensor-ids.h
@@ -22,9 +22,6 @@
#define HID_USAGE_SENSOR_DATA_LIGHT 0x2004d0
#define HID_USAGE_SENSOR_LIGHT_ILLUM 0x2004d1
#define HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE 0x2004d2
-#define HID_USAGE_SENSOR_LIGHT_CHROMATICITY 0x2004d3
-#define HID_USAGE_SENSOR_LIGHT_CHROMATICITY_X 0x2004d4
-#define HID_USAGE_SENSOR_LIGHT_CHROMATICITY_Y 0x2004d5

/* PROX (200011) */
#define HID_USAGE_SENSOR_PROX 0x200011
--
2.43.0


2023-12-17 20:07:46

by Srinivas Pandruvada

[permalink] [raw]
Subject: [PATCH 2/2] Revert "iio: hid-sensor-als: Add light color temperature support"

This reverts commit 5f05285df691b1e82108eead7165feae238c95ef.

This commit assumes that every HID descriptor for ALS sensor has
presence of usage id ID HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE.
When the above usage id is absent, driver probe fails. This breaks
ALS sensor functionality on many platforms.

Till we have a good solution, revert this commit.

Reported-by: Thomas Weißschuh <[email protected]>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218223
Signed-off-by: Srinivas Pandruvada <[email protected]>
Cc: [email protected]
---
drivers/iio/light/hid-sensor-als.c | 37 ++----------------------------
include/linux/hid-sensor-ids.h | 1 -
2 files changed, 2 insertions(+), 36 deletions(-)

diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index d44b3f30ae4a..5cd27f04b45e 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -14,9 +14,8 @@
#include "../common/hid-sensors/hid-sensor-trigger.h"

enum {
- CHANNEL_SCAN_INDEX_INTENSITY,
- CHANNEL_SCAN_INDEX_ILLUM,
- CHANNEL_SCAN_INDEX_COLOR_TEMP,
+ CHANNEL_SCAN_INDEX_INTENSITY = 0,
+ CHANNEL_SCAN_INDEX_ILLUM = 1,
CHANNEL_SCAN_INDEX_MAX
};

@@ -66,16 +65,6 @@ static const struct iio_chan_spec als_channels[] = {
BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
.scan_index = CHANNEL_SCAN_INDEX_ILLUM,
},
- {
- .type = IIO_COLORTEMP,
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
- .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
- BIT(IIO_CHAN_INFO_SCALE) |
- BIT(IIO_CHAN_INFO_SAMP_FREQ) |
- BIT(IIO_CHAN_INFO_HYSTERESIS) |
- BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
- .scan_index = CHANNEL_SCAN_INDEX_COLOR_TEMP,
- },
IIO_CHAN_SOFT_TIMESTAMP(CHANNEL_SCAN_INDEX_TIMESTAMP)
};

@@ -114,11 +103,6 @@ static int als_read_raw(struct iio_dev *indio_dev,
min = als_state->als[chan->scan_index].logical_minimum;
address = HID_USAGE_SENSOR_LIGHT_ILLUM;
break;
- case CHANNEL_SCAN_INDEX_COLOR_TEMP:
- report_id = als_state->als[chan->scan_index].report_id;
- min = als_state->als[chan->scan_index].logical_minimum;
- address = HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE;
- break;
default:
report_id = -1;
break;
@@ -239,10 +223,6 @@ static int als_capture_sample(struct hid_sensor_hub_device *hsdev,
als_state->scan.illum[CHANNEL_SCAN_INDEX_ILLUM] = sample_data;
ret = 0;
break;
- case HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE:
- als_state->scan.illum[CHANNEL_SCAN_INDEX_COLOR_TEMP] = sample_data;
- ret = 0;
- break;
case HID_USAGE_SENSOR_TIME_TIMESTAMP:
als_state->timestamp = hid_sensor_convert_timestamp(&als_state->common_attributes,
*(s64 *)raw_data);
@@ -278,19 +258,6 @@ static int als_parse_report(struct platform_device *pdev,
st->als[i].report_id);
}

- ret = sensor_hub_input_get_attribute_info(hsdev, HID_INPUT_REPORT,
- 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);
-
- 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);
-
st->scale_precision = hid_sensor_format_scale(usage_id,
&st->als[CHANNEL_SCAN_INDEX_INTENSITY],
&st->scale_pre_decml, &st->scale_post_decml);
diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-sensor-ids.h
index 8af4fb3e0254..13b1e65fbdcc 100644
--- a/include/linux/hid-sensor-ids.h
+++ b/include/linux/hid-sensor-ids.h
@@ -21,7 +21,6 @@
#define HID_USAGE_SENSOR_ALS 0x200041
#define HID_USAGE_SENSOR_DATA_LIGHT 0x2004d0
#define HID_USAGE_SENSOR_LIGHT_ILLUM 0x2004d1
-#define HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE 0x2004d2

/* PROX (200011) */
#define HID_USAGE_SENSOR_PROX 0x200011
--
2.43.0


2023-12-18 10:13:29

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 0/2] Fix regression in ALS

On Sun, 17 Dec 2023 12:07:01 -0800
Srinivas Pandruvada <[email protected]> wrote:

> Addition of color temperature and chromaticity support breaks ALS sensor
> on several platforms. Till we have a good solution, revert these commits
> for 6.7 cycle.
>
> Srinivas Pandruvada (2):
> Revert "iio: hid-sensor-als: Add light chromaticity support"
> Revert "iio: hid-sensor-als: Add light color temperature support"
>
> drivers/iio/light/hid-sensor-als.c | 100 +----------------------------
> include/linux/hid-sensor-ids.h | 4 --
> 2 files changed, 2 insertions(+), 102 deletions(-)

+CC Greg KH.

Hi Greg,

This is a regression fix that I'd like to get in asap. Currently light sensors
on a wide range of laptops are broken. I was hoping we'd fix the the problem
rather than need to revert, but time is running out so revert it is.

I don't have anything else that needs to be rushed in before the merge cycle,
so if you are happy to pick these two reverts directly that would be great.

Message ID of the cover letter is

[email protected]

Acked-by: Jonathan Cameron <[email protected]>

If not I should be able to do a pull request in next couple of days
with these in.

Thanks,

Jonathan


>


2023-12-18 16:28:41

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 0/2] Fix regression in ALS

On Sun, 17 Dec 2023 12:07:01 -0800
Srinivas Pandruvada <[email protected]> wrote:

> Addition of color temperature and chromaticity support breaks ALS sensor
> on several platforms. Till we have a good solution, revert these commits
> for 6.7 cycle.
>
> Srinivas Pandruvada (2):
> Revert "iio: hid-sensor-als: Add light chromaticity support"
> Revert "iio: hid-sensor-als: Add light color temperature support"
>
> drivers/iio/light/hid-sensor-als.c | 100 +----------------------------
> include/linux/hid-sensor-ids.h | 4 --
> 2 files changed, 2 insertions(+), 102 deletions(-)

+CC Greg KH. (resent as I messed up Greg's address first time around)

Hi Greg,

This is a regression fix that I'd like to get in asap. Currently light sensors
on a wide range of laptops are broken. I was hoping we'd fix the the problem
rather than need to revert, but time is running out so revert it is.

I don't have anything else that needs to be rushed in before the merge cycle,
so if you are happy to pick these two reverts directly that would be great.

Message ID of the cover letter is

[email protected]

Acked-by: Jonathan Cameron <[email protected]>

If not I should be able to do a pull request in next couple of days
with these in.

Thanks,

Jonathan


>


2023-12-19 07:08:55

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 0/2] Fix regression in ALS

On Mon, Dec 18, 2023 at 04:22:27PM +0000, Jonathan Cameron wrote:
> On Sun, 17 Dec 2023 12:07:01 -0800
> Srinivas Pandruvada <[email protected]> wrote:
>
> > Addition of color temperature and chromaticity support breaks ALS sensor
> > on several platforms. Till we have a good solution, revert these commits
> > for 6.7 cycle.
> >
> > Srinivas Pandruvada (2):
> > Revert "iio: hid-sensor-als: Add light chromaticity support"
> > Revert "iio: hid-sensor-als: Add light color temperature support"
> >
> > drivers/iio/light/hid-sensor-als.c | 100 +----------------------------
> > include/linux/hid-sensor-ids.h | 4 --
> > 2 files changed, 2 insertions(+), 102 deletions(-)
>
> +CC Greg KH. (resent as I messed up Greg's address first time around)
>
> Hi Greg,
>
> This is a regression fix that I'd like to get in asap. Currently light sensors
> on a wide range of laptops are broken. I was hoping we'd fix the the problem
> rather than need to revert, but time is running out so revert it is.
>
> I don't have anything else that needs to be rushed in before the merge cycle,
> so if you are happy to pick these two reverts directly that would be great.
>
> Message ID of the cover letter is
>
> [email protected]
>
> Acked-by: Jonathan Cameron <[email protected]>
>
> If not I should be able to do a pull request in next couple of days
> with these in.

Now queued up, thanks.

greg k-h