Hi Jonathan,
> -----Original Message-----
> From: Jonathan Cameron <jic23@xxxxxxxxxx>
> Sent: Thursday, October 28, 2021 12:31 PM
> To: Miclaus, Antoniu <Antoniu.Miclaus@xxxxxxxxxx>
> Cc: robh+dt@xxxxxxxxxx; linux-iio@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Sa, Nuno
> <Nuno.Sa@xxxxxxxxxx>; Lars-Peter Clausen <lars@xxxxxxxxxx>
> Subject: Re: [PATCH v2 1/2] iio: frequency: admv1013: add support for
> ADMV1013
>
> On Thu, 28 Oct 2021 10:08:08 +0000
> "Miclaus, Antoniu" <Antoniu.Miclaus@xxxxxxxxxx> wrote:
>
> > Hello Jonathan,
> >
> > Thanks for the review!
> >
> > Regarding the interface for the Mixer Offset adjustments:
> > ADMV1013_MIXER_OFF_ADJ_P
> > ADMV1013_MIXER_OFF_ADJ_N
> >
> > These parameters are related to the LO feedthrough offset
> calibration.
> > (LO and sideband suppression)
> >
> > On this matter, my suggestion would be to add IIO calibration types,
> something like:
> > IIO_CHAN_INFO_CALIBFEEDTROUGH_POS
> > IIO_CHAN_INFO_CALIBFEEDTROUGH_NEG
>
> These sound too specific to me - we want an interface that is
> potentially useful
> in other places. They are affecting the 'channel' which here is
> simply an alt voltage channel, but I'm assuming it's something like
> separate analog tweaks to the positive and negative of the differential
> pair?
That's also my understanding. This kind of calibration seems to be very
specific for TX local oscillators...
> Current channel is represented as a single index, but one route to this
> would be
> to have it as a differential pair.
>
> out_altvoltage0-1_phase
> for the attribute that applies at the level of the differential pair and
>
> out_altvoltage0_calibbias
> out_altvoltage1_calibbias
> For the P and N signal specific attributes.
Honestly, I'm not very enthusiastic with having out_altvoltage{0|1} as the
this applies to a single channel... Having this with separate indexes feels
odd to me. Even though we have the phase with "0-1", this can be a place
for misuse and confusion...
I was thinking about modifiers (to kind of represent differential channels)
but I don't think it would work out here... What about IIO_CHAN_INFO_CALIBBIAS_P
and IIO_CHAN_INFO_CALIBBIAS_N? Or maybe just something like
IIO_CHAN_INFO_CALIBBIAS_DIFF and internally in IIO we would automatically
create both P and N sysfs files since I don't think it makes senses in any case to
just define one of the calibrations... Anyways, these are my 5 cents :)
- Nuno Sá
