Re: [PATCH 3/3] iio: cdc: add Texas Instruments FDC1004 driver

[Victor Chernyakin]

Anything I don't reply to I've addressed in v2.


> Given offset is applied prior to scale. Is this in appropriate
> units?  I'm not following that description.

I believe the units are correct, but I've changed the wording so
it's hopefully more clear what's going on.

> common convention to add the error condition to the exit one rather than do it
> in here.  Then check ret after the exit from teh poll.
> Tends to give nice code to read.

I think I understand the change you want, but please check v2 to make sure.

> For a capacitance channel I'm going to guess that expectation is
> wiring is such that only a subset of channels matter for a given
> configuration?  It may be more user friendly to put that in
> dt (see how we do it in adc/adc.yaml for instance).

Does it buy us much to not expose disconnected channels? Whatever code is
reading from the sensor presumably has knowledge of the configuration
anyway (otherwise, how would it know what "channel 1" even represents?), so
does it need to be protected from reading the disconnected channels?

> Interesting that you have these only for the single ends.  I guess they
> are tweaks on the analog side?

Well, the sensor doesn't allow you to set calibration values on
a differential channel, only separately on the two channels involved.

> Putting that aside, the base unit of capacitance in the ABI
> is a nano farad. The part is documented at 0.5 femto farad
> resolution so I'd kind of expect a scale of only 0.5e-6.
>
> Perhaps the issue here is desire for a very high degree of
> precision on the scale factors?  I don't mind that if it
> is useful but with a documented 0.2% gain error I'm not
> sure it is.  For next version add some reasoning for
> why such high precision is useful.
>
> We frequently round scale factors as often they are swamped
> by error / noise.  Perhaps that doesn't apply here.

I think you're right that in this case the extra precision doesn't really
make a practical difference, but does it hurt? We have to use *some* unit,
so why not just use one that doesn't introduce rounding? To me, that seems
like the default choice. It's rounding that I would expect to require
special justification (like what you're doing by citing the resolution and
error values).

Even if we decide that a given amount of rounding is okay for *this*
sensor, imagine TI releases a new one that's a drop-in replacement but
with better precision. We can't be sure that the rounding is still okay
for this new sensor, unless we don't do any rounding in the first place.

> I'd look at using regmap even though you only have a single bus. It provides
> a rich set of helpers and the caching functionality that will save a bunch
> of bus traffic.

Ah... is that a strong preference in new drivers? That would be quite
a bit of rewrite work...

> Why not build these of channel and channel 2 at the place where you need them?
> This feels unnecessarily complex in this structure. Much better where we can
> see the reason to do this used.

That's fair, changed it to what you suggest.