On Tue, 2 Jul 2024 17:26:48 +0530
Abhash Jha <abhashkumarjha123@xxxxxxxxx> wrote:
> Proximity and gesture offset registers perform offset correction to improve
> cross-talk performance. Add support for reading from and writing to
> proximity and gesture offset registers. Create sysfs attributes for
> them via iio to expose them to userspace.
>
> Signed-off-by: Abhash Jha <abhashkumarjha123@xxxxxxxxx>
All custom ABI needs documentation in
Documentation/ABI/testing/sysfs-bus-iio-*
where * is appropriate scoped - here probably device.
Key thing is that custom documentation == custom userspace code ==
never used by 99% of users.
So we need to think very hard about whether we can map it to standard
ABI, or if it deserves new standard ABI (which will eventually make it
into userspace tools).
That's a hard discussion to have without documentation
Consider in particular if calibbias can work for these as it sounds
superficially similar to what the datasheet is talking about.
> ---
> drivers/iio/light/apds9960.c | 245 +++++++++++++++++++++++++++++++++++
> 1 file changed, 245 insertions(+)
>
> diff --git a/drivers/iio/light/apds9960.c b/drivers/iio/light/apds9960.c
> index 1065a340b..bccf9a8c7 100644
> --- a/drivers/iio/light/apds9960.c
> +++ b/drivers/iio/light/apds9960.c
> @@ -101,6 +101,10 @@
> #define APDS9960_MAX_ALS_THRES_VAL 0xffff
> #define APDS9960_MAX_INT_TIME_IN_US 1000000
>
> +/* MIN and MAX offset from pg: 26 of the datasheet */
> +#define MIN_OFFSET -127
> +#define MAX_OFFSET 127
S8_MIN, S8_MAX
probably appropriate here.
> +static IIO_DEVICE_ATTR(proximity_offset_ur, S_IRUGO | S_IWUSR, apds9960_proximity_offset_ur_show, apds9960_proximity_offset_ur_store, 0);
> +static IIO_DEVICE_ATTR(proximity_offset_dl, S_IRUGO | S_IWUSR, apds9960_proximity_offset_dl_show, apds9960_proximity_offset_dl_store, 0);
> +static IIO_DEVICE_ATTR(gesture_offset_u, S_IRUGO | S_IWUSR, apds9960_gesture_offset_u_show, apds9960_gesture_offset_u_store, 0);
> +static IIO_DEVICE_ATTR(gesture_offset_d, S_IRUGO | S_IWUSR, apds9960_gesture_offset_d_show, apds9960_gesture_offset_d_store, 0);
> +static IIO_DEVICE_ATTR(gesture_offset_l, S_IRUGO | S_IWUSR, apds9960_gesture_offset_l_show, apds9960_gesture_offset_l_store, 0);
> +static IIO_DEVICE_ATTR(gesture_offset_r, S_IRUGO | S_IWUSR, apds9960_gesture_offset_r_show, apds9960_gesture_offset_r_store, 0);
Very long lines. Try to keep under 80 chars unless there is a good reason
(readability mainly). here I'm not seeing one.
Also, the kernel is moving away from symbols for permissions as
checkpatch would I think have told you..
> +
> static struct attribute *apds9960_attributes[] = {
> &iio_const_attr_proximity_scale_available.dev_attr.attr,
> &iio_const_attr_intensity_scale_available.dev_attr.attr,
> &iio_const_attr_integration_time_available.dev_attr.attr,
> + &iio_dev_attr_proximity_offset_ur.dev_attr.attr,
> + &iio_dev_attr_proximity_offset_dl.dev_attr.attr,
> + &iio_dev_attr_gesture_offset_u.dev_attr.attr,
> + &iio_dev_attr_gesture_offset_d.dev_attr.attr,
> + &iio_dev_attr_gesture_offset_l.dev_attr.attr,
> + &iio_dev_attr_gesture_offset_r.dev_attr.attr,
> NULL,
> };
>
