Re: [PATCH v4 1/1] iio: stx104: Add IIO support for the ADC channels

[Jonathan Cameron <jic23@kernel.org>]

On 22/08/16 14:24, William Breathitt Gray wrote:
> On Sun, Aug 21, 2016 at 08:39:01PM +0100, Jonathan Cameron wrote:
>> On 25/07/16 18:54, William Breathitt Gray wrote:
>>> The Apex Embedded Systems STX104 features 16 channels of single-ended (8
>>> channels of true differential) 16-bit analog input. Differential input
>>> configuration may be selected via a physical jumper on the device.
>>> Similarly, input polarity (unipolar/bipolar) is configured via a
>>> physical jumper on the device.
>>>
>>> Input gain selection is available to the user via software, thus
>>> allowing eight possible input ranges: +-10V, +-5V, +-2.5V, +-1.25V,
>>> 0 to 10V, 0 to 5V, 0 to 2.5V, and 0 to 1.25V. Four input gain
>>> configurations are supported: x1, x2, x4, and x8.
>>>
>>> This ADC resolution is 16-bits (1/65536 of full scale). Analog input
>>> samples are taken on software trigger; neither FIFO sampling nor
>>> interrupt triggering is supported by this driver.
>>>
>>> The Apex Embedded Systems STX104 is primarily an analog-to-digital
>>> converter device. The STX104 IIO driver was initially placed in the DAC
>>> directory because only the DAC portion of the STX104 was supported at
>>> the time. Now that ADC support has been added to the STX104 IIO driver,
>>> the driver should be moved to the more appropriate ADC directory.
>>>
>>> Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
>> Hi William,
>>
>> I'd missed the odd use of an output channel to control scale.
>> I'm guessing this is because we are looking at actual control of a front
>> end separate amplifier on the board?  However as far as the driver is
>> confirmed this is no different from any multi range ADC and so
>> should be done with IIO_INFO_SCALE.
>>
>> Sorry for taking so long to get back to you.  Holiday season I'm afraid.
>>
>> Jonathan
> 
>>> +#define STX104_GAIN_CHAN {					\
>>> +	.type = IIO_VOLTAGE,					\
>>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_HARDWAREGAIN),	\
>>> +	.output = 1						\
>>>  }
>>
>> I'm confused.  You have a separate output channel to control gain?
>> Why... Variable gain devices are normally just represented via the
>> info_mask element IIO_CHAN_INFO_SCALE as an info_mask_shared_by_type.
> 
> Hey Jonathan,
> 
> Thanks for getting back to me. I did have trouble figuring out the
> proper way to expose the gain control for this device. Perhaps if I
> describe the register interface it may make more sense.
> 
> The STX104 ADC input range may be probed with a read to register offset
> 11; bit 2 represents the ADC bipolar/unipolar configuration (ADBU),
> while bits 1 and 0 represent the ADC gain setting. ADBU is configured by
> physical jumper on the device, while the ADC gain setting is configured
> in software via a register write.
> 
> ADC gain may be set with a write of bits 1 and 0 to register offset 11;
> 00 = gain of x1, 01 = gain of x2, 10 = gain of x4, 11 = gain of x8.
> Writing to this register sets the analog input gain for all 8/16 analog
> inputs.
> 
> Since the ADC gain setting applies to all analog inputs rather than
> individual inputs separately, what is the proper way to expose this
> option? Should I remove the STX104_GAIN_CHAN #define entirely and use
> something like the following change instead:
> 
> 	#define STX104_IN_CHAN(chan, diff) {				\
> 		.type = IIO_VOLTAGE,					\
> 		.channel = chan,					\
> 		.channel2 = chan,					\
> 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
> 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> 		.indexed = 1,						\
> 		.output = 0,						\
> 		.differential = diff					\
> 	}
> 
> Where the IIO_CHAN_INFO_SCALE now exposes the gain setting affecting all
> analog inputs.
Yes.  Basically using shared_by_type gives it an appropriate name
in_voltage_scale and though you have the entry every time, only one attribute
is created.  The others act more as documentation in the code than anything
else (so they should be there).

Jonathan
> 
> Thanks,
> 
> William Breathitt Gray
> --
> 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
>