linux-kernel - Re: [PATCH v2 3/9] iio: backend adi-axi-dac: extend features

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240908161136.501a9c86@jic23-huawei>
Date: Sun, 8 Sep 2024 16:11:36 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Angelo Dureghello <adureghello@...libre.com>
Cc: Lars-Peter Clausen <lars@...afoo.de>, Michael Hennerich
 <Michael.Hennerich@...log.com>, Nuno Sá <nuno.sa@...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, David Lechner
 <dlechner@...libre.com>
Subject: Re: [PATCH v2 3/9] iio: backend adi-axi-dac: extend features

On Thu, 05 Sep 2024 17:17:33 +0200
Angelo Dureghello <adureghello@...libre.com> wrote:

> From: Angelo Dureghello <adureghello@...libre.com>
> 
> Extend DAC backend with new features required for the AXI driver
> version for the ad3552r DAC. Mainly, a new compatible string has
> been added to support a DAC IP very similar to the generic DAC IP
> but with some customizations to work with the ad3552r.
> 
> Then, a serie of generic functions has been added to match with
> ad3552r needs. Function names has been kept generic as much as
> possible, to allow re-utilization from other frontend drivers.
> 
> Signed-off-by: Angelo Dureghello <adureghello@...libre.com>
> Co-developed-by: David Lechner <dlechner@...libre.com>
> Co-developed-by: Nuno Sá <nuno.sa@...log.com>
A few comments below.

> ---
>  drivers/iio/dac/adi-axi-dac.c | 267 ++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 257 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/iio/dac/adi-axi-dac.c b/drivers/iio/dac/adi-axi-dac.c
> index 0cb00f3bec04..cc31e1dcd1df 100644
> --- a/drivers/iio/dac/adi-axi-dac.c
> +++ b/drivers/iio/dac/adi-axi-dac.c
> @@ -44,11 +44,34 @@
>  #define   AXI_DAC_RSTN_MMCM_RSTN	BIT(1)
>  #define   AXI_DAC_RSTN_RSTN		BIT(0)
>  #define AXI_DAC_REG_CNTRL_1		0x0044
> +#define   AXI_DAC_EXT_SYNC_ARM		BIT(1)
> +#define   AXI_DAC_EXT_SYNC_DISARM	BIT(2)
>  #define   AXI_DAC_SYNC			BIT(0)
>  #define AXI_DAC_REG_CNTRL_2		0x0048
> -#define	  ADI_DAC_R1_MODE		BIT(4)
> +#define   AXI_DAC_SDR_DDR_N		BIT(16)
> +#define   AXI_DAC_SYMB_8B		BIT(14)
> +#define	  ADI_DAC_R1_MODE		BIT(5)

Bug?  Either was wrong before or after this change. I've no
idea which.


> +
> +static int axi_dac_bus_reg_write(struct iio_backend *back, u32 reg,
> +				 unsigned int val, size_t data_size)
> +{
> +	struct axi_dac_state *st = iio_backend_get_priv(back);
> +
> +	switch (st->info->bus_type) {
> +	case AXI_DAC_BUS_TYPE_DDR_QSPI: {

...

> +
> +		return regmap_clear_bits(st->regmap, AXI_DAC_REG_CUSTOM_CTRL,
> +					  AXI_DAC_TRANSFER_DATA);
> +		}
> +		break;

Can't get here so drop the break;


> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int axi_dac_bus_reg_read(struct iio_backend *back, u32 reg,
> +				unsigned int *val, size_t data_size)
> +{
> +	struct axi_dac_state *st = iio_backend_get_priv(back);
> +
> +	switch (st->info->bus_type) {
> +	case AXI_DAC_BUS_TYPE_DDR_QSPI: {
> +		int ret;
> +		u32 bval;
> +
> +		bval = 0;
> +		ret = axi_dac_bus_reg_write(back, AXI_DAC_RD_ADDR(reg), 0,
> +					    data_size);
> +		if (ret)
> +			return ret;
> +
> +		ret = regmap_read_poll_timeout(st->regmap, AXI_DAC_UI_STATUS,
> +					       bval, bval != AXI_DAC_BUSY,
> +					       10, 100);
> +		if (ret)
> +			return ret;
> +
> +		return regmap_read(st->regmap, AXI_DAC_CNTRL_DATA_RD, val);
> +		}
> +		break;
Can't get here.

> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}