Re: [PATCH V5 2/2] iio: proximity: aw9610x: Add support for aw9610x proximity sensor

[Krzysztof Kozlowski]

On 26/07/2024 08:13, wangshuaijie@xxxxxxxxxx wrote:
> From: shuaijie wang <wangshuaijie@xxxxxxxxxx>
> 
> AW9610X is a low power consumption capacitive touch and proximity controller.
> Each channel can be independently config as sensor input, shield output.
> 
> Channel Information:
>   aw96103: 3-channel
>   aw96105: 5-channel
> 
> Signed-off-by: shuaijie wang <wangshuaijie@xxxxxxxxxx>
> ---
>  drivers/iio/proximity/Kconfig   |  11 +
>  drivers/iio/proximity/Makefile  |   1 +
>  drivers/iio/proximity/aw9610x.c | 791 ++++++++++++++++++++++++++++++++
>  drivers/iio/proximity/aw9610x.h | 140 ++++++
>  4 files changed, 943 insertions(+)
>  create mode 100644 drivers/iio/proximity/aw9610x.c
>  create mode 100644 drivers/iio/proximity/aw9610x.h
> 
> diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
> index 2ca3b0bc5eba..ca1b8bde2def 100644
> --- a/drivers/iio/proximity/Kconfig
> +++ b/drivers/iio/proximity/Kconfig
> @@ -219,4 +219,15 @@ config VL53L0X_I2C
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called vl53l0x-i2c.
>  
> +config AW9610X
> +	tristate "Awinic AW9610X proximity sensor"
> +	select REGMAP_I2C
> +	depends on I2C
> +	help
> +	  Say Y here to build a driver for Awinic's AW9610X capacitive
> +	  proximity sensor.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called aw9610x.
> +
>  endmenu
> diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
> index f36598380446..483f5bf1ac8b 100644
> --- a/drivers/iio/proximity/Makefile
> +++ b/drivers/iio/proximity/Makefile
> @@ -21,4 +21,5 @@ obj-$(CONFIG_SX_COMMON) 	+= sx_common.o
>  obj-$(CONFIG_SX9500)		+= sx9500.o
>  obj-$(CONFIG_VCNL3020)		+= vcnl3020.o
>  obj-$(CONFIG_VL53L0X_I2C)	+= vl53l0x-i2c.o
> +obj-$(CONFIG_AW9610X)		+= aw9610x.o
>  
> diff --git a/drivers/iio/proximity/aw9610x.c b/drivers/iio/proximity/aw9610x.c
> new file mode 100644
> index 000000000000..de5be3918e4f
> --- /dev/null
> +++ b/drivers/iio/proximity/aw9610x.c
> @@ -0,0 +1,791 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * AWINIC aw9610x proximity sensor driver
> + *
> + * Author: Wang Shuaijie <wangshuaijie@xxxxxxxxxx>
> + *
> + * Copyright (c) 2024 awinic Technology CO., LTD
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/bitfield.h>
> +#include <linux/delay.h>
> +#include <linux/firmware.h>
> +#include <linux/i2c.h>
> +#include <linux/input.h>

Where do you use it?

> +#include <linux/interrupt.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/events.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/of_gpio.h>

Where do you use it?

> +#include <linux/power_supply.h>
> +#include <linux/pinctrl/consumer.h>

Where do you use it?

> +#include <linux/regulator/consumer.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>

Maybe several others are not used...

> +#include "aw9610x.h"
> +
> +static unsigned int aw9610x_reg_default[] = {

Why this cannot be const?

> +	0x0000, 0x00003f3f, 0x0004, 0x00000064, 0x0008, 0x0017c11e,
> +	0x000c, 0x05000000, 0x0010, 0x00093ffd, 0x0014, 0x19240009,
> +	0x0018, 0xd81c0207, 0x001c, 0xff000000, 0x0020, 0x00241900,
> +	0x0024, 0x00093ff7, 0x0028, 0x58020009, 0x002c, 0xd81c0207,
> +	0x0030, 0xff000000, 0x0034, 0x00025800, 0x0038, 0x00093fdf,
> +	0x003c, 0x7d3b0009, 0x0040, 0xd81c0207,	0x0044, 0xff000000,
> +	0x0048, 0x003b7d00, 0x004c, 0x00093f7f, 0x0050, 0xe9310009,
> +	0x0054, 0xd81c0207, 0x0058, 0xff000000,	0x005c, 0x0031e900,
> +	0x0060, 0x00093dff, 0x0064, 0x1a0c0009,	0x0068, 0xd81c0207,
> +	0x006c, 0xff000000, 0x0070, 0x000c1a00,	0x0074, 0x80093fff,
> +	0x0078, 0x043d0009, 0x007c, 0xd81c0207,	0x0080, 0xff000000,
> +	0x0084, 0x003d0400, 0x00a0, 0xe6400000,	0x00a4, 0x00000000,
> +	0x00a8, 0x010408d2, 0x00ac, 0x00000000,	0x00b0, 0x00000000,
> +	0x00b8, 0x00005fff, 0x00bc, 0x00000000,	0x00c0, 0x00000000,
> +	0x00c4, 0x00000000, 0x00c8, 0x00000000,	0x00cc, 0x00000000,
> +	0x00d0, 0x00000000, 0x00d4, 0x00000000, 0x00d8, 0x00000000,
> +	0x00dc, 0xe6447800, 0x00e0, 0x78000000,	0x00e4, 0x010408d2,
> +	0x00e8, 0x00000000, 0x00ec, 0x00000000,	0x00f4, 0x00005fff,
> +	0x00f8, 0x00000000, 0x00fc, 0x00000000,	0x0100, 0x00000000,
> +	0x0104, 0x00000000, 0x0108, 0x00000000,	0x010c, 0x02000000,
> +	0x0110, 0x00000000, 0x0114, 0x00000000,	0x0118, 0xe6447800,
> +	0x011c, 0x78000000, 0x0120, 0x010408d2,	0x0124, 0x00000000,
> +	0x0128, 0x00000000, 0x0130, 0x00005fff,	0x0134, 0x00000000,
> +	0x0138, 0x00000000, 0x013c, 0x00000000,	0x0140, 0x00000000,
> +	0x0144, 0x00000000, 0x0148, 0x02000000,	0x014c, 0x00000000,
> +	0x0150, 0x00000000, 0x0154, 0xe6447800,	0x0158, 0x78000000,
> +	0x015c, 0x010408d2, 0x0160, 0x00000000,	0x0164, 0x00000000,
> +	0x016c, 0x00005fff, 0x0170, 0x00000000,	0x0174, 0x00000000,
> +	0x0178, 0x00000000, 0x017c, 0x00000000,	0x0180, 0x00000000,
> +	0x0184, 0x02000000, 0x0188, 0x00000000,	0x018c, 0x00000000,
> +	0x0190, 0xe6447800, 0x0194, 0x78000000,	0x0198, 0x010408d2,
> +	0x019c, 0x00000000, 0x01a0, 0x00000000,	0x01a8, 0x00005fff,
> +	0x01ac, 0x00000000, 0x01b0, 0x00000000,	0x01b4, 0x00000000,
> +	0x01b8, 0x00000000, 0x01bc, 0x00000000,	0x01c0, 0x02000000,
> +	0x01c4, 0x00000000, 0x01c8, 0x00000000,	0x01cc, 0xe6407800,
> +	0x01d0, 0x78000000, 0x01d4, 0x010408d2,	0x01d8, 0x00000000,
> +	0x01dc, 0x00000000, 0x01e4, 0x00005fff,	0x01e8, 0x00000000,
> +	0x01ec, 0x00000000, 0x01f0, 0x00000000,	0x01f4, 0x00000000,
> +	0x01f8, 0x00000000, 0x01fc, 0x02000000,	0x0200, 0x00000000,
> +	0x0204, 0x00000000, 0x0208, 0x00000008,	0x020c, 0x0000000d,
> +	0x41fc, 0x00000000, 0x4400, 0x00000000,	0x4410, 0x00000000,
> +	0x4420, 0x00000000, 0x4430, 0x00000000,	0x4440, 0x00000000,
> +	0x4450, 0x00000000, 0x4460, 0x00000000,	0x4470, 0x00000000,
> +	0xf080, 0x00003018, 0xf084, 0x00000fff,	0xf800, 0x00000000,
> +	0xf804, 0x00002e00, 0xf8d0, 0x00000001,	0xf8d4, 0x00000000,
> +	0xff00, 0x00000301, 0xff0c, 0x01000000,	0xffe0, 0x00000000,
> +	0xfff4, 0x00004011, 0x0090, 0x00000000,	0x0094, 0x00000000,
> +	0x0098, 0x00000000, 0x009c, 0x3f3f3f3f,
> +};
> +

...

> +static int aw9610x_read_chipid(struct aw9610x *aw9610x)
> +{
> +	unsigned char cnt = 0;
> +	u32 reg_val;
> +	int ret;
> +
> +	while (cnt < AW_READ_CHIPID_RETRIES) {
> +		ret = aw9610x_i2c_read(aw9610x, REG_CHIPID, &reg_val);
> +		if (ret < 0) {
> +			cnt++;
> +			usleep_range(2000, 3000);
> +		} else {
> +			reg_val = FIELD_GET(AW9610X_CHIPID_MASK, reg_val);
> +			break;
> +		}
> +	}
> +
> +	if (reg_val == AW9610X_CHIP_ID)
> +		return 0;

So devices are detectable? Encode this in the bindings (oneOf and a
fallback compatible) and drop unneeded entry from ID tables.

Best regards,
Krzysztof