Re: [PATCH v9 6/7] iio: accel: kionix-kx022a: Add a function to retrieve number of bytes in buffer

[Jonathan Cameron]

On Sat, 16 Sep 2023 14:38:52 +0200
Mehdi Djait <mehdi.djait.k@xxxxxxxxx> wrote:

> Since Kionix accelerometers use various numbers of bits to report data, a
> device-specific function is required.
> Implement the function as a callback in the device-specific chip_info structure
> 
> Reviewed-by: Matti Vaittinen <mazziesaccount@xxxxxxxxx>
> Signed-off-by: Mehdi Djait <mehdi.djait.k@xxxxxxxxx>


>  const struct kx022a_chip_info kx022a_chip_info = {
> -	.name		  = "kx022-accel",
> -	.regmap_config	  = &kx022a_regmap_config,
> -	.channels	  = kx022a_channels,
> -	.num_channels	  = ARRAY_SIZE(kx022a_channels),
> -	.fifo_length	  = KX022A_FIFO_LENGTH,
> -	.who		  = KX022A_REG_WHO,
> -	.id		  = KX022A_ID,
> -	.cntl		  = KX022A_REG_CNTL,
> -	.cntl2		  = KX022A_REG_CNTL2,
> -	.odcntl		  = KX022A_REG_ODCNTL,
> -	.buf_cntl1	  = KX022A_REG_BUF_CNTL1,
> -	.buf_cntl2	  = KX022A_REG_BUF_CNTL2,
> -	.buf_clear	  = KX022A_REG_BUF_CLEAR,
> -	.buf_status1	  = KX022A_REG_BUF_STATUS_1,
> -	.buf_read	  = KX022A_REG_BUF_READ,
> -	.inc1		  = KX022A_REG_INC1,
> -	.inc4		  = KX022A_REG_INC4,
> -	.inc5		  = KX022A_REG_INC5,
> -	.inc6		  = KX022A_REG_INC6,
> -	.xout_l		  = KX022A_REG_XOUT_L,
This is a very good illustration of why aligning value assignments is not
(in my opinion) a good idea. I'll tweak the earlier patch whilst
applying to add the extra indent.

However, in future look through your complete series for cases where
code added in an earlier patch is simply reformatted in a later one.

Same applies to the structure kdoc below.


> +	.name				= "kx022-accel",
> +	.regmap_config			= &kx022a_regmap_config,
> +	.channels			= kx022a_channels,
> +	.num_channels			= ARRAY_SIZE(kx022a_channels),
> +	.fifo_length			= KX022A_FIFO_LENGTH,
> +	.who				= KX022A_REG_WHO,
> +	.id				= KX022A_ID,
> +	.cntl				= KX022A_REG_CNTL,
> +	.cntl2				= KX022A_REG_CNTL2,
> +	.odcntl				= KX022A_REG_ODCNTL,
> +	.buf_cntl1			= KX022A_REG_BUF_CNTL1,
> +	.buf_cntl2			= KX022A_REG_BUF_CNTL2,
> +	.buf_clear			= KX022A_REG_BUF_CLEAR,
> +	.buf_status1			= KX022A_REG_BUF_STATUS_1,
> +	.buf_read			= KX022A_REG_BUF_READ,
> +	.inc1				= KX022A_REG_INC1,
> +	.inc4				= KX022A_REG_INC4,
> +	.inc5				= KX022A_REG_INC5,
> +	.inc6				= KX022A_REG_INC6,
> +	.xout_l				= KX022A_REG_XOUT_L,
> +	.get_fifo_bytes_available	= kx022a_get_fifo_bytes_available,
>  };
>  EXPORT_SYMBOL_NS_GPL(kx022a_chip_info, IIO_KX022A);
>  
> diff --git a/drivers/iio/accel/kionix-kx022a.h b/drivers/iio/accel/kionix-kx022a.h
> index 0e5026019213..7ca48e6f2c49 100644
> --- a/drivers/iio/accel/kionix-kx022a.h
> +++ b/drivers/iio/accel/kionix-kx022a.h
> @@ -76,29 +76,34 @@
>  
>  struct device;
>  
> +struct kx022a_data;
> +
>  /**
>   * 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
> - * @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
> + * @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
> + * @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
> + * @get_fifo_bytes_available:	function pointer to get amount of acceleration
> + *				data bytes currently stored in the sensor's FIFO
> + *				buffer
>   */
>  struct kx022a_chip_info {
>  	const char *name;
> @@ -121,6 +126,7 @@ struct kx022a_chip_info {
>  	u8 inc5;
>  	u8 inc6;
>  	u8 xout_l;
> +	int (*get_fifo_bytes_available)(struct kx022a_data *);
>  };
>  
>  int kx022a_probe_internal(struct device *dev, const struct kx022a_chip_info *chip_info);