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! >> 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@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> > > -- 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
