On Fri, 26 May 2023 16:30:46 +0200
Mehdi Djait <mehdi.djait.k@xxxxxxxxx> wrote:
> Add the chip_info structure to the driver's private data to hold all
> the device specific infos.
> Refactor the kx022a driver implementation to make it more generic and
> extensible.
>
> Signed-off-by: Mehdi Djait <mehdi.djait.k@xxxxxxxxx>
Hi Mehdi,
Just one trivial comment from me about missing docs / unused element.
Thanks,
Jonathan
...
> diff --git a/drivers/iio/accel/kionix-kx022a.h b/drivers/iio/accel/kionix-kx022a.h
> index 12424649d438..c23b6f03409e 100644
> --- a/drivers/iio/accel/kionix-kx022a.h
> +++ b/drivers/iio/accel/kionix-kx022a.h
> @@ -76,7 +76,56 @@
>
> struct device;
>
> -int kx022a_probe_internal(struct device *dev);
> -extern const struct regmap_config kx022a_regmap;
> +/**
> + * struct kx022a_chip_info - Kionix accelerometer chip specific information
> + *
> + * @name: name of the device
> + * @regmap_config: pointer to register map configuration
> + * @channels: pointer to iio_chan_spec array
> + * @num_channels: number of iio_chan_spec channels
> + * @fifo_length: number of 16-bit samples in a full buffer
> + * @who: WHO_AM_I register
> + * @id: WHO_AM_I register value
> + * @cntl: control register 1
> + * @cntl2: control register 2
> + * @odcntl: output data control register
> + * @buf_cntl1: buffer control register 1
> + * @buf_cntl2: buffer control register 2
> + * @buf_clear: buffer clear register
> + * @buf_status1: buffer status register 1
Missing entry for buf_smp_lvl_mask.
Mind you it's also not used in this patch and only seems to be used in the
get_fifo_bytes() function in patch 7, so might not be needed at all?
If it is, then introduce it in patch 7 where we can see how it is used.
> + * @buf_read: buffer read register
> + * @inc1: interrupt control register 1
> + * @inc4: interrupt control register 4
> + * @inc5: interrupt control register 5
> + * @inc6: interrupt control register 6
> + * @xout_l: x-axis output least significant byte
> + */
> +struct kx022a_chip_info {
> + const char *name;
> + const struct regmap_config *regmap_config;
> + const struct iio_chan_spec *channels;
> + unsigned int num_channels;
> + unsigned int fifo_length;
> + u8 who;
> + u8 id;
> + u8 cntl;
> + u8 cntl2;
> + u8 odcntl;
> + u8 buf_cntl1;
> + u8 buf_cntl2;
> + u8 buf_clear;
> + u8 buf_status1;
> + u16 buf_smp_lvl_mask;
> + u8 buf_read;
> + u8 inc1;
> + u8 inc4;
> + u8 inc5;
> + u8 inc6;
> + u8 xout_l;
> +};
> +
> +int kx022a_probe_internal(struct device *dev, const struct kx022a_chip_info *chip_info);
> +
> +extern const struct kx022a_chip_info kx022a_chip_info;
>
> #endif
