Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 28 May 2023 17:59:09 +0100
From:   Jonathan Cameron <jic23@kernel.org>
To:     Mehdi Djait <mehdi.djait.k@gmail.com>
Cc:     mazziesaccount@gmail.com, krzysztof.kozlowski+dt@linaro.org,
        andriy.shevchenko@linux.intel.com, robh+dt@kernel.org,
        lars@metafoo.de, linux-iio@vger.kernel.org,
        linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v4 7/7] iio: accel: Add support for Kionix/ROHM
 KX132-1211 accelerometer
Message-ID: <20230528175909.245d5bfc@jic23-huawei>
In-Reply-To: <d776c1dd5beef6ef812a4f7e4958eb6cb0f5e58e.1685111274.git.mehdi.djait.k@gmail.com>
References: <cover.1685111274.git.mehdi.djait.k@gmail.com>
        <d776c1dd5beef6ef812a4f7e4958eb6cb0f5e58e.1685111274.git.mehdi.djait.k@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

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

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

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 mos=
t of the
>   * Linux CPUs can handle without dropping samples. Also, the low power m=
ode is
> @@ -613,6 +731,24 @@ static int kx022a_get_fifo_bytes(struct kx022a_data =
*data)
>  	return fifo_bytes;
>  }
> =20
> +static int kx132_get_fifo_bytes(struct kx022a_data *data)
> +{
> +	__le16 buf_status;
> +	int ret, fifo_bytes;
> +
> +	ret =3D 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 =3D le16_to_cpu(buf_status);
> +	fifo_bytes &=3D data->chip_info->buf_smp_lvl_mask;

See below.

> +
> +	return fifo_bytes;
> +}
> +
>  static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int sample=
s,
>  			       bool irq)
>  {
> @@ -1036,6 +1172,32 @@ const struct kx022a_chip_info kx022a_chip_info =3D=
 {
>  };
>  EXPORT_SYMBOL_NS_GPL(kx022a_chip_info, IIO_KX022A);
> =20
> +const struct kx022a_chip_info kx132_chip_info =3D {
> +	.name		  =3D "kx132-1211",
> +	.regmap_config	  =3D &kx132_regmap_config,
> +	.channels	  =3D kx132_channels,
> +	.num_channels	  =3D ARRAY_SIZE(kx132_channels),
> +	.fifo_length	  =3D KX132_FIFO_LENGTH,
> +	.who		  =3D KX132_REG_WHO,
> +	.id		  =3D KX132_ID,
> +	.cntl		  =3D KX132_REG_CNTL,
> +	.cntl2		  =3D KX132_REG_CNTL2,
> +	.odcntl		  =3D KX132_REG_ODCNTL,
> +	.buf_cntl1	  =3D KX132_REG_BUF_CNTL1,
> +	.buf_cntl2	  =3D KX132_REG_BUF_CNTL2,
> +	.buf_clear	  =3D KX132_REG_BUF_CLEAR,
> +	.buf_status1	  =3D KX132_REG_BUF_STATUS_1,
> +	.buf_smp_lvl_mask =3D KX132_MASK_BUF_SMP_LVL,
As per earlier comment I think this is only set for this device...
> +	.buf_read	  =3D KX132_REG_BUF_READ,
> +	.inc1		  =3D KX132_REG_INC1,
> +	.inc4		  =3D KX132_REG_INC4,
> +	.inc5		  =3D KX132_REG_INC5,
> +	.inc6		  =3D KX132_REG_INC6,
> +	.xout_l		  =3D KX132_REG_XOUT_L,
> +	.get_fifo_bytes	  =3D 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);
> +