Re: [PATCH v2 5/5] iio: hid-sensor-prox: Add support for more channels

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 28 Oct 2024 10:12:25 +0000
Ricardo Ribalda <ribalda@chromium.org> wrote:

> Egis620 supports 3 channels: presense, proximity and attention.
> 
> Modify the driver so it can read those channels as well.
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Hi Ricardo, one comment inline that I'm fine fixing up if nothing else
comes up. 

Given this series is introducing a new channel type, I'd like to leave
it on the list for a little while to see if there are any comments on that
definition beyond the one I had...

Jonathan

> ---
>  drivers/iio/light/hid-sensor-prox.c | 171 ++++++++++++++++++++----------------
>  1 file changed, 95 insertions(+), 76 deletions(-)
> 
> diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c
> index 0f12a8a83790..e6f81d40a492 100644
> --- a/drivers/iio/light/hid-sensor-prox.c
> +++ b/drivers/iio/light/hid-sensor-prox.c
> @@ -13,16 +13,31 @@
>  #include <linux/iio/buffer.h>
>  #include "../common/hid-sensors/hid-sensor-trigger.h"
>  
> -#define CHANNEL_SCAN_INDEX_PRESENCE 0
> +static const u32 prox_usage_ids[] = {
> +	HID_USAGE_SENSOR_HUMAN_PRESENCE,
> +	HID_USAGE_SENSOR_HUMAN_PROXIMITY,
> +	HID_USAGE_SENSOR_HUMAN_ATTENTION,
> +};
> +#define MAX_CHANNELS ARRAY_SIZE(prox_usage_ids)
> +
> +enum {
> +	HID_HUMAN_PRESENCE,
> +	HID_HUMAN_PROXIMITY,
> +	HID_HUMAN_ATTENTION,
> +};
>  
>  struct prox_state {
>  	struct hid_sensor_hub_callbacks callbacks;
>  	struct hid_sensor_common common_attributes;
> -	struct hid_sensor_hub_attribute_info prox_attr;
> -	u32 human_presence;
> +	struct hid_sensor_hub_attribute_info prox_attr[MAX_CHANNELS];
> +	struct iio_chan_spec channels[MAX_CHANNELS];
> +	u32 channel2usage[MAX_CHANNELS];
> +	u32 human_presence[MAX_CHANNELS];
>  	int scale_pre_decml;
>  	int scale_post_decml;
>  	int scale_precision;
> +	unsigned long scan_mask[2]; // One entry plus one terminator.
Really trivial, but in IIO and most of the kernel comment are still
'c style' /* One entry plus one terminator */

> +	int num_channels;
>  };
>  
>  static const u32 prox_sensitivity_addresses[] = {
> @@ -30,18 +45,23 @@ static const u32 prox_sensitivity_addresses[] = {
>  	HID_USAGE_SENSOR_DATA_PRESENCE,
>  };

>