From: Michael Hennerich <michael.hennerich@analog.com>
To: Jonathan Cameron <jic23@cam.ac.uk>
Cc: Jean Delvare <khali@linux-fr.org>,
"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
Drivers <Drivers@analog.com>,
"device-drivers-devel@blackfin.uclinux.org"
<device-drivers-devel@blackfin.uclinux.org>,
Guenter Roeck <guenter.roeck@ericsson.com>
Subject: Re: [Device-drivers-devel] Oddities and how to handle them.
Date: Tue, 3 May 2011 11:26:05 +0200 [thread overview]
Message-ID: <4DBFCA2D.3080105@analog.com> (raw)
In-Reply-To: <4DBEC4C1.3070009@cam.ac.uk>
On 05/02/2011 04:50 PM, Jonathan Cameron wrote:
>
>>>>>>
>>>>> We could prefix all inputs with in and all outputs with out, assuming
>>>>> we move voltages out of the way. Ultimately we didn't have any output
>>>>> devices when we started hammering the interfaces into shape.
>>>>>
>>>>>
>>>>>
>>>> That sounds right to me.
>>>>
>>>>
>>> We may need to do this gradually, or on the move from staging out into the
>>> main tree. Whilst we are in staging, I know there are mainstream users
>>> of a few drivers. Perhaps we just support old interface for them on a
>>> case by case basis.
>>>
>>> This will want a full proposal to lkml.
>>>
>>>
>>>>>
>>>>>> In addition we need to proper naming for what is input or output -
>>>>>> current, voltage, etc.
>>>>>>
>>>>>> The three power values can't be three different channels.
>>>>>> They are alternatives all on the same physical input channel, and the
>>>>>> naming should express this.
>>>>>>
>>>>>>
>>>>>>
>>>>> Then it will have to be as modifiers. Right now we tend to use them to
>>>>> group things. So for accelerometers we can in theory have:
>>>>>
>>>>> accel0_x,
>>>>> accel0_y,
>>>>> accel1_x, etc. for chips implementing more than one sensor in a given
>>>>> direction.
>>>>>
>>>>> If we insist on same number meaning same physical ping then for typical
>>>>> inertial sensor the channel number would have to be unique.
>>>>> Thus take adis16400 we would need.
>>>>>
>>>>> in0_supply_raw
>>>>> gyro1_x_raw
>>>>> gyro2_y_raw
>>>>> gyro3_z_raw
>>>>> accel4_x_raw
>>>>> etc...
>>>>> which, whilst looking odd, wouldn't be a fundamental problem.
>>>>>
>>>>>
>>>>>
>>>> Agreed - that looks odd. And yes modifiers should work as well.
>>>> So we get to -
>>>>
>>>> in_powerX_Y_apparent_raw
>>>>
>>>> in_volatgeX_Y_rms_raw
>>>>
>>>> or
>>>>
>>>> inX_powerY_apparent_raw
>>>> inX_volatgeY_rms_raw
>>>> outX_volatgeY_raw
>>>>
>>>>
>>>>
>>> I'm a little confused on what the Y is? I would imagine we can only have
>>> one apparent power measure per channel. The modifier will be into an enum
>>> associated with that 'apparent' label, much as we have 'x'
>>> for axis in devices where that makes sense. We may want to move away from
>>> the passing a character approach for those modifiers as well so we have
>>> just one path.
>>>
>>>
>> Hi Jonathan,
>>
>> I'm now getting confused as well.
>> Yes one apparent power measure per channel is enough.
>> Didn't you say that the 3 power values will need to be different channels?
>> My point was that we need a modifier that identifies the physical
>> input/output channel.
>>
> I was thinking of this other way around. We have perfectly good channel
> numbers. Lets use them for the physical channel, then use the modifiers
> to distinguish what we are dealing with. Thus, here we have:
>
> Channel types
> Power,
> Voltage,
> Current,
> (for now keep voltage as inX as it will easier to do a separate series converting
> all drivers to new naming)
>
> for power, we define modifiers, apparent, active, reactive.
>
> for voltage and current we will define the modifier rms
>
> (this is a change to what I proposed earlier so as to allow for
> events on RMS values. For consistency we will probably want to move
> the existing channel info element peak_raw over to be a modifier
> as well - what we currently do with that is a dirty hack that will
> bite us at some point)
>
> We then define channel numbering, to be an 'indicator' of shared physical
> channel (i.e. pin). I only say indicator so as to avoid a mass change of
> the tree in this driver patch. As with the channel renames, that wants
> to be a separate series. It actually effects only a few channels on a few
> devices so isn't a big problem.
>
> By saying channel numbers indicate physical channels iff they are present
> we get around having to assign the to axes on the IMU's and accelerometers.
>
> So we end up with here (I've gone for raw everywhere to avoid reading the
> datasheet thoroughly!)
>
> power0_apparent_raw
> power0_active_raw
> power0_reactive_raw
> in0_raw (probably become voltage0_raw at a later date, from waveform register?)
>
Not sure if we need voltage0_raw and current0_raw as a none buffer channel.
These actual values are only interesting when they are sampled at a
fixed frequency.
> in0_rms_raw
> in0_peak_raw (max value from set number of wave cycles - probably needs in0_peak_cycles as well?)
> curr0_raw (from waveform register?)
> curr0_rms_raw
> curr0_peak_raw (max value from set number of wave cycles..)
>
> Would this cover your requirements? It generalizes well (I think) so I'm quite
> keen on doing it roughly like this...
>
Thanks, this covers things - and makes a lot of sense.
> As a follow up series, I'll (or some one else) also move the accelerometers etc
> to not specify their modifiers with 'x' as channel but rather the modifier
> code in channel2 of iio_chan_spec.
>
> Thanks for knocking this driver into shape!
>
> Hope it doesn't prove too painful.
>
> 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
next prev parent reply other threads:[~2011-05-03 9:35 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-04-26 16:13 Oddities and how to handle them Jonathan Cameron
2011-04-27 11:32 ` Michael Hennerich
2011-04-27 14:42 ` Jonathan Cameron
2011-04-27 15:03 ` Hennerich, Michael
2011-04-27 15:11 ` Jonathan Cameron
2011-04-28 8:36 ` Hennerich, Michael
2011-04-28 9:31 ` Jonathan Cameron
2011-04-28 10:04 ` Michael Hennerich
2011-04-28 10:19 ` Jonathan Cameron
2011-04-28 13:49 ` Guenter Roeck
2011-04-28 13:51 ` Jean Delvare
2011-04-28 14:21 ` Jonathan Cameron
2011-04-28 14:23 ` Guenter Roeck
2011-04-28 14:35 ` Jonathan Cameron
2011-04-28 14:58 ` [Device-drivers-devel] " Michael Hennerich
2011-04-28 15:46 ` Jonathan Cameron
2011-04-29 14:21 ` Michael Hennerich
2011-04-29 15:03 ` Jonathan Cameron
2011-05-02 8:02 ` Michael Hennerich
2011-05-02 14:50 ` Jonathan Cameron
2011-05-03 9:26 ` Michael Hennerich [this message]
2011-05-03 9:46 ` Jonathan Cameron
2011-05-03 18:07 ` Jonathan Cameron
2011-05-04 10:56 ` Jonathan Cameron
2011-05-04 18:45 ` Hennerich, Michael
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4DBFCA2D.3080105@analog.com \
--to=michael.hennerich@analog.com \
--cc=Drivers@analog.com \
--cc=device-drivers-devel@blackfin.uclinux.org \
--cc=guenter.roeck@ericsson.com \
--cc=jic23@cam.ac.uk \
--cc=khali@linux-fr.org \
--cc=linux-iio@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.