On 08/01/11 12:23, Michael Hennerich wrote:
> On 07/28/2011 05:54 PM, Jonathan Cameron wrote:
>> On 07/28/11 16:19, Michael Hennerich wrote:
>>> On 07/28/2011 04:03 PM, Jonathan Cameron wrote:
>>>> On 07/28/11 12:32, michael.hennerich@xxxxxxxxxx wrote:
>>>>> From: Michael Hennerich<michael.hennerich@xxxxxxxxxx>
>>>>>
>>>>> The AD5933 is a high precision impedance converter system solution
>>>>> that combines an on-board frequency generator with a 12-bit, 1 MSPS,
>>>>> analog-to-digital converter (ADC). The frequency generator allows an
>>>>> external complex impedance to be excited with a known frequency.
>>>>>
>>>>> The response signal from the impedance is sampled by the on-board ADC
>>>>> and a discrete Fourier transform (DFT) is processed by an on-chip DSP engine.
>>>>> The DFT algorithm returns a real (R) and imaginary (I) data-word at each
>>>>> output frequency.
>>>>>
>>>> Mostly looks good. Couple of nitpicks / queries in line. I'm particularly
>>>> bothered about the SCALE info elements in chan_spec that aren't matched
>>>> in the read_raw (or a write_raw if relevant).
>>> in0_real_raw and in0_imag_raw only exist as scan elements.
>>> They aren't present in indio_dev->channels. I therefore couldn't use
>>> scale info elements from channel spec.
>> Ah. I hadn't noticed that (obviously). Why is this the case? Does it just
>> not make sense to read them directly? If so, set their channel number to -1
>> and they won't appear. Hmm. Perhaps we need to make that test a little
>> more refined to make this work. iio_device_add_channel_sysfs drops the channel
>> immediately if it's number is negative. Maybe move the magic value into channel2?
>>
>> That way the only uggliness will come if we have a differential channel that only exists
>> for scans. Or use a negative index (rather than just -1). That's probably the cleanest
>> option, but will require a few abs() insertions in the code and some testing
>> to make sure we got them all.
>>
>
> Well this works well for channels abs(chan->channel) > 0.
> But how about the case where I need no channel sysfs but the scale attributes of channel# 0?
> Unfortunately there is no -0 :-)
oops. Good point. I'll drop this for now. Could enforce that any such channel isn't given index
zero, but that would be clunky.
>
> Since the AD5933 is a corner case - I suggest we leave it as is - and I add some comments to the source... ?
Good plan. We'll rethink this if it turns up again.
>
>
>> Something like:
>>
>> diff --git a/drivers/staging/iio/industrialio-core.c b/drivers/staging/iio/industrialio-core.c
>> index b49db92..aeeb5e6 100644
>> --- a/drivers/staging/iio/industrialio-core.c
>> +++ b/drivers/staging/iio/industrialio-core.c
>> @@ -480,7 +480,7 @@ int __iio_device_attr_init(struct device_attribute *dev_attr,
>> = kasprintf(GFP_KERNEL, "%s_%s%d_%s",
>> iio_direction[chan->output],
>> iio_chan_type_name_spec_shared[chan->type],
>> - chan->channel,
>> + (int)abs(chan->channel),
>> full_postfix);
>>
>> if (name_format == NULL) {
>> @@ -489,7 +489,7 @@ int __iio_device_attr_init(struct device_attribute *dev_attr,
>> }
>> dev_attr->attr.name = kasprintf(GFP_KERNEL,
>> name_format,
>> - chan->channel,
>> + (int)abs(chan->channel),
>> chan->channel2);
>> if (dev_attr->attr.name == NULL) {
>> ret = -ENOMEM;
>> @@ -585,27 +585,27 @@ static int iio_device_add_channel_sysfs(struct iio_dev *dev_info,
>> {
>> int ret, i;
>>
>> - if (chan->channel< 0)
>> - return 0;
>> - if (chan->processed_val)
>> - ret = __iio_add_chan_devattr("input", NULL, chan,
>> -&iio_read_channel_info,
>> - NULL,
>> - 0,
>> - 0,
>> -&dev_info->dev,
>> -&dev_info->channel_attr_list);
>> - else
>> - ret = __iio_add_chan_devattr("raw", NULL, chan,
>> -&iio_read_channel_info,
>> - (chan->output ?
>> -&iio_write_channel_info : NULL),
>> - 0,
>> - 0,
>> -&dev_info->dev,
>> -&dev_info->channel_attr_list);
>> - if (ret)
>> - goto error_ret;
>> + if (chan->channel>= 0) {
>> + if (chan->processed_val)
>> + ret = __iio_add_chan_devattr("input", NULL, chan,
>> +&iio_read_channel_info,
>> + NULL,
>> + 0,
>> + 0,
>> +&dev_info->dev,
>> +&dev_info->channel_attr_list);
>> + else
>> + ret = __iio_add_chan_devattr("raw", NULL, chan,
>> +&iio_read_channel_info,
>> + (chan->output ?
>> +&iio_write_channel_info : NULL),
>> + 0,
>> + 0,
>> +&dev_info->dev,
>> +&dev_info->channel_attr_list);
>> + if (ret)
>> + goto error_ret;
>> + }
>>
>> for_each_set_bit(i,&chan->info_mask, sizeof(long)*8) {
>> ret = __iio_add_chan_devattr(iio_chan_info_postfix[i/2],
>>
>>
>
>
--
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
