| 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
| ||
|
Message-ID: <8f869b3b-df3f-49a9-9b6e-640697aa91dd@wanadoo.fr>
Date: Sat, 14 Sep 2024 20:21:56 +0200
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: Mariel Tinaco <Mariel.Tinaco@...log.com>
Cc: linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, Jonathan Cameron <jic23@...nel.org>,
Lars-Peter Clausen <lars@...afoo.de>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Michael Hennerich <Michael.Hennerich@...log.com>,
Conor Dooley <conor+dt@...nel.org>,
Marcelo Schmitt <marcelo.schmitt1@...il.com>,
Dimitri Fedrau <dima.fedrau@...il.com>, David Lechner
<dlechner@...libre.com>, Nuno Sá <noname.nuno@...il.com>
Subject: Re: [PATCH v4 2/2] iio: dac: support the ad8460 Waveform DAC
Le 12/09/2024 à 11:54, Mariel Tinaco a écrit :
> The AD8460 is a “bits in, power out” high voltage, high-power,
> high-speed driver optimized for large output current (up to ±1 A)
> and high slew rate (up to ±1800 V/μs) at high voltage (up to ±40 V)
> into capacitive loads.
>
> 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 ±55 V and a single low voltage
> supply of 5 V.
>
> Signed-off-by: Mariel Tinaco <Mariel.Tinaco-OyLXuOCK7orQT0dZR+AlfA@...lic.gmane.org>
> ---
Hi,
...
> +#define AD8460_CHAN_EXT_INFO(_name, _what, _read, _write) { \
> + .name = _name, \
> + .read = (_read), \
> + .write = (_write), \
> + .private = (_what), \
Why () for _read, _write, _what?
(or why no () for _name?)
> + .shared = IIO_SEPARATE, \
> +}
> +
> +static struct iio_chan_spec_ext_info ad8460_ext_info[] = {
I think this can be static const struct.
> + AD8460_CHAN_EXT_INFO("raw0", 0, ad8460_dac_input_read,
> + ad8460_dac_input_write),
> + AD8460_CHAN_EXT_INFO("raw1", 1, ad8460_dac_input_read,
> + ad8460_dac_input_write),
> + AD8460_CHAN_EXT_INFO("raw2", 2, ad8460_dac_input_read,
> + ad8460_dac_input_write),
> + AD8460_CHAN_EXT_INFO("raw3", 3, ad8460_dac_input_read,
> + ad8460_dac_input_write),
> + AD8460_CHAN_EXT_INFO("raw4", 4, ad8460_dac_input_read,
> + ad8460_dac_input_write),
> + AD8460_CHAN_EXT_INFO("raw5", 5, ad8460_dac_input_read,
> + ad8460_dac_input_write),
> + AD8460_CHAN_EXT_INFO("raw6", 6, ad8460_dac_input_read,
> + ad8460_dac_input_write),
> + AD8460_CHAN_EXT_INFO("raw7", 7, ad8460_dac_input_read,
> + ad8460_dac_input_write),
> + AD8460_CHAN_EXT_INFO("raw8", 8, ad8460_dac_input_read,
> + ad8460_dac_input_write),
> + AD8460_CHAN_EXT_INFO("raw9", 9, ad8460_dac_input_read,
> + ad8460_dac_input_write),
> + AD8460_CHAN_EXT_INFO("raw10", 10, ad8460_dac_input_read,
> + ad8460_dac_input_write),
> + AD8460_CHAN_EXT_INFO("raw11", 11, ad8460_dac_input_read,
> + ad8460_dac_input_write),
> + AD8460_CHAN_EXT_INFO("raw12", 12, ad8460_dac_input_read,
> + ad8460_dac_input_write),
> + AD8460_CHAN_EXT_INFO("raw13", 13, ad8460_dac_input_read,
> + ad8460_dac_input_write),
> + AD8460_CHAN_EXT_INFO("raw14", 14, ad8460_dac_input_read,
> + ad8460_dac_input_write),
> + AD8460_CHAN_EXT_INFO("raw15", 15, ad8460_dac_input_read,
> + ad8460_dac_input_write),
> + AD8460_CHAN_EXT_INFO("toggle_en", 0, ad8460_read_toggle_en,
> + ad8460_write_toggle_en),
> + AD8460_CHAN_EXT_INFO("symbol", 0, ad8460_read_symbol,
> + ad8460_write_symbol),
> + AD8460_CHAN_EXT_INFO("powerdown", 0, ad8460_read_powerdown,
> + ad8460_write_powerdown),
> + IIO_ENUM("powerdown_mode", IIO_SEPARATE, &ad8460_powerdown_mode_enum),
> + IIO_ENUM_AVAILABLE("powerdown_mode", IIO_SHARED_BY_TYPE,
> + &ad8460_powerdown_mode_enum),
> + {}
> +};
...
> +static int ad8460_probe(struct spi_device *spi)
> +{
> + struct ad8460_state *state;
> + struct iio_dev *indio_dev;
> + struct device *dev;
> + u32 tmp[2], temp;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*state));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + state = iio_priv(indio_dev);
> +
> + indio_dev->name = "ad8460";
> + indio_dev->info = &ad8460_info;
> +
> + state->spi = spi;
> + dev = &spi->dev;
> +
> + state->regmap = 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");
> +
> + devm_mutex_init(dev, &state->lock);
> +
> + state->sync_clk = 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 = devm_iio_channel_get(dev, "ad8460-tmp");
> + if (IS_ERR(state->tmp_adc_channel)) {
> + if (PTR_ERR(state->tmp_adc_channel) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> + indio_dev->channels = ad8460_channels;
> + indio_dev->num_channels = ARRAY_SIZE(ad8460_channels);
> + } else {
> + indio_dev->channels = ad8460_channels_with_tmp_adc;
> + indio_dev->num_channels = ARRAY_SIZE(ad8460_channels_with_tmp_adc);
> + }
> +
> + ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(ad8460_supplies),
> + ad8460_supplies);
> + if (ret) {
> + dev_err(dev, "Failed to enable power supplies\n");
> + return ret;
Nitpick: return dev_err_probe() as done in other places?
> + }
> +
> + ret = devm_regulator_get_enable_read_voltage(dev, "refio_1p2v");
> + if (ret < 0 && ret != -ENODEV)
> + return dev_err_probe(dev, ret, "Failed to get reference voltage\n");
> +
> + state->refio_1p2v_mv = ret == -ENODEV ? 1200 : ret / 1000;
...
CJ
Powered by blists - more mailing lists