linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Lechner <dlechner@baylibre.com>
To: Marcelo Schmitt <marcelo.schmitt@analog.com>,
	linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-doc@vger.kernel.org, devicetree@vger.kernel.org,
	linux-spi@vger.kernel.org
Cc: jic23@kernel.org, Michael.Hennerich@analog.com,
	nuno.sa@analog.com, eblanc@baylibre.com, andy@kernel.org,
	corbet@lwn.net, robh@kernel.org, krzk+dt@kernel.org,
	conor+dt@kernel.org, broonie@kernel.org,
	Jonathan.Cameron@huawei.com, andriy.shevchenko@linux.intel.com,
	ahaslam@baylibre.com, sergiu.cuciurean@analog.com,
	marcelo.schmitt1@gmail.com
Subject: Re: [PATCH 09/15] iio: adc: ad4030: Support multiple data lanes per channel
Date: Sat, 30 Aug 2025 12:19:04 -0500	[thread overview]
Message-ID: <a9b64ad7-41ff-4428-b47a-e3b3e50670a3@baylibre.com> (raw)
In-Reply-To: <4e2b2d07a255bb249a1dc40a4470c7e123d4213f.1756511030.git.marcelo.schmitt@analog.com>

On 8/29/25 7:43 PM, Marcelo Schmitt wrote:
> AD4030 and similar chips can output ADC sample data through 1, 2, or 4
> lines per channel. The number of SPI lines the device uses to output data
> is specified in firmware. Parse SPI read bus width setting from firmware
> and configure the device to use that amount of lines to output data.
> 
> Co-developed-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> ---
>  drivers/iio/adc/ad4030.c | 33 ++++++++++++++++++++++++++++++---
>  1 file changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad4030.c b/drivers/iio/adc/ad4030.c
> index 68f76432dbfd..e6c1c9be1632 100644
> --- a/drivers/iio/adc/ad4030.c
> +++ b/drivers/iio/adc/ad4030.c
> @@ -20,6 +20,7 @@
>  #include <linux/iio/iio.h>
>  #include <linux/iio/trigger_consumer.h>
>  #include <linux/iio/triggered_buffer.h>
> +#include <linux/log2.h>
>  #include <linux/pwm.h>
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
> @@ -258,6 +259,10 @@ struct ad4030_state {
>  #define AD4030_OFFLOAD_CHAN_DIFF(_idx, _scan_type)			\
>  	__AD4030_CHAN_DIFF(_idx, _scan_type, 1)
>  
> +static const int ad4030_rx_bus_width[] = {
> +	1, 2, 4, 8,
> +};
> +
>  static const int ad4030_average_modes[] = {
>  	1, 2, 4, 8, 16, 32, 64, 128,
>  	256, 512, 1024, 2048, 4096, 8192, 16384, 32768,
> @@ -1197,7 +1202,7 @@ static void ad4030_prepare_offload_msg(struct ad4030_state *st)
>  		 */
>  		offload_bpw = data_width * st->chip->num_voltage_inputs;
>  	else
> -		offload_bpw  = data_width;
> +		offload_bpw  = data_width / (1 << st->lane_mode);

To make proper use of 2 or 4 lines for a single channel, we should be
using the SPI APIs correctly and set rx_nbits in struct spi_transfer
instead of providing an inaccurate bits per word.

>  
>  	st->offload_xfer.speed_hz = AD4030_SPI_MAX_REG_XFER_SPEED;
>  	st->offload_xfer.bits_per_word = offload_bpw;
> @@ -1208,6 +1213,10 @@ static void ad4030_prepare_offload_msg(struct ad4030_state *st)
>  
>  static int ad4030_config(struct ad4030_state *st)
>  {
> +	struct device *dev = &st->spi->dev;
> +	const char *propname;
> +	u32 rx_bus_width;
> +	unsigned int i;
>  	int ret;
>  	u8 reg_modes;
>  
> @@ -1215,10 +1224,28 @@ static int ad4030_config(struct ad4030_state *st)
>  	st->offset_avail[1] = 1;
>  	st->offset_avail[2] = BIT(st->chip->precision_bits - 1) - 1;
>  
> -	if (st->chip->num_voltage_inputs > 1)
> +	/* Optional property specifying the number of lanes to read ADC data */
> +	propname = "spi-rx-bus-width";

This property is already handled by the core SPI code and will set
spi->mode flags SPI_RX_DUAL, SPI_RX_QUAD or SPI_RX_OCTAL. So we don't
need to read the property again.

> +	rx_bus_width = ad4030_rx_bus_width[0]; /* Default to 1 rx lane. */
> +	device_property_read_u32(dev, propname, &rx_bus_width);
> +	/* Check the rx bus width is valid */
> +	for (i = 0; i < ARRAY_SIZE(ad4030_rx_bus_width); i++)
> +		if (ad4030_rx_bus_width[i] == rx_bus_width)
> +			break;
> +
> +	if (i >= ARRAY_SIZE(ad4030_rx_bus_width))
> +		return dev_err_probe(dev, -EINVAL, "Invalid %s: %u\n",
> +				     propname, rx_bus_width);
> +
> +	rx_bus_width = ad4030_rx_bus_width[i];
> +
> +	if (rx_bus_width == 8 && st->chip->num_voltage_inputs == 1)
> +		return dev_err_probe(dev, -EINVAL, "1 channel with 8 lanes?\n");

As mentioned in the dt-bindings patch review, we really should consider
the 2 channel case separate as 2 SPI buses rather than 8 lines on a
single bus. 

Only using "spi-rx-bus-width" also has the shortcoming that if we specify
4, we don't know if is it 4 lines on 1 channel or is it 2 lines each on 2
channels? There is no way to tell from just that information.

> +
> +	if (rx_bus_width == 1 && st->chip->num_voltage_inputs > 1)
>  		st->lane_mode = AD4030_LANE_MD_INTERLEAVED;
>  	else
> -		st->lane_mode = AD4030_LANE_MD_1_PER_CH;
> +		st->lane_mode = ilog2(rx_bus_width / st->chip->num_voltage_inputs);
>  
>  	reg_modes = FIELD_PREP(AD4030_REG_MODES_MASK_LANE_MODE, st->lane_mode);
>  


  parent reply	other threads:[~2025-08-30 17:19 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-30  0:39 [PATCH 00/15] Add SPI offload support to AD4030 Marcelo Schmitt
2025-08-30  0:40 ` [PATCH 01/15] iio: adc: ad4030: Fix _scale for when oversampling is enabled Marcelo Schmitt
2025-08-30  5:00   ` Andy Shevchenko
2025-08-30 18:43   ` Jonathan Cameron
2025-08-30 18:48     ` David Lechner
2025-09-02 13:18     ` Marcelo Schmitt
2025-08-30  0:40 ` [PATCH 02/15] dt-bindings: iio: adc: adi,ad4030: Reference spi-peripheral-props Marcelo Schmitt
2025-08-30  0:41 ` [PATCH 03/15] Documentation: iio: ad4030: Add double PWM SPI offload doc Marcelo Schmitt
2025-08-30 16:49   ` David Lechner
2025-08-30  0:41 ` [PATCH 04/15] dt-bindings: iio: adc: adi,ad4030: Add PWM Marcelo Schmitt
2025-08-30  0:42 ` [PATCH 05/15] spi: offload: types: add offset parameter Marcelo Schmitt
2025-08-30  5:01   ` Andy Shevchenko
2025-08-30  0:42 ` [PATCH 06/15] spi: spi-offload-trigger-pwm: Use duty offset Marcelo Schmitt
2025-08-30  5:02   ` Andy Shevchenko
2025-08-30 16:41   ` David Lechner
2025-08-30  0:42 ` [PATCH 07/15] iio: adc: ad4030: Add SPI offload support Marcelo Schmitt
2025-08-30  7:36   ` Andy Shevchenko
2025-08-30 12:08   ` kernel test robot
2025-08-30 19:11   ` Jonathan Cameron
2025-08-30 20:14   ` David Lechner
2025-09-02 14:52     ` Marcelo Schmitt
2025-08-30  0:43 ` [PATCH 08/15] dt-bindings: iio: adc: adi,ad4030: Add 4-lane per channel bus width option Marcelo Schmitt
2025-08-30 17:01   ` David Lechner
2025-08-30  0:43 ` [PATCH 09/15] iio: adc: ad4030: Support multiple data lanes per channel Marcelo Schmitt
2025-08-30  7:38   ` Andy Shevchenko
2025-08-30 17:19   ` David Lechner [this message]
2025-08-30  0:43 ` [PATCH 10/15] dt-bindings: iio: adc: adi,ad4030: Add adi,clock-mode Marcelo Schmitt
2025-08-30 18:02   ` David Lechner
2025-08-30  0:44 ` [PATCH 11/15] iio: adc: ad4030: Add clock mode option parse and setup Marcelo Schmitt
2025-08-30  7:42   ` Andy Shevchenko
2025-08-30  0:44 ` [PATCH 12/15] dt-bindings: iio: adc: adi,ad4030: Add adi,dual-data-rate Marcelo Schmitt
2025-08-30 17:27   ` David Lechner
2025-08-30  0:45 ` [PATCH 13/15] iio: adc: ad4030: Enable dual data rate Marcelo Schmitt
2025-08-30  7:46   ` Andy Shevchenko
2025-08-30 17:33   ` David Lechner
2025-08-30  0:45 ` [PATCH 14/15] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216 and ADAQ4224 Marcelo Schmitt
2025-08-30 18:45   ` David Lechner
2025-08-30  0:45 ` [PATCH 15/15] iio: adc: ad4030: Add support for " Marcelo Schmitt
2025-08-30  7:57   ` Andy Shevchenko
2025-09-02 15:22     ` Marcelo Schmitt
2025-08-30 19:17   ` David Lechner
2025-09-01 11:47   ` Dan Carpenter
2025-08-30  2:48 ` [PATCH 00/15] Add SPI offload support to AD4030 Marcelo Schmitt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a9b64ad7-41ff-4428-b47a-e3b3e50670a3@baylibre.com \
    --to=dlechner@baylibre.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=Michael.Hennerich@analog.com \
    --cc=ahaslam@baylibre.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=andy@kernel.org \
    --cc=broonie@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=corbet@lwn.net \
    --cc=devicetree@vger.kernel.org \
    --cc=eblanc@baylibre.com \
    --cc=jic23@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=marcelo.schmitt1@gmail.com \
    --cc=marcelo.schmitt@analog.com \
    --cc=nuno.sa@analog.com \
    --cc=robh@kernel.org \
    --cc=sergiu.cuciurean@analog.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).