Re: [RFC] iio: amplifiers: New driver for AD8366 Dual-Digital Variable Gain Amplifier

[Michael Hennerich <michael.hennerich@analog.com>]

On 05/08/2012 04:53 PM, Jonathan Cameron wrote:
> On 5/8/2012 3:48 PM, Michael Hennerich wrote:
>> On 05/08/2012 03:32 PM, Jonathan Cameron wrote:
>>> On 5/8/2012 2:26 PM, Hennerich, Michael wrote:
>>>> Jonathan Cameron wrote on 2012-05-08:
>>>>> On 5/7/2012 4:17 PM, Michael Hennerich wrote:
>>>>>> On 05/07/2012 05:00 PM, Michael Hennerich wrote:
>>>>>>> On 03/22/2012 10:10 AM, Jonathan Cameron wrote:
>>>>>>>> On 3/22/2012 8:52 AM, Michael Hennerich wrote:
>>>>>>>>> On 03/21/2012 08:34 PM, Jonathan Cameron wrote:
>>>>>>>>>> On 02/22/2012 12:36 PM, michael.hennerich@analog.com wrote:
>>>>>>>>>>> From: Michael Hennerich<michael.hennerich@analog.com>
>>>>>>>>>> Sorry for the slow response on this one. Been off sick...
>>>>>>>>>>
>>>>>>>>>> Anyhow, I'm still not sure what the right interface for this type
>>>>>>>>>> of device is.
>>>>>>>>>>
>>>>>>>>>> The obvious options are:
>>>>>>>>>>
>>>>>>>>>> 1) Make gain an IIO type (doesn't make much sense as gain is only
>>>>>>>>>> going
>>>>>>>>>> to be of one particular existing type).
>>>>>>>>>> 2) Have it as an IIO_ALTVOLTAGE channel as you have here and use
>>>>>>>>>> extend
>>>>>>>>>> name.  Any real reason for picking altvoltage rather than
>>>>> voltage?
>>>>>>>>> I'm open for advice. Since I made the amplifier being an OUT type
>>>>>>>>> device
>>>>>>>>> I chose IIO_ALTVOLTAGE analogous to our DDS/PLL drivers.
>>>>>>>>> Some VGAs/PGAs work from DC, but typically VGAs are HF devices.
>>>>>>>> Hmm.. Don't suppose it really matters but we ought to aim for
>>>>>>>> consistency
>>>>>>>> (by review) at least.  This particular part is DC through to
>>>>> 600MHz.
>>>>>>>>>> Clearly gain has the same meaning in either case (assuming it's
>>>>>>>>>> linear). 3) Make a change to core to allow a channel to have
>>>>>>>>>> elements in info_mask but not actually to have a raw access.  Not
>>>>>>>>>> entirely sure how we will do that cleanly.  Also it's not clear
>>>>>>>>>> whether the gain would be an IN or an OUT channel type!
>>>>>>>>> Well - having the ability for channels without raw access element
>>>>>>>>> would be of interest.
>>>>>>>> True enough.  Cleanest way to do this that I can think of is to
>>>>>>>> make
>>>>>>>> a tree wide change to add the raw element to the info_mask.  We
>>>>>>>> could
>>>>>>>> allow for a zero info_mask value actually being the equivalent of
>>>>>>>> having only a raw channel.  It's invasive but if we agreee it
>>>>>>>> should
>>>>>>>> be done now is probably the best time to do it (just post merge
>>>>>>>> window etc).
>>>>>>> Hi Jonathan,
>>>>>>>
>>>>>>> Thanks for getting this in place.
>>>>>>>
>>>>>>>> Whilst here, we clearly need way of destinguishing values in DB
>>>>>>>> from
>>>>>>>> linear gains.  Could add a new return type for read_raw callbacks?
>>>>>>> Does something like this work for you?
>>>>>>> Also wondering if we should introduce IIO_CHAN_INFO_GAIN
>>>>>>> for amplifier type devices?
>>>>>> Thinking about it a bit more - why not have iio_chan_type:IIO_GAIN?
>>>>> Lack of consistency with other devices.  If we have a pga on the front
>>>>> of an adc then the type is voltage and control is done via relevant
>>>>> info
>>>>> element. How is this any different?
>>>> Well there can be several types of amplifiers voltage, current or
>>>> none-electronic
>>>> such as optical amplifiers.
>>> Quite.  But a given amplier only does one type.  Hence arguement still
>>> stands...  The only
>>> exception is an analog device such as an accelerometer wired up to a
>>> conventional adc,
>>> but then we don't handle that anyway...
>>>> The key element GAIN of an amplifier is only the ratio between input
>>>> and output.
>>>> The unit cancels out, so why bother with voltage or current?
>>> Far as I know you can't use a voltage amplifier for current
>>> amplificiation or the other way around,
>>> so still makes sence to keep them alongside the type.  The voltage vs
>>> altvoltage is a bit of
>>> a pain but I guess no cunning plan is perfect..
>> It really depends on the external circuit.
>> But I agree programmable current amplifiers are rare.
>> More overlap is between IIO_VOLTAGE and IIO_POWER.
> Fair point... Whilst there is an issue here, i'm still unconvinced that
> an IIO_GAIN type makes sense..
>>>> That's why I proposed IIO_GAIN.
>>>> Anyways I'm fine with IIO_VOLTAGE for now, but I would like to use
>>>> IIO_CHAN_INFO_GAIN
>>>> and not IIO_CHAN_INFO_SCALE?
>>> Its internally applied so actually IIO_CHAN_INFO_CALIBSCALE is the
>>> relevant one.
>>> I'm really keen to stick to the existing ones purely because this does
>>> look exactly like a
>>> pga front end as found on numerous adcs.  Why treat it differently just
>>> because it is
>>> in a different package?  We may need to be clever about routing between
>>> parts sometime
>>> in the future but would be best to keep it simple for now?
>> I know it's important to stay consistent.
>> But when talking about characteristics of an amplifier -
>> CALIBSCALE doesn't really sound well and SCALE is for sure not
>> applicable.
>>
>> What        /sys/bus/iio/devices/iio:deviceX/in_voltageY_calibscale
>> Description:
>>          Hardware applied calibration scale factor. (assumed to fix
>>          production inaccuracies).  If shared across all channels,
>> <type>_calibscale is used.
>>
>> Apart from 'calibration' and  'production inaccuracies' -
>> Technically seen it does the right thing.
>> But you need some imagination to use CALIBSCALE for gain.
> Perhaps we can rename this?  Maybe hardwaregain or
> something like that?   Can always allow both for a bit and
> do a smooth transition over to the new choice?
Hardwargain sound good to me. I've no problem keeping CALIBSCALE
where it's applicable and matches the purpose.

-Michael

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


-- 
Greetings,
Michael

--
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif