Skip to contentSkip to conversation

Oddities and how to handle them.

DISCUSSION Jonathan Cameron 4 participants 25 messages Apr 26, 2011 – May 4, 2011 IIO

lore
Share & tools
lore
#0 lore raw

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

#1 lore raw
Quoted 12 lines · 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!)

Quoted 6 lines

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.

Quoted 10 lines

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.

Quoted 6 lines

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?

Quoted 7 lines

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

Signature · 8 lines
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
#2 lore raw
Quoted 19 lines · Michael Hennerich
Quoted 12 lines · 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!)

Quoted 6 lines

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!

Quoted 3 lines

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.

Quoted 11 lines
Quoted 11 lines

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.

Quoted 4 lines

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

Quoted 8 lines
Quoted 6 lines

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!

Quoted 7 lines

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

#3 lore raw

Jonathan Cameron wrote on 2011-04-27:

Quoted 22 lines · Michael Hennerich
Quoted 11 lines · Jonathan Cameron
Quoted 8 lines

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!

Quoted 8 lines

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

Quoted 5 lines

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

#4 lore raw
Quoted 29 lines · Michael

Jonathan Cameron wrote on 2011-04-27:

Quoted 22 lines · Michael Hennerich
Quoted 11 lines · Jonathan Cameron
Quoted 8 lines

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!

Quoted 8 lines

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

Quoted 5 lines

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.

Quoted 3 lines

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

#5 lore raw

Jonathan Cameron wrote on 2011-04-27:

Quoted 24 lines · Michael
Quoted 23 lines

Jonathan Cameron wrote on 2011-04-27:

Quoted 22 lines · Michael Hennerich
Quoted 11 lines · Jonathan Cameron
Quoted 8 lines

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!

Quoted 8 lines

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

Quoted 1 line
Quoted 1 line
Quoted 1 line

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

#6 lore raw

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

Quoted 26 lines
Quoted 20 lines
Quoted 20 lines
Quoted 20 lines
Quoted 10 lines
Quoted 8 lines

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!

Quoted 8 lines

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

Quoted 1 line
Quoted 1 line
Quoted 1 line

or.

Hi Jonathan,

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

Quoted 7 lines

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.

Quoted 1 line

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

Quoted 7 lines

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

#7 lore raw
Quoted 42 lines · 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.

Quoted 30 lines
Quoted 24 lines
Quoted 24 lines
Quoted 24 lines
Quoted 9 lines
Quoted 9 lines

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

Quoted 3 lines

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!

Quoted 9 lines

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

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

or.

Hi Jonathan,

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

Quoted 8 lines

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.

Quoted 8 lines

Call it 'root mean square' rather than quadratic in the docs. They have different meanings in English.

Quoted 2 lines

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

Quoted 32 lines

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

Quoted 8 lines

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

Signature · 8 lines
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
#8 lore raw
Quoted 48 lines · Michael Hennerich
Quoted 47 lines · 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.

Quoted 30 lines
Quoted 24 lines
Quoted 24 lines
Quoted 24 lines
Quoted 9 lines
Quoted 9 lines

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

Quoted 3 lines

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!

Quoted 9 lines

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

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

or.

Hi Jonathan,

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

Quoted 8 lines

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.

Quoted 2 lines

inIX I is the current.

Quoted 8 lines
Quoted 3 lines

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

Quoted 35 lines
Quoted 33 lines

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

Quoted 8 lines

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

#9 lore raw
Quoted 52 lines · Jonathan Cameron
Quoted 51 lines · Michael Hennerich
Quoted 50 lines · 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.

Quoted 30 lines
Quoted 24 lines
Quoted 24 lines
Quoted 24 lines
Quoted 9 lines
Quoted 9 lines

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

Quoted 3 lines

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!

Quoted 9 lines

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

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

or.

Hi Jonathan,

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

Quoted 8 lines

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.

Quoted 2 lines

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

Quoted 8 lines
Quoted 5 lines

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.

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

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

#10 lore raw
In reply to Jonathan Cameron
Quoted 12 lines · 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.

Quoted 9 lines

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.

Quoted 7 lines

Call it 'root mean square' rather than quadratic in the docs. They have different meanings in English.

Quoted 1 line

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.

Quoted 3 lines

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.

Signature · 1 line
Jean Delvare
#11 lore raw
Quoted 37 lines · Jean Delvare
Quoted 12 lines · 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.

Quoted 9 lines

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.

Quoted 7 lines

Call it 'root mean square' rather than quadratic in the docs. They have different meanings in English.

Quoted 1 line

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.

Quoted 3 lines

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.

Quoted 5 lines

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?

#12 lore raw
Quoted 51 lines · Jonathan Cameron
Quoted 37 lines · Jean Delvare
Quoted 12 lines · 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.

Quoted 9 lines

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.

Quoted 7 lines

Call it 'root mean square' rather than quadratic in the docs. They have different meanings in English.

Quoted 1 line

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.

Quoted 3 lines

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.

Quoted 5 lines

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.

Quoted 8 lines

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.

Quoted 11 lines

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

#13 lore raw
Quoted 55 lines · Guenter Roeck
Quoted 54 lines · Jonathan Cameron
Quoted 37 lines · Jean Delvare
Quoted 12 lines · 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.

Quoted 9 lines

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.

Quoted 7 lines

Call it 'root mean square' rather than quadratic in the docs. They have different meanings in English.

Quoted 1 line

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.

Quoted 3 lines

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.

Quoted 5 lines

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

Quoted 6 lines
Quoted 5 lines

Then define two new (to us) channel types.

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

power%d, presumably.

Ooops. Thanks.

Quoted 12 lines
Quoted 5 lines

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:

Quoted 7 lines
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.
Quoted 2 lines

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

#14 lore raw
In reply to Jonathan Cameron
Quoted 54 lines · Jonathan Cameron
Quoted 43 lines · Jean Delvare
Quoted 17 lines · 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.

Quoted 10 lines

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.

Quoted 7 lines
Quoted 2 lines

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.

Quoted 4 lines

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.

Quoted 6 lines

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.

Quoted 8 lines

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.

Quoted 5 lines

Then define two new (to us) channel types.

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

power?

Quoted 9 lines

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.

Quoted 15 lines

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
[email protected]
https://blackfin.uclinux.org/mailman/listinfo/device-drivers-devel

Signature · 8 lines
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
#15 lore raw
Quoted 60 lines · Michael Hennerich
Quoted 59 lines · Jonathan Cameron
Quoted 43 lines · Jean Delvare
Quoted 17 lines · 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.

Quoted 10 lines

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.

Quoted 7 lines
Quoted 2 lines

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.

Quoted 4 lines

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.

Quoted 6 lines

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.

Quoted 8 lines
Quoted 3 lines

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?

Quoted 8 lines
Quoted 7 lines

Then define two new (to us) channel types.

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

power?

Oopsy.

Quoted 4 lines
Quoted 4 lines

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:

Quoted 8 lines
Quoted 5 lines
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!

Quoted 4 lines

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.

Quoted 6 lines

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.

Quoted 18 lines
Quoted 16 lines

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
[email protected]
https://blackfin.uclinux.org/mailman/listinfo/device-drivers-devel

#16 lore raw
Quoted 77 lines · Jonathan Cameron
Quoted 75 lines · Michael Hennerich
Quoted 73 lines · Jonathan Cameron
Quoted 51 lines · Jean Delvare
Quoted 20 lines · 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.

Quoted 11 lines

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.

Quoted 9 lines
Quoted 3 lines

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.

Quoted 5 lines

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.

Quoted 7 lines

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

Quoted 7 lines
Quoted 5 lines

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.

Quoted 24 lines

Surely they need the same scaling as in0_raw?

Quoted 10 lines
Quoted 8 lines

Then define two new (to us) channel types.

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

power?

Oopsy.

Quoted 10 lines
Quoted 10 lines

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.
Quoted 9 lines
Quoted 3 lines

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.

Quoted 10 lines
Quoted 6 lines

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.

Quoted 20 lines
Quoted 8 lines

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.

Quoted 8 lines

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

?

Quoted 21 lines
Quoted 19 lines
Quoted 17 lines

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
[email protected]
https://blackfin.uclinux.org/mailman/listinfo/device-drivers-devel

Signature · 8 lines
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
#17 lore raw
Quoted 85 lines · Michael Hennerich
Quoted 77 lines · Jonathan Cameron
Quoted 75 lines · Michael Hennerich
Quoted 73 lines · Jonathan Cameron
Quoted 51 lines · Jean Delvare
Quoted 20 lines · 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.

Quoted 11 lines

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.

Quoted 9 lines
Quoted 3 lines

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.

Quoted 5 lines

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.

Quoted 7 lines

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

Quoted 7 lines
Quoted 5 lines

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

Quoted 7 lines

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

Quoted 28 lines
Quoted 28 lines

Surely they need the same scaling as in0_raw?

Quoted 10 lines
Quoted 8 lines

Then define two new (to us) channel types.

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

power?

Oopsy.

Quoted 13 lines
Quoted 10 lines

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 ?

Quoted 8 lines
Quoted 6 lines

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)

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

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.

Quoted 5 lines
Quoted 4 lines

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.

Quoted 33 lines
Quoted 20 lines
Quoted 8 lines

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.

Quoted 8 lines

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

Quoted 8 lines

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.

Quoted 24 lines

?

Quoted 21 lines
Quoted 19 lines
Quoted 17 lines

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
[email protected]
https://blackfin.uclinux.org/mailman/listinfo/device-drivers-devel

#18 lore raw
Quoted 229 lines · Jonathan Cameron
Quoted 101 lines · Michael Hennerich
Quoted 90 lines · Jonathan Cameron
Quoted 87 lines · Michael Hennerich
Quoted 84 lines · Jonathan Cameron
Quoted 59 lines · Jean Delvare
Quoted 23 lines · 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.

Quoted 12 lines

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.

Quoted 11 lines
Quoted 4 lines

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.

Quoted 6 lines

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.

Quoted 8 lines

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

Quoted 9 lines
Quoted 6 lines

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

Quoted 8 lines

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

Quoted 41 lines
Quoted 34 lines

Surely they need the same scaling as in0_raw?

Quoted 12 lines
Quoted 9 lines

Then define two new (to us) channel types.

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

power?

Oopsy.

Quoted 11 lines
Quoted 11 lines

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.
Quoted 4 lines

If we want to be more verbose, why do we use 'in' for voltage and not voltageX ?

Quoted 7 lines

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!

Quoted 3 lines

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)

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

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.

Quoted 7 lines
Quoted 5 lines

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.

Quoted 35 lines
Quoted 21 lines
Quoted 9 lines

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.

Quoted 9 lines

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

Quoted 9 lines

in_volatgeX_Y_rms_raw

or

inX_powerY_apparent_raw
inX_volatgeY_rms_raw
outX_volatgeY_raw

Quoted 7 lines

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.

Quoted 28 lines
Quoted 26 lines

?

Quoted 22 lines
Quoted 20 lines
Quoted 18 lines

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
[email protected]
https://blackfin.uclinux.org/mailman/listinfo/device-drivers-devel

Signature · 8 lines
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
#19 lore raw
Quoted 68 lines
Quoted 60 lines
Quoted 9 lines
Quoted 7 lines
Quoted 2 lines

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.

Quoted 35 lines
Quoted 21 lines
Quoted 9 lines

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.

Quoted 9 lines

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

Quoted 9 lines

in_volatgeX_Y_rms_raw

or

inX_powerY_apparent_raw
inX_volatgeY_rms_raw
outX_volatgeY_raw

Quoted 7 lines

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.

Quoted 7 lines

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

#20 lore raw
Quoted 112 lines · Jonathan Cameron
Quoted 74 lines
Quoted 66 lines
Quoted 11 lines
Quoted 8 lines
Quoted 2 lines

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.

Quoted 37 lines
Quoted 22 lines
Quoted 10 lines

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.

Quoted 10 lines

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

Quoted 10 lines

in_volatgeX_Y_rms_raw

or

inX_powerY_apparent_raw
inX_volatgeY_rms_raw
outX_volatgeY_raw

Quoted 8 lines

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.

Quoted 8 lines

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.

Quoted 8 lines

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.

Quoted 9 lines

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.

Quoted 15 lines

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

Signature · 8 lines
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
#21 lore raw
Quoted 116 lines · Michael Hennerich
Quoted 115 lines · Jonathan Cameron
Quoted 74 lines
Quoted 66 lines
Quoted 11 lines
Quoted 8 lines
Quoted 2 lines

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.

Quoted 37 lines
Quoted 22 lines
Quoted 10 lines

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.

Quoted 10 lines

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

Quoted 10 lines

in_volatgeX_Y_rms_raw

or

inX_powerY_apparent_raw
inX_volatgeY_rms_raw
outX_volatgeY_raw

Quoted 8 lines

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.

Quoted 8 lines

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

Quoted 8 lines
Quoted 5 lines

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

Quoted 10 lines
Quoted 9 lines

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!

Quoted 17 lines
Quoted 15 lines

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

#22 lore raw
Quoted 135 lines · Jonathan Cameron
Quoted 116 lines · Michael Hennerich
Quoted 115 lines · Jonathan Cameron
Quoted 74 lines
Quoted 66 lines
Quoted 11 lines
Quoted 8 lines
Quoted 2 lines

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.

Quoted 37 lines
Quoted 22 lines
Quoted 10 lines

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.

Quoted 10 lines

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

Quoted 10 lines

in_volatgeX_Y_rms_raw

or

inX_powerY_apparent_raw
inX_volatgeY_rms_raw
outX_volatgeY_raw

Quoted 8 lines

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.

Quoted 8 lines

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

Quoted 8 lines
Quoted 5 lines

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

Quoted 6 lines
Quoted 6 lines

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

Quoted 8 lines
Quoted 4 lines
Quoted 3 lines

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!

Quoted 24 lines
Quoted 9 lines
Quoted 9 lines

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.

Quoted 9 lines
Quoted 7 lines

Jonathan
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author Applied
#23 lore raw
Quoted 145 lines · Jonathan Cameron
Quoted 135 lines · Jonathan Cameron
Quoted 116 lines · Michael Hennerich
Quoted 115 lines · Jonathan Cameron
Quoted 74 lines
Quoted 66 lines
Quoted 11 lines
Quoted 8 lines
Quoted 2 lines

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.

Quoted 37 lines
Quoted 22 lines
Quoted 10 lines

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.

Quoted 10 lines

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

Quoted 10 lines

in_volatgeX_Y_rms_raw

or

inX_powerY_apparent_raw
inX_volatgeY_rms_raw
outX_volatgeY_raw

Quoted 8 lines

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.

Quoted 8 lines

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

Quoted 8 lines
Quoted 5 lines

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

Quoted 6 lines
Quoted 6 lines

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

Quoted 8 lines
Quoted 4 lines
Quoted 3 lines

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!

Quoted 6 lines

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.

Quoted 1 line
#24 lore raw

Jonathan Cameron wrote on 2011-05-04:

Quoted 154 lines · Jonathan Cameron
Quoted 138 lines · Jonathan Cameron
Quoted 137 lines · Michael Hennerich
Quoted 123 lines · Jonathan Cameron
Quoted 113 lines
Quoted 74 lines
Quoted 66 lines
Quoted 12 lines
Quoted 9 lines
Quoted 2 lines

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.

Quoted 36 lines
Quoted 21 lines
Quoted 10 lines

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.

Quoted 10 lines

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

Quoted 10 lines

in_volatgeX_Y_rms_raw

or

inX_powerY_apparent_raw
inX_volatgeY_rms_raw
outX_volatgeY_raw

Quoted 8 lines

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.

Quoted 7 lines

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.

Quoted 9 lines

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

Quoted 3 lines

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

Quoted 5 lines
Quoted 5 lines

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

Quoted 8 lines
Quoted 8 lines
Quoted 4 lines
Quoted 3 lines

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!

Quoted 6 lines

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.

Quoted 8 lines
Quoted 2 lines

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