Re: [PATCH v4 7/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer

[Jonathan Cameron]

On Fri, 26 May 2023 16:30:48 +0200
Mehdi Djait <mehdi.djait.k@xxxxxxxxx> wrote:

> Kionix KX132-1211 is a tri-axis 16-bit accelerometer that can support
> ranges from ±2G to ±16G, digital output through I²C/SPI.
> Add support for basic accelerometer features such as reading acceleration
> via IIO using raw reads, triggered buffer (data-ready), or the WMI IRQ.
> 
> Datasheet: https://kionixfs.azureedge.net/en/document/KX132-1211-Technical-Reference-Manual-Rev-5.0.pdf
> Signed-off-by: Mehdi Djait <mehdi.djait.k@xxxxxxxxx>

Hi Mehdi,

Comment is a follow on from the earlier one about buf_smp_lvl_mask.

Jonathan

>  /*
>   * The sensor HW can support ODR up to 1600 Hz, which is beyond what most of the
>   * Linux CPUs can handle without dropping samples. Also, the low power mode is
> @@ -613,6 +731,24 @@ static int kx022a_get_fifo_bytes(struct kx022a_data *data)
>  	return fifo_bytes;
>  }
>  
> +static int kx132_get_fifo_bytes(struct kx022a_data *data)
> +{
> +	__le16 buf_status;
> +	int ret, fifo_bytes;
> +
> +	ret = regmap_bulk_read(data->regmap, data->chip_info->buf_status1,
> +			       &buf_status, sizeof(buf_status));
> +	if (ret) {
> +		dev_err(data->dev, "Error reading buffer status\n");
> +		return ret;
> +	}
> +
> +	fifo_bytes = le16_to_cpu(buf_status);
> +	fifo_bytes &= data->chip_info->buf_smp_lvl_mask;

See below.

> +
> +	return fifo_bytes;
> +}
> +
>  static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples,
>  			       bool irq)
>  {
> @@ -1036,6 +1172,32 @@ const struct kx022a_chip_info kx022a_chip_info = {
>  };
>  EXPORT_SYMBOL_NS_GPL(kx022a_chip_info, IIO_KX022A);
>  
> +const struct kx022a_chip_info kx132_chip_info = {
> +	.name		  = "kx132-1211",
> +	.regmap_config	  = &kx132_regmap_config,
> +	.channels	  = kx132_channels,
> +	.num_channels	  = ARRAY_SIZE(kx132_channels),
> +	.fifo_length	  = KX132_FIFO_LENGTH,
> +	.who		  = KX132_REG_WHO,
> +	.id		  = KX132_ID,
> +	.cntl		  = KX132_REG_CNTL,
> +	.cntl2		  = KX132_REG_CNTL2,
> +	.odcntl		  = KX132_REG_ODCNTL,
> +	.buf_cntl1	  = KX132_REG_BUF_CNTL1,
> +	.buf_cntl2	  = KX132_REG_BUF_CNTL2,
> +	.buf_clear	  = KX132_REG_BUF_CLEAR,
> +	.buf_status1	  = KX132_REG_BUF_STATUS_1,
> +	.buf_smp_lvl_mask = KX132_MASK_BUF_SMP_LVL,
As per earlier comment I think this is only set for this device...
> +	.buf_read	  = KX132_REG_BUF_READ,
> +	.inc1		  = KX132_REG_INC1,
> +	.inc4		  = KX132_REG_INC4,
> +	.inc5		  = KX132_REG_INC5,
> +	.inc6		  = KX132_REG_INC6,
> +	.xout_l		  = KX132_REG_XOUT_L,
> +	.get_fifo_bytes	  = kx132_get_fifo_bytes,
and only used in this device specific callback.
Probably can just use it directly in the callback.
> +};
> +EXPORT_SYMBOL_NS_GPL(kx132_chip_info, IIO_KX022A);
> +