[PATCH 0/8] Multiple light sensor support

[PATCH 0/8] Multiple light sensor support

[Basavaraj Natikar]

This series adds support for light color temperature and chromaticity.

Basavaraj Natikar (8):
  iio: hid-sensor-als: Use channel index to support more hub attributes
  iio: hid-sensor-als: Add light color temperature support
  HID: amd_sfh: Add support for light color temperature
  HID: amd_sfh: Add support for SFH1.1 light color temperature
  iio: Add channel for chromaticity
  iio: hid-sensor-als: Add light chromaticity support
  HID: amd_sfh: Add light chromaticity support
  HID: amd_sfh: Add light chromaticity for SFH1.1

 Documentation/ABI/testing/sysfs-bus-iio       |   8 ++
 .../hid_descriptor/amd_sfh_hid_desc.c         |   7 +
 .../hid_descriptor/amd_sfh_hid_desc.h         |   3 +
 .../hid_descriptor/amd_sfh_hid_report_desc.h  |  21 +++
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c |   9 ++
 .../amd-sfh-hid/sfh1_1/amd_sfh_interface.h    |  15 +++
 drivers/iio/industrialio-core.c               |   1 +
 drivers/iio/light/hid-sensor-als.c            | 124 +++++++++++++++---
 include/linux/hid-sensor-ids.h                |   4 +
 include/uapi/linux/iio/types.h                |   1 +
 tools/iio/iio_event_monitor.c                 |   2 +
 11 files changed, 180 insertions(+), 15 deletions(-)

-- 
2.25.1

[PATCH 1/8] iio: hid-sensor-als: Use channel index to support more hub attributes

[Basavaraj Natikar]

Sensor hub attributes can be extended to support more channels. So in
order to support more sensor hub attributes for ALS use channel index to
get specific sensor hub attributes.

Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
 drivers/iio/light/hid-sensor-als.c | 38 +++++++++++++++---------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index eb1aedad7edc..48879e233aec 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -24,7 +24,7 @@ enum {
 struct als_state {
 	struct hid_sensor_hub_callbacks callbacks;
 	struct hid_sensor_common common_attributes;
-	struct hid_sensor_hub_attribute_info als_illum;
+	struct hid_sensor_hub_attribute_info als_illum[CHANNEL_SCAN_INDEX_MAX];
 	struct {
 		u32 illum[CHANNEL_SCAN_INDEX_MAX];
 		u64 timestamp __aligned(8);
@@ -99,8 +99,8 @@ static int als_read_raw(struct iio_dev *indio_dev,
 		switch (chan->scan_index) {
 		case  CHANNEL_SCAN_INDEX_INTENSITY:
 		case  CHANNEL_SCAN_INDEX_ILLUM:
-			report_id = als_state->als_illum.report_id;
-			min = als_state->als_illum.logical_minimum;
+			report_id = als_state->als_illum[chan->scan_index].report_id;
+			min = als_state->als_illum[chan->scan_index].logical_minimum;
 			address = HID_USAGE_SENSOR_LIGHT_ILLUM;
 			break;
 		default:
@@ -242,23 +242,23 @@ static int als_parse_report(struct platform_device *pdev,
 				struct als_state *st)
 {
 	int ret;
+	int i;
+
+	for (i = 0; i <= CHANNEL_SCAN_INDEX_ILLUM; ++i) {
+		ret = sensor_hub_input_get_attribute_info(hsdev, HID_INPUT_REPORT, usage_id,
+							  HID_USAGE_SENSOR_LIGHT_ILLUM,
+							  &st->als_illum[i]);
+		if (ret < 0)
+			return ret;
+		als_adjust_channel_bit_mask(channels, i, st->als_illum[i].size);
+
+		dev_dbg(&pdev->dev, "als %x:%x\n", st->als_illum[i].index,
+			st->als_illum[i].report_id);
+	}
 
-	ret = sensor_hub_input_get_attribute_info(hsdev, HID_INPUT_REPORT,
-			usage_id,
-			HID_USAGE_SENSOR_LIGHT_ILLUM,
-			&st->als_illum);
-	if (ret < 0)
-		return ret;
-	als_adjust_channel_bit_mask(channels, CHANNEL_SCAN_INDEX_INTENSITY,
-				    st->als_illum.size);
-	als_adjust_channel_bit_mask(channels, CHANNEL_SCAN_INDEX_ILLUM,
-					st->als_illum.size);
-
-	dev_dbg(&pdev->dev, "als %x:%x\n", st->als_illum.index,
-			st->als_illum.report_id);
-
-	st->scale_precision = hid_sensor_format_scale(usage_id, &st->als_illum,
-				&st->scale_pre_decml, &st->scale_post_decml);
+	st->scale_precision = hid_sensor_format_scale(usage_id,
+						      &st->als_illum[CHANNEL_SCAN_INDEX_INTENSITY],
+						      &st->scale_pre_decml, &st->scale_post_decml);
 
 	return ret;
 }
-- 
2.25.1

[PATCH 2/8] iio: hid-sensor-als: Add light color temperature support

[Basavaraj Natikar]

In most cases, ambient color sensors also support light color temperature.
As a result, add support of light color temperature.

Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
 drivers/iio/light/hid-sensor-als.c | 33 ++++++++++++++++++++++++++++++
 include/linux/hid-sensor-ids.h     |  1 +
 2 files changed, 34 insertions(+)

diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index 48879e233aec..220fb93fea6d 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -16,6 +16,7 @@
 enum {
 	CHANNEL_SCAN_INDEX_INTENSITY = 0,
 	CHANNEL_SCAN_INDEX_ILLUM = 1,
+	CHANNEL_SCAN_INDEX_COLOR_TEMP,
 	CHANNEL_SCAN_INDEX_MAX
 };
 
@@ -65,6 +66,18 @@ static const struct iio_chan_spec als_channels[] = {
 		BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
 		.scan_index = CHANNEL_SCAN_INDEX_ILLUM,
 	},
+	{
+		.type = IIO_TEMP,
+		.modified = 1,
+		.channel2 = IIO_MOD_TEMP_AMBIENT,
+		.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)
 };
 
@@ -103,6 +116,11 @@ static int als_read_raw(struct iio_dev *indio_dev,
 			min = als_state->als_illum[chan->scan_index].logical_minimum;
 			address = HID_USAGE_SENSOR_LIGHT_ILLUM;
 			break;
+		case  CHANNEL_SCAN_INDEX_COLOR_TEMP:
+			report_id = als_state->als_illum[chan->scan_index].report_id;
+			min = als_state->als_illum[chan->scan_index].logical_minimum;
+			address = HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE;
+			break;
 		default:
 			report_id = -1;
 			break;
@@ -223,6 +241,10 @@ 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);
@@ -256,6 +278,17 @@ static int als_parse_report(struct platform_device *pdev,
 			st->als_illum[i].report_id);
 	}
 
+	ret = sensor_hub_input_get_attribute_info(hsdev, HID_INPUT_REPORT, usage_id,
+						  HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE,
+						  &st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP]);
+	if (ret < 0)
+		return ret;
+	als_adjust_channel_bit_mask(channels, CHANNEL_SCAN_INDEX_COLOR_TEMP,
+				    st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP].size);
+
+	dev_dbg(&pdev->dev, "als %x:%x\n", st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP].index,
+		st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP].report_id);
+
 	st->scale_precision = hid_sensor_format_scale(usage_id,
 						      &st->als_illum[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 13b1e65fbdcc..8af4fb3e0254 100644
--- a/include/linux/hid-sensor-ids.h
+++ b/include/linux/hid-sensor-ids.h
@@ -21,6 +21,7 @@
 #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.25.1

[PATCH 3/8] HID: amd_sfh: Add support for light color temperature

[Basavaraj Natikar]

In most cases, ambient color sensors also support light color temperature.
As a result, add support of light color temperature.

Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
 drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c  | 4 ++++
 drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h  | 1 +
 .../amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h   | 7 +++++++
 3 files changed, 12 insertions(+)

diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c
index 8716a05950c8..b7e732ec4806 100644
--- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c
+++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c
@@ -257,6 +257,10 @@ static u8 get_input_report(u8 current_index, int sensor_idx, int report_id,
 		else
 			als_input.illuminance_value =
 				(int)sensor_virt_addr[0] / AMD_SFH_FW_MULTIPLIER;
+
+		if (sensor_idx == ACS_IDX)
+			als_input.light_color_temp = sensor_virt_addr[1];
+
 		report_size = sizeof(als_input);
 		memcpy(input_report, &als_input, sizeof(als_input));
 		break;
diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h
index ebd55675eb62..a7fc50deca4d 100644
--- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h
+++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h
@@ -99,6 +99,7 @@ struct als_input_report {
 	struct common_input_property common_property;
 	/* values specific to this sensor */
 	int illuminance_value;
+	int light_color_temp;
 } __packed;
 
 struct hpd_feature_report {
diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h
index 697f2791ea9c..26e994e54ded 100644
--- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h
+++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h
@@ -641,6 +641,13 @@ static const u8 als_report_descriptor[] = {
 0x75, 32,		/* HID report size(32) */
 0x95, 1,		/* HID report count (1) */
 0X81, 0x02,		/* HID Input (Data_Arr_Abs) */
+0x0A, 0xD2, 0x04,	/* HID usage sensor data light temperature */
+0x17, 0x00, 0x00, 0x01, 0x80,	 /* HID logical Min_32 */
+0x27, 0xFF, 0xFF, 0xFF, 0x7F,	 /* HID logical Max_32 */
+0x55, 0x0,		/* HID unit exponent(0x0) */
+0x75, 32,		/* HID report size(32) */
+0x95, 1,		/* HID report count (1) */
+0X81, 0x02,		/* HID Input (Data_Arr_Abs) */
 0xC0			/* HID end collection */
 };
 
-- 
2.25.1

[PATCH 4/8] HID: amd_sfh: Add support for SFH1.1 light color temperature

[Basavaraj Natikar]

In most cases, ambient color sensors also support light color temperature.
As a result, add support of light color temperature for SFH1.1.

Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c      |  6 ++++++
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h | 13 +++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
index 06bdcf072d10..f100aaafa167 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
@@ -188,6 +188,7 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
 	struct sfh_mag_data mag_data;
 	struct sfh_als_data als_data;
 	struct hpd_status hpdstatus;
+	struct sfh_base_info binfo;
 	void __iomem *sensoraddr;
 	u8 report_size = 0;
 
@@ -235,6 +236,11 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
 		memcpy_fromio(&als_data, sensoraddr, sizeof(struct sfh_als_data));
 		get_common_inputs(&als_input.common_property, report_id);
 		als_input.illuminance_value = float_to_int(als_data.lux);
+
+		memcpy_fromio(&binfo, mp2->vsbase, sizeof(struct sfh_base_info));
+		if (binfo.sbase.s_prop[ALS_IDX].sf.feat & 0x2)
+			als_input.light_color_temp = als_data.light_color_temp;
+
 		report_size = sizeof(als_input);
 		memcpy(input_report, &als_input, sizeof(als_input));
 		break;
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
index 9d31d5b510eb..6f6f5db150c3 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
@@ -88,6 +88,16 @@ struct sfh_sensor_list {
 	};
 };
 
+struct sfh_sensor_prop {
+	union {
+		u32 sprop;
+		struct {
+			u32 elist	: 16;
+			u32 feat	: 16;
+		} sf;
+	};
+};
+
 struct sfh_base_info {
 	union {
 		u32 sfh_base[24];
@@ -95,6 +105,8 @@ struct sfh_base_info {
 			struct sfh_platform_info plat_info;
 			struct sfh_firmware_info  fw_info;
 			struct sfh_sensor_list s_list;
+			u32 rsvd;
+			struct sfh_sensor_prop s_prop[16];
 		} sbase;
 	};
 };
@@ -134,6 +146,7 @@ struct sfh_mag_data {
 struct sfh_als_data {
 	struct sfh_common_data commondata;
 	u32 lux;
+	u32 light_color_temp;
 };
 
 struct hpd_status {
-- 
2.25.1

[PATCH 5/8] iio: Add channel for chromaticity

[Basavaraj Natikar]

In most cases, ambient color sensors also support the x and y light
colors, which represent the coordinates on the CIE 1931 chromaticity
diagram. Thus, add channel for chromaticity.

Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
 Documentation/ABI/testing/sysfs-bus-iio | 8 ++++++++
 drivers/iio/industrialio-core.c         | 1 +
 include/uapi/linux/iio/types.h          | 1 +
 tools/iio/iio_event_monitor.c           | 2 ++
 4 files changed, 12 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index a2854dc9a839..6a810771f5e4 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -2179,3 +2179,11 @@ Contact:	linux-iio@vger.kernel.org
 Description:
 		Number of conditions that must occur, during a running
 		period, before an event is generated.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_chromaticity_x_raw
+What:		/sys/bus/iio/devices/iio:deviceX/in_chromaticity_y_raw
+KernelVersion:	6.6
+Contact:	linux-iio@vger.kernel.org
+Description:
+		The x and y light color coordinate on the CIE 1931 chromaticity
+		diagram.
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index d752e9c0499b..c1df66cdfdf6 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -90,6 +90,7 @@ static const char * const iio_chan_type_name_spec[] = {
 	[IIO_POSITIONRELATIVE]  = "positionrelative",
 	[IIO_PHASE] = "phase",
 	[IIO_MASSCONCENTRATION] = "massconcentration",
+	[IIO_CHROMATICITY] = "chromaticity",
 };
 
 static const char * const iio_modifier_names[] = {
diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
index c79f2f046a0b..8952d48cfc64 100644
--- a/include/uapi/linux/iio/types.h
+++ b/include/uapi/linux/iio/types.h
@@ -47,6 +47,7 @@ enum iio_chan_type {
 	IIO_POSITIONRELATIVE,
 	IIO_PHASE,
 	IIO_MASSCONCENTRATION,
+	IIO_CHROMATICITY,
 };
 
 enum iio_modifier {
diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c
index 0a5c2bb60030..115ba1fbf3ac 100644
--- a/tools/iio/iio_event_monitor.c
+++ b/tools/iio/iio_event_monitor.c
@@ -59,6 +59,7 @@ static const char * const iio_chan_type_name_spec[] = {
 	[IIO_POSITIONRELATIVE] = "positionrelative",
 	[IIO_PHASE] = "phase",
 	[IIO_MASSCONCENTRATION] = "massconcentration",
+	[IIO_CHROMATICITY] = "chromaticity",
 };
 
 static const char * const iio_ev_type_text[] = {
@@ -173,6 +174,7 @@ static bool event_is_known(struct iio_event_data *event)
 	case IIO_POSITIONRELATIVE:
 	case IIO_PHASE:
 	case IIO_MASSCONCENTRATION:
+	case IIO_CHROMATICITY:
 		break;
 	default:
 		return false;
-- 
2.25.1

[PATCH 6/8] iio: hid-sensor-als: Add light chromaticity support

[Basavaraj Natikar]

In most cases, ambient color sensors also support the x and y light
colors, which represent the coordinates on the CIE 1931 chromaticity
diagram. Thus, add light chromaticity x and y.

Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
 drivers/iio/light/hid-sensor-als.c | 61 ++++++++++++++++++++++++++++++
 include/linux/hid-sensor-ids.h     |  3 ++
 2 files changed, 64 insertions(+)

diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index 220fb93fea6d..e1a2cc83b892 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -17,6 +17,8 @@ enum {
 	CHANNEL_SCAN_INDEX_INTENSITY = 0,
 	CHANNEL_SCAN_INDEX_ILLUM = 1,
 	CHANNEL_SCAN_INDEX_COLOR_TEMP,
+	CHANNEL_SCAN_INDEX_CHROMATICITY_X,
+	CHANNEL_SCAN_INDEX_CHROMATICITY_Y,
 	CHANNEL_SCAN_INDEX_MAX
 };
 
@@ -78,6 +80,30 @@ 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)
 };
 
@@ -121,6 +147,16 @@ static int als_read_raw(struct iio_dev *indio_dev,
 			min = als_state->als_illum[chan->scan_index].logical_minimum;
 			address = HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE;
 			break;
+		case  CHANNEL_SCAN_INDEX_CHROMATICITY_X:
+			report_id = als_state->als_illum[chan->scan_index].report_id;
+			min = als_state->als_illum[chan->scan_index].logical_minimum;
+			address = HID_USAGE_SENSOR_LIGHT_CHROMATICITY_X;
+			break;
+		case  CHANNEL_SCAN_INDEX_CHROMATICITY_Y:
+			report_id = als_state->als_illum[chan->scan_index].report_id;
+			min = als_state->als_illum[chan->scan_index].logical_minimum;
+			address = HID_USAGE_SENSOR_LIGHT_CHROMATICITY_Y;
+			break;
 		default:
 			report_id = -1;
 			break;
@@ -245,6 +281,14 @@ 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);
@@ -289,6 +333,23 @@ static int als_parse_report(struct platform_device *pdev,
 	dev_dbg(&pdev->dev, "als %x:%x\n", st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP].index,
 		st->als_illum[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_illum[next_scan_index]);
+		if (ret < 0)
+			return ret;
+
+		als_adjust_channel_bit_mask(channels, CHANNEL_SCAN_INDEX_CHROMATICITY_X + i,
+					    st->als_illum[next_scan_index].size);
+
+		dev_dbg(&pdev->dev, "als %x:%x\n",
+			st->als_illum[next_scan_index].index,
+			st->als_illum[next_scan_index].report_id);
+	}
+
 	st->scale_precision = hid_sensor_format_scale(usage_id,
 						      &st->als_illum[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..6730ee900ee1 100644
--- a/include/linux/hid-sensor-ids.h
+++ b/include/linux/hid-sensor-ids.h
@@ -22,6 +22,9 @@
 #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.25.1

[PATCH 7/8] HID: amd_sfh: Add light chromaticity support

[Basavaraj Natikar]

In most cases, ambient color sensors also support the x and y light
colors, which represent the coordinates on the CIE 1931 chromaticity
diagram. Thus, add light chromaticity x and y.

Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
 .../amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c  |  5 ++++-
 .../amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h  |  2 ++
 .../hid_descriptor/amd_sfh_hid_report_desc.h       | 14 ++++++++++++++
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c
index b7e732ec4806..ef1f9be8b893 100644
--- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c
+++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c
@@ -258,8 +258,11 @@ static u8 get_input_report(u8 current_index, int sensor_idx, int report_id,
 			als_input.illuminance_value =
 				(int)sensor_virt_addr[0] / AMD_SFH_FW_MULTIPLIER;
 
-		if (sensor_idx == ACS_IDX)
+		if (sensor_idx == ACS_IDX) {
 			als_input.light_color_temp = sensor_virt_addr[1];
+			als_input.chromaticity_x_value = sensor_virt_addr[2];
+			als_input.chromaticity_y_value = sensor_virt_addr[3];
+		}
 
 		report_size = sizeof(als_input);
 		memcpy(input_report, &als_input, sizeof(als_input));
diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h
index a7fc50deca4d..882434b1501f 100644
--- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h
+++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h
@@ -100,6 +100,8 @@ struct als_input_report {
 	/* values specific to this sensor */
 	int illuminance_value;
 	int light_color_temp;
+	int chromaticity_x_value;
+	int chromaticity_y_value;
 } __packed;
 
 struct hpd_feature_report {
diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h
index 26e994e54ded..67ec2d6a417d 100644
--- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h
+++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h
@@ -648,6 +648,20 @@ static const u8 als_report_descriptor[] = {
 0x75, 32,		/* HID report size(32) */
 0x95, 1,		/* HID report count (1) */
 0X81, 0x02,		/* HID Input (Data_Arr_Abs) */
+0x0A, 0xD4, 0x04,       /* HID usage sensor data light chromaticity_x */
+0x17, 0x00, 0x00, 0x01, 0x80,	 /* HID logical Min_32 */
+0x27, 0xFF, 0xFF, 0xFF, 0x7F,	 /* HID logical Max_32 */
+0x55, 0x0,		/* HID unit exponent(0x0) */
+0x75, 32,		/* HID report size(32) */
+0x95, 1,		/* HID report count(1) */
+0X81, 0x02,		/* HID Input (Data_Var_Abs) */
+0x0A, 0xD5, 0x04,       /* HID usage sensor data light chromaticity_y */
+0x17, 0x00, 0x00, 0x01, 0x80,	 /* HID logical Min_32 */
+0x27, 0xFF, 0xFF, 0xFF, 0x7F,	 /* HID logical Max_32 */
+0x55, 0x0,		/* HID unit exponent(0x0) */
+0x75, 32,		/* HID report size(32) */
+0x95, 1,		/* HID report count (1) */
+0X81, 0x02,		/* HID Input (Data_Var_Abs) */
 0xC0			/* HID end collection */
 };
 
-- 
2.25.1

[PATCH 8/8] HID: amd_sfh: Add light chromaticity for SFH1.1

[Basavaraj Natikar]

In most cases, ambient color sensors also support the x and y light
colors, which represent the coordinates on the CIE 1931 chromaticity
diagram. Thus, add light chromaticity x and y for SFH1.1.

Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c      | 5 ++++-
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h | 2 ++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
index f100aaafa167..8a037de08e92 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
@@ -238,8 +238,11 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
 		als_input.illuminance_value = float_to_int(als_data.lux);
 
 		memcpy_fromio(&binfo, mp2->vsbase, sizeof(struct sfh_base_info));
-		if (binfo.sbase.s_prop[ALS_IDX].sf.feat & 0x2)
+		if (binfo.sbase.s_prop[ALS_IDX].sf.feat & 0x2) {
 			als_input.light_color_temp = als_data.light_color_temp;
+			als_input.chromaticity_x_value = float_to_int(als_data.chromaticity_x);
+			als_input.chromaticity_y_value = float_to_int(als_data.chromaticity_y);
+		}
 
 		report_size = sizeof(als_input);
 		memcpy(input_report, &als_input, sizeof(als_input));
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
index 6f6f5db150c3..656c3e95ef8c 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
@@ -147,6 +147,8 @@ struct sfh_als_data {
 	struct sfh_common_data commondata;
 	u32 lux;
 	u32 light_color_temp;
+	u32 chromaticity_x;
+	u32 chromaticity_y;
 };
 
 struct hpd_status {
-- 
2.25.1

Re: [PATCH 1/8] iio: hid-sensor-als: Use channel index to support more hub attributes

[Jonathan Cameron]

On Fri, 15 Sep 2023 10:46:56 +0530
Basavaraj Natikar <Basavaraj.Natikar@amd.com> wrote:

> Sensor hub attributes can be extended to support more channels. So in
> order to support more sensor hub attributes for ALS use channel index to
> get specific sensor hub attributes.
> 
> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>

Hi Basavaraj,

> ---
>  drivers/iio/light/hid-sensor-als.c | 38 +++++++++++++++---------------
>  1 file changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
> index eb1aedad7edc..48879e233aec 100644
> --- a/drivers/iio/light/hid-sensor-als.c
> +++ b/drivers/iio/light/hid-sensor-als.c
> @@ -24,7 +24,7 @@ enum {
>  struct als_state {
>  	struct hid_sensor_hub_callbacks callbacks;
>  	struct hid_sensor_common common_attributes;
> -	struct hid_sensor_hub_attribute_info als_illum;
> +	struct hid_sensor_hub_attribute_info als_illum[CHANNEL_SCAN_INDEX_MAX];

If being used for other channels, probably want to rename it as no longer
als_*illum*
>  	struct {
>  		u32 illum[CHANNEL_SCAN_INDEX_MAX];
>  		u64 timestamp __aligned(8);
> @@ -99,8 +99,8 @@ static int als_read_raw(struct iio_dev *indio_dev,
>  		switch (chan->scan_index) {
>  		case  CHANNEL_SCAN_INDEX_INTENSITY:
>  		case  CHANNEL_SCAN_INDEX_ILLUM:
> -			report_id = als_state->als_illum.report_id;
> -			min = als_state->als_illum.logical_minimum;
> +			report_id = als_state->als_illum[chan->scan_index].report_id;
> +			min = als_state->als_illum[chan->scan_index].logical_minimum;
>  			address = HID_USAGE_SENSOR_LIGHT_ILLUM;
>  			break;
>  		default:
> @@ -242,23 +242,23 @@ static int als_parse_report(struct platform_device *pdev,
>  				struct als_state *st)
>  {
>  	int ret;
> +	int i;
> +
> +	for (i = 0; i <= CHANNEL_SCAN_INDEX_ILLUM; ++i) {

> +		ret = sensor_hub_input_get_attribute_info(hsdev, HID_INPUT_REPORT, usage_id,
> +							  HID_USAGE_SENSOR_LIGHT_ILLUM,
> +							  &st->als_illum[i]);

I would call out either as a comment here or in the patch description that we repeat
the same reading for the two existing channels and hence whilst we store them in
separate entries things continue to work as before where there was just one entry.

> +		if (ret < 0)
> +			return ret;
> +		als_adjust_channel_bit_mask(channels, i, st->als_illum[i].size);
> +
> +		dev_dbg(&pdev->dev, "als %x:%x\n", st->als_illum[i].index,
> +			st->als_illum[i].report_id);
> +	}
>  
> -	ret = sensor_hub_input_get_attribute_info(hsdev, HID_INPUT_REPORT,
> -			usage_id,
> -			HID_USAGE_SENSOR_LIGHT_ILLUM,
> -			&st->als_illum);
> -	if (ret < 0)
> -		return ret;
> -	als_adjust_channel_bit_mask(channels, CHANNEL_SCAN_INDEX_INTENSITY,
> -				    st->als_illum.size);
> -	als_adjust_channel_bit_mask(channels, CHANNEL_SCAN_INDEX_ILLUM,
> -					st->als_illum.size);
> -
> -	dev_dbg(&pdev->dev, "als %x:%x\n", st->als_illum.index,
> -			st->als_illum.report_id);
> -
> -	st->scale_precision = hid_sensor_format_scale(usage_id, &st->als_illum,
> -				&st->scale_pre_decml, &st->scale_post_decml);
> +	st->scale_precision = hid_sensor_format_scale(usage_id,
> +						      &st->als_illum[CHANNEL_SCAN_INDEX_INTENSITY],
> +						      &st->scale_pre_decml, &st->scale_post_decml);

Keep line lengths shorter anywhere it doesn't hurt readability.  Whilst the hard limit has
gone up we still prefer to keep under 80chars where it is easy to do.
Sometimes that means relaxing the alignment with opening bracket.

>  
>  	return ret;
>  }

Re: [PATCH 2/8] iio: hid-sensor-als: Add light color temperature support

[Jonathan Cameron]

On Fri, 15 Sep 2023 10:46:57 +0530
Basavaraj Natikar <Basavaraj.Natikar@amd.com> wrote:

> In most cases, ambient color sensors also support light color temperature.
> As a result, add support of light color temperature.
> 
> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> ---
>  drivers/iio/light/hid-sensor-als.c | 33 ++++++++++++++++++++++++++++++
>  include/linux/hid-sensor-ids.h     |  1 +
>  2 files changed, 34 insertions(+)
> 
> diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
> index 48879e233aec..220fb93fea6d 100644
> --- a/drivers/iio/light/hid-sensor-als.c
> +++ b/drivers/iio/light/hid-sensor-als.c
> @@ -16,6 +16,7 @@
>  enum {
>  	CHANNEL_SCAN_INDEX_INTENSITY = 0,
>  	CHANNEL_SCAN_INDEX_ILLUM = 1,

Either drop, the = 1 or keep consistency for TEMP. 
I don't think the = 1 is useful so I'd drop it.

> +	CHANNEL_SCAN_INDEX_COLOR_TEMP,
>  	CHANNEL_SCAN_INDEX_MAX
>  };
>  
> @@ -65,6 +66,18 @@ static const struct iio_chan_spec als_channels[] = {
>  		BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
>  		.scan_index = CHANNEL_SCAN_INDEX_ILLUM,
>  	},
> +	{
> +		.type = IIO_TEMP,

Using a temperature channel for color temp is a bit of a stretch,
Particularly as it's likely we will see light sensors with actual
air temperature sensors in them at somepoint even if we don't have
any already.

So this needs a new channel type
IIO_COLORTEMP or similar.

Units for this probably want to be kelvin unlike the mili decrees centigrade
used for IIO_TEMP.

> +		.modified = 1,
> +		.channel2 = IIO_MOD_TEMP_AMBIENT,

I don't really see the modifier as useful here. That exists for thermocouple
type systems where it is necessary to know ambient vs sample temperatures.


> +		.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)
>  };
>  
> @@ -103,6 +116,11 @@ static int als_read_raw(struct iio_dev *indio_dev,
>  			min = als_state->als_illum[chan->scan_index].logical_minimum;
>  			address = HID_USAGE_SENSOR_LIGHT_ILLUM;
>  			break;
> +		case  CHANNEL_SCAN_INDEX_COLOR_TEMP:
> +			report_id = als_state->als_illum[chan->scan_index].report_id;
> +			min = als_state->als_illum[chan->scan_index].logical_minimum;
> +			address = HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE;
> +			break;
>  		default:
>  			report_id = -1;
>  			break;
> @@ -223,6 +241,10 @@ 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);
> @@ -256,6 +278,17 @@ static int als_parse_report(struct platform_device *pdev,
>  			st->als_illum[i].report_id);
>  	}
>  
> +	ret = sensor_hub_input_get_attribute_info(hsdev, HID_INPUT_REPORT, usage_id,
> +						  HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE,
> +						  &st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP]);
> +	if (ret < 0)
> +		return ret;
> +	als_adjust_channel_bit_mask(channels, CHANNEL_SCAN_INDEX_COLOR_TEMP,
> +				    st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP].size);
> +
> +	dev_dbg(&pdev->dev, "als %x:%x\n", st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP].index,
> +		st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP].report_id);
> +
>  	st->scale_precision = hid_sensor_format_scale(usage_id,
>  						      &st->als_illum[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 13b1e65fbdcc..8af4fb3e0254 100644
> --- a/include/linux/hid-sensor-ids.h
> +++ b/include/linux/hid-sensor-ids.h
> @@ -21,6 +21,7 @@
>  #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

Re: [PATCH 5/8] iio: Add channel for chromaticity

[Jonathan Cameron]

On Fri, 15 Sep 2023 10:47:00 +0530
Basavaraj Natikar <Basavaraj.Natikar@amd.com> wrote:

> In most cases, ambient color sensors also support the x and y light
> colors, which represent the coordinates on the CIE 1931 chromaticity
> diagram. Thus, add channel for chromaticity.

Adding a channel type.

Otherwise looks reasonable to me.

> 
> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> ---
>  Documentation/ABI/testing/sysfs-bus-iio | 8 ++++++++
>  drivers/iio/industrialio-core.c         | 1 +
>  include/uapi/linux/iio/types.h          | 1 +
>  tools/iio/iio_event_monitor.c           | 2 ++
>  4 files changed, 12 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index a2854dc9a839..6a810771f5e4 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -2179,3 +2179,11 @@ Contact:	linux-iio@vger.kernel.org
>  Description:
>  		Number of conditions that must occur, during a running
>  		period, before an event is generated.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_chromaticity_x_raw
> +What:		/sys/bus/iio/devices/iio:deviceX/in_chromaticity_y_raw
> +KernelVersion:	6.6
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		The x and y light color coordinate on the CIE 1931 chromaticity
> +		diagram.
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index d752e9c0499b..c1df66cdfdf6 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -90,6 +90,7 @@ static const char * const iio_chan_type_name_spec[] = {
>  	[IIO_POSITIONRELATIVE]  = "positionrelative",
>  	[IIO_PHASE] = "phase",
>  	[IIO_MASSCONCENTRATION] = "massconcentration",
> +	[IIO_CHROMATICITY] = "chromaticity",
>  };
>  
>  static const char * const iio_modifier_names[] = {
> diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
> index c79f2f046a0b..8952d48cfc64 100644
> --- a/include/uapi/linux/iio/types.h
> +++ b/include/uapi/linux/iio/types.h
> @@ -47,6 +47,7 @@ enum iio_chan_type {
>  	IIO_POSITIONRELATIVE,
>  	IIO_PHASE,
>  	IIO_MASSCONCENTRATION,
> +	IIO_CHROMATICITY,
>  };
>  
>  enum iio_modifier {
> diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c
> index 0a5c2bb60030..115ba1fbf3ac 100644
> --- a/tools/iio/iio_event_monitor.c
> +++ b/tools/iio/iio_event_monitor.c
> @@ -59,6 +59,7 @@ static const char * const iio_chan_type_name_spec[] = {
>  	[IIO_POSITIONRELATIVE] = "positionrelative",
>  	[IIO_PHASE] = "phase",
>  	[IIO_MASSCONCENTRATION] = "massconcentration",
> +	[IIO_CHROMATICITY] = "chromaticity",
>  };
>  
>  static const char * const iio_ev_type_text[] = {
> @@ -173,6 +174,7 @@ static bool event_is_known(struct iio_event_data *event)
>  	case IIO_POSITIONRELATIVE:
>  	case IIO_PHASE:
>  	case IIO_MASSCONCENTRATION:
> +	case IIO_CHROMATICITY:
>  		break;
>  	default:
>  		return false;

Re: [PATCH 0/8] Multiple light sensor support

[Jonathan Cameron]

On Fri, 15 Sep 2023 10:46:55 +0530
Basavaraj Natikar <Basavaraj.Natikar@amd.com> wrote:

> This series adds support for light color temperature and chromaticity.

Hi Basavaraj,

I'd rename the series to make it clearer that this is about adding
colour temperature and chromaticity support rather than simply lots
of boring ambient light sensors.

Jonathan

> 
> Basavaraj Natikar (8):
>   iio: hid-sensor-als: Use channel index to support more hub attributes
>   iio: hid-sensor-als: Add light color temperature support
>   HID: amd_sfh: Add support for light color temperature
>   HID: amd_sfh: Add support for SFH1.1 light color temperature
>   iio: Add channel for chromaticity
>   iio: hid-sensor-als: Add light chromaticity support
>   HID: amd_sfh: Add light chromaticity support
>   HID: amd_sfh: Add light chromaticity for SFH1.1
> 
>  Documentation/ABI/testing/sysfs-bus-iio       |   8 ++
>  .../hid_descriptor/amd_sfh_hid_desc.c         |   7 +
>  .../hid_descriptor/amd_sfh_hid_desc.h         |   3 +
>  .../hid_descriptor/amd_sfh_hid_report_desc.h  |  21 +++
>  drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c |   9 ++
>  .../amd-sfh-hid/sfh1_1/amd_sfh_interface.h    |  15 +++
>  drivers/iio/industrialio-core.c               |   1 +
>  drivers/iio/light/hid-sensor-als.c            | 124 +++++++++++++++---
>  include/linux/hid-sensor-ids.h                |   4 +
>  include/uapi/linux/iio/types.h                |   1 +
>  tools/iio/iio_event_monitor.c                 |   2 +
>  11 files changed, 180 insertions(+), 15 deletions(-)
> 

Re: [PATCH 2/8] iio: hid-sensor-als: Add light color temperature support

[Basavaraj Natikar]

On 9/17/2023 4:30 PM, Jonathan Cameron wrote:
> On Fri, 15 Sep 2023 10:46:57 +0530
> Basavaraj Natikar <Basavaraj.Natikar@amd.com> wrote:
>
>> In most cases, ambient color sensors also support light color temperature.
>> As a result, add support of light color temperature.
>>
>> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
>> ---
>>  drivers/iio/light/hid-sensor-als.c | 33 ++++++++++++++++++++++++++++++
>>  include/linux/hid-sensor-ids.h     |  1 +
>>  2 files changed, 34 insertions(+)
>>
>> diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
>> index 48879e233aec..220fb93fea6d 100644
>> --- a/drivers/iio/light/hid-sensor-als.c
>> +++ b/drivers/iio/light/hid-sensor-als.c
>> @@ -16,6 +16,7 @@
>>  enum {
>>  	CHANNEL_SCAN_INDEX_INTENSITY = 0,
>>  	CHANNEL_SCAN_INDEX_ILLUM = 1,
> Either drop, the = 1 or keep consistency for TEMP. 
> I don't think the = 1 is useful so I'd drop it.
>
>> +	CHANNEL_SCAN_INDEX_COLOR_TEMP,
>>  	CHANNEL_SCAN_INDEX_MAX
>>  };
>>  
>> @@ -65,6 +66,18 @@ static const struct iio_chan_spec als_channels[] = {
>>  		BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
>>  		.scan_index = CHANNEL_SCAN_INDEX_ILLUM,
>>  	},
>> +	{
>> +		.type = IIO_TEMP,
> Using a temperature channel for color temp is a bit of a stretch,
> Particularly as it's likely we will see light sensors with actual
> air temperature sensors in them at somepoint even if we don't have
> any already.
>
> So this needs a new channel type
> IIO_COLORTEMP or similar.
>
> Units for this probably want to be kelvin unlike the mili decrees centigrade
> used for IIO_TEMP.
>
>> +		.modified = 1,
>> +		.channel2 = IIO_MOD_TEMP_AMBIENT,
> I don't really see the modifier as useful here. That exists for thermocouple
> type systems where it is necessary to know ambient vs sample temperatures.

Sure Jonathan, I will address all comments in this series in v2.
Also, can i add new channel type IIO_COLORTEMP with following channel index
for light color temperature ?
       {
                .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,
        }

Thanks,
--
Basavaraj

>
>
>> +		.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)
>>  };
>>  
>> @@ -103,6 +116,11 @@ static int als_read_raw(struct iio_dev *indio_dev,
>>  			min = als_state->als_illum[chan->scan_index].logical_minimum;
>>  			address = HID_USAGE_SENSOR_LIGHT_ILLUM;
>>  			break;
>> +		case  CHANNEL_SCAN_INDEX_COLOR_TEMP:
>> +			report_id = als_state->als_illum[chan->scan_index].report_id;
>> +			min = als_state->als_illum[chan->scan_index].logical_minimum;
>> +			address = HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE;
>> +			break;
>>  		default:
>>  			report_id = -1;
>>  			break;
>> @@ -223,6 +241,10 @@ 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);
>> @@ -256,6 +278,17 @@ static int als_parse_report(struct platform_device *pdev,
>>  			st->als_illum[i].report_id);
>>  	}
>>  
>> +	ret = sensor_hub_input_get_attribute_info(hsdev, HID_INPUT_REPORT, usage_id,
>> +						  HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE,
>> +						  &st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP]);
>> +	if (ret < 0)
>> +		return ret;
>> +	als_adjust_channel_bit_mask(channels, CHANNEL_SCAN_INDEX_COLOR_TEMP,
>> +				    st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP].size);
>> +
>> +	dev_dbg(&pdev->dev, "als %x:%x\n", st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP].index,
>> +		st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP].report_id);
>> +
>>  	st->scale_precision = hid_sensor_format_scale(usage_id,
>>  						      &st->als_illum[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 13b1e65fbdcc..8af4fb3e0254 100644
>> --- a/include/linux/hid-sensor-ids.h
>> +++ b/include/linux/hid-sensor-ids.h
>> @@ -21,6 +21,7 @@
>>  #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

Re: [PATCH 2/8] iio: hid-sensor-als: Add light color temperature support

[Jonathan Cameron]

On Sun, 17 Sep 2023 20:13:46 +0530
Basavaraj Natikar <bnatikar@amd.com> wrote:

> On 9/17/2023 4:30 PM, Jonathan Cameron wrote:
> > On Fri, 15 Sep 2023 10:46:57 +0530
> > Basavaraj Natikar <Basavaraj.Natikar@amd.com> wrote:
> >  
> >> In most cases, ambient color sensors also support light color temperature.
> >> As a result, add support of light color temperature.
> >>
> >> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> >> ---
> >>  drivers/iio/light/hid-sensor-als.c | 33 ++++++++++++++++++++++++++++++
> >>  include/linux/hid-sensor-ids.h     |  1 +
> >>  2 files changed, 34 insertions(+)
> >>
> >> diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
> >> index 48879e233aec..220fb93fea6d 100644
> >> --- a/drivers/iio/light/hid-sensor-als.c
> >> +++ b/drivers/iio/light/hid-sensor-als.c
> >> @@ -16,6 +16,7 @@
> >>  enum {
> >>  	CHANNEL_SCAN_INDEX_INTENSITY = 0,
> >>  	CHANNEL_SCAN_INDEX_ILLUM = 1,  
> > Either drop, the = 1 or keep consistency for TEMP. 
> > I don't think the = 1 is useful so I'd drop it.
> >  
> >> +	CHANNEL_SCAN_INDEX_COLOR_TEMP,
> >>  	CHANNEL_SCAN_INDEX_MAX
> >>  };
> >>  
> >> @@ -65,6 +66,18 @@ static const struct iio_chan_spec als_channels[] = {
> >>  		BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
> >>  		.scan_index = CHANNEL_SCAN_INDEX_ILLUM,
> >>  	},
> >> +	{
> >> +		.type = IIO_TEMP,  
> > Using a temperature channel for color temp is a bit of a stretch,
> > Particularly as it's likely we will see light sensors with actual
> > air temperature sensors in them at somepoint even if we don't have
> > any already.
> >
> > So this needs a new channel type
> > IIO_COLORTEMP or similar.
> >
> > Units for this probably want to be kelvin unlike the mili decrees centigrade
> > used for IIO_TEMP.
> >  
> >> +		.modified = 1,
> >> +		.channel2 = IIO_MOD_TEMP_AMBIENT,  
> > I don't really see the modifier as useful here. That exists for thermocouple
> > type systems where it is necessary to know ambient vs sample temperatures.  
> 
> Sure Jonathan, I will address all comments in this series in v2.
> Also, can i add new channel type IIO_COLORTEMP with following channel index
> for light color temperature ?

Looks good.

Jonathan

>        {
>                 .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,
>         }
> 
> Thanks,
> --
> Basavaraj
> 
> >
> >  
> >> +		.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)
> >>  };
> >>  
> >> @@ -103,6 +116,11 @@ static int als_read_raw(struct iio_dev *indio_dev,
> >>  			min = als_state->als_illum[chan->scan_index].logical_minimum;
> >>  			address = HID_USAGE_SENSOR_LIGHT_ILLUM;
> >>  			break;
> >> +		case  CHANNEL_SCAN_INDEX_COLOR_TEMP:
> >> +			report_id = als_state->als_illum[chan->scan_index].report_id;
> >> +			min = als_state->als_illum[chan->scan_index].logical_minimum;
> >> +			address = HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE;
> >> +			break;
> >>  		default:
> >>  			report_id = -1;
> >>  			break;
> >> @@ -223,6 +241,10 @@ 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);
> >> @@ -256,6 +278,17 @@ static int als_parse_report(struct platform_device *pdev,
> >>  			st->als_illum[i].report_id);
> >>  	}
> >>  
> >> +	ret = sensor_hub_input_get_attribute_info(hsdev, HID_INPUT_REPORT, usage_id,
> >> +						  HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE,
> >> +						  &st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP]);
> >> +	if (ret < 0)
> >> +		return ret;
> >> +	als_adjust_channel_bit_mask(channels, CHANNEL_SCAN_INDEX_COLOR_TEMP,
> >> +				    st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP].size);
> >> +
> >> +	dev_dbg(&pdev->dev, "als %x:%x\n", st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP].index,
> >> +		st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP].report_id);
> >> +
> >>  	st->scale_precision = hid_sensor_format_scale(usage_id,
> >>  						      &st->als_illum[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 13b1e65fbdcc..8af4fb3e0254 100644
> >> --- a/include/linux/hid-sensor-ids.h
> >> +++ b/include/linux/hid-sensor-ids.h
> >> @@ -21,6 +21,7 @@
> >>  #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  
>