linux-kernel - Re: [RFC 1/3] IIO: Direct digital synthesis abi documentation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Thu, 09 Dec 2010 10:57:43 +0000
From:	Jonathan Cameron <jic23@....ac.uk>
To:	"Hennerich, Michael" <Michael.Hennerich@...log.com>
CC:	"linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Drivers <Drivers@...log.com>,
	"device-drivers-devel@...ckfin.uclinux.org" 
	<device-drivers-devel@...ckfin.uclinux.org>
Subject: Re: [RFC 1/3] IIO: Direct digital synthesis abi documentation

On 12/09/10 05:48, Hennerich, Michael wrote:
> Jonathan Cameron wrote on 2010-12-07:
>> On 12/07/10 05:18, Hennerich, Michael wrote:
>>>> Jonathan Cameron wrote on 2010-12-03:
>>>> On 12/02/10 12:21, michael.hennerich@...log.com wrote:
>>>>> From: Michael Hennerich <michael.hennerich@...log.com>
>>>>>
>>>>> Proposed ABI documentation
>>>>>
> 
>>>>> What:              /sys/bus/iio/devices/device[n]/ddsX_freqY
>>>> Here we deviate a little from what we did with input channels. In that
>>>> case there was the existing interface (from hwmon) to match so we
>>>> already had an _input designation to tell us that the number was in
>>>> the relevant base units (here it would be Hz).  Hence we added a _raw
>>>> label to say it wasn't and tell userspace to apply scale and offset.
>>>> This is stretching a point somewhat, but looking at the hwmon docs,
>>>> they have pwmX_freq as a value in Hz. That's obviously going to make
>>>> consistency rather tricky to achieve!
>>>>
>>>> Do you think we should leave all _freq without modifier as being in Hz
>>>> and have ddsX_freqY_raw. Or should we rely on userspace verifying if
>>>> there are appropriate scale / offset parameters to be applied and
>>>> hence working out for itself whether the value in ddsX_freqY is in Hz
>>>> or not?
>>>>
>>>> I'm think I marginally favour leaving it as you have it here but
>>>> others may have different opinions.
>>>
>>> Offset is not likely to be used here - but these devices actually
>>> provide sub Hertz resolution. It's very likely to occur, that we
>>> want to have scale being 1000 and the user writes frequency in mHz.
>>> I might even consider using mHz for the sample driver as well.
>> I guess it's a question of whether doing the fixed point arithmetic in
>> kernel is cheap enough to use Hz but allow for a decimal point?  That
>> would remove the need for the _scale parameter which would simplify
>> the user interface slightly.
>>
>> Lets go with what you originally suggested. It works and with clear
>> documentation the difference between it and some of our other
>> 'frequency' elements shouldn't confuse anyone.
> 
> I prefer the _scale parameter.
Fine with me.
> Not aware of any other sysfs files, taking decimal points.
Hmm.. There is at least on in IIO that takes decimal inputs (hmc5834),
though I think only in the category where there is an _available attribute
to list the possibilities. Output wise quite a few use decimals
(your ad799x for starters!)  IIRC there are a few more cases in drivers
Alan Cox has submitted recently (not sure what merge status is on those).



>>>>> +What:              /sys/bus/iio/devices/device[n]/ddsX_out_disable
>>>>> +KernelVersion:     2.6.37
>>>>> +Contact:   linux-iio@...r.kernel.org
>>>>> +Description:
>>>>> +           Disables any signal generation on all outputs.
>>>> With the X in there you need to say for dds X. On everything else so
>>>> far we have tended to go with enable attributes rather than this way
>>>> around.  Why do it as disable here?
>>>
>>> We can change the logic. The sample driver enables the output once the
>>> ddsX_outY_wavetype file is written.
>> Is that a good idea? What if a device is dependant on having a very
>> particular frequency fed to it and the user not knowing that wavetype
>> turns things on might set that before the frequency? The semantics
>> don't make it clear that one must set the wavetype last.  I would
>> think separating the configuration from enable/disable might be more
>> intuitive.
> 
> I agree.

Looks like we've pinned this down so that we are both happy. No one else
has picked up on it, so I guess no one else is currently interested in
DDS support.

I'll be happy to do a final review of an updated patch set.

Thanks,

Jonathan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/