Oddities and how to handle them.

Oddities and how to handle them.

[Jonathan Cameron]

Hi Michael,

I'm trying to squash the remaining drivers that don't build after
all the changes in iio-onwards.  A couple of odd cases have come
up.

ad7291 - has two similar concurrent event measurements on the same temperature channel.
One is almost a windowed average, but not quite. The other based on raw value.
I'm not sure what event code we should generate for them to differentiate between
them.  So for now I just have both producing a generic temperature threshold event.
(filtering on data isn't covered by our abi's yet, let alone on event detectors!)

ad7745 - slow capacitance adc.  This one actually makes a possibly valid use of the
buffer event codes.  It is slow enough that it may make sense to notify userspace
that a new reading is available (90Hz).  I'd prefer to see this happen by just allowing
polling on the sysfs attributes though. I have no means to testing this one, and such
a change is a little more invasive than I like to make without hardware!

adt7310 - should be in hwmon as far as I can see.... + it's abusing our interfaces
so I don't like it ;)  For now I've squashed the events into basic temp events.
adt7410 - the same.

adt7316 - The fault condition on the external temperature sensor isn't an event that
one should ever see unless something is horribly wrong.  I'd be inclined to copy the
hwmon interface for this and do it as an alarm sysfs attribute... I don't think it
belongs in out main event path...  For now I've removed it's ability to be reported at all.

 
ade7758 - Complex driver I'm not that keen on touching without a lot of testing support.
Don't suppose you want to take this one Michael? (*looks hopeful*)  At lease blugeoning
it into more or less current interfaces would be a great help. I can do it, but then I
suspect I'll break it in a few exciting ways :(

So all in all, only ad7745 and ade7758 no longer build.

Lots more clean up to be done all over the tree, but I think we are making good progress
in the right direction.

Jonathan

Re: Oddities and how to handle them.

[Michael Hennerich]

On 04/26/2011 06:13 PM, Jonathan Cameron wrote:
> Hi Michael,
>
> I'm trying to squash the remaining drivers that don't build after
> all the changes in iio-onwards.  A couple of odd cases have come
> up.
>
> ad7291 - has two similar concurrent event measurements on the same temperature channel.
> One is almost a windowed average, but not quite. The other based on raw value.
> I'm not sure what event code we should generate for them to differentiate between
> them.  So for now I just have both producing a generic temperature threshold event.
> (filtering on data isn't covered by our abi's yet, let alone on event detectors!)
>
> ad7745 - slow capacitance adc.  This one actually makes a possibly valid use of the
> buffer event codes.  It is slow enough that it may make sense to notify userspace
> that a new reading is available (90Hz).  I'd prefer to see this happen by just allowing
> polling on the sysfs attributes though. I have no means to testing this one, and such
> a change is a little more invasive than I like to make without hardware!
>   
The /RDY interrupt in this driver should be a data available interrupt,
used by a buffer to trigger the readout.
Implementing it as an event is pointless.

> adt7310 - should be in hwmon as far as I can see.... + it's abusing our interfaces
> so I don't like it ;)  For now I've squashed the events into basic temp events.
> adt7410 - the same.
>
> adt7316 - The fault condition on the external temperature sensor isn't an event that
> one should ever see unless something is horribly wrong.  I'd be inclined to copy the
> hwmon interface for this and do it as an alarm sysfs attribute... I don't think it
> belongs in out main event path...  For now I've removed it's ability to be reported at all.
>
>   
Fixing these so that they build is fine for now.
There are a few more oddities with these drivers.
I have ordered hardware and there are some open tasks to test and
cleanup those drivers.
>  
> ade7758 - Complex driver I'm not that keen on touching without a lot of testing support.
> Don't suppose you want to take this one Michael? (*looks hopeful*)  At lease blugeoning
> it into more or less current interfaces would be a great help. I can do it, but then I
> suspect I'll break it in a few exciting ways :(
>   
I can fix building on this one. However I currently don't have enough
time to fix and document the API.
The buffer scan attribute naming is a bit complicated on this one. Do
you think we can stick with wform?
There is some interaction with the WAVEFORM MODE Register. Ideally we
have enable files for all possible waveform selection possibilities,
which are numerous, 3 sources (phases)  * 5 measurement options
(Current, Voltage, Active Power, Reactive Power and VA). Only one
combination can be enabled at a given time, since they are exclusive or.

Thoughts?
   
> So all in all, only ad7745 and ade7758 no longer build.
>
> Lots more clean up to be done all over the tree, but I think we are making good progress
> in the right direction.
>
> Jonathan
>   

-- 
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

Re: Oddities and how to handle them.

[Jonathan Cameron]

On 04/27/11 12:32, Michael Hennerich wrote:
> On 04/26/2011 06:13 PM, Jonathan Cameron wrote:
>> Hi Michael,
>>
>> I'm trying to squash the remaining drivers that don't build after
>> all the changes in iio-onwards.  A couple of odd cases have come
>> up.
>>
>> ad7291 - has two similar concurrent event measurements on the same temperature channel.
>> One is almost a windowed average, but not quite. The other based on raw value.
>> I'm not sure what event code we should generate for them to differentiate between
>> them.  So for now I just have both producing a generic temperature threshold event.
>> (filtering on data isn't covered by our abi's yet, let alone on event detectors!)
>>
>> ad7745 - slow capacitance adc.  This one actually makes a possibly valid use of the
>> buffer event codes.  It is slow enough that it may make sense to notify userspace
>> that a new reading is available (90Hz).  I'd prefer to see this happen by just allowing
>> polling on the sysfs attributes though. I have no means to testing this one, and such
>> a change is a little more invasive than I like to make without hardware!
>>   
> The /RDY interrupt in this driver should be a data available interrupt,
> used by a buffer to trigger the readout.
> Implementing it as an event is pointless.
Definitely pointless as an event, but as the device is so slow, I can see some merit
in allowing poll on the sysfs attributes rather than insisting on a full buffer implementation.
> 
>> adt7310 - should be in hwmon as far as I can see.... + it's abusing our interfaces
>> so I don't like it ;)  For now I've squashed the events into basic temp events.
>> adt7410 - the same.
>>
>> adt7316 - The fault condition on the external temperature sensor isn't an event that
>> one should ever see unless something is horribly wrong.  I'd be inclined to copy the
>> hwmon interface for this and do it as an alarm sysfs attribute... I don't think it
>> belongs in out main event path...  For now I've removed it's ability to be reported at all.
>>
>>   
> Fixing these so that they build is fine for now.
> There are a few more oddities with these drivers.
> I have ordered hardware and there are some open tasks to test and
> cleanup those drivers.
Cool
>>  
>> ade7758 - Complex driver I'm not that keen on touching without a lot of testing support.
>> Don't suppose you want to take this one Michael? (*looks hopeful*)  At lease blugeoning
>> it into more or less current interfaces would be a great help. I can do it, but then I
>> suspect I'll break it in a few exciting ways :(
>>   
> I can fix building on this one. However I currently don't have enough
> time to fix and document the API.
That's fine.  We won't be pushing any of the energy meter drivers out of staging
for a while yet anyway!
> The buffer scan attribute naming is a bit complicated on this one. Do
> you think we can stick with wform?
> There is some interaction with the WAVEFORM MODE Register. Ideally we
> have enable files for all possible waveform selection possibilities,
> which are numerous, 3 sources (phases)  * 5 measurement options
> (Current, Voltage, Active Power, Reactive Power and VA). Only one
> combination can be enabled at a given time, since they are exclusive or.
That sounds right.  One _en attribute per combination.  Probably need to clarify
the documentation to say that any of these attributes can effect any other and
userspace should check all values after it has configured what it wants.
Thanks

Jonathan

RE: Oddities and how to handle them.

[Hennerich, Michael]

Jonathan Cameron wrote on 2011-04-27:
> On 04/27/11 12:32, Michael Hennerich wrote:
>> On 04/26/2011 06:13 PM, Jonathan Cameron wrote:
>>> Hi Michael,

>>> ade7758 - Complex driver I'm not that keen on touching without a lot
>>> of testing support. Don't suppose you want to take this one Michael?
>>> (*looks hopeful*) At lease blugeoning it into more or less current
>>> interfaces would be a great help. I can do it, but then I suspect I'll
>>> break it in a few exciting ways :(
>>>
>> I can fix building on this one. However I currently don't have
>> enough time to fix and document the API.
> That's fine.  We won't be pushing any of the energy meter drivers out
> of staging for a while yet anyway!
>> The buffer scan attribute naming is a bit complicated on this one.
>> Do you think we can stick with wform?
>> There is some interaction with the WAVEFORM MODE Register. Ideally
>> we have enable files for all possible waveform selection
>> possibilities, which are numerous, 3 sources (phases)  * 5
>> measurement options (Current, Voltage, Active Power, Reactive Power
>> and VA). Only one combination can be enabled at a given time, since
>> they are exclusive
> or.
> That sounds right.  One _en attribute per combination.  Probably need
> to clarify the documentation to say that any of these attributes can
> effect any other and userspace should check all values after it has
> configured what it wants.

I guess with the new channel registration I need to use
indio_dev->available_scan_masks for this. And the user has to explicitly
disable the enabled channel and then enable the targeted one...


Greetings,
Michael

--
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368; Gesch=
aeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin, Margaret=
 Seif

Re: Oddities and how to handle them.

[Jonathan Cameron]

On 04/27/11 16:03, Hennerich, Michael wrote:
> Jonathan Cameron wrote on 2011-04-27:
>> On 04/27/11 12:32, Michael Hennerich wrote:
>>> On 04/26/2011 06:13 PM, Jonathan Cameron wrote:
>>>> Hi Michael,
> 
>>>> ade7758 - Complex driver I'm not that keen on touching without a lot
>>>> of testing support. Don't suppose you want to take this one Michael?
>>>> (*looks hopeful*) At lease blugeoning it into more or less current
>>>> interfaces would be a great help. I can do it, but then I suspect I'll
>>>> break it in a few exciting ways :(
>>>>
>>> I can fix building on this one. However I currently don't have
>>> enough time to fix and document the API.
>> That's fine.  We won't be pushing any of the energy meter drivers out
>> of staging for a while yet anyway!
>>> The buffer scan attribute naming is a bit complicated on this one.
>>> Do you think we can stick with wform?
>>> There is some interaction with the WAVEFORM MODE Register. Ideally
>>> we have enable files for all possible waveform selection
>>> possibilities, which are numerous, 3 sources (phases)  * 5
>>> measurement options (Current, Voltage, Active Power, Reactive Power
>>> and VA). Only one combination can be enabled at a given time, since
>>> they are exclusive
>> or.
>> That sounds right.  One _en attribute per combination.  Probably need
>> to clarify the documentation to say that any of these attributes can
>> effect any other and userspace should check all values after it has
>> configured what it wants.
> 
> I guess with the new channel registration I need to use
> indio_dev->available_scan_masks for this. And the user has to explicitly
> disable the enabled channel and then enable the targeted one...
Yes.  Nearest example is the max1363 but that's a fair bit more complex...

RE: Oddities and how to handle them.

[Hennerich, Michael]

Jonathan Cameron wrote on 2011-04-27:
> On 04/27/11 16:03, Hennerich, Michael wrote:
>> Jonathan Cameron wrote on 2011-04-27:
>>> On 04/27/11 12:32, Michael Hennerich wrote:
>>>> On 04/26/2011 06:13 PM, Jonathan Cameron wrote:
>>>>> Hi Michael,
>>>>>
>>>>> ade7758 - Complex driver I'm not that keen on touching without a lot
>>>>> of testing support. Don't suppose you want to take this one Michael?
>>>>> (*looks hopeful*) At lease blugeoning it into more or less current
>>>>> interfaces would be a great help. I can do it, but then I suspect
>>>>> I'll break it in a few exciting ways :(
>>>>>
>>>> I can fix building on this one. However I currently don't have
>>>> enough time to fix and document the API.
>>> That's fine.  We won't be pushing any of the energy meter drivers out
>>> of staging for a while yet anyway!
>>>> The buffer scan attribute naming is a bit complicated on this one.
>>>> Do you think we can stick with wform?
>>>> There is some interaction with the WAVEFORM MODE Register. Ideally
>>>> we have enable files for all possible waveform selection
>>>> possibilities, which are numerous, 3 sources (phases)  * 5
>>>> measurement options (Current, Voltage, Active Power, Reactive
>>>> Power and VA). Only one combination can be enabled at a given
>>>> time, since they are exclusive
>>> or.

Hi Jonathan,

For the metering parts I think we need to define a few more channel types.

How about this ones

inSX            S is the apparent power.
inPX            P is the active power.
inQX            Q is the reactive power.
inVX            V is the voltage. (only inX ?)
inVRMSX VRMS is the quadratic mean voltage.
inIX            I is the current.
inIRMSX IRMS is the quadratic mean current.

Since they won't be really popular

How about using a string in iio_chan_type_name_spec

        [IIO_IN_GENERIC] = "in%s",


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

Re: Oddities and how to handle them.

[Jonathan Cameron]

Guenter / Jean - cc'd you two because we have an sysfs interface naming question for
AC sensors that touches on the edge of hwmon.

>>>>>> Hi Michael,
>>>>>>
>>>>>> ade7758 - Complex driver I'm not that keen on touching without a lot
>>>>>> of testing support. Don't suppose you want to take this one Michael?
>>>>>> (*looks hopeful*) At lease blugeoning it into more or less current
>>>>>> interfaces would be a great help. I can do it, but then I suspect
>>>>>> I'll break it in a few exciting ways :(
>>>>>>
>>>>> I can fix building on this one. However I currently don't have
>>>>> enough time to fix and document the API.
>>>> That's fine.  We won't be pushing any of the energy meter drivers out
>>>> of staging for a while yet anyway!
>>>>> The buffer scan attribute naming is a bit complicated on this one.
>>>>> Do you think we can stick with wform?
>>>>> There is some interaction with the WAVEFORM MODE Register. Ideally
>>>>> we have enable files for all possible waveform selection
>>>>> possibilities, which are numerous, 3 sources (phases)  * 5
>>>>> measurement options (Current, Voltage, Active Power, Reactive
>>>>> Power and VA). Only one combination can be enabled at a given
>>>>> time, since they are exclusive
>>>> or.
> 
> Hi Jonathan,
> 
> For the metering parts I think we need to define a few more channel types.
> 
> How about this ones
> 
> inSX            S is the apparent power.
> inPX            P is the active power.
> inQX            Q is the reactive power.
> inVX            V is the voltage. (only inX ?)
> inVRMSX VRMS is the quadratic mean voltage.
Call it 'root mean square' rather than quadratic in the docs. They have different
meanings in English.
> inIX            I is the current.
currX as per hwmon?  They also define a power attribute, but only 1 (as DC
I guess). Cc'd Guenter and Jean to see if we can / want to share an interface...

Guenter/ Jean, do you think hwmon will ever handle AC sensors? Maybe we want to be
well clear of your interfaces just to avoid confusion? Or define a new set of shared
names for the above that we will both use (when it becomes relevant?)
> inIRMSX IRMS is the quadratic mean current.
> 
> Since they won't be really popular
> 
> How about using a string in iio_chan_type_name_spec
> 
>         [IIO_IN_GENERIC] = "in%s",
Firstly, perhaps, IIO_IN_MOD will correspond better with naming?

Been thinking about exactly this question for the light sensors...
Initially strings seemed like the obvious thing to do, but then I thought
some more.  For all of these we will need to have matching event codes.
The easiest there would be to do them as IIO_EV_MOD_POWER_APPARENT etc.
Once we have those defines, we might as well roll the names into the core
as a string table referenced by value stored in channel2.
Actually, arguably we shouldn't be doing the same for 'x','y','z' as well
for consistency. Right now we allow for directions without corresponding event
codes. That way lies doom!

I know there aren't likely to be hundreds of these devices any time soon,
but we don't really want to set a precedence for allowing free naming.
That was part of the justification for moving to chan_spec in the first
place!

Jonathan

Re: Oddities and how to handle them.

[Michael Hennerich]

On 04/28/2011 11:31 AM, Jonathan Cameron wrote:
> Guenter / Jean - cc'd you two because we have an sysfs interface naming question for
> AC sensors that touches on the edge of hwmon.
>
>   
>>>>>>> Hi Michael,
>>>>>>>
>>>>>>> ade7758 - Complex driver I'm not that keen on touching without a lot
>>>>>>> of testing support. Don't suppose you want to take this one Michael?
>>>>>>> (*looks hopeful*) At lease blugeoning it into more or less current
>>>>>>> interfaces would be a great help. I can do it, but then I suspect
>>>>>>> I'll break it in a few exciting ways :(
>>>>>>>
>>>>>>>               
>>>>>> I can fix building on this one. However I currently don't have
>>>>>> enough time to fix and document the API.
>>>>>>             
>>>>> That's fine.  We won't be pushing any of the energy meter drivers out
>>>>> of staging for a while yet anyway!
>>>>>           
>>>>>> The buffer scan attribute naming is a bit complicated on this one.
>>>>>> Do you think we can stick with wform?
>>>>>> There is some interaction with the WAVEFORM MODE Register. Ideally
>>>>>> we have enable files for all possible waveform selection
>>>>>> possibilities, which are numerous, 3 sources (phases)  * 5
>>>>>> measurement options (Current, Voltage, Active Power, Reactive
>>>>>> Power and VA). Only one combination can be enabled at a given
>>>>>> time, since they are exclusive
>>>>>>             
>>>>> or.
>>>>>           
>> Hi Jonathan,
>>
>> For the metering parts I think we need to define a few more channel types.
>>
>> How about this ones
>>
>> inSX            S is the apparent power.
>> inPX            P is the active power.
>> inQX            Q is the reactive power.
>> inVX            V is the voltage. (only inX ?)
>> inVRMSX VRMS is the quadratic mean voltage.
>>     
> Call it 'root mean square' rather than quadratic in the docs. They have different
> meanings in English.
>   
>> inIX            I is the current.
>>     
> currX as per hwmon?  They also define a power attribute, but only 1 (as DC
> I guess). Cc'd Guenter and Jean to see if we can / want to share an interface...
>   
Honestly I don't like 'curr' - In my opinion we should use SI until
symbols (as well as standardized derived SI units where applicable).
In this particular case current is a SI unit.
For everything further we should use symbols commonly used in
Anglo-American EE and Physics literature. (NIST, IEEE, ...)
 
> Guenter/ Jean, do you think hwmon will ever handle AC sensors? Maybe we want to be
> well clear of your interfaces just to avoid confusion? Or define a new set of shared
> names for the above that we will both use (when it becomes relevant?)
>   
>> inIRMSX IRMS is the quadratic mean current.
>>
>> Since they won't be really popular
>>
>> How about using a string in iio_chan_type_name_spec
>>
>>         [IIO_IN_GENERIC] = "in%s",
>>     
> Firstly, perhaps, IIO_IN_MOD will correspond better with naming?
>   
> Been thinking about exactly this question for the light sensors...
> Initially strings seemed like the obvious thing to do, but then I thought
> some more.  For all of these we will need to have matching event codes.
> The easiest there would be to do them as IIO_EV_MOD_POWER_APPARENT etc.
> Once we have those defines, we might as well roll the names into the core
> as a string table referenced by value stored in channel2.
> Actually, arguably we shouldn't be doing the same for 'x','y','z' as well
> for consistency. Right now we allow for directions without corresponding event
> codes. That way lies doom!
>
> I know there aren't likely to be hundreds of these devices any time soon,
> but we don't really want to set a precedence for allowing free naming.
> That was part of the justification for moving to chan_spec in the first
> place!
>
> Jonathan
>
>   


-- 
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

Re: Oddities and how to handle them.

[Jonathan Cameron]

On 04/28/11 11:04, Michael Hennerich wrote:
> On 04/28/2011 11:31 AM, Jonathan Cameron wrote:
>> Guenter / Jean - cc'd you two because we have an sysfs interface naming question for
>> AC sensors that touches on the edge of hwmon.
>>
>>   
>>>>>>>> Hi Michael,
>>>>>>>>
>>>>>>>> ade7758 - Complex driver I'm not that keen on touching without a lot
>>>>>>>> of testing support. Don't suppose you want to take this one Michael?
>>>>>>>> (*looks hopeful*) At lease blugeoning it into more or less current
>>>>>>>> interfaces would be a great help. I can do it, but then I suspect
>>>>>>>> I'll break it in a few exciting ways :(
>>>>>>>>
>>>>>>>>               
>>>>>>> I can fix building on this one. However I currently don't have
>>>>>>> enough time to fix and document the API.
>>>>>>>             
>>>>>> That's fine.  We won't be pushing any of the energy meter drivers out
>>>>>> of staging for a while yet anyway!
>>>>>>           
>>>>>>> The buffer scan attribute naming is a bit complicated on this one.
>>>>>>> Do you think we can stick with wform?
>>>>>>> There is some interaction with the WAVEFORM MODE Register. Ideally
>>>>>>> we have enable files for all possible waveform selection
>>>>>>> possibilities, which are numerous, 3 sources (phases)  * 5
>>>>>>> measurement options (Current, Voltage, Active Power, Reactive
>>>>>>> Power and VA). Only one combination can be enabled at a given
>>>>>>> time, since they are exclusive
>>>>>>>             
>>>>>> or.
>>>>>>           
>>> Hi Jonathan,
>>>
>>> For the metering parts I think we need to define a few more channel types.
>>>
>>> How about this ones
>>>
>>> inSX            S is the apparent power.
>>> inPX            P is the active power.
>>> inQX            Q is the reactive power.
>>> inVX            V is the voltage. (only inX ?)
>>> inVRMSX VRMS is the quadratic mean voltage.
>>>     
>> Call it 'root mean square' rather than quadratic in the docs. They have different
>> meanings in English.
>>   
>>> inIX            I is the current.
>>>     
>> currX as per hwmon?  They also define a power attribute, but only 1 (as DC
>> I guess). Cc'd Guenter and Jean to see if we can / want to share an interface...
>>   
> Honestly I don't like 'curr' - In my opinion we should use SI until
> symbols (as well as standardized derived SI units where applicable).
> In this particular case current is a SI unit.
> For everything further we should use symbols commonly used in
> Anglo-American EE and Physics literature. (NIST, IEEE, ...)
Agreed, but it's an interface that is in place so best to match where
we can. That way far fewer argument lie (we can always blame someone
else :)
>  
>> Guenter/ Jean, do you think hwmon will ever handle AC sensors? Maybe we want to be
>> well clear of your interfaces just to avoid confusion? Or define a new set of shared
>> names for the above that we will both use (when it becomes relevant?)
>>   
>>> inIRMSX IRMS is the quadratic mean current.
>>>
>>> Since they won't be really popular
>>>
>>> How about using a string in iio_chan_type_name_spec
>>>
>>>         [IIO_IN_GENERIC] = "in%s",
>>>     
>> Firstly, perhaps, IIO_IN_MOD will correspond better with naming?
>>   
>> Been thinking about exactly this question for the light sensors...
>> Initially strings seemed like the obvious thing to do, but then I thought
>> some more.  For all of these we will need to have matching event codes.
>> The easiest there would be to do them as IIO_EV_MOD_POWER_APPARENT etc.
>> Once we have those defines, we might as well roll the names into the core
>> as a string table referenced by value stored in channel2.
>> Actually, arguably we shouldn't be doing the same for 'x','y','z' as well
>> for consistency. Right now we allow for directions without corresponding event
>> codes. That way lies doom!
>>
>> I know there aren't likely to be hundreds of these devices any time soon,
>> but we don't really want to set a precedence for allowing free naming.
>> That was part of the justification for moving to chan_spec in the first
>> place!
>>
>> Jonathan
>>
>>   
> 
> 

Re: Oddities and how to handle them.

[Guenter Roeck]

On Thu, Apr 28, 2011 at 06:19:09AM -0400, Jonathan Cameron wrote:
> On 04/28/11 11:04, Michael Hennerich wrote:
> > On 04/28/2011 11:31 AM, Jonathan Cameron wrote:
> >> Guenter / Jean - cc'd you two because we have an sysfs interface naming question for
> >> AC sensors that touches on the edge of hwmon.
> >>
> >>   
> >>>>>>>> Hi Michael,
> >>>>>>>>
> >>>>>>>> ade7758 - Complex driver I'm not that keen on touching without a lot
> >>>>>>>> of testing support. Don't suppose you want to take this one Michael?
> >>>>>>>> (*looks hopeful*) At lease blugeoning it into more or less current
> >>>>>>>> interfaces would be a great help. I can do it, but then I suspect
> >>>>>>>> I'll break it in a few exciting ways :(
> >>>>>>>>
> >>>>>>>>               
> >>>>>>> I can fix building on this one. However I currently don't have
> >>>>>>> enough time to fix and document the API.
> >>>>>>>             
> >>>>>> That's fine.  We won't be pushing any of the energy meter drivers out
> >>>>>> of staging for a while yet anyway!
> >>>>>>           
> >>>>>>> The buffer scan attribute naming is a bit complicated on this one.
> >>>>>>> Do you think we can stick with wform?
> >>>>>>> There is some interaction with the WAVEFORM MODE Register. Ideally
> >>>>>>> we have enable files for all possible waveform selection
> >>>>>>> possibilities, which are numerous, 3 sources (phases)  * 5
> >>>>>>> measurement options (Current, Voltage, Active Power, Reactive
> >>>>>>> Power and VA). Only one combination can be enabled at a given
> >>>>>>> time, since they are exclusive
> >>>>>>>             
> >>>>>> or.
> >>>>>>           
> >>> Hi Jonathan,
> >>>
> >>> For the metering parts I think we need to define a few more channel types.
> >>>
> >>> How about this ones
> >>>
> >>> inSX            S is the apparent power.
> >>> inPX            P is the active power.
> >>> inQX            Q is the reactive power.
> >>> inVX            V is the voltage. (only inX ?)
> >>> inVRMSX VRMS is the quadratic mean voltage.
> >>>     
> >> Call it 'root mean square' rather than quadratic in the docs. They have different
> >> meanings in English.
> >>   
> >>> inIX            I is the current.
> >>>     
> >> currX as per hwmon?  They also define a power attribute, but only 1 (as DC
> >> I guess). Cc'd Guenter and Jean to see if we can / want to share an interface...
> >>   
> > Honestly I don't like 'curr' - In my opinion we should use SI until
> > symbols (as well as standardized derived SI units where applicable).
> > In this particular case current is a SI unit.
> > For everything further we should use symbols commonly used in
> > Anglo-American EE and Physics literature. (NIST, IEEE, ...)
> Agreed, but it's an interface that is in place so best to match where
> we can. That way far fewer argument lie (we can always blame someone
> else :)

hwmon only uses inX for voltages. As such, usage of currX would only make sense
if you also use powerX for power, energyX for energy and so on.

> >  
> >> Guenter/ Jean, do you think hwmon will ever handle AC sensors? Maybe we want to be
> >> well clear of your interfaces just to avoid confusion? Or define a new set of shared
> >> names for the above that we will both use (when it becomes relevant?)
> >>   
Depends if this would be for hardware monitoring purposes, which seems to be unlikely.

If it were, any ABI extensions or driver-specific attributes would presumably be based
on the existing hwmon ABI.

Guenter

Re: Oddities and how to handle them.

[Jean Delvare]

On Thu, 28 Apr 2011 10:31:15 +0100, Jonathan Cameron wrote:
> Guenter / Jean - cc'd you two because we have an sysfs interface naming question for
> AC sensors that touches on the edge of hwmon.
>
> > For the metering parts I think we need to define a few more channel types.
> > 
> > How about this ones
> > 
> > inSX            S is the apparent power.
> > inPX            P is the active power.
> > inQX            Q is the reactive power.
> > inVX            V is the voltage. (only inX ?)
> > inVRMSX VRMS is the quadratic mean voltage.
> Call it 'root mean square' rather than quadratic in the docs. They have different
> meanings in English.
> > inIX            I is the current.
> currX as per hwmon?  They also define a power attribute, but only 1 (as DC
> I guess). Cc'd Guenter and Jean to see if we can / want to share an interface...
> 
> Guenter/ Jean, do you think hwmon will ever handle AC sensors?

Well, never say never ;) While I've never heard of such sensors, I
guess it would make some sense for a computer PSU to include a sensor
chip monitoring both the AC input and the DC output to measure the
efficiency of the unit.

> Maybe we want to be
> well clear of your interfaces just to avoid confusion? Or define a new set of shared
> names for the above that we will both use (when it becomes relevant?)

It's hard to tell in advance what hwmon would implement, as we lack
actual examples. All I can say is that we would never use "in" prefixes
for power or current. The above power examples would most probably be
named powerX_apparent_input, powerX_active_input and
powerX_reactive_input, if we ever have to support these. And
inX_rms_input for the root mean square voltage input.

But then again, I'm not sure if there is any point in sharing anything,
or forcing any difference, between hwmon and iio interfaces. They are
different by nature, and if we don't strictly enforce their relation,
they are bound to randomly diverge and converge anyway.

-- 
Jean Delvare

Re: Oddities and how to handle them.

[Jonathan Cameron]

On 04/28/11 14:51, Jean Delvare wrote:
> On Thu, 28 Apr 2011 10:31:15 +0100, Jonathan Cameron wrote:
>> Guenter / Jean - cc'd you two because we have an sysfs interface naming question for
>> AC sensors that touches on the edge of hwmon.
>>
>>> For the metering parts I think we need to define a few more channel types.
>>>
>>> How about this ones
>>>
>>> inSX            S is the apparent power.
>>> inPX            P is the active power.
>>> inQX            Q is the reactive power.
>>> inVX            V is the voltage. (only inX ?)
>>> inVRMSX VRMS is the quadratic mean voltage.
>> Call it 'root mean square' rather than quadratic in the docs. They have different
>> meanings in English.
>>> inIX            I is the current.
>> currX as per hwmon?  They also define a power attribute, but only 1 (as DC
>> I guess). Cc'd Guenter and Jean to see if we can / want to share an interface...
>>
>> Guenter/ Jean, do you think hwmon will ever handle AC sensors?
> 
> Well, never say never ;) While I've never heard of such sensors, I
> guess it would make some sense for a computer PSU to include a sensor
> chip monitoring both the AC input and the DC output to measure the
> efficiency of the unit.
> 
>> Maybe we want to be
>> well clear of your interfaces just to avoid confusion? Or define a new set of shared
>> names for the above that we will both use (when it becomes relevant?)
> 
> It's hard to tell in advance what hwmon would implement, as we lack
> actual examples. All I can say is that we would never use "in" prefixes
> for power or current. The above power examples would most probably be
> named powerX_apparent_input, powerX_active_input and
> powerX_reactive_input, if we ever have to support these. And
> inX_rms_input for the root mean square voltage input.
> 
> But then again, I'm not sure if there is any point in sharing anything,
> or forcing any difference, between hwmon and iio interfaces. They are
> different by nature, and if we don't strictly enforce their relation,
> they are bound to randomly diverge and converge anyway.
> 
Agreed to a certain extent, but saying that there is no point in reinventing
the wheel.  I think the naming you've just suggested is clearer anyway.

Ultimately I don't insist on keeping to your interfaces when it really doesn't
make sense, but when it does, it makes our life easier as we aren't starting from
scratch. Also, politically it was suggested we do this by a few people I'd like
to keep on side.

Michael - howabout doing the rms values as done with peak_raw - as chan info parameters
(directly derived from raw values anyway).

Then define two new (to us) channel types.

IIO_CURRENT = "curr%d"
IIO_POWER = "curr%d"

Using the rfc I'm going to post after I send this email, we can add additional names to a channel.
The only slight annoyance is the 3 power values will need to be different channels, so under that
we will have:

power0_apparent_raw           is the apparent power.
power1_active_raw             is the active power.
power2_reactive_raw           is the reactive power.
in0                           is the voltage.
in0_rms_raw                   is the rms voltage.
curr0_raw                     is the current.

We may want to think about how to indicate which of these are measured on the same physical pin.
If we were to have all the power measures as power0_* then we need another way of differentiating
their event codes.  Could use modifiers I suppose... I've just sanity checked that there are no
other issues, by modifying tsl2563 to claim both intensity readings are on the same channel.

What do you think?

Re: Oddities and how to handle them.

[Guenter Roeck]

On Thu, Apr 28, 2011 at 10:21:38AM -0400, Jonathan Cameron wrote:
> On 04/28/11 14:51, Jean Delvare wrote:
> > On Thu, 28 Apr 2011 10:31:15 +0100, Jonathan Cameron wrote:
> >> Guenter / Jean - cc'd you two because we have an sysfs interface naming question for
> >> AC sensors that touches on the edge of hwmon.
> >>
> >>> For the metering parts I think we need to define a few more channel types.
> >>>
> >>> How about this ones
> >>>
> >>> inSX            S is the apparent power.
> >>> inPX            P is the active power.
> >>> inQX            Q is the reactive power.
> >>> inVX            V is the voltage. (only inX ?)
> >>> inVRMSX VRMS is the quadratic mean voltage.
> >> Call it 'root mean square' rather than quadratic in the docs. They have different
> >> meanings in English.
> >>> inIX            I is the current.
> >> currX as per hwmon?  They also define a power attribute, but only 1 (as DC
> >> I guess). Cc'd Guenter and Jean to see if we can / want to share an interface...
> >>
> >> Guenter/ Jean, do you think hwmon will ever handle AC sensors?
> > 
> > Well, never say never ;) While I've never heard of such sensors, I
> > guess it would make some sense for a computer PSU to include a sensor
> > chip monitoring both the AC input and the DC output to measure the
> > efficiency of the unit.
> > 
> >> Maybe we want to be
> >> well clear of your interfaces just to avoid confusion? Or define a new set of shared
> >> names for the above that we will both use (when it becomes relevant?)
> > 
> > It's hard to tell in advance what hwmon would implement, as we lack
> > actual examples. All I can say is that we would never use "in" prefixes
> > for power or current. The above power examples would most probably be
> > named powerX_apparent_input, powerX_active_input and
> > powerX_reactive_input, if we ever have to support these. And
> > inX_rms_input for the root mean square voltage input.
> > 
> > But then again, I'm not sure if there is any point in sharing anything,
> > or forcing any difference, between hwmon and iio interfaces. They are
> > different by nature, and if we don't strictly enforce their relation,
> > they are bound to randomly diverge and converge anyway.
> > 
> Agreed to a certain extent, but saying that there is no point in reinventing
> the wheel.  I think the naming you've just suggested is clearer anyway.
> 
> Ultimately I don't insist on keeping to your interfaces when it really doesn't
> make sense, but when it does, it makes our life easier as we aren't starting from
> scratch. Also, politically it was suggested we do this by a few people I'd like
> to keep on side.
> 
> Michael - howabout doing the rms values as done with peak_raw - as chan info parameters
> (directly derived from raw values anyway).
> 
> Then define two new (to us) channel types.
> 
> IIO_CURRENT = "curr%d"
> IIO_POWER = "curr%d"
> 
power%d, presumably.

> Using the rfc I'm going to post after I send this email, we can add additional names to a channel.
> The only slight annoyance is the 3 power values will need to be different channels, so under that
> we will have:
> 
> power0_apparent_raw           is the apparent power.
> power1_active_raw             is the active power.
> power2_reactive_raw           is the reactive power.
> in0                           is the voltage.
> in0_rms_raw                   is the rms voltage.
> curr0_raw                     is the current.
> 
Note that in hwmon, all input sensors have _input at the end, as in Jean's example.
What does the _raw stand for ?

Guenter

Re: Oddities and how to handle them.

[Jonathan Cameron]

On 04/28/11 15:23, Guenter Roeck wrote:
> On Thu, Apr 28, 2011 at 10:21:38AM -0400, Jonathan Cameron wrote:
>> On 04/28/11 14:51, Jean Delvare wrote:
>>> On Thu, 28 Apr 2011 10:31:15 +0100, Jonathan Cameron wrote:
>>>> Guenter / Jean - cc'd you two because we have an sysfs interface naming question for
>>>> AC sensors that touches on the edge of hwmon.
>>>>
>>>>> For the metering parts I think we need to define a few more channel types.
>>>>>
>>>>> How about this ones
>>>>>
>>>>> inSX            S is the apparent power.
>>>>> inPX            P is the active power.
>>>>> inQX            Q is the reactive power.
>>>>> inVX            V is the voltage. (only inX ?)
>>>>> inVRMSX VRMS is the quadratic mean voltage.
>>>> Call it 'root mean square' rather than quadratic in the docs. They have different
>>>> meanings in English.
>>>>> inIX            I is the current.
>>>> currX as per hwmon?  They also define a power attribute, but only 1 (as DC
>>>> I guess). Cc'd Guenter and Jean to see if we can / want to share an interface...
>>>>
>>>> Guenter/ Jean, do you think hwmon will ever handle AC sensors?
>>>
>>> Well, never say never ;) While I've never heard of such sensors, I
>>> guess it would make some sense for a computer PSU to include a sensor
>>> chip monitoring both the AC input and the DC output to measure the
>>> efficiency of the unit.
>>>
>>>> Maybe we want to be
>>>> well clear of your interfaces just to avoid confusion? Or define a new set of shared
>>>> names for the above that we will both use (when it becomes relevant?)
>>>
>>> It's hard to tell in advance what hwmon would implement, as we lack
>>> actual examples. All I can say is that we would never use "in" prefixes
>>> for power or current. The above power examples would most probably be
>>> named powerX_apparent_input, powerX_active_input and
>>> powerX_reactive_input, if we ever have to support these. And
>>> inX_rms_input for the root mean square voltage input.
>>>
>>> But then again, I'm not sure if there is any point in sharing anything,
>>> or forcing any difference, between hwmon and iio interfaces. They are
>>> different by nature, and if we don't strictly enforce their relation,
>>> they are bound to randomly diverge and converge anyway.
>>>
>> Agreed to a certain extent, but saying that there is no point in reinventing
>> the wheel.  I think the naming you've just suggested is clearer anyway.
>>
>> Ultimately I don't insist on keeping to your interfaces when it really doesn't
>> make sense, but when it does, it makes our life easier as we aren't starting from
>> scratch. Also, politically it was suggested we do this by a few people I'd like
>> to keep on side.
>>
>> Michael - howabout doing the rms values as done with peak_raw - as chan info parameters
>> (directly derived from raw values anyway).
>>
>> Then define two new (to us) channel types.
>>
>> IIO_CURRENT = "curr%d"
>> IIO_POWER = "curr%d"
>>
> power%d, presumably.
Ooops. Thanks.
> 
>> Using the rfc I'm going to post after I send this email, we can add additional names to a channel.
>> The only slight annoyance is the 3 power values will need to be different channels, so under that
>> we will have:
>>
>> power0_apparent_raw           is the apparent power.
>> power1_active_raw             is the active power.
>> power2_reactive_raw           is the reactive power.
>> in0                           is the voltage.
>> in0_rms_raw                   is the rms voltage.
>> curr0_raw                     is the current.
>>
> Note that in hwmon, all input sensors have _input at the end, as in Jean's example.
> What does the _raw stand for ?
Means unprocessed.  We use _input for anything that is already in the right units.
If it's a linear transform than it's up to userspace to do it (using other attributes
that tell it what the conversion factors are).  The main drivers to convert to
SI units for output are light sensors where this transform is truly hideous!
They then use illuminanceX_input as the attribute name.

Some devices have no known conversion to any helpful units (tend to be calibrated
to a specific task - proximity sensors are probably the worst for this).

With sysfs access whether we process in kernel or in userspace doesn't matter much
(though it does push some bits into user space where simpler floating point code
can deal with them).

It is more important with the other route for data from devices where we have
buffered access via a more or less overhead free software or hardware (ring/fifo)
buffers.

Also, we do abuse your _input sytax by allowing floating point values (irrc, you
insist on integer?)

Jonathan

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

[Michael Hennerich]

On 04/28/2011 04:21 PM, Jonathan Cameron wrote:
> On 04/28/11 14:51, Jean Delvare wrote:
>   
>> On Thu, 28 Apr 2011 10:31:15 +0100, Jonathan Cameron wrote:
>>     
>>> Guenter / Jean - cc'd you two because we have an sysfs interface naming question for
>>> AC sensors that touches on the edge of hwmon.
>>>
>>>       
>>>> For the metering parts I think we need to define a few more channel types.
>>>>
>>>> How about this ones
>>>>
>>>> inSX            S is the apparent power.
>>>> inPX            P is the active power.
>>>> inQX            Q is the reactive power.
>>>> inVX            V is the voltage. (only inX ?)
>>>> inVRMSX VRMS is the quadratic mean voltage.
>>>>         
>>> Call it 'root mean square' rather than quadratic in the docs. They have different
>>> meanings in English.
>>>       
>>>> inIX            I is the current.
>>>>         
>>> currX as per hwmon?  They also define a power attribute, but only 1 (as DC
>>> I guess). Cc'd Guenter and Jean to see if we can / want to share an interface...
>>>
>>> Guenter/ Jean, do you think hwmon will ever handle AC sensors?
>>>       
>> Well, never say never ;) While I've never heard of such sensors, I
>> guess it would make some sense for a computer PSU to include a sensor
>> chip monitoring both the AC input and the DC output to measure the
>> efficiency of the unit.
>>
>>     
>>> Maybe we want to be
>>> well clear of your interfaces just to avoid confusion? Or define a new set of shared
>>> names for the above that we will both use (when it becomes relevant?)
>>>       
>> It's hard to tell in advance what hwmon would implement, as we lack
>> actual examples. All I can say is that we would never use "in" prefixes
>> for power or current. The above power examples would most probably be
>> named powerX_apparent_input, powerX_active_input and
>> powerX_reactive_input, if we ever have to support these. And
>> inX_rms_input for the root mean square voltage input.
>>
>> But then again, I'm not sure if there is any point in sharing anything,
>> or forcing any difference, between hwmon and iio interfaces. They are
>> different by nature, and if we don't strictly enforce their relation,
>> they are bound to randomly diverge and converge anyway.
>>
>>     
> Agreed to a certain extent, but saying that there is no point in reinventing
> the wheel.  I think the naming you've just suggested is clearer anyway.
>
> Ultimately I don't insist on keeping to your interfaces when it really doesn't
> make sense, but when it does, it makes our life easier as we aren't starting from
> scratch. Also, politically it was suggested we do this by a few people I'd like
> to keep on side.
>
> Michael - howabout doing the rms values as done with peak_raw - as chan info parameters
> (directly derived from raw values anyway).
>   

I don't understand - why would the RMS value being a _raw value?
V and Vrms are calculated inside the metering IC.
And so far we use the _raw stuff only if the value needs to be scaled
somehow. 


> Then define two new (to us) channel types.
>
> IIO_CURRENT = "curr%d"
> IIO_POWER = "curr%d"
>   
power?
> Using the rfc I'm going to post after I send this email, we can add additional names to a channel.
> The only slight annoyance is the 3 power values will need to be different channels, so under that
> we will have:
>
> power0_apparent_raw           is the apparent power.
> power1_active_raw             is the active power.
> power2_reactive_raw           is the reactive power.
> in0                           is the voltage.
>   

If we want to be more verbose, why do we use 'in' for voltage and not
voltageX ?
It looks to me we're messing something up here. For DACs we use outX for
voltage!
I think we have to come up with something that allows us to specify
whether something is an output or input.
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.

> in0_rms_raw                   is the rms voltage.
> curr0_raw                     is the current.
>
> We may want to think about how to indicate which of these are measured on the same physical pin.
> If we were to have all the power measures as power0_* then we need another way of differentiating
> their event codes.  Could use modifiers I suppose... I've just sanity checked that there are no
> other issues, by modifying tsl2563 to claim both intensity readings are on the same channel.
>
> What do you think?
> _______________________________________________
> Device-drivers-devel mailing list
> Device-drivers-devel@blackfin.uclinux.org
> https://blackfin.uclinux.org/mailman/listinfo/device-drivers-devel
>
>   


-- 
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

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

[Jonathan Cameron]

On 04/28/11 15:58, Michael Hennerich wrote:
> On 04/28/2011 04:21 PM, Jonathan Cameron wrote:
>> On 04/28/11 14:51, Jean Delvare wrote:
>>   
>>> On Thu, 28 Apr 2011 10:31:15 +0100, Jonathan Cameron wrote:
>>>     
>>>> Guenter / Jean - cc'd you two because we have an sysfs interface naming question for
>>>> AC sensors that touches on the edge of hwmon.
>>>>
>>>>       
>>>>> For the metering parts I think we need to define a few more channel types.
>>>>>
>>>>> How about this ones
>>>>>
>>>>> inSX            S is the apparent power.
>>>>> inPX            P is the active power.
>>>>> inQX            Q is the reactive power.
>>>>> inVX            V is the voltage. (only inX ?)
>>>>> inVRMSX VRMS is the quadratic mean voltage.
>>>>>         
>>>> Call it 'root mean square' rather than quadratic in the docs. They have different
>>>> meanings in English.
>>>>       
>>>>> inIX            I is the current.
>>>>>         
>>>> currX as per hwmon?  They also define a power attribute, but only 1 (as DC
>>>> I guess). Cc'd Guenter and Jean to see if we can / want to share an interface...
>>>>
>>>> Guenter/ Jean, do you think hwmon will ever handle AC sensors?
>>>>       
>>> Well, never say never ;) While I've never heard of such sensors, I
>>> guess it would make some sense for a computer PSU to include a sensor
>>> chip monitoring both the AC input and the DC output to measure the
>>> efficiency of the unit.
>>>
>>>     
>>>> Maybe we want to be
>>>> well clear of your interfaces just to avoid confusion? Or define a new set of shared
>>>> names for the above that we will both use (when it becomes relevant?)
>>>>       
>>> It's hard to tell in advance what hwmon would implement, as we lack
>>> actual examples. All I can say is that we would never use "in" prefixes
>>> for power or current. The above power examples would most probably be
>>> named powerX_apparent_input, powerX_active_input and
>>> powerX_reactive_input, if we ever have to support these. And
>>> inX_rms_input for the root mean square voltage input.
>>>
>>> But then again, I'm not sure if there is any point in sharing anything,
>>> or forcing any difference, between hwmon and iio interfaces. They are
>>> different by nature, and if we don't strictly enforce their relation,
>>> they are bound to randomly diverge and converge anyway.
>>>
>>>     
>> Agreed to a certain extent, but saying that there is no point in reinventing
>> the wheel.  I think the naming you've just suggested is clearer anyway.
>>
>> Ultimately I don't insist on keeping to your interfaces when it really doesn't
>> make sense, but when it does, it makes our life easier as we aren't starting from
>> scratch. Also, politically it was suggested we do this by a few people I'd like
>> to keep on side.
>>
>> Michael - howabout doing the rms values as done with peak_raw - as chan info parameters
>> (directly derived from raw values anyway).
>>   
> 
> I don't understand - why would the RMS value being a _raw value?
> V and Vrms are calculated inside the metering IC.
> And so far we use the _raw stuff only if the value needs to be scaled
> somehow.
So these are already in Volts? (or I guess millivolts?)
Surely they need the same scaling as in0_raw?
> 
> 
>> Then define two new (to us) channel types.
>>
>> IIO_CURRENT = "curr%d"
>> IIO_POWER = "curr%d"
>>   
> power?
Oopsy.
>> Using the rfc I'm going to post after I send this email, we can add additional names to a channel.
>> The only slight annoyance is the 3 power values will need to be different channels, so under that
>> we will have:
>>
>> power0_apparent_raw           is the apparent power.
>> power1_active_raw             is the active power.
>> power2_reactive_raw           is the reactive power.
>> in0                           is the voltage.
>>   
> 
> If we want to be more verbose, why do we use 'in' for voltage and not
> voltageX ?
Matching hwmon, Greg KH and others pushed for this early on. I didn't care
really as long as it didn't get in the way.  So when you get down to
it combination of trying to limit variation of interfaces in kernel
and politics.  Jean has always argued (as here) that it probably isn't
going to work anyway!

> It looks to me we're messing something up here. For DACs we use outX for
> voltage!
> I think we have to come up with something that allows us to specify
> whether something is an output or input.
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.

> 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.
> 
>> in0_rms_raw                   is the rms voltage.
>> curr0_raw                     is the current.
>>
>> We may want to think about how to indicate which of these are measured on the same physical pin.
>> If we were to have all the power measures as power0_* then we need another way of differentiating
>> their event codes.  Could use modifiers I suppose... I've just sanity checked that there are no
>> other issues, by modifying tsl2563 to claim both intensity readings are on the same channel.
>>
>> What do you think?
>> _______________________________________________
>> Device-drivers-devel mailing list
>> Device-drivers-devel@blackfin.uclinux.org
>> https://blackfin.uclinux.org/mailman/listinfo/device-drivers-devel
>>
>>   
> 
> 

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

[Michael Hennerich]

On 04/28/2011 05:46 PM, Jonathan Cameron wrote:
> On 04/28/11 15:58, Michael Hennerich wrote:
>   
>> On 04/28/2011 04:21 PM, Jonathan Cameron wrote:
>>     
>>> On 04/28/11 14:51, Jean Delvare wrote:
>>>
>>>       
>>>> On Thu, 28 Apr 2011 10:31:15 +0100, Jonathan Cameron wrote:
>>>>
>>>>         
>>>>> Guenter / Jean - cc'd you two because we have an sysfs interface naming question for
>>>>> AC sensors that touches on the edge of hwmon.
>>>>>
>>>>>
>>>>>           
>>>>>> For the metering parts I think we need to define a few more channel types.
>>>>>>
>>>>>> How about this ones
>>>>>>
>>>>>> inSX            S is the apparent power.
>>>>>> inPX            P is the active power.
>>>>>> inQX            Q is the reactive power.
>>>>>> inVX            V is the voltage. (only inX ?)
>>>>>> inVRMSX VRMS is the quadratic mean voltage.
>>>>>>
>>>>>>             
>>>>> Call it 'root mean square' rather than quadratic in the docs. They have different
>>>>> meanings in English.
>>>>>
>>>>>           
>>>>>> inIX            I is the current.
>>>>>>
>>>>>>             
>>>>> currX as per hwmon?  They also define a power attribute, but only 1 (as DC
>>>>> I guess). Cc'd Guenter and Jean to see if we can / want to share an interface...
>>>>>
>>>>> Guenter/ Jean, do you think hwmon will ever handle AC sensors?
>>>>>
>>>>>           
>>>> Well, never say never ;) While I've never heard of such sensors, I
>>>> guess it would make some sense for a computer PSU to include a sensor
>>>> chip monitoring both the AC input and the DC output to measure the
>>>> efficiency of the unit.
>>>>
>>>>
>>>>         
>>>>> Maybe we want to be
>>>>> well clear of your interfaces just to avoid confusion? Or define a new set of shared
>>>>> names for the above that we will both use (when it becomes relevant?)
>>>>>
>>>>>           
>>>> It's hard to tell in advance what hwmon would implement, as we lack
>>>> actual examples. All I can say is that we would never use "in" prefixes
>>>> for power or current. The above power examples would most probably be
>>>> named powerX_apparent_input, powerX_active_input and
>>>> powerX_reactive_input, if we ever have to support these. And
>>>> inX_rms_input for the root mean square voltage input.
>>>>
>>>> But then again, I'm not sure if there is any point in sharing anything,
>>>> or forcing any difference, between hwmon and iio interfaces. They are
>>>> different by nature, and if we don't strictly enforce their relation,
>>>> they are bound to randomly diverge and converge anyway.
>>>>
>>>>
>>>>         
>>> Agreed to a certain extent, but saying that there is no point in reinventing
>>> the wheel.  I think the naming you've just suggested is clearer anyway.
>>>
>>> Ultimately I don't insist on keeping to your interfaces when it really doesn't
>>> make sense, but when it does, it makes our life easier as we aren't starting from
>>> scratch. Also, politically it was suggested we do this by a few people I'd like
>>> to keep on side.
>>>
>>> Michael - howabout doing the rms values as done with peak_raw - as chan info parameters
>>> (directly derived from raw values anyway).
>>>
>>>       
>> I don't understand - why would the RMS value being a _raw value?
>> V and Vrms are calculated inside the metering IC.
>> And so far we use the _raw stuff only if the value needs to be scaled
>> somehow.
>>     
> So these are already in Volts? (or I guess millivolts?)
>   
Need to check the exact data representation.
There are offset and gain control registers associated. Ideally they are
properly set and then we get the voltage in millivolts.
The none RMS values are the absolute voltages that get sampled into the
ring buffer.
The RMS voltages are calculated inside the part and provided to the user
as sysfs attributes files.
 
> Surely they need the same scaling as in0_raw?
>   
>>
>>     
>>> Then define two new (to us) channel types.
>>>
>>> IIO_CURRENT = "curr%d"
>>> IIO_POWER = "curr%d"
>>>
>>>       
>> power?
>>     
> Oopsy.
>   
>>> Using the rfc I'm going to post after I send this email, we can add additional names to a channel.
>>> The only slight annoyance is the 3 power values will need to be different channels, so under that
>>> we will have:
>>>
>>> power0_apparent_raw           is the apparent power.
>>> power1_active_raw             is the active power.
>>> power2_reactive_raw           is the reactive power.
>>> in0                           is the voltage.
>>>
>>>       
>> If we want to be more verbose, why do we use 'in' for voltage and not
>> voltageX ?
>>     
> Matching hwmon, Greg KH and others pushed for this early on. I didn't care
> really as long as it didn't get in the way.  So when you get down to
> it combination of trying to limit variation of interfaces in kernel
> and politics.  Jean has always argued (as here) that it probably isn't
> going to work anyway!
>   
IIO and HWMON servers different purposes. I think we already got to the
point of divergent interfaces.
 
>   
>> It looks to me we're messing something up here. For DACs we use outX for
>> voltage!
>> I think we have to come up with something that allows us to specify
>> whether something is an output or input.
>>     
> 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.
>   
>> 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

?
>>     
>>> in0_rms_raw                   is the rms voltage.
>>> curr0_raw                     is the current.
>>>
>>> We may want to think about how to indicate which of these are measured on the same physical pin.
>>> If we were to have all the power measures as power0_* then we need another way of differentiating
>>> their event codes.  Could use modifiers I suppose... I've just sanity checked that there are no
>>> other issues, by modifying tsl2563 to claim both intensity readings are on the same channel.
>>>
>>> What do you think?
>>> _______________________________________________
>>> Device-drivers-devel mailing list
>>> Device-drivers-devel@blackfin.uclinux.org
>>> https://blackfin.uclinux.org/mailman/listinfo/device-drivers-devel
>>>
>>>
>>>       
>>
>>     
>
>   


-- 
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

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

[Jonathan Cameron]

On 04/29/11 15:21, Michael Hennerich wrote:
> On 04/28/2011 05:46 PM, Jonathan Cameron wrote:
>> On 04/28/11 15:58, Michael Hennerich wrote:
>>   
>>> On 04/28/2011 04:21 PM, Jonathan Cameron wrote:
>>>     
>>>> On 04/28/11 14:51, Jean Delvare wrote:
>>>>
>>>>       
>>>>> On Thu, 28 Apr 2011 10:31:15 +0100, Jonathan Cameron wrote:
>>>>>
>>>>>         
>>>>>> Guenter / Jean - cc'd you two because we have an sysfs interface naming question for
>>>>>> AC sensors that touches on the edge of hwmon.
>>>>>>
>>>>>>
>>>>>>           
>>>>>>> For the metering parts I think we need to define a few more channel types.
>>>>>>>
>>>>>>> How about this ones
>>>>>>>
>>>>>>> inSX            S is the apparent power.
>>>>>>> inPX            P is the active power.
>>>>>>> inQX            Q is the reactive power.
>>>>>>> inVX            V is the voltage. (only inX ?)
>>>>>>> inVRMSX VRMS is the quadratic mean voltage.
>>>>>>>
>>>>>>>             
>>>>>> Call it 'root mean square' rather than quadratic in the docs. They have different
>>>>>> meanings in English.
>>>>>>
>>>>>>           
>>>>>>> inIX            I is the current.
>>>>>>>
>>>>>>>             
>>>>>> currX as per hwmon?  They also define a power attribute, but only 1 (as DC
>>>>>> I guess). Cc'd Guenter and Jean to see if we can / want to share an interface...
>>>>>>
>>>>>> Guenter/ Jean, do you think hwmon will ever handle AC sensors?
>>>>>>
>>>>>>           
>>>>> Well, never say never ;) While I've never heard of such sensors, I
>>>>> guess it would make some sense for a computer PSU to include a sensor
>>>>> chip monitoring both the AC input and the DC output to measure the
>>>>> efficiency of the unit.
>>>>>
>>>>>
>>>>>         
>>>>>> Maybe we want to be
>>>>>> well clear of your interfaces just to avoid confusion? Or define a new set of shared
>>>>>> names for the above that we will both use (when it becomes relevant?)
>>>>>>
>>>>>>           
>>>>> It's hard to tell in advance what hwmon would implement, as we lack
>>>>> actual examples. All I can say is that we would never use "in" prefixes
>>>>> for power or current. The above power examples would most probably be
>>>>> named powerX_apparent_input, powerX_active_input and
>>>>> powerX_reactive_input, if we ever have to support these. And
>>>>> inX_rms_input for the root mean square voltage input.
>>>>>
>>>>> But then again, I'm not sure if there is any point in sharing anything,
>>>>> or forcing any difference, between hwmon and iio interfaces. They are
>>>>> different by nature, and if we don't strictly enforce their relation,
>>>>> they are bound to randomly diverge and converge anyway.
>>>>>
>>>>>
>>>>>         
>>>> Agreed to a certain extent, but saying that there is no point in reinventing
>>>> the wheel.  I think the naming you've just suggested is clearer anyway.
>>>>
>>>> Ultimately I don't insist on keeping to your interfaces when it really doesn't
>>>> make sense, but when it does, it makes our life easier as we aren't starting from
>>>> scratch. Also, politically it was suggested we do this by a few people I'd like
>>>> to keep on side.
>>>>
>>>> Michael - howabout doing the rms values as done with peak_raw - as chan info parameters
>>>> (directly derived from raw values anyway).
>>>>
>>>>       
>>> I don't understand - why would the RMS value being a _raw value?
>>> V and Vrms are calculated inside the metering IC.
>>> And so far we use the _raw stuff only if the value needs to be scaled
>>> somehow.
>>>     
>> So these are already in Volts? (or I guess millivolts?)
>>   
> Need to check the exact data representation.
> There are offset and gain control registers associated. Ideally they are
> properly set and then we get the voltage in millivolts.
> The none RMS values are the absolute voltages that get sampled into the
> ring buffer.
> The RMS voltages are calculated inside the part and provided to the user
> as sysfs attributes files.
Cool. In that case we don't need the raw.  However, we do perhaps need to make it
clear it is processed and put _input at the end instead...
>  
>> Surely they need the same scaling as in0_raw?
>>   
>>>
>>>     
>>>> Then define two new (to us) channel types.
>>>>
>>>> IIO_CURRENT = "curr%d"
>>>> IIO_POWER = "curr%d"
>>>>
>>>>       
>>> power?
>>>     
>> Oopsy.
>>   
>>>> Using the rfc I'm going to post after I send this email, we can add additional names to a channel.
>>>> The only slight annoyance is the 3 power values will need to be different channels, so under that
>>>> we will have:
>>>>
>>>> power0_apparent_raw           is the apparent power.
>>>> power1_active_raw             is the active power.
>>>> power2_reactive_raw           is the reactive power.
>>>> in0                           is the voltage.
>>>>
>>>>       
>>> If we want to be more verbose, why do we use 'in' for voltage and not
>>> voltageX ?
>>>     
>> Matching hwmon, Greg KH and others pushed for this early on. I didn't care
>> really as long as it didn't get in the way.  So when you get down to
>> it combination of trying to limit variation of interfaces in kernel
>> and politics.  Jean has always argued (as here) that it probably isn't
>> going to work anyway!
>>   
> IIO and HWMON servers different purposes. I think we already got to the
> point of divergent interfaces.
True.  But so far it is only for things where we really have to do so,
such as threshold interrupts.  Hwmon's alarms are fine for their usecases
but only cover a fraction of what we have on inertial sensors for starters.
(as you'd expect)
>  
>>   
>>> It looks to me we're messing something up here. For DACs we use outX for
>>> voltage!
>>> I think we have to come up with something that allows us to specify
>>> whether something is an output or input.
>>>     
>> 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.
> ?
>>>     
>>>> in0_rms_raw                   is the rms voltage.
>>>> curr0_raw                     is the current.
>>>>
>>>> We may want to think about how to indicate which of these are measured on the same physical pin.
>>>> If we were to have all the power measures as power0_* then we need another way of differentiating
>>>> their event codes.  Could use modifiers I suppose... I've just sanity checked that there are no
>>>> other issues, by modifying tsl2563 to claim both intensity readings are on the same channel.
>>>>
>>>> What do you think?
>>>> _______________________________________________
>>>> Device-drivers-devel mailing list
>>>> Device-drivers-devel@blackfin.uclinux.org
>>>> https://blackfin.uclinux.org/mailman/listinfo/device-drivers-devel
>>>>
>>>>
>>>>       
>>>
>>>     
>>
>>   
> 
> 

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

[Michael Hennerich]

On 04/29/2011 05:03 PM, Jonathan Cameron wrote:
> On 04/29/11 15:21, Michael Hennerich wrote:
>   
>> On 04/28/2011 05:46 PM, Jonathan Cameron wrote:
>>     
>>> On 04/28/11 15:58, Michael Hennerich wrote:
>>>
>>>       
>>>> On 04/28/2011 04:21 PM, Jonathan Cameron wrote:
>>>>
>>>>         
>>>>> On 04/28/11 14:51, Jean Delvare wrote:
>>>>>
>>>>>
>>>>>           
>>>>>> On Thu, 28 Apr 2011 10:31:15 +0100, Jonathan Cameron wrote:
>>>>>>
>>>>>>
>>>>>>             
>>>>>>> Guenter / Jean - cc'd you two because we have an sysfs interface naming question for
>>>>>>> AC sensors that touches on the edge of hwmon.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>               
>>>>>>>> For the metering parts I think we need to define a few more channel types.
>>>>>>>>
>>>>>>>> How about this ones
>>>>>>>>
>>>>>>>> inSX            S is the apparent power.
>>>>>>>> inPX            P is the active power.
>>>>>>>> inQX            Q is the reactive power.
>>>>>>>> inVX            V is the voltage. (only inX ?)
>>>>>>>> inVRMSX VRMS is the quadratic mean voltage.
>>>>>>>>
>>>>>>>>
>>>>>>>>                 
>>>>>>> Call it 'root mean square' rather than quadratic in the docs. They have different
>>>>>>> meanings in English.
>>>>>>>
>>>>>>>
>>>>>>>               
>>>>>>>> inIX            I is the current.
>>>>>>>>
>>>>>>>>
>>>>>>>>                 
>>>>>>> currX as per hwmon?  They also define a power attribute, but only 1 (as DC
>>>>>>> I guess). Cc'd Guenter and Jean to see if we can / want to share an interface...
>>>>>>>
>>>>>>> Guenter/ Jean, do you think hwmon will ever handle AC sensors?
>>>>>>>
>>>>>>>
>>>>>>>               
>>>>>> Well, never say never ;) While I've never heard of such sensors, I
>>>>>> guess it would make some sense for a computer PSU to include a sensor
>>>>>> chip monitoring both the AC input and the DC output to measure the
>>>>>> efficiency of the unit.
>>>>>>
>>>>>>
>>>>>>
>>>>>>             
>>>>>>> Maybe we want to be
>>>>>>> well clear of your interfaces just to avoid confusion? Or define a new set of shared
>>>>>>> names for the above that we will both use (when it becomes relevant?)
>>>>>>>
>>>>>>>
>>>>>>>               
>>>>>> It's hard to tell in advance what hwmon would implement, as we lack
>>>>>> actual examples. All I can say is that we would never use "in" prefixes
>>>>>> for power or current. The above power examples would most probably be
>>>>>> named powerX_apparent_input, powerX_active_input and
>>>>>> powerX_reactive_input, if we ever have to support these. And
>>>>>> inX_rms_input for the root mean square voltage input.
>>>>>>
>>>>>> But then again, I'm not sure if there is any point in sharing anything,
>>>>>> or forcing any difference, between hwmon and iio interfaces. They are
>>>>>> different by nature, and if we don't strictly enforce their relation,
>>>>>> they are bound to randomly diverge and converge anyway.
>>>>>>
>>>>>>
>>>>>>
>>>>>>             
>>>>> Agreed to a certain extent, but saying that there is no point in reinventing
>>>>> the wheel.  I think the naming you've just suggested is clearer anyway.
>>>>>
>>>>> Ultimately I don't insist on keeping to your interfaces when it really doesn't
>>>>> make sense, but when it does, it makes our life easier as we aren't starting from
>>>>> scratch. Also, politically it was suggested we do this by a few people I'd like
>>>>> to keep on side.
>>>>>
>>>>> Michael - howabout doing the rms values as done with peak_raw - as chan info parameters
>>>>> (directly derived from raw values anyway).
>>>>>
>>>>>
>>>>>           
>>>> I don't understand - why would the RMS value being a _raw value?
>>>> V and Vrms are calculated inside the metering IC.
>>>> And so far we use the _raw stuff only if the value needs to be scaled
>>>> somehow.
>>>>
>>>>         
>>> So these are already in Volts? (or I guess millivolts?)
>>>
>>>       
>> Need to check the exact data representation.
>> There are offset and gain control registers associated. Ideally they are
>> properly set and then we get the voltage in millivolts.
>> The none RMS values are the absolute voltages that get sampled into the
>> ring buffer.
>> The RMS voltages are calculated inside the part and provided to the user
>> as sysfs attributes files.
>>     
> Cool. In that case we don't need the raw.  However, we do perhaps need to make it
> clear it is processed and put _input at the end instead...
>   
>>     
>>> Surely they need the same scaling as in0_raw?
>>>
>>>       
>>>>
>>>>         
>>>>> Then define two new (to us) channel types.
>>>>>
>>>>> IIO_CURRENT = "curr%d"
>>>>> IIO_POWER = "curr%d"
>>>>>
>>>>>
>>>>>           
>>>> power?
>>>>
>>>>         
>>> Oopsy.
>>>
>>>       
>>>>> Using the rfc I'm going to post after I send this email, we can add additional names to a channel.
>>>>> The only slight annoyance is the 3 power values will need to be different channels, so under that
>>>>> we will have:
>>>>>
>>>>> power0_apparent_raw           is the apparent power.
>>>>> power1_active_raw             is the active power.
>>>>> power2_reactive_raw           is the reactive power.
>>>>> in0                           is the voltage.
>>>>>
>>>>>
>>>>>           
>>>> If we want to be more verbose, why do we use 'in' for voltage and not
>>>> voltageX ?
>>>>
>>>>         
>>> Matching hwmon, Greg KH and others pushed for this early on. I didn't care
>>> really as long as it didn't get in the way.  So when you get down to
>>> it combination of trying to limit variation of interfaces in kernel
>>> and politics.  Jean has always argued (as here) that it probably isn't
>>> going to work anyway!
>>>
>>>       
>> IIO and HWMON servers different purposes. I think we already got to the
>> point of divergent interfaces.
>>     
> True.  But so far it is only for things where we really have to do so,
> such as threshold interrupts.  Hwmon's alarms are fine for their usecases
> but only cover a fraction of what we have on inertial sensors for starters.
> (as you'd expect)
>   
>>     
>>>       
>>>> It looks to me we're messing something up here. For DACs we use outX for
>>>> voltage!
>>>> I think we have to come up with something that allows us to specify
>>>> whether something is an output or input.
>>>>
>>>>         
>>> 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.

>> ?
>>     
>>>>         
>>>>> in0_rms_raw                   is the rms voltage.
>>>>> curr0_raw                     is the current.
>>>>>
>>>>> We may want to think about how to indicate which of these are measured on the same physical pin.
>>>>> If we were to have all the power measures as power0_* then we need another way of differentiating
>>>>> their event codes.  Could use modifiers I suppose... I've just sanity checked that there are no
>>>>> other issues, by modifying tsl2563 to claim both intensity readings are on the same channel.
>>>>>
>>>>> What do you think?
>>>>> _______________________________________________
>>>>> Device-drivers-devel mailing list
>>>>> Device-drivers-devel@blackfin.uclinux.org
>>>>> https://blackfin.uclinux.org/mailman/listinfo/device-drivers-devel
>>>>>
>>>>>
>>>>>
>>>>>           
>>>>
>>>>         
>>>
>>>       
>>
>>     
>
>   


-- 
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

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

[Jonathan Cameron]

>>>>>         
>>>> 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?)
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...

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

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

[Michael Hennerich]

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

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

[Jonathan Cameron]

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

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

[Jonathan Cameron]

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!
> 
>>> 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
>>>
>>>   
>>
>>
> 
> --
> 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
> 

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

[Jonathan Cameron]

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.


> 

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

[Hennerich, Michael]

Jonathan Cameron wrote on 2011-05-04:
> 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.

Thanks for the update - I'll switch to the work branch tomorrow.
Still doing the ADE7758 cleanups on your michex branch.


Greetings,
Michael

--
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368; Gesch=
aeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin, Margaret=
 Seif