linux-kernel - Re: [PATCH v6 7/8] iio: dac: ad3552r: add high-speed platform driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20241019162437.500b6ac4@jic23-huawei>
Date: Sat, 19 Oct 2024 16:24:37 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Angelo Dureghello <adureghello@...libre.com>
Cc: Nuno Sá <nuno.sa@...log.com>, Lars-Peter Clausen
 <lars@...afoo.de>, Michael Hennerich <Michael.Hennerich@...log.com>, Rob
 Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor
 Dooley <conor+dt@...nel.org>, Olivier Moysan <olivier.moysan@...s.st.com>,
 linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, dlechner@...libre.com, Mark Brown
 <broonie@...nel.org>
Subject: Re: [PATCH v6 7/8] iio: dac: ad3552r: add high-speed platform
 driver

On Mon, 14 Oct 2024 12:08:13 +0200
Angelo Dureghello <adureghello@...libre.com> wrote:

> From: Angelo Dureghello <adureghello@...libre.com>
> 
> Add High Speed ad3552r platform driver.
> 
> The ad3552r DAC is controlled by a custom (fpga-based) DAC IP
> through the current AXI backend, or similar alternative IIO backend.
> 
> Compared to the existing driver (ad3552r.c), that is a simple SPI
> driver, this driver is coupled with a DAC IIO backend that finally
> controls the ad3552r by a fpga-based "QSPI+DDR" interface, to reach
> maximum transfer rate of 33MUPS using dma stream capabilities.
> 
> All commands involving QSPI bus read/write are delegated to the backend
> through the provided APIs for bus read/write.
> 
> Signed-off-by: Angelo Dureghello <adureghello@...libre.com>
Hi Angelo,

You have lots of review already. Just one trivial addition from me.

Jonathan

> diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
> new file mode 100644
> index 000000000000..cb29a600e141
> --- /dev/null
> +++ b/drivers/iio/dac/ad3552r-hs.c
> @@ -0,0 +1,526 @@

> +static int ad3552r_hs_buffer_postenable(struct iio_dev *indio_dev)
> +{

> +
> +	switch (*indio_dev->active_scan_mask) {
> +	case AD3552R_CH0_ACTIVE:
> +		st->single_channel = true;
> +		loop_len = 2;
> +		val = AD3552R_REG_ADDR_CH_DAC_16B(0);
> +		break;
> +	case AD3552R_CH1_ACTIVE:
> +		st->single_channel = true;
> +		loop_len = 2;
> +		val = AD3552R_REG_ADDR_CH_DAC_16B(1);
> +		break;
> +	case AD3552R_CH0_CH1_ACTIVE:
	case AD3552R_CH0_ACTIVE | AD3552R_CH1_ACTIVE:


> diff --git a/drivers/iio/dac/ad3552r.h b/drivers/iio/dac/ad3552r.h
> index 088eb8ecfac6..fc00ed4c2565 100644
> --- a/drivers/iio/dac/ad3552r.h
> +++ b/drivers/iio/dac/ad3552r.h

> @@ -129,6 +131,11 @@
>  #define AD3552R_GAIN_SCALE				1000
>  #define AD3552R_LDAC_PULSE_US				100
>  
> +#define AD3552R_CH0_ACTIVE				BIT(0)
> +#define AD3552R_CH1_ACTIVE				BIT(1)
> +#define AD3552R_CH0_CH1_ACTIVE				(AD3552R_CH0_ACTIVE | \
> +							 AD3552R_CH1_ACTIVE)

I'd just put that one inline in the case statement.

> +
>  #define AD3552R_MAX_RANGES	5
>  #define AD3542R_MAX_RANGES	6
>  
>