From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 F243115D5BB; Sat, 7 Sep 2024 17:15:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725729313; cv=none; b=jLU+v1P/DvNt8EHueRyKEgkfL8cDqdV0pplSYNl72Hz5JLHsOMP4RZ6tMa5PAup78HH8GBmgUh7NWmrTjElKvxjIln/zPL3w8TXz6bdmEFWj3pRdTK1o2dXZnXD5apL3JhtrtKQe5RrfQwPSSfqXFjlHLIWRfxFo+jxI6SItV5E= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725729313; c=relaxed/simple; bh=tcjsLNQyJNWKbgceFHvvMiUd+8SAPr95G7SirxnFvyQ=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=nVRQdOGU/fWf1ovS94zwx8Bw8k5xzAlPwZrVEM4y/2nhXSCcCMjHRQB2lh9R4OAnUngIvRNCNNvUciAeYMTTDTRQClf2PTzG6a1pNSSErEBlTuIsBEGM27pZhz6U7FQchlia3tebnI7krhfCw5ht6+KKUrwqOf61RTVhRtn30Ak= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=HsokSaVz; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="HsokSaVz" Received: by smtp.kernel.org (Postfix) with ESMTPSA id E0F83C4CEC2; Sat, 7 Sep 2024 17:15:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1725729312; bh=tcjsLNQyJNWKbgceFHvvMiUd+8SAPr95G7SirxnFvyQ=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=HsokSaVzpFt5vwQ3cyd0lZcxahqzecC6z/NlNokz3ietSnA5+TVLJNJll+GWhjWyy EsDG1R21gwh9vWLWu0rHEx6daVbYWvNP+qDhZzLo2eba8tjMep9/pxPjb7bVk7Bz5s Zi0lEPyu79aueh6A7d8KR9SgxFVjkBb0tUohhf6CttPRKuvFNcNxarhdPMHIiX2UkL AlwDfUdLX0I9nPpqXztUliu8ZmJl8KLrosblAb6AV06Z+VTknehqzsSkH2ypY6jDQM q5VAdQE0KLQ7tyUWprB2/31s72/Zp0DZfTn34s/dKOrsLc+6Q67LK/z28IPrzWQdEf kRwi12gy5iNdA== Date: Sat, 7 Sep 2024 18:14:49 +0100 From: Jonathan Cameron To: Mariel Tinaco Cc: , , , Lars-Peter Clausen , Rob Herring , Krzysztof Kozlowski , Michael Hennerich , Conor Dooley , Marcelo Schmitt , Dimitri Fedrau , David Lechner , Nuno =?UTF-8?B?U8Oh?= Subject: Re: [PATCH v3 2/2] iio: dac: support the ad8460 Waveform DAC Message-ID: <20240907181449.1453bd41@jic23-huawei> In-Reply-To: <20240904023040.23352-3-Mariel.Tinaco@analog.com> References: <20240904023040.23352-1-Mariel.Tinaco@analog.com> <20240904023040.23352-3-Mariel.Tinaco@analog.com> X-Mailer: Claws Mail 4.3.0 (GTK 3.24.43; x86_64-pc-linux-gnu) Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On Wed, 4 Sep 2024 10:30:40 +0800 Mariel Tinaco wrote: > The AD8460 is a =E2=80=9Cbits in, power out=E2=80=9D high voltage, high-p= ower, > high-speed driver optimized for large output current (up to =C2=B11 A) > and high slew rate (up to =C2=B11800 V/=CE=BCs) at high voltage (up to = =C2=B140 V) > into capacitive loads. >=20 > A digital engine implements user-configurable features: modes for > digital input, programmable supply current, and fault monitoring > and programmable protection settings for output current, > output voltage, and junction temperature. The AD8460 operates on > high voltage dual supplies up to =C2=B155 V and a single low voltage > supply of 5 V. >=20 > Signed-off-by: Mariel Tinaco A few comments inline. Jonathan > obj-$(CONFIG_ADI_AXI_DAC) +=3D adi-axi-dac.o > diff --git a/drivers/iio/dac/ad8460.c b/drivers/iio/dac/ad8460.c > new file mode 100644 > index 000000000000..6428bfaf0bd7 > --- /dev/null > +++ b/drivers/iio/dac/ad8460.c > @@ -0,0 +1,932 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * AD8460 Waveform generator DAC Driver > + * > + * Copyright (C) 2024 Analog Devices, Inc. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include Given there are lots of IIO specific includes, probably a case where pulling them out as a separate block after the main includes makes sense. > +#include > +#include > +#include > +#include > +#include > +#include > ... > + > + state =3D iio_priv(indio_dev); > + mutex_init(&state->lock); Trivial but there is no a devm_mutex_init() that deals with the obscure debug case where mutex uninit makes sense. Might as well use it here. > + > + indio_dev->name =3D "ad8460"; > + indio_dev->info =3D &ad8460_info; > + > + state->spi =3D spi; > + dev =3D &spi->dev; > + > + state->regmap =3D devm_regmap_init_spi(spi, &ad8460_regmap_config); > + if (IS_ERR(state->regmap)) > + return dev_err_probe(dev, PTR_ERR(state->regmap), > + "Failed to initialize regmap"); > + > + state->sync_clk =3D devm_clk_get_enabled(dev, NULL); > + if (IS_ERR(state->sync_clk)) > + return dev_err_probe(dev, PTR_ERR(state->sync_clk), > + "Failed to get sync clk\n"); > + > + state->tmp_adc_channel =3D devm_iio_channel_get(dev, "ad8460-tmp"); > + if (IS_ERR_OR_NULL(state->tmp_adc_channel)) { > + indio_dev->channels =3D ad8460_channels; > + indio_dev->num_channels =3D ARRAY_SIZE(ad8460_channels); > + } else { > + indio_dev->channels =3D ad8460_channels_with_tmp_adc; > + indio_dev->num_channels =3D ARRAY_SIZE(ad8460_channels_with_tmp_adc); > + } > + Add and enable the various other supplies. They are probably always on in which case the regulator framework will work it's magic to avoid use having to care that they aren't in the dts. > + ret =3D devm_regulator_get_enable_read_voltage(dev, "refio_1p2v"); > + if (ret =3D=3D -ENODEV) > + state->refio_1p2v_mv =3D 1200; > + else if (ret >=3D 0) > + state->refio_1p2v_mv =3D ret / 1000; > + else > + return dev_err_probe(dev, ret, "Failed to get reference voltage\n"); > + ... > + ret =3D device_property_read_u32_array(dev, "adi,range-microamp", > + tmp, ARRAY_SIZE(tmp)); > + if (!ret) { > + if (in_range(tmp[1], 0, AD8460_ABS_MAX_OVERCURRENT_UA)) > + regmap_write(state->regmap, AD8460_CTRL_REG(0x08), > + FIELD_PREP(AD8460_FAULT_ARM_MSK, 1) | > + AD8460_CURRENT_LIMIT_CONV(tmp[1])); Your binding has a fixed set of values. Yet this is anything in range. Which is correct? Based on datasheet I think it is flexible but that you have listed the example values instead of the limits. > + > + if (in_range(tmp[0], -AD8460_ABS_MAX_OVERCURRENT_UA, 0)) > + regmap_write(state->regmap, AD8460_CTRL_REG(0x09), > + FIELD_PREP(AD8460_FAULT_ARM_MSK, 1) | > + AD8460_CURRENT_LIMIT_CONV(abs(tmp[0]))); > + } > + > + ret =3D device_property_read_u32_array(dev, "adi,range-microvolt", > + tmp, ARRAY_SIZE(tmp)); > + if (!ret) { > + if (in_range(tmp[1], 0, AD8460_ABS_MAX_OVERVOLTAGE_UV)) > + regmap_write(state->regmap, AD8460_CTRL_REG(0x0A), > + FIELD_PREP(AD8460_FAULT_ARM_MSK, 1) | > + AD8460_VOLTAGE_LIMIT_CONV(tmp[1])); > + > + if (in_range(tmp[0], -AD8460_ABS_MAX_OVERVOLTAGE_UV, 0)) > + regmap_write(state->regmap, AD8460_CTRL_REG(0x0B), > + FIELD_PREP(AD8460_FAULT_ARM_MSK, 1) | > + AD8460_VOLTAGE_LIMIT_CONV(abs(tmp[0]))); > + } > + > + ret =3D device_property_read_u32(dev, "adi,max-millicelsius", &temp); > + if (!ret) { > + if (in_range(temp, AD8460_MIN_OVERTEMPERATURE_MC, > + AD8460_MAX_OVERTEMPERATURE_MC)) > + regmap_write(state->regmap, AD8460_CTRL_REG(0x0C), > + FIELD_PREP(AD8460_FAULT_ARM_MSK, 1) | > + AD8460_TEMP_LIMIT_CONV(abs(temp))); > + } > + > + ret =3D ad8460_reset(state); > + if (ret) > + return ret; > + > + /* Enables DAC by default */ > + ret =3D regmap_update_bits(state->regmap, AD8460_CTRL_REG(0x01), > + AD8460_HVDAC_SLEEP_MSK, > + FIELD_PREP(AD8460_HVDAC_SLEEP_MSK, 0)); > + if (ret) > + return ret; > + > + indio_dev->modes =3D INDIO_DIRECT_MODE; > + indio_dev->setup_ops =3D &ad8460_buffer_setup_ops; > + > + ret =3D devm_iio_dmaengine_buffer_setup_ext(dev, indio_dev, "tx", > + IIO_BUFFER_DIRECTION_OUT); > + if (ret) > + return dev_err_probe(dev, ret, > + "Failed to get DMA buffer\n"); > + > + return devm_iio_device_register(dev, indio_dev); > +}