diff mbox series

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

Message ID 20230915051703.1689578-2-Basavaraj.Natikar@amd.com
State Superseded
Headers show
Series [1/8] iio: hid-sensor-als: Use channel index to support more hub attributes | expand

Commit Message

Basavaraj Natikar Sept. 15, 2023, 5:16 a.m. UTC
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(-)

Comments

Jonathan Cameron Sept. 17, 2023, 10:56 a.m. UTC | #1
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;
>  }
diff mbox series

Patch

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;
 }