Re: [Device-drivers-devel] Oddities and how to handle them.

[Jonathan Cameron <jic23@cam.ac.uk>]

On 05/03/11 19:07, Jonathan Cameron wrote:
> On 05/03/11 10:46, Jonathan Cameron wrote:
>> On 05/03/11 10:26, Michael Hennerich wrote:
>>> 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.
>> Cool. I wasn't sure about those.  Can conceive of devices that look at the exact
>> wave form which want to do this, but agreed, it doesn't make sense for this one..
>> (and I have no idea if such a detailed device exists - can only think of being useful
>> for looking at various DC to AC convertors...)
>>>> 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.
>> I'm pushing the updated code all the way through the tree.  It will take a little while
>> as this touches about half the driver updates. Note I'm also scrapping all but one of
>> the IIO_CHAN macros as per the other branch of this thread.  As Arnd predicted they have
>> turned into a maintenance nightmare!
> 
> I've pushed a fairly rough branch 'work' on the iio-onwards tree.  As the names of the last
> few commits show, it has some elements I should have hacked off the top, but I've run out
> of time (pesky students) + not sure what time I'll have for a few days.
> 
> Still basic principal of what I described is there in the meantime.
> Sorry it's touch messy!
Cleaned up somewhat and remembered to tack in the ability to do _input attributes
that I forgot in the last version (core support was partly there - but not in the macros).

Pushed to the work branch. I need to do some doc fixes before switching master over to this.


>