[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240629174003.60738d4b@jic23-huawei>
Date: Sat, 29 Jun 2024 17:40:03 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Nuno Sá <noname.nuno@...il.com>
Cc: Esteban Blanc <eblanc@...libre.com>, baylibre-upstreaming@...ups.io,
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>, Nuno
Sa <nuno.sa@...log.com>, linux-iio@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, David Lechner
<dlechner@...libre.com>
Subject: Re: [PATCH RFC 2/5] iio: adc: ad4030: add driver for ad4030-24
> > +#define AD4030_CHAN_CMO(_idx) { \
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> > + .type = IIO_VOLTAGE, \
> > + .indexed = 1, \
> > + .channel = _idx, \
> > + .scan_index = _idx, \
> > + .scan_type = { \
> > + .sign = 'u', \
> > + .storagebits = 8, \
> > + .realbits = 8, \
> > + .endianness = IIO_BE, \
> > + }, \
> > +}
> > +
>
> So, from the datasheet, figure 39 we have something like a multiplexer where we
> can have:
>
> - averaged data;
> - normal differential;
> - test pattern (btw, useful to have it in debugfs - but can come later);
Can consider doing test pattern as simply a different channel, data is afterall
coming from a very different place.
> - 8 common mode bits;
>
> While the average, normal and test pattern are really mutual exclusive, the
> common mode voltage is different in the way that it's appended to differential
> sample. Making it kind of an aggregated thingy. Thus I guess it can make sense
> for us to see them as different channels from a SW perspective (even more since
> gain and offset only apply to the differential data). But there are a couple of
> things I don't like (have concerns):
>
> * You're pushing the CM channels into the end. So when we a 2 channel device
> we'll have:
>
> in_voltage0 - diff
> in_voltage1 - diff
> in_voltage2 - CM associated with chan0
> in_voltage0 - CM associated with chan1
>
> I think we could make it so the CM channel comes right after the channel where
> it's data belongs too. So for example, odd channels would be CM channels (and
> labels could also make sense).
>
> Other thing that came to mind is if we could somehow use differential = true
> here. Having something like:
>
> in_voltage1_in_voltage0_raw - diff data
in_voltage1-voltage0_raw so normal differential channel.
> ...
> And the only thing for CM would be:
>
> in_voltage1_raw
> in_voltage1_scale
you can do that with normal channels.
>
> (not sure if the above is doable with ext_info - maybe only with device_attrs)
We 'could' do the somewhat nasty
in_voltage1-voltage0_raw - so normal differential channel.
in_voltage1+voltage0_raw via a custom attribute. Don't try to hammer that in
as channel related though.
>
> The downside of the above is that we don't have a way to separate the scan
> elements. Meaning that we don't have a way to specify the scan_type for both the
> common mode and differential voltage. That said, I wonder if it is that useful
> to buffer the common mode stuff? Alternatively, we could just have the scan_type
> for the diff data and apps really wanting the CM voltage could still access the
> raw data. Not pretty, I know...
Hmm. That is indeed a nasty corner if we don't use a real channel.
This may be a case for just using labels for the relationship.
>
> However, even if we go with the two separate channels there's one thing that
> concerns me. Right now we have diff data with 32 for storage bits and CM data
> with 8 storage bits which means the sample will be 40 bits and in reality we
> just have 32. Sure, now we have SW buffering so we can make it work but the
> ultimate goal is to support HW buffering where we won't be able to touch the
> sample and thus we can't lie about the sample size. Could you run any test with
> this approach on a HW buffer setup?
>
> I did not gave too much thought on it but I'm not sure there's a sane way to
> have multiple scan_types associated with the same channel.
Not at the same time.
Jonathan
Powered by blists - more mailing lists