Re: [PATCH] iio: pll: New driver for ADF4350/ADF4351 Wideband Synthesizers (PLL)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Dec 21 2011, Michael Hennerich wrote:

On 12/20/2011 12:45 PM, J.I. Cameron wrote:
On Dec 20 2011, Michael Hennerich wrote:

On 12/14/2011 09:29 PM, Jonathan Cameron wrote:
On 12/12/2011 10:17 AM, Michael Hennerich wrote:
On 12/10/2011 03:41 PM, Jonathan Cameron wrote:
On 12/09/2011 01:22 PM, michael.hennerich@xxxxxxxxxx wrote:

There is also the double indexes that we don't allow anywhere else.
They are here because we have different waveforms with same underlying
channel.  We could had a bonus optional index to all channels I suppose
and have this as the only current user?  Or maybe just a flag to say its
a 'subchannel' and use channel2.  Afterall these channels are neither
going to have modifiers or to be differential so I can't see a clash
occuring on the use of that...
Well one of the indexes with the DDS parts is used to distinguish
the tuning words options, in combination with the pincontrol feature.
PLL devices typically doesn't feature this.

So bottom line - can we agree on:

out_altvoltageX_freqY
out_altvoltageX_phaseY
out_altvoltageX_scale
out_altvoltageX_enable
...
Where X stands for the output channel, and Y for some sort of option.
Looks good as far as I am concerned. Might be worth a quick check through
of dds and meter devices to see how we can extend this to cover them.

Hi Jonathan,

Looked at the various use cases. And the only user for the second
subindex seems to be the DDS parts.

I now wondering if we should handle freqY and phaseY by using extended_name,
rather than adding new postfixes for them. Which is going to be difficult anyways,
Use extend name will make for some hideous logic in the read_raw, write_raw
functions.  If we need it (and it looks like we do), lets make whatever
changes are necessary to do this cleanly in the chan spec structure.
I'd been avoiding the DDS parts so far because I have little idea of
how to make them play nicely with iio_chan_spec.

Ideally freq and phase would be chaninfo elements. Given that is a mask
there is no obvious way of storing the associated index.  This also effects
the write_raw and read_raw functions as they have no way to get to that index
either.  That one could be avoided by changing them to take a u64 and embedding
the index in a few bits of that
(fiddly carrying that through everywhere but not too bad).

We could simply define a set of freq0 freq1 freq2 as separate elements of the
info mask, but that' really nasty and going to bite us at some point.

How about adding (with better naming);

int *infomask_nums;
//could drop next one if we define a magic termination value.
int infomask_nums_number;
to iio_chan_spec.

where for example

static int inf_mask_nums[] = {
[IIO_CHAN_INFO_FREQUENCY] = 7;
...
};
static struct iio_chan_spec bobschannels[] = {
{
 ...usual stuff.
 .info_mask = IIO_CHAN_INFO_FREQUENCY_SEPARATE_BIT,
 .infomask_nums = inf_mask_nums,
 .infomask_nums_number = ARRAY_SIZE(inf_mask_nums),
}
};

The parsing logic then create

in_accelX_freq0...in_accelX_freq6

This functionality may also be useful for filter taps...

Obviously the above creates large tables, so for space saving we
may want to use a structure pairing IIO_CHAN_INFO_WWWWW with the
number.  Can terminate that with a 0,0 entry and loose the counter
as well.  (I like this version better come to think of it).

One other thought is that we may run into issues with the number
of elements in the infomask.  Only split I can think of is
to put the shared elements in one mask and the separate ones in another.

since I may need the current postfixes such as _scale as well.

out_altvoltageX_freqY_scale
out_altvoltageX_phaseY_scale
My initial thought on this is that these are ulikely to be written
in a particularly fast path, so we could avoid scale etc for them by
dictating that they are written in the correct units?  We already do
this for things like sampling frequency.

If we take things to this many levels it is going to get very confusing.
The raw and scale stuff only makes sense for stuff on fast paths.
(Now I expect you will come up with an application where this is
pushed from a buffer just to prove me wrong ;)

An alternative could be to introduce an indexed extended_name, using the index passed in channel2?
Not sure if it is worth adding it?
I would rather not as stated above.  Handling logic will rely on reading that name
which we have been desperately avoiding so far.




--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux