[v11,5/8] iio: adc: adi-axi-adc: set data format

Message ID	20250127105726.6314-6-antoniu.miclaus@analog.com (mailing list archive)
State	Changes Requested
Headers	show Received: from mx0a-00128a01.pphosted.com (mx0a-00128a01.pphosted.com [148.163.135.77]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8A2D71FF1D6; Mon, 27 Jan 2025 10:57:54 +0000 (UTC) From: Antoniu Miclaus <antoniu.miclaus@analog.com> To: <jic23@kernel.org>, <robh@kernel.org>, <conor+dt@kernel.org>, <linux-iio@vger.kernel.org>, <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <linux-pwm@vger.kernel.org> CC: Antoniu Miclaus <antoniu.miclaus@analog.com>, Nuno Sa <nuno.sa@analog.com> Subject: [PATCH v11 5/8] iio: adc: adi-axi-adc: set data format Date: Mon, 27 Jan 2025 12:57:23 +0200 Message-ID: <20250127105726.6314-6-antoniu.miclaus@analog.com> In-Reply-To: <20250127105726.6314-1-antoniu.miclaus@analog.com> References: <20250127105726.6314-1-antoniu.miclaus@analog.com> Precedence: bulk MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain
Series	Add support for AD485x DAS Family \| expand [v11,0/8] Add support for AD485x DAS Family [v11,1/8] iio: backend: add API for interface get [v11,2/8] iio: backend: add support for data size set [v11,3/8] iio: backend: add API for oversampling [v11,4/8] iio: adc: adi-axi-adc: add interface type [v11,5/8] iio: adc: adi-axi-adc: set data format [v11,6/8] iio: adc: adi-axi-adc: add oversampling [v11,7/8] dt-bindings: iio: adc: add ad4851 [v11,8/8] iio: adc: ad4851: add ad485x driver

Message ID

20250127105726.6314-6-antoniu.miclaus@analog.com (mailing list archive)

State

Changes Requested

Headers

From: Antoniu Miclaus <antoniu.miclaus@analog.com>
To: <jic23@kernel.org>, <robh@kernel.org>, <conor+dt@kernel.org>,
        <linux-iio@vger.kernel.org>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <linux-pwm@vger.kernel.org>
CC: Antoniu Miclaus <antoniu.miclaus@analog.com>, Nuno Sa <nuno.sa@analog.com>
Subject: [PATCH v11 5/8] iio: adc: adi-axi-adc: set data format
Date: Mon, 27 Jan 2025 12:57:23 +0200
Message-ID: <20250127105726.6314-6-antoniu.miclaus@analog.com>
In-Reply-To: <20250127105726.6314-1-antoniu.miclaus@analog.com>
References: <20250127105726.6314-1-antoniu.miclaus@analog.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Type: text/plain

Series

Add support for AD485x DAS Family | expand

Commit Message

Miclaus, Antoniu Jan. 27, 2025, 10:57 a.m. UTC

Add support for selecting the data format within the AXI ADC ip.

Reviewed-by: Nuno Sa <nuno.sa@analog.com>
Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
no changes in v11.
 drivers/iio/adc/adi-axi-adc.c | 46 +++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

Comments

David Lechner Jan. 28, 2025, 4:16 p.m. UTC | #1

On 1/27/25 4:57 AM, Antoniu Miclaus wrote:
> Add support for selecting the data format within the AXI ADC ip.
> 
> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> no changes in v11.
>  drivers/iio/adc/adi-axi-adc.c | 46 +++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
> index d2e1dc63775c..3c213ca5ff8e 100644
> --- a/drivers/iio/adc/adi-axi-adc.c
> +++ b/drivers/iio/adc/adi-axi-adc.c
> @@ -45,6 +45,12 @@
>  #define ADI_AXI_ADC_REG_CTRL			0x0044
>  #define    ADI_AXI_ADC_CTRL_DDR_EDGESEL_MASK	BIT(1)
>  
> +#define ADI_AXI_ADC_REG_CNTRL_3			0x004c
> +#define   AD485X_CNTRL_3_PACKET_FORMAT_MSK	GENMASK(1, 0)
> +#define   AD485X_PACKET_FORMAT_20BIT		0x0
> +#define   AD485X_PACKET_FORMAT_24BIT		0x1
> +#define   AD485X_PACKET_FORMAT_32BIT		0x2
> +
>  #define ADI_AXI_ADC_REG_DRP_STATUS		0x0074
>  #define   ADI_AXI_ADC_DRP_LOCKED		BIT(17)
>  
> @@ -312,6 +318,45 @@ static int axi_adc_interface_type_get(struct iio_backend *back,
>  	return 0;
>  }
>  
> +static int axi_adc_data_size_set(struct iio_backend *back, unsigned int size)
> +{
> +	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
> +	unsigned int val;
> +
> +	switch (size) {
> +	/*
> +	 * There are two different variants of the AXI AD485X IP block, a 16-bit
> +	 * and a 20-bit variant.
> +	 * The 0x0 value (AD485X_PACKET_FORMAT_20BIT) is corresponding also to
> +	 * the 16-bit variant of the IP block.
> +	 */
> +	case 16:
> +	case 20:
> +		val = AD485X_PACKET_FORMAT_20BIT;
> +		break;
> +	case 24:
> +		val = AD485X_PACKET_FORMAT_24BIT;
> +		break;
> +	/*
> +	 * The 0x2 (AD485X_PACKET_FORMAT_32BIT) corresponds only to the 20-bit
> +	 * variant of the IP block. Setting this value properly is ensured by
> +	 * the upper layers of the drivers calling the axi-adc functions.
> +	 * Also, for 16-bit IP block, the 0x2 (AD485X_PACKET_FORMAT_32BIT)
> +	 * value is handled as maximum size available which is 24-bit for this
> +	 * configuration.
> +	 */
> +	case 32:
> +		val = AD485X_PACKET_FORMAT_32BIT;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return regmap_update_bits(st->regmap, ADI_AXI_ADC_REG_CNTRL_3,
> +				  AD485X_CNTRL_3_PACKET_FORMAT_MSK,
> +				  FIELD_PREP(AD485X_CNTRL_3_PACKET_FORMAT_MSK, val));
> +}
> +
>  static struct iio_buffer *axi_adc_request_buffer(struct iio_backend *back,
>  						 struct iio_dev *indio_dev)
>  {
> @@ -360,6 +405,7 @@ static const struct iio_backend_ops adi_axi_adc_ops = {
>  	.test_pattern_set = axi_adc_test_pattern_set,
>  	.chan_status = axi_adc_chan_status,
>  	.interface_type_get = axi_adc_interface_type_get,
> +	.data_size_set = axi_adc_data_size_set,
>  	.debugfs_reg_access = iio_backend_debugfs_ptr(axi_adc_reg_access),
>  	.debugfs_print_chan_status = iio_backend_debugfs_ptr(axi_adc_debugfs_print_chan_status),
>  };

Why was [1] not addressed?

[1]: https://lore.kernel.org/linux-iio/9c262f599fb9b42feac99cfb541723a0a6f50e6b.camel@gmail.com/

Miclaus, Antoniu Feb. 3, 2025, 11:02 a.m. UTC | #2

> On 1/27/25 4:57 AM, Antoniu Miclaus wrote:
> > Add support for selecting the data format within the AXI ADC ip.
> >
> > Reviewed-by: Nuno Sa <nuno.sa@analog.com>
> > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > ---
> > no changes in v11.
> >  drivers/iio/adc/adi-axi-adc.c | 46
> +++++++++++++++++++++++++++++++++++
> >  1 file changed, 46 insertions(+)
> >
> > diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
> > index d2e1dc63775c..3c213ca5ff8e 100644
> > --- a/drivers/iio/adc/adi-axi-adc.c
> > +++ b/drivers/iio/adc/adi-axi-adc.c
> > @@ -45,6 +45,12 @@
> >  #define ADI_AXI_ADC_REG_CTRL			0x0044
> >  #define    ADI_AXI_ADC_CTRL_DDR_EDGESEL_MASK	BIT(1)
> >
> > +#define ADI_AXI_ADC_REG_CNTRL_3			0x004c
> > +#define   AD485X_CNTRL_3_PACKET_FORMAT_MSK	GENMASK(1, 0)
> > +#define   AD485X_PACKET_FORMAT_20BIT		0x0
> > +#define   AD485X_PACKET_FORMAT_24BIT		0x1
> > +#define   AD485X_PACKET_FORMAT_32BIT		0x2
> > +
> >  #define ADI_AXI_ADC_REG_DRP_STATUS		0x0074
> >  #define   ADI_AXI_ADC_DRP_LOCKED		BIT(17)
> >
> > @@ -312,6 +318,45 @@ static int axi_adc_interface_type_get(struct
> iio_backend *back,
> >  	return 0;
> >  }
> >
> > +static int axi_adc_data_size_set(struct iio_backend *back, unsigned int size)
> > +{
> > +	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
> > +	unsigned int val;
> > +
> > +	switch (size) {
> > +	/*
> > +	 * There are two different variants of the AXI AD485X IP block, a 16-bit
> > +	 * and a 20-bit variant.
> > +	 * The 0x0 value (AD485X_PACKET_FORMAT_20BIT) is corresponding
> also to
> > +	 * the 16-bit variant of the IP block.
> > +	 */
> > +	case 16:
> > +	case 20:
> > +		val = AD485X_PACKET_FORMAT_20BIT;
> > +		break;
> > +	case 24:
> > +		val = AD485X_PACKET_FORMAT_24BIT;
> > +		break;
> > +	/*
> > +	 * The 0x2 (AD485X_PACKET_FORMAT_32BIT) corresponds only to
> the 20-bit
> > +	 * variant of the IP block. Setting this value properly is ensured by
> > +	 * the upper layers of the drivers calling the axi-adc functions.
> > +	 * Also, for 16-bit IP block, the 0x2
> (AD485X_PACKET_FORMAT_32BIT)
> > +	 * value is handled as maximum size available which is 24-bit for this
> > +	 * configuration.
> > +	 */
> > +	case 32:
> > +		val = AD485X_PACKET_FORMAT_32BIT;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	return regmap_update_bits(st->regmap,
> ADI_AXI_ADC_REG_CNTRL_3,
> > +				  AD485X_CNTRL_3_PACKET_FORMAT_MSK,
> > +
> FIELD_PREP(AD485X_CNTRL_3_PACKET_FORMAT_MSK, val));
> > +}
> > +
> >  static struct iio_buffer *axi_adc_request_buffer(struct iio_backend *back,
> >  						 struct iio_dev *indio_dev)
> >  {
> > @@ -360,6 +405,7 @@ static const struct iio_backend_ops adi_axi_adc_ops
> = {
> >  	.test_pattern_set = axi_adc_test_pattern_set,
> >  	.chan_status = axi_adc_chan_status,
> >  	.interface_type_get = axi_adc_interface_type_get,
> > +	.data_size_set = axi_adc_data_size_set,
> >  	.debugfs_reg_access = iio_backend_debugfs_ptr(axi_adc_reg_access),
> >  	.debugfs_print_chan_status =
> iio_backend_debugfs_ptr(axi_adc_debugfs_print_chan_status),
> >  };
> 
> Why was [1] not addressed?
> 
> [1]: https://urldefense.com/v3/__https://lore.kernel.org/linux-
> iio/9c262f599fb9b42feac99cfb541723a0a6f50e6b.camel@gmail.com/__;!!A
> 3Ni8CS0y2Y!6uVytAwWUCsEazOUTACecMQkbMuHBF95sbla50CbTUFkZkyxS
> -S7jMOCczpoyKCjtAKvMOyrt0ukYwcXC_l5q60$

Indeed it was not addressed. I remained with the impression that adding part prefix
in the macro definitions was enough. I will add the compatible string support.
Although I have a question in order to minimize the number of versions to be sent
In the future. Should I add a separate patch for the compatible support (which
will not add value independently) or should I include it in this patch which adds
custom function for data format for the AD485x IP core?

Jonathan Cameron Feb. 3, 2025, 3:25 p.m. UTC | #3

On Mon, 3 Feb 2025 11:02:58 +0000
"Miclaus, Antoniu" <Antoniu.Miclaus@analog.com> wrote:

>  
> > On 1/27/25 4:57 AM, Antoniu Miclaus wrote:  
> > > Add support for selecting the data format within the AXI ADC ip.
> > >
> > > Reviewed-by: Nuno Sa <nuno.sa@analog.com>
> > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > > ---
> > > no changes in v11.
> > >  drivers/iio/adc/adi-axi-adc.c | 46  
> > +++++++++++++++++++++++++++++++++++  
> > >  1 file changed, 46 insertions(+)
> > >
> > > diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
> > > index d2e1dc63775c..3c213ca5ff8e 100644
> > > --- a/drivers/iio/adc/adi-axi-adc.c
> > > +++ b/drivers/iio/adc/adi-axi-adc.c
> > > @@ -45,6 +45,12 @@
> > >  #define ADI_AXI_ADC_REG_CTRL			0x0044
> > >  #define    ADI_AXI_ADC_CTRL_DDR_EDGESEL_MASK	BIT(1)
> > >
> > > +#define ADI_AXI_ADC_REG_CNTRL_3			0x004c
> > > +#define   AD485X_CNTRL_3_PACKET_FORMAT_MSK	GENMASK(1, 0)
> > > +#define   AD485X_PACKET_FORMAT_20BIT		0x0
> > > +#define   AD485X_PACKET_FORMAT_24BIT		0x1
> > > +#define   AD485X_PACKET_FORMAT_32BIT		0x2
> > > +
> > >  #define ADI_AXI_ADC_REG_DRP_STATUS		0x0074
> > >  #define   ADI_AXI_ADC_DRP_LOCKED		BIT(17)
> > >
> > > @@ -312,6 +318,45 @@ static int axi_adc_interface_type_get(struct  
> > iio_backend *back,  
> > >  	return 0;
> > >  }
> > >
> > > +static int axi_adc_data_size_set(struct iio_backend *back, unsigned int size)
> > > +{
> > > +	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
> > > +	unsigned int val;
> > > +
> > > +	switch (size) {
> > > +	/*
> > > +	 * There are two different variants of the AXI AD485X IP block, a 16-bit
> > > +	 * and a 20-bit variant.
> > > +	 * The 0x0 value (AD485X_PACKET_FORMAT_20BIT) is corresponding  
> > also to  
> > > +	 * the 16-bit variant of the IP block.
> > > +	 */
> > > +	case 16:
> > > +	case 20:
> > > +		val = AD485X_PACKET_FORMAT_20BIT;
> > > +		break;
> > > +	case 24:
> > > +		val = AD485X_PACKET_FORMAT_24BIT;
> > > +		break;
> > > +	/*
> > > +	 * The 0x2 (AD485X_PACKET_FORMAT_32BIT) corresponds only to  
> > the 20-bit  
> > > +	 * variant of the IP block. Setting this value properly is ensured by
> > > +	 * the upper layers of the drivers calling the axi-adc functions.
> > > +	 * Also, for 16-bit IP block, the 0x2  
> > (AD485X_PACKET_FORMAT_32BIT)  
> > > +	 * value is handled as maximum size available which is 24-bit for this
> > > +	 * configuration.
> > > +	 */
> > > +	case 32:
> > > +		val = AD485X_PACKET_FORMAT_32BIT;
> > > +		break;
> > > +	default:
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	return regmap_update_bits(st->regmap,  
> > ADI_AXI_ADC_REG_CNTRL_3,  
> > > +				  AD485X_CNTRL_3_PACKET_FORMAT_MSK,
> > > +  
> > FIELD_PREP(AD485X_CNTRL_3_PACKET_FORMAT_MSK, val));  
> > > +}
> > > +
> > >  static struct iio_buffer *axi_adc_request_buffer(struct iio_backend *back,
> > >  						 struct iio_dev *indio_dev)
> > >  {
> > > @@ -360,6 +405,7 @@ static const struct iio_backend_ops adi_axi_adc_ops  
> > = {  
> > >  	.test_pattern_set = axi_adc_test_pattern_set,
> > >  	.chan_status = axi_adc_chan_status,
> > >  	.interface_type_get = axi_adc_interface_type_get,
> > > +	.data_size_set = axi_adc_data_size_set,
> > >  	.debugfs_reg_access = iio_backend_debugfs_ptr(axi_adc_reg_access),
> > >  	.debugfs_print_chan_status =  
> > iio_backend_debugfs_ptr(axi_adc_debugfs_print_chan_status),  
> > >  };  
> > 
> > Why was [1] not addressed?
> > 
> > [1]: https://urldefense.com/v3/__https://lore.kernel.org/linux-
> > iio/9c262f599fb9b42feac99cfb541723a0a6f50e6b.camel@gmail.com/__;!!A
> > 3Ni8CS0y2Y!6uVytAwWUCsEazOUTACecMQkbMuHBF95sbla50CbTUFkZkyxS
> > -S7jMOCczpoyKCjtAKvMOyrt0ukYwcXC_l5q60$  
> 
> Indeed it was not addressed. I remained with the impression that adding part prefix
> in the macro definitions was enough. I will add the compatible string support.
> Although I have a question in order to minimize the number of versions to be sent
> In the future. Should I add a separate patch for the compatible support (which
> will not add value independently) or should I include it in this patch which adds
> custom function for data format for the AD485x IP core?

Binding docs update needs to be a separate patch.

Also, we should probably only set axi_adc_data_size_set in iio_backend_ops for
that ID.  So you'll need to pick from two copies of adi_axi_adc_ops
which probably means two iio_backend_info structures.
That data_size_set callback should not be set for cases that don't use it
(so the generic IP if I understand this correctly).

Similar to that part of:
https://lore.kernel.org/all/20250129-wip-bl-ad7606_add_backend_sw_mode-v3-7-c3aec77c0ab7@baylibre.com/

Hmm. This is looking like a messy merge.

Angelo, Antoniu,

Please figure out between you an order to the series so who is going to have
to rebase.  If this one goes first, may be worth pulling part of
patch 6 from Angelo's set to introduce struct axi_adc_info with what
this patch needs (just the backend_info pointer and maybe version?)

Thanks,

Jonathan

Angelo Dureghello Feb. 3, 2025, 5:13 p.m. UTC | #4

On 03.02.2025 15:25, Jonathan Cameron wrote:
> On Mon, 3 Feb 2025 11:02:58 +0000
> "Miclaus, Antoniu" <Antoniu.Miclaus@analog.com> wrote:
> 
> >  
> > > On 1/27/25 4:57 AM, Antoniu Miclaus wrote:  
> > > > Add support for selecting the data format within the AXI ADC ip.
> > > >
> > > > Reviewed-by: Nuno Sa <nuno.sa@analog.com>
> > > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > > > ---
> > > > no changes in v11.
> > > >  drivers/iio/adc/adi-axi-adc.c | 46  
> > > +++++++++++++++++++++++++++++++++++  
> > > >  1 file changed, 46 insertions(+)
> > > >
> > > > diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
> > > > index d2e1dc63775c..3c213ca5ff8e 100644
> > > > --- a/drivers/iio/adc/adi-axi-adc.c
> > > > +++ b/drivers/iio/adc/adi-axi-adc.c
> > > > @@ -45,6 +45,12 @@
> > > >  #define ADI_AXI_ADC_REG_CTRL			0x0044
> > > >  #define    ADI_AXI_ADC_CTRL_DDR_EDGESEL_MASK	BIT(1)
> > > >
> > > > +#define ADI_AXI_ADC_REG_CNTRL_3			0x004c
> > > > +#define   AD485X_CNTRL_3_PACKET_FORMAT_MSK	GENMASK(1, 0)
> > > > +#define   AD485X_PACKET_FORMAT_20BIT		0x0
> > > > +#define   AD485X_PACKET_FORMAT_24BIT		0x1
> > > > +#define   AD485X_PACKET_FORMAT_32BIT		0x2
> > > > +
> > > >  #define ADI_AXI_ADC_REG_DRP_STATUS		0x0074
> > > >  #define   ADI_AXI_ADC_DRP_LOCKED		BIT(17)
> > > >
> > > > @@ -312,6 +318,45 @@ static int axi_adc_interface_type_get(struct  
> > > iio_backend *back,  
> > > >  	return 0;
> > > >  }
> > > >
> > > > +static int axi_adc_data_size_set(struct iio_backend *back, unsigned int size)
> > > > +{
> > > > +	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
> > > > +	unsigned int val;
> > > > +
> > > > +	switch (size) {
> > > > +	/*
> > > > +	 * There are two different variants of the AXI AD485X IP block, a 16-bit
> > > > +	 * and a 20-bit variant.
> > > > +	 * The 0x0 value (AD485X_PACKET_FORMAT_20BIT) is corresponding  
> > > also to  
> > > > +	 * the 16-bit variant of the IP block.
> > > > +	 */
> > > > +	case 16:
> > > > +	case 20:
> > > > +		val = AD485X_PACKET_FORMAT_20BIT;
> > > > +		break;
> > > > +	case 24:
> > > > +		val = AD485X_PACKET_FORMAT_24BIT;
> > > > +		break;
> > > > +	/*
> > > > +	 * The 0x2 (AD485X_PACKET_FORMAT_32BIT) corresponds only to  
> > > the 20-bit  
> > > > +	 * variant of the IP block. Setting this value properly is ensured by
> > > > +	 * the upper layers of the drivers calling the axi-adc functions.
> > > > +	 * Also, for 16-bit IP block, the 0x2  
> > > (AD485X_PACKET_FORMAT_32BIT)  
> > > > +	 * value is handled as maximum size available which is 24-bit for this
> > > > +	 * configuration.
> > > > +	 */
> > > > +	case 32:
> > > > +		val = AD485X_PACKET_FORMAT_32BIT;
> > > > +		break;
> > > > +	default:
> > > > +		return -EINVAL;
> > > > +	}
> > > > +
> > > > +	return regmap_update_bits(st->regmap,  
> > > ADI_AXI_ADC_REG_CNTRL_3,  
> > > > +				  AD485X_CNTRL_3_PACKET_FORMAT_MSK,
> > > > +  
> > > FIELD_PREP(AD485X_CNTRL_3_PACKET_FORMAT_MSK, val));  
> > > > +}
> > > > +
> > > >  static struct iio_buffer *axi_adc_request_buffer(struct iio_backend *back,
> > > >  						 struct iio_dev *indio_dev)
> > > >  {
> > > > @@ -360,6 +405,7 @@ static const struct iio_backend_ops adi_axi_adc_ops  
> > > = {  
> > > >  	.test_pattern_set = axi_adc_test_pattern_set,
> > > >  	.chan_status = axi_adc_chan_status,
> > > >  	.interface_type_get = axi_adc_interface_type_get,
> > > > +	.data_size_set = axi_adc_data_size_set,
> > > >  	.debugfs_reg_access = iio_backend_debugfs_ptr(axi_adc_reg_access),
> > > >  	.debugfs_print_chan_status =  
> > > iio_backend_debugfs_ptr(axi_adc_debugfs_print_chan_status),  
> > > >  };  
> > > 
> > > Why was [1] not addressed?
> > > 
> > > [1]: https://urldefense.com/v3/__https://lore.kernel.org/linux-
> > > iio/9c262f599fb9b42feac99cfb541723a0a6f50e6b.camel@gmail.com/__;!!A
> > > 3Ni8CS0y2Y!6uVytAwWUCsEazOUTACecMQkbMuHBF95sbla50CbTUFkZkyxS
> > > -S7jMOCczpoyKCjtAKvMOyrt0ukYwcXC_l5q60$  
> > 
> > Indeed it was not addressed. I remained with the impression that adding part prefix
> > in the macro definitions was enough. I will add the compatible string support.
> > Although I have a question in order to minimize the number of versions to be sent
> > In the future. Should I add a separate patch for the compatible support (which
> > will not add value independently) or should I include it in this patch which adds
> > custom function for data format for the AD485x IP core?
> 
> Binding docs update needs to be a separate patch.
> 
> Also, we should probably only set axi_adc_data_size_set in iio_backend_ops for
> that ID.  So you'll need to pick from two copies of adi_axi_adc_ops
> which probably means two iio_backend_info structures.
> That data_size_set callback should not be set for cases that don't use it
> (so the generic IP if I understand this correctly).
> 
> Similar to that part of:
> https://lore.kernel.org/all/20250129-wip-bl-ad7606_add_backend_sw_mode-v3-7-c3aec77c0ab7@baylibre.com/
> 
> Hmm. This is looking like a messy merge.
> 
> Angelo, Antoniu,
> 
> Please figure out between you an order to the series so who is going to have
> to rebase.  If this one goes first, may be worth pulling part of
> patch 6 from Angelo's set to introduce struct axi_adc_info with what
> this patch needs (just the backend_info pointer and maybe version?)
>

Hi,

yes, above suggestion seems good to me.
I go on with my patchset, than i can rebase in case this go first.

Regards,
angelo
 
> Thanks,
> 
> Jonathan
>

diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index d2e1dc63775c..3c213ca5ff8e 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -45,6 +45,12 @@ 
 #define ADI_AXI_ADC_REG_CTRL			0x0044
 #define    ADI_AXI_ADC_CTRL_DDR_EDGESEL_MASK	BIT(1)
 
+#define ADI_AXI_ADC_REG_CNTRL_3			0x004c
+#define   AD485X_CNTRL_3_PACKET_FORMAT_MSK	GENMASK(1, 0)
+#define   AD485X_PACKET_FORMAT_20BIT		0x0
+#define   AD485X_PACKET_FORMAT_24BIT		0x1
+#define   AD485X_PACKET_FORMAT_32BIT		0x2
+
 #define ADI_AXI_ADC_REG_DRP_STATUS		0x0074
 #define   ADI_AXI_ADC_DRP_LOCKED		BIT(17)
 
@@ -312,6 +318,45 @@  static int axi_adc_interface_type_get(struct iio_backend *back,
 	return 0;
 }
 
+static int axi_adc_data_size_set(struct iio_backend *back, unsigned int size)
+{
+	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
+	unsigned int val;
+
+	switch (size) {
+	/*
+	 * There are two different variants of the AXI AD485X IP block, a 16-bit
+	 * and a 20-bit variant.
+	 * The 0x0 value (AD485X_PACKET_FORMAT_20BIT) is corresponding also to
+	 * the 16-bit variant of the IP block.
+	 */
+	case 16:
+	case 20:
+		val = AD485X_PACKET_FORMAT_20BIT;
+		break;
+	case 24:
+		val = AD485X_PACKET_FORMAT_24BIT;
+		break;
+	/*
+	 * The 0x2 (AD485X_PACKET_FORMAT_32BIT) corresponds only to the 20-bit
+	 * variant of the IP block. Setting this value properly is ensured by
+	 * the upper layers of the drivers calling the axi-adc functions.
+	 * Also, for 16-bit IP block, the 0x2 (AD485X_PACKET_FORMAT_32BIT)
+	 * value is handled as maximum size available which is 24-bit for this
+	 * configuration.
+	 */
+	case 32:
+		val = AD485X_PACKET_FORMAT_32BIT;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return regmap_update_bits(st->regmap, ADI_AXI_ADC_REG_CNTRL_3,
+				  AD485X_CNTRL_3_PACKET_FORMAT_MSK,
+				  FIELD_PREP(AD485X_CNTRL_3_PACKET_FORMAT_MSK, val));
+}
+
 static struct iio_buffer *axi_adc_request_buffer(struct iio_backend *back,
 						 struct iio_dev *indio_dev)
 {
@@ -360,6 +405,7 @@  static const struct iio_backend_ops adi_axi_adc_ops = {
 	.test_pattern_set = axi_adc_test_pattern_set,
 	.chan_status = axi_adc_chan_status,
 	.interface_type_get = axi_adc_interface_type_get,
+	.data_size_set = axi_adc_data_size_set,
 	.debugfs_reg_access = iio_backend_debugfs_ptr(axi_adc_reg_access),
 	.debugfs_print_chan_status = iio_backend_debugfs_ptr(axi_adc_debugfs_print_chan_status),
 };

[v11,5/8] iio: adc: adi-axi-adc: set data format

Commit Message

Comments

Patch