> -----Original Message-----
> From: Jonathan Cameron <jic23@xxxxxxxxxx>
> Sent: Saturday, September 28, 2024 10:20 PM
> To: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>
> Cc: Tinaco, Mariel <Mariel.Tinaco@xxxxxxxxxx>; linux-iio@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Lars-Peter Clausen
> <lars@xxxxxxxxxx>; Rob Herring <robh@xxxxxxxxxx>; Krzysztof Kozlowski
> <krzk+dt@xxxxxxxxxx>; Hennerich, Michael
> <Michael.Hennerich@xxxxxxxxxx>; Conor Dooley <conor+dt@xxxxxxxxxx>;
> Marcelo Schmitt <marcelo.schmitt1@xxxxxxxxx>; Dimitri Fedrau
> <dima.fedrau@xxxxxxxxx>; David Lechner <dlechner@xxxxxxxxxxxx>; Nuno Sá
> <noname.nuno@xxxxxxxxx>
> Subject: Re: [PATCH v4 2/2] iio: dac: support the ad8460 Waveform DAC
>
> [External]
>
> On Sat, 14 Sep 2024 20:21:56 +0200
> Christophe JAILLET <christophe.jaillet@xxxxxxxxxx> wrote:
>
> > 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@xxxxxxxxxxxxxxxx>
> > > ---
>
> Rather than go around again, I fixed up all the comments made and the
> autobuilder issues then applied this.
>
> Diff follows. The only bit I'm not 100% sure on was your intent with the
> temperature channel. I've made it an input but shout if I'm missing something.
>
> With this diff applied on top, applied to the togreg branch of iio.git which is
> only pushed out as testing for now as I'll rebase on rc1 once available.
>
> Thanks,
>
> Jonathan
>
Hi Jonathan,
Thank you for finding the time to fix up the inline comments from the
previous rounds! I have created a patch for that but was unable to send it
yet because I'm still clueless about the temp channel. Apologies about that
About the temperature channel, it does make sense to set it as input since the
value is read-only. About the implementation of the channel, I'm wondering
what would happen if the consumer-get-channel would throw -EPROBE_DEFER
half the time? Is it not possible to skip it over since the channel is optional
anyway? Or does this defer error from the consumer mean that the other
configurations for the succeeding attributes will be blocked, which is why we
have to return probe instantly?
state->tmp_adc_channel = devm_iio_channel_get(dev, "ad8460-tmp");
if (IS_ERR(state->tmp_adc_channel)) {
state->tmp_adc_channel = NULL;
indio_dev->channels = ad8460_channels;
indio_dev->num_channels = ARRAY_SIZE(ad8460_channels);
} else {
ret = iio_get_channel_type(state->tmp_adc_channel, &temp);
if (ret < 0)
return ret;
if (temp != IIO_TEMP)
return dev_err_probe(dev, -EINVAL,
"Incompatible channel type %d\n", temp);
indio_dev->channels = ad8460_channels_with_tmp_adc;
indio_dev->num_channels = ARRAY_SIZE(ad8460_channels_with_tmp_adc);
}
I also found other implementations where the type of channel is checked. Thought
That maybe it's a good addition for security.
Thanks,
Mariel
