Re: [PATCH] iio: dac: Add support for ltc2632 DACs

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

 



Hi Jonathan,

Thanks for the great feedback. I am working with Maxime on this driver.

> On Mar 4, 2017, at 14:03, Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> 
>> +static const struct iio_chan_spec_ext_info ltc2632_ext_info[] = {
>> +	{
>> +		.name = "powerdown",
> Why have this as a sysfs attribute? It's new ABI so should be documented
> and justified.
> 
> Normally having explicit power up and powerdown of devices is somewhat
> frowned upon as it can often be implicitly done depending on the state
> of the machine etc (runtime pm and similar). If that's not the case here
> please provide some explanatory comments.

I understand what you mean. We just copy/pasted from a neighbouring driver but
plan on using the feature. I will give you our use case as an example. We use
this DAC to drive amplifiers in an acquisition subsystem. This subsystem may be
offline for long durations (when no acquisition is taking place). The system
still needs to be fully awake as the user is simply doing something else with
the unit at that moment.

Trying to bear with you I would ask: are there ways to define different alert
states with more granularity than "sleeping" and "awake", which would allow
us to tie the powerdown state of the chips? If so, quick pointers would be
appreciated.

>> 
>> +#define LTC2632_CHANNEL(_chan, _bits) { \
>> +		.type = IIO_VOLTAGE, \
>> +		.indexed = 1, \
>> +		.output = 1, \
>> +		.channel = (_chan), \
>> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
>> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
>> +		.address = (_chan), \
>> +		.scan_type = { \
>> +			.sign		= 'u', \
>> +			.realbits	= (_bits), \
>> +			.storagebits	= 16, \
>> +			.shift		= 16 - (_bits), \
> Hmm. Without buffers for output devices some of this is never used, but
> it does provide 'documentation' of a type so fine to leave it here.

Guilty of plagiarism, we do not master the iio subsystem yet. We copied
5624r_spi.c and tweaked it for this chip. We did not intend to document by doing
this. Can you suggest a different version of this struct?

Thanks again!--
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