Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)

[Jonathan Cameron]

On Sun, 25 Mar 2018 13:53:02 -0700
John Syne  wrote:

> > On Mar 25, 2018, at 9:54 AM, Jonathan Cameron  wrote:
> > 
> > On Sun, 25 Mar 2018 01:29:41 -0700
> > John Syne  wrote:
> >   
> >> Hi Jonathan,  
> > Hi John,
> > 
> > Please wrap your normal emails (excepting tables) to 80 chars.  
> Yeah, I’m trying to do that, but I use a Mac and Apple Mail doesn’t
> have a feature to wrap at 80 automatically so I have to do
> this manually.
> >   
> >> 
> >> I was speaking with Rodrigo and here is what I think must be done to move
> >> ADE7878 out of staging
> >> 
> >> Here are the steps as I see them:
> >> 
> >> 1) Define the IIO Attributes so they are consistent with the IIO ABI. This
> >>   should be pretty simple given agreement on the naming convention.  
> > Please don't go just changing attribute names - the driver needs to
> > use the standard approach of iio_chan_spec and create the vast
> > majority of attributes automatically via that.  There 'may' be
> > a few corner cases wee don't want to make generic and they 'might'
> > be exposed as attributes.
> > 
> > I suspect this change is the 'big' job.  The core support necessary
> > for RMS and MAV (computedtype or whatever we call it) will need adding
> > as well.  That is a separate patch set with support for some examples
> > in the dummy driver.  
> Great that you mentioned this, because I though we only needed to
> modify the IIO_ATTR naming. I’ll take a look at iio_chan_spec and
> the examples in the dummy driver.

That is how it was originally done and this is an 'old' driver,
but the chan spec stuff does three things :
1) Saves on lots of boiler plate code.
2) Makes it sane to access the device from other driver (hwmon now does 
something
similar btw).
3) Avoids a lot of ABI typos and careful review.

> >   
> >> 2) Map the ADE7854 interrupt status to IIO events. This requires an
> >>   interrupt processing section.
> >> 3) Add DeviceTree support.
> >> 4) Create DeviceTree overlay for the ADE7854.  
> > More a case of bindings for now.  If those are used via an overlay
> > fine but given we don't have any boards with one one in mainline,
> > this is an implementation detail for the user rather than part
> > of moving this driver out of staging.  
> I was thinking more along the lines of documentation to show a
> developer what was needed to get the driver working.
Do it as an example in the device tree bindings doc.

> >   
> >> 5) Update ADE7854 probe to read in the DeviceTree register settings.
> >> 6) Add support for power modes (PM1, PM2).  
> > This isn't necessary for a move out of staging (nice to have though).
> >   
> >> 7) Not sure if we will support measurement streaming on the ADE7854.
> >>   The problem is ADE7854 is designed as an SPI master, which means
> >>   it controls the SPI clock, so the driver must support SPI slave
> >>   mode. However, the Linux Kernel does not currently support SPI
> >>   slave mode. We have three choices to make this work and they
> >>   are all a lot of work: 1) Add support for SPI Slave mode to the
> >>   kernel,  2) Use hardware to convert SPI signals to I2S signals
> >>   and with the use of a custom codec, use the ALSA framework to
> >>   stream the samples (this is an approach I used, but I don’t like
> >>   it), 3) Move the I2S driver out of the sound subsystem and use it
> >>   together with DMA to stream samples directly into the ADE7854 driver
> >>   (my preferred solutions). Perhaps Mark Brown has some ideas on how
> >>to make this work.   
> > I'll be honest, this is an end of line part and frankly more than
> > a little crazy. I would go with simply not supporting the measurement
> > streaming at all for this part.  If you really need it we can then
> > move onto the how part, but from what you have said I'm guessing you
> > don't care except in an abstract 'it would be nice' sort of a way?  
> Yeah, I’m moving to the ADE9000 so I’ll drop this.
Agreed.  We let someone else pick it up if they care.

> >   
> >> 
> >> The ADE9000 will be much easier because it uses an SPI Slave interface. 
> >> 
> >> I hope I have captured everything, but let me know if I have missed 
> >> anything.
> >>   
> > 
> > That will do for now ;)  I'm sure there will be details that need
> > tidying up once we have the above done, but that's true for any new
> > driver (and this will be nearly a new driver before things are done).  
> Thank you again for all the detailed feedback.
> > 
> > Jonathan  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)

[Jonathan Cameron]

On Sun, 25 Mar 2018 13:36:40 -0700
John Syne  wrote:

> > On Mar 25, 2018, at 9:29 AM, Jonathan Cameron  wrote:
> > 
> > On Sat, 24 Mar 2018 15:45:21 -0700
> > John Syne  wrote:
> >   
> >>> On Mar 24, 2018, at 8:02 AM, Jonathan Cameron  wrote:
> >>> 
> >>> On Mon, 19 Mar 2018 22:57:16 -0700
> >>> John Syne  wrote:
> >>>   
>  Hi Jonathan,
>  
>  Thank you for all your hard work. Your feedback is really helpful. I’m 
>  surprised that no one from Analog Device has offered any suggestions.
>    
> >>> 
> >>> Sadly those active in the mainline linux kernel from ADI are focused in
> >>> other areas currently :(
> >> Good point. I will reach out to Analog Devices and get a name of someone 
> >> who is knowledgeable in this area. Perhaps Lars-Peters has a suggestion.   
> > Yes, Lars or Michael Hennerich (+CC) are my normal contact points.
> > I believe this is outside both of their areas, but they may be able to
> > put you in contact with the right person.  If nothing else it would be
> > good to make someone in ADI aware that people care about linux support for
> > these parts!  
> I sent in a request to ADI for a contact person to help with the review.

Cool.

> >   
>  Anyway, please see my comments inline below
>  
>  Regards,
>  John
>  
>  
>  
>  
>    
> > On Mar 18, 2018, at 5:23 AM, Jonathan Cameron  wrote:
> > 
> > On Sat, 17 Mar 2018 23:11:45 -0700
> > John Syne  wrote:
> >   
> >> Hi Jonathan,  
> > Hi John and All,
> > 
> > I'd love to get some additional input on this from anyone interested.
> > There are a lot of weird and wonderful derived quantities in an energy
> > meter and it seems we need to make some fundamental changes to support
> > them - including potentially a userspace breaking change to the event
> > code description.
> >   
> >> 
> >> Here is the complete list of registers for the ADE7878 which I copied 
> >> from the data sheet. I added a column “IIO Attribute” which I hope 
> >> follows your IIO ABI. Please make any changes you feel are incorrect. 
> >> BTW, there are several registers that cannot be generalized and are 
> >> used purely for chip configuration. I think we should add a new naming 
> >> convention, namely {register}_{}. Also, I see in 
> >> the sys_bus_iio doc
> > 
> > No, registers can always be broken up in to real meaningful values if
> > they are exposed to userspace and userspace has a reason to tweak
> > them.
> > 
> > We never expose raw registers to userspace except through
> > debugfs and the clue is in the name - purely for debug.  
> OK
> > 
> >   
> >>> 
> >>> 
> >>> 
> >>>   
> >> in_accel_x_peak_raw
> >> 
> >> so shouldn’t the phase be represented as follows:
> >> 
> >> in_current_A_scale  
> > I'm still confused.  What does A represent here?  I assumed that was a 
> > wild
> > card for the channel number before but clearly not.
> > 
> > Ah, you are labelling the 3 separate phases as A, B and C. Hmm.
> > I guess they sort of look like axis, and sort of like independent 
> > channels.
> > So could be indexed or done via modifiers depending on how you look at 
> > it.  
>  In metering terminology, the three phases are commonly referred to as 
>  RED, GREEN, BLUE or PhaseA, PhaseB, PhaseC and Neutral. Analog Devices 
>  uses the ABCN nomenclature so anyone using this driver will probably 
>  have referenced the Analog Devices datasheet so this terminology should 
>  be familiar. 
> >>> Which is more commonly used in general?  We are designing what we hope
> >>> will be a general interface and last thing we want is to have everyone
> >>> not using ADI parts confused!  Personally not assigning 'colours' makes
> >>> more sense to me.
> >> I did a little research and here is the naming used by vendor:
> >> 
> >> ST Microelectronics: P1, P2, P3, N
> >> http://www.st.com/content/ccc/resource/technical/document/application_note/5e/f4/22/4d/90/96/4c/c4/CD00153264.pdf/files/CD00153264.pdf/jcr:content/translations/en.CD00153264.pdf
> >> 
> >> Teridian: ABCN
> >> https://www.mouser.com/ds/2/256/71M6513-71M6513H-22603.pdf
> >> 
> >> Maxim Integrated: ABCN
> >> https://datasheets.maximintegrated.com/en/ds/78M6631.pdf
> >> 
> >> Microchip: ABCN
> >> http://ww1.microchip.com/downloads/en/DeviceDoc/51643b.pdf
> >> 
> >> NXP: L1, L2, L3, N
> >> https://www.nxp.com/support/developer-resources/reference-designs/kinetis-km3x-256-mcu-three-phase-metering-reference-design:RD-KM3x-256-3PH-METERING
> >> 
> >> Renesas: ABCN
> >> https://www.renesas.com/en-us/solutions/home/metering/power-meter-three.html
> >> 
> >> So the most consistent would be ABCN  
> > Cool.   Makes sense to go with that.  

Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)

[John Syne]

> On Mar 25, 2018, at 9:54 AM, Jonathan Cameron  wrote:
> 
> On Sun, 25 Mar 2018 01:29:41 -0700
> John Syne  wrote:
> 
>> Hi Jonathan,
> Hi John,
> 
> Please wrap your normal emails (excepting tables) to 80 chars.
Yeah, I’m trying to do that, but I use a Mac and Apple Mail doesn’t
have a feature to wrap at 80 automatically so I have to do
this manually.
> 
>> 
>> I was speaking with Rodrigo and here is what I think must be done to move
>> ADE7878 out of staging
>> 
>> Here are the steps as I see them:
>> 
>> 1) Define the IIO Attributes so they are consistent with the IIO ABI. This
>>   should be pretty simple given agreement on the naming convention.
> Please don't go just changing attribute names - the driver needs to
> use the standard approach of iio_chan_spec and create the vast
> majority of attributes automatically via that.  There 'may' be
> a few corner cases wee don't want to make generic and they 'might'
> be exposed as attributes.
> 
> I suspect this change is the 'big' job.  The core support necessary
> for RMS and MAV (computedtype or whatever we call it) will need adding
> as well.  That is a separate patch set with support for some examples
> in the dummy driver.
Great that you mentioned this, because I though we only needed to
modify the IIO_ATTR naming. I’ll take a look at iio_chan_spec and
the examples in the dummy driver.
> 
>> 2) Map the ADE7854 interrupt status to IIO events. This requires an
>>   interrupt processing section.
>> 3) Add DeviceTree support.
>> 4) Create DeviceTree overlay for the ADE7854.
> More a case of bindings for now.  If those are used via an overlay
> fine but given we don't have any boards with one one in mainline,
> this is an implementation detail for the user rather than part
> of moving this driver out of staging.
I was thinking more along the lines of documentation to show a
developer what was needed to get the driver working.
> 
>> 5) Update ADE7854 probe to read in the DeviceTree register settings.
>> 6) Add support for power modes (PM1, PM2).
> This isn't necessary for a move out of staging (nice to have though).
> 
>> 7) Not sure if we will support measurement streaming on the ADE7854.
>>   The problem is ADE7854 is designed as an SPI master, which means
>>   it controls the SPI clock, so the driver must support SPI slave
>>   mode. However, the Linux Kernel does not currently support SPI
>>   slave mode. We have three choices to make this work and they
>>   are all a lot of work: 1) Add support for SPI Slave mode to the
>>   kernel,  2) Use hardware to convert SPI signals to I2S signals
>>   and with the use of a custom codec, use the ALSA framework to
>>   stream the samples (this is an approach I used, but I don’t like
>>   it), 3) Move the I2S driver out of the sound subsystem and use it
>>   together with DMA to stream samples directly into the ADE7854 driver
>>   (my preferred solutions). Perhaps Mark Brown has some ideas on how
>>to make this work. 
> I'll be honest, this is an end of line part and frankly more than
> a little crazy. I would go with simply not supporting the measurement
> streaming at all for this part.  If you really need it we can then
> move onto the how part, but from what you have said I'm guessing you
> don't care except in an abstract 'it would be nice' sort of a way?
Yeah, I’m moving to the ADE9000 so I’ll drop this.
> 
>> 
>> The ADE9000 will be much easier because it uses an SPI Slave interface. 
>> 
>> I hope I have captured everything, but let me know if I have missed anything.
>> 
> 
> That will do for now ;)  I'm sure there will be details that need
> tidying up once we have the above done, but that's true for any new
> driver (and this will be nearly a new driver before things are done).
Thank you again for all the detailed feedback.
> 
> Jonathan

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)

[John Syne]

> 
> On Mar 25, 2018, at 9:44 AM, Jonathan Cameron  wrote:
> 
> On Sat, 24 Mar 2018 16:06:17 -0700
> John Syne  wrote:
> 
> This thread is becoming unmanageable so I am cropping this down to just
> the questions that remain open.
> 
 Probably easier to copy and paste this table into a spreadsheet. Let me 
 know if there is anything I got wrong. Thank you again for all your help.  
>>> Yeah, we need to shrink this if we do it again.  
>> I’ll send an updated copy after this e-mail. Can you accept a spreadsheet 
>> attachment or a CSV file?
> We need to keep the discussion visible on list so it needs to stay in
> plain text.  Just need to drop any columns we aren't caring about to make
> it easier to read.
Done
>>> 
> ...
 0x439C CVAROS  in_powerreactive0_phaseC_offset in  
 powerreactive   0   phaseC  offset  R/W 24  32 ZPSE S   
 0x00Phase C total reactive power offset adjust (ADE7858, 
 ADE7868, and ADE7878).
 0x439D AFWGAIN in_power0_phaseA_fundamental_scale  in  
 power   0   phaseA_fundamental  scale   R/W 24  32 ZPSE S  
  0x00Phase A fundamental active power gain adjust. 
 Location reserved for ADE7854, ADE7858, and ADE7868.  
>>> Hmm. Fundamental needs to be represented using a separate channel index
>>> and description of the frequency filters applied.  That should map it
>>> a generic way.  
>> How do I do this?
> Define additional channels with different index and for them
> use the the infomask elements
> IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY,
> IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY
> and provide suitable values from the read_raw callbacks for that
> channel.
> 
> ...
 0x43C0 AIRMS   in_current0_phaseA_rms  in  current 0   
 phaseA_rms  R   24  32 ZP   S   N/A4Phase A 
 current rms value.  
>>> in_current0_phaseA_rms_raw as otherwise we don't know we need to apply
>>> in_current0_phaseA_rms_scale to it (or the shared value that maps to that). 
>>>  
>> Yeah, this is still confusion to me. This should read 
>> in_current0_phaseA_rms_gain 
>> as it directly affects the value in_current0_phaseA_rms_raw. We still have 
>> to apply
>> a scale value to turn this cryptic number into something meaningful. 
> So I'm a little lost. We have variable gain fine.
> Does it effect the necessary scale factor to go from raw to real value or not?
Yes
> 1) Yes it does - then roll it as appropriate into the _scale attribute. 
>  It should not be separated.  This often requires some interesting maths
>  but is a onetime thing as the value isn't changing dynamically.


> 
> 2) No it doesn't - then it is calibgain as it represents a necessary
>  parameter to change the incoming circuit to compensate for external effects.
The hardwaregain and calibgain are setup only once and do not vary.
The ADE7854 can accept various input ranges such as 0-250mV, 0-5V, etc
The calibgain is used to compensate for the inaccuracies of the sensors
and the termination components. 
> 
> It is possible you have a mixture of the two and hence need both but that
> is normally only the case with devices where the calibgain is about fixing
> the factory calibration.
> 
> ...
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)

[John Syne]

> On Mar 25, 2018, at 9:44 AM, Jonathan Cameron  wrote:
> 
> On Sat, 24 Mar 2018 16:06:17 -0700
> John Syne  wrote:
> 
> This thread is becoming unmanageable so I am cropping this down to just
> the questions that remain open.
> 
 Probably easier to copy and paste this table into a spreadsheet. Let me 
 know if there is anything I got wrong. Thank you again for all your help.  
>>> Yeah, we need to shrink this if we do it again.  
>> I’ll send an updated copy after this e-mail. Can you accept a spreadsheet 
>> attachment or a CSV file?
> We need to keep the discussion visible on list so it needs to stay in
> plain text.  Just need to drop any columns we aren't caring about to make
> it easier to read.
Done
>>> 
> ...
 0x439C CVAROS  in_powerreactive0_phaseC_offset in  
 powerreactive   0   phaseC  offset  R/W 24  32 ZPSE S   
 0x00Phase C total reactive power offset adjust (ADE7858, 
 ADE7868, and ADE7878).
 0x439D AFWGAIN in_power0_phaseA_fundamental_scale  in  
 power   0   phaseA_fundamental  scale   R/W 24  32 ZPSE S  
  0x00Phase A fundamental active power gain adjust. 
 Location reserved for ADE7854, ADE7858, and ADE7868.  
>>> Hmm. Fundamental needs to be represented using a separate channel index
>>> and description of the frequency filters applied.  That should map it
>>> a generic way.  
>> How do I do this?
> Define additional channels with different index and for them
> use the the infomask elements
> IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY,
> IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY
> and provide suitable values from the read_raw callbacks for that
> channel.
> 
> ...
 0x43C0 AIRMS   in_current0_phaseA_rms  in  current 0   
 phaseA_rms  R   24  32 ZP   S   N/A4Phase A 
 current rms value.  
>>> in_current0_phaseA_rms_raw as otherwise we don't know we need to apply
>>> in_current0_phaseA_rms_scale to it (or the shared value that maps to that). 
>>>  
>> Yeah, this is still confusion to me. This should read 
>> in_current0_phaseA_rms_gain 
>> as it directly affects the value in_current0_phaseA_rms_raw. We still have 
>> to apply
>> a scale value to turn this cryptic number into something meaningful. 
> So I'm a little lost. We have variable gain fine.
> Does it effect the necessary scale factor to go from raw to real value or not?
Yes
> 1) Yes it does - then roll it as appropriate into the _scale attribute. 
>   It should not be separated.  This often requires some interesting maths
>   but is a onetime thing as the value isn't changing dynamically.


> 
> 2) No it doesn't - then it is calibgain as it represents a necessary
>   parameter to change the incoming circuit to compensate for external effects.
The hardwaregain and calibgain are setup only once and do not vary.
The ADE7854 can accept various input ranges such as 0-250mV, 0-5V, etc
The calibgain is used to compensate for the inaccuracies of the sensors
and the termination components. 
> 
> It is possible you have a mixture of the two and hence need both but that
> is normally only the case with devices where the calibgain is about fixing
> the factory calibration.
> 
> ...

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)

[John Syne]

> On Mar 25, 2018, at 9:29 AM, Jonathan Cameron  wrote:
> 
> On Sat, 24 Mar 2018 15:45:21 -0700
> John Syne  wrote:
> 
>>> On Mar 24, 2018, at 8:02 AM, Jonathan Cameron  wrote:
>>> 
>>> On Mon, 19 Mar 2018 22:57:16 -0700
>>> John Syne  wrote:
>>> 
 Hi Jonathan,
 
 Thank you for all your hard work. Your feedback is really helpful. I’m 
 surprised that no one from Analog Device has offered any suggestions.
 
>>> 
>>> Sadly those active in the mainline linux kernel from ADI are focused in
>>> other areas currently :(  
>> Good point. I will reach out to Analog Devices and get a name of someone 
>> who is knowledgeable in this area. Perhaps Lars-Peters has a suggestion. 
> Yes, Lars or Michael Hennerich (+CC) are my normal contact points.
> I believe this is outside both of their areas, but they may be able to
> put you in contact with the right person.  If nothing else it would be
> good to make someone in ADI aware that people care about linux support for
> these parts!
I sent in a request to ADI for a contact person to help with the review.
> 
 Anyway, please see my comments inline below
 
 Regards,
 John
 
 
 
 
 
> On Mar 18, 2018, at 5:23 AM, Jonathan Cameron  wrote:
> 
> On Sat, 17 Mar 2018 23:11:45 -0700
> John Syne  wrote:
> 
>> Hi Jonathan,
> Hi John and All,
> 
> I'd love to get some additional input on this from anyone interested.
> There are a lot of weird and wonderful derived quantities in an energy
> meter and it seems we need to make some fundamental changes to support
> them - including potentially a userspace breaking change to the event
> code description.
> 
>> 
>> Here is the complete list of registers for the ADE7878 which I copied 
>> from the data sheet. I added a column “IIO Attribute” which I hope 
>> follows your IIO ABI. Please make any changes you feel are incorrect. 
>> BTW, there are several registers that cannot be generalized and are used 
>> purely for chip configuration. I think we should add a new naming 
>> convention, namely {register}_{}. Also, I see in the 
>> sys_bus_iio doc  
> 
> No, registers can always be broken up in to real meaningful values if
> they are exposed to userspace and userspace has a reason to tweak
> them.
> 
> We never expose raw registers to userspace except through
> debugfs and the clue is in the name - purely for debug.
OK
> 
> 
>>> 
>>> 
>>> 
>>> 
>> in_accel_x_peak_raw
>> 
>> so shouldn’t the phase be represented as follows:
>> 
>> in_current_A_scale
> I'm still confused.  What does A represent here?  I assumed that was a 
> wild
> card for the channel number before but clearly not.
> 
> Ah, you are labelling the 3 separate phases as A, B and C. Hmm.
> I guess they sort of look like axis, and sort of like independent 
> channels.
> So could be indexed or done via modifiers depending on how you look at 
> it.
 In metering terminology, the three phases are commonly referred to as RED, 
 GREEN, BLUE or PhaseA, PhaseB, PhaseC and Neutral. Analog Devices uses the 
 ABCN nomenclature so anyone using this driver will probably have 
 referenced the Analog Devices datasheet so this terminology should be 
 familiar.   
>>> Which is more commonly used in general?  We are designing what we hope
>>> will be a general interface and last thing we want is to have everyone
>>> not using ADI parts confused!  Personally not assigning 'colours' makes
>>> more sense to me.  
>> I did a little research and here is the naming used by vendor:
>> 
>> ST Microelectronics: P1, P2, P3, N
>> http://www.st.com/content/ccc/resource/technical/document/application_note/5e/f4/22/4d/90/96/4c/c4/CD00153264.pdf/files/CD00153264.pdf/jcr:content/translations/en.CD00153264.pdf
>> 
>> Teridian: ABCN
>> https://www.mouser.com/ds/2/256/71M6513-71M6513H-22603.pdf
>> 
>> Maxim Integrated: ABCN
>> https://datasheets.maximintegrated.com/en/ds/78M6631.pdf
>> 
>> Microchip: ABCN
>> http://ww1.microchip.com/downloads/en/DeviceDoc/51643b.pdf
>> 
>> NXP: L1, L2, L3, N
>> https://www.nxp.com/support/developer-resources/reference-designs/kinetis-km3x-256-mcu-three-phase-metering-reference-design:RD-KM3x-256-3PH-METERING
>> 
>> Renesas: ABCN
>> https://www.renesas.com/en-us/solutions/home/metering/power-meter-three.html
>> 
>> So the most consistent would be ABCN
> Cool.   Makes sense to go with that.  Thanks for checking this out.
> 
>> 
>> 
>> All vendor providing three phase energy metering ICs use the ABCN 
>> terminology. 
>>> 
>>> (btw please wrap any lines that don't need to be long to around 80 
>>> characters).
>>> 
> 
> Hmm. With neutral in there as well I guess we need to make them
> modifiers (but might change my mind later ;)
> 

Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)

[Jonathan Cameron]

On Sat, 24 Mar 2018 16:06:17 -0700
John Syne  wrote:

This thread is becoming unmanageable so I am cropping this down to just
the questions that remain open.

> >> Probably easier to copy and paste this table into a spreadsheet. Let me 
> >> know if there is anything I got wrong. Thank you again for all your help.  
> > Yeah, we need to shrink this if we do it again.  
> I’ll send an updated copy after this e-mail. Can you accept a spreadsheet 
> attachment or a CSV file?
We need to keep the discussion visible on list so it needs to stay in
plain text.  Just need to drop any columns we aren't caring about to make
it easier to read.
> > 
...
> >> 0x439C CVAROS  in_powerreactive0_phaseC_offset in  
> >> powerreactive   0   phaseC  offset  R/W 24  32 ZPSE S   
> >> 0x00Phase C total reactive power offset adjust (ADE7858, 
> >> ADE7868, and ADE7878).
> >> 0x439D AFWGAIN in_power0_phaseA_fundamental_scale  in  
> >> power   0   phaseA_fundamental  scale   R/W 24  32 ZPSE S  
> >>  0x00Phase A fundamental active power gain adjust. 
> >> Location reserved for ADE7854, ADE7858, and ADE7868.  
> > Hmm. Fundamental needs to be represented using a separate channel index
> > and description of the frequency filters applied.  That should map it
> > a generic way.  
> How do I do this?
Define additional channels with different index and for them
use the the infomask elements
IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY,
IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY
and provide suitable values from the read_raw callbacks for that
channel.

...
> >> 0x43C0 AIRMS   in_current0_phaseA_rms  in  current 0   
> >> phaseA_rms  R   24  32 ZP   S   N/A4Phase A 
> >> current rms value.  
> > in_current0_phaseA_rms_raw as otherwise we don't know we need to apply
> > in_current0_phaseA_rms_scale to it (or the shared value that maps to that). 
> >  
> Yeah, this is still confusion to me. This should read 
> in_current0_phaseA_rms_gain 
> as it directly affects the value in_current0_phaseA_rms_raw. We still have to 
> apply
> a scale value to turn this cryptic number into something meaningful. 
So I'm a little lost. We have variable gain fine.
Does it effect the necessary scale factor to go from raw to real value or not?
1) Yes it does - then roll it as appropriate into the _scale attribute. 
   It should not be separated.  This often requires some interesting maths
   but is a onetime thing as the value isn't changing dynamically.

2) No it doesn't - then it is calibgain as it represents a necessary
   parameter to change the incoming circuit to compensate for external effects.

It is possible you have a mixture of the two and hence need both but that
is normally only the case with devices where the calibgain is about fixing
the factory calibration.

...
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)

[Jonathan Cameron]

On Sat, 24 Mar 2018 15:45:21 -0700
John Syne  wrote:

> > On Mar 24, 2018, at 8:02 AM, Jonathan Cameron  wrote:
> > 
> > On Mon, 19 Mar 2018 22:57:16 -0700
> > John Syne  wrote:
> >   
> >> Hi Jonathan,
> >> 
> >> Thank you for all your hard work. Your feedback is really helpful. I’m 
> >> surprised that no one from Analog Device has offered any suggestions.
> >>   
> > 
> > Sadly those active in the mainline linux kernel from ADI are focused in
> > other areas currently :(  
> Good point. I will reach out to Analog Devices and get a name of someone 
> who is knowledgeable in this area. Perhaps Lars-Peters has a suggestion. 
Yes, Lars or Michael Hennerich (+CC) are my normal contact points.
I believe this is outside both of their areas, but they may be able to
put you in contact with the right person.  If nothing else it would be
good to make someone in ADI aware that people care about linux support for
these parts!

> >> Anyway, please see my comments inline below
> >> 
> >> Regards,
> >> John
> >> 
> >> 
> >> 
> >> 
> >>   
> >>> On Mar 18, 2018, at 5:23 AM, Jonathan Cameron  wrote:
> >>> 
> >>> On Sat, 17 Mar 2018 23:11:45 -0700
> >>> John Syne  wrote:
> >>>   
>  Hi Jonathan,
> >>> Hi John and All,
> >>> 
> >>> I'd love to get some additional input on this from anyone interested.
> >>> There are a lot of weird and wonderful derived quantities in an energy
> >>> meter and it seems we need to make some fundamental changes to support
> >>> them - including potentially a userspace breaking change to the event
> >>> code description.
> >>>   
>  
>  Here is the complete list of registers for the ADE7878 which I copied 
>  from the data sheet. I added a column “IIO Attribute” which I hope 
>  follows your IIO ABI. Please make any changes you feel are incorrect. 
>  BTW, there are several registers that cannot be generalized and are used 
>  purely for chip configuration. I think we should add a new naming 
>  convention, namely {register}_{}. Also, I see in the 
>  sys_bus_iio doc  

No, registers can always be broken up in to real meaningful values if
they are exposed to userspace and userspace has a reason to tweak
them.

We never expose raw registers to userspace except through
debugfs and the clue is in the name - purely for debug.


> > 
> > 
> > 
> >   
>  in_accel_x_peak_raw
>  
>  so shouldn’t the phase be represented as follows:
>  
>  in_current_A_scale
> >>> I'm still confused.  What does A represent here?  I assumed that was a 
> >>> wild
> >>> card for the channel number before but clearly not.
> >>> 
> >>> Ah, you are labelling the 3 separate phases as A, B and C. Hmm.
> >>> I guess they sort of look like axis, and sort of like independent 
> >>> channels.
> >>> So could be indexed or done via modifiers depending on how you look at 
> >>> it.
> >> In metering terminology, the three phases are commonly referred to as RED, 
> >> GREEN, BLUE or PhaseA, PhaseB, PhaseC and Neutral. Analog Devices uses the 
> >> ABCN nomenclature so anyone using this driver will probably have 
> >> referenced the Analog Devices datasheet so this terminology should be 
> >> familiar.   
> > Which is more commonly used in general?  We are designing what we hope
> > will be a general interface and last thing we want is to have everyone
> > not using ADI parts confused!  Personally not assigning 'colours' makes
> > more sense to me.  
> I did a little research and here is the naming used by vendor:
> 
> ST Microelectronics: P1, P2, P3, N
> http://www.st.com/content/ccc/resource/technical/document/application_note/5e/f4/22/4d/90/96/4c/c4/CD00153264.pdf/files/CD00153264.pdf/jcr:content/translations/en.CD00153264.pdf
> 
> Teridian: ABCN
> https://www.mouser.com/ds/2/256/71M6513-71M6513H-22603.pdf
> 
> Maxim Integrated: ABCN
> https://datasheets.maximintegrated.com/en/ds/78M6631.pdf
> 
> Microchip: ABCN
> http://ww1.microchip.com/downloads/en/DeviceDoc/51643b.pdf
> 
> NXP: L1, L2, L3, N
> https://www.nxp.com/support/developer-resources/reference-designs/kinetis-km3x-256-mcu-three-phase-metering-reference-design:RD-KM3x-256-3PH-METERING
> 
> Renesas: ABCN
> https://www.renesas.com/en-us/solutions/home/metering/power-meter-three.html
> 
> So the most consistent would be ABCN
Cool.   Makes sense to go with that.  Thanks for checking this out.

> 
> 
> All vendor providing three phase energy metering ICs use the ABCN 
> terminology. 
> > 
> > (btw please wrap any lines that don't need to be long to around 80 
> > characters).
> >   
> >>> 
> >>> Hmm. With neutral in there as well I guess we need to make them
> >>> modifiers (but might change my mind later ;)
> >>> 
> >>> Particularly as we are using the the modifier for RMS under the previous
> >>> plan... It appears we should treat that instead like we did for peak
> >>> and do it as an additional info mask element. 

Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)

[John Syne]

Hi Jonathan,

I was speaking with Rodrigo and here is what I think must be done to move 
ADE7878 out of staging

Here are the steps as I see them:

1) Define the IIO Attributes so they are consistent with the IIO ABI. This 
should be pretty simple given agreement on the naming convention. 
2) Map the ADE7854 interrupt status to IIO events. This requires an interrupt 
processing section.
3) Add DeviceTree support.
4) Create DeviceTree overlay for the ADE7854.
5) Update ADE7854 probe to read in the DeviceTree register settings.
6) Add support for power modes (PM1, PM2).
7) Not sure if we will support measurement streaming on the ADE7854. The 
problem is ADE7854 is designed as an SPI master, which means it controls the 
SPI clock, so the driver must support SPI slave mode. However, the Linux Kernel 
does not currently support SPI slave mode. We have three choices to make this 
work and they are all a lot of work: 1) Add support for SPI Slave mode to the 
kernel,  2) Use hardware to convert SPI signals to I2S signals and with the use 
of a custom codec, use the ALSA framework to stream the samples (this is an 
approach I used, but I don’t like it), 3) Move the I2S driver out of the sound 
subsystem and use it together with DMA to stream samples directly into the 
ADE7854 driver (my preferred solutions). Perhaps Mark Brown has some ideas on 
how to make this work. 

The ADE9000 will be much easier because it uses an SPI Slave interface. 

I hope I have captured everything, but let me know if I have missed anything.

Regards,
John





___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)

[John Syne]

> On Mar 24, 2018, at 8:18 AM, Jonathan Cameron  wrote:
> 
> On Mon, 19 Mar 2018 23:28:45 -0700
> John Syne  wrote:
> 
>> Hi Jonathan,
>> 
>> I broke out the {Direction}_{Type}_{Index}_{Modifier}_{Info_Mask} into 
>> separate columns to make sure I understand your instructions. Good way to 
>> check the results. 
>> 
>> Probably easier to copy and paste this table into a spreadsheet. Let me know 
>> if there is anything I got wrong. Thank you again for all your help.
> Yeah, we need to shrink this if we do it again.
I’ll send an updated copy after this e-mail. Can you accept a spreadsheet 
attachment or a CSV file?
> 
> Can drop anything not related to ABI (so RW mask address etc and just have the
> register name and the bits around ABI.
Done
> 
> Note I mentioned the altvoltage stuff in the previous email, so we need
> to think about whether that is useful to distinguish the instantaneous
> measurements from the multi cycle AC ones.
> Actually I think we are fine here as we explicitly describe all 
> alt measurements as mav or rms which going to be handled in our
> new computedvalue description.
Yeah, I don’t see how anyone would use these registers, so I propose
we just drop them.
> 
> 
>> 
>> Address  RegisterIIO Attribute   Device Tree or Code 
>> Direction   TypeIndex   ModifierInfo Mask   R/W Bit 
>> Length  Bit Length During CommunicationsTypeDefault Value   
>> Description
>> 0x4380   AIGAIN  in_current0_phaseA_scalein  current 
>> 0   phaseA  scale   R/W 24  32 ZPSE S   0x00
>> Phase A current gain adjust.
>> 0x4381   AVGAIN  in_voltage0_phaseA_scalein  voltage 
>> 0   phaseA  scale   R/W 24  32 ZPSE S   0x00
>> Phase A voltage gain adjust.
>> 0x4382   BIGAIN  in_current0_phaseB_scalein  current 
>> 0   phaseB  scale   R/W 24  32 ZPSE S   0x00
>> Phase B current gain adjust.
>> 0x4383   BVGAIN  in_voltage0_phaseB_scalein  voltage 
>> 0   phaseB  scale   R/W 24  32 ZPSE S   0x00
>> Phase B voltage gain adjust.
>> 0x4384   CIGAIN  in_current0_phaseC_scalein  current 
>> 0   phaseC  scale   R/W 24  32 ZPSE S   0x00
>> Phase C current gain adjust.
>> 0x4385   CVGAIN  in_voltage0_phaseC_scalein  voltage 
>> 0   phaseC  scale   R/W 24  32 ZPSE S   0x00
>> Phase C voltage gain adjust.
>> 0x4386   NIGAIN  in_current0_neutral_scale   in  current 
>> 0   neutral scale   R/W 24  32 ZPSE S   0x00
>> Neutral current gain adjust (ADE7868 and ADE7878 only).
>> 0x4387   AIRMSOS in_current0_phaseA_rms_offset   in  current 
>> 0   phaseA  offset  R/W 24  32 ZPSE S   0x00
>> Phase A current rms offset.
>> 0x4388   AVRMSOS in_voltage0_phaseA_rms_offset   in  voltage 
>> 0   phaseA  offset  R/W 24  32 ZPSE S   0x00
>> Phase A voltage rms offset.
>> 0x4389   BIRMSOS in_current0_phaseB_rms_offset   in  current 
>> 0   phaseB  offset  R/W 24  32 ZPSE S   0x00
>> Phase B current rms offset.
>> 0x438A   BVRMSOS in_voltage0_phaseB_rms_offset   in  voltage 
>> 0   phaseB  offset  R/W 24  32 ZPSE S   0x00
>> Phase B voltage rms offset.
>> 0x438B   CIRMSOS in_current0_phaseC_rms_offset   in  current 
>> 0   phaseC  offset  R/W 24  32 ZPSE S   0x00
>> Phase C current rms offset.
>> 0x438C   CVRMSOS in_voltage0_phaseC_rms_offset   in  voltage 
>> 0   phaseC  offset  R/W 24  32 ZPSE S   0x00
>> Phase C voltage rms offset.
>> 0x438D   NIRMSOS in_current0_neutral_rms_offset  in  current 
>> 0   neutral offset  R/W 24  32 ZPSE S   0x00
>> Neutral current rms offset (ADE7868 and ADE7878 only).
>> 0x438E   AVAGAIN in_powerapparent0_phaseA_scale  in  
>> powerapparent   0   phaseA  scale   R/W 24  32 ZPSE S   
>> 0x00Phase A apparent power gain adjust.
>> 0x438F   BVAGAIN in_powerapparent0_phaseB_scale  in  
>> powerapparent   0   phaseB  scale   R/W 24  32 ZPSE S   
>> 0x00Phase B apparent power gain adjust.
>> 0x4390   CVAGAIN in_powerapparent0_phaseC_scale  in  
>> powerapparent   0   phaseC  scale   R/W 24  32 ZPSE S   
>> 0x00Phase C apparent power gain adjust.
>> 0x4391   AWGAIN  in_power0_phaseA_scale  in  power   0   
>> phaseA  scale   R/W 24  32 ZPSE S   0x00Phase A 
>> total active power gain adjust.
>> 

Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)

[John Syne]

Hi Jonathan,

I have attached a CSV version of this table below. Hopefully that will make 
things easier for you.

RegisterIIO Attribute   Device Tree or Code Direction   type
Index   ModifierInfo Mask
AIGAIN  in_current0_phaseA_scalein  current 0   phaseA  
scale
AVGAIN  in_voltage0_phaseA_scalein  voltage 0   phaseA  
scale
BIGAIN  in_current0_phaseB_scalein  current 0   phaseB  
scale
BVGAIN  in_voltage0_phaseB_scalein  voltage 0   phaseB  
scale
CIGAIN  in_current0_phaseC_scalein  current 0   phaseC  
scale
CVGAIN  in_voltage0_phaseC_scalein  voltage 0   phaseC  
scale
NIGAIN  in_current0_neutral_scale   in  current 0   neutral 
scale
AIRMSOS in_current0_phaseA_rms_offset   in  current 0   phaseA  
offset
AVRMSOS in_voltage0_phaseA_rms_offset   in  voltage 0   phaseA  
offset
BIRMSOS in_current0_phaseB_rms_offset   in  current 0   phaseB  
offset
BVRMSOS in_voltage0_phaseB_rms_offset   in  voltage 0   phaseB  
offset
CIRMSOS in_current0_phaseC_rms_offset   in  current 0   phaseC  
offset
CVRMSOS in_voltage0_phaseC_rms_offset   in  voltage 0   phaseC  
offset
NIRMSOS in_current0_neutral_rms_offset  in  current 0   neutral 
offset
AVAGAIN in_powerapparent0_phaseA_scale  in  powerapparent   0   
phaseA  scale
BVAGAIN in_powerapparent0_phaseB_scale  in  powerapparent   0   
phaseB  scale
CVAGAIN in_powerapparent0_phaseC_scale  in  powerapparent   0   
phaseC  scale
AWGAIN  in_power0_phaseA_scale  in  power   0   phaseA  scale
AWATTOS in_power0_phaseA_offset in  power   0   phaseA  offset
BWGAIN  in_power0_phaseB_scale  in  power   0   phaseB  scale
BWATTOS in_power0_phaseB_offset in  power   0   phaseB  offset
CWGAIN  in_power0_PhaseC_scale  in  power   0   phaseC  scale
CWATTOS in_power0_phaseC_offset in  power   0   phaseC  offset
AVARGAINin_powerreactive0_phaseA_scale  in  powerreactive   
0   phaseA  scale
AVAROS  in_powerreactive0_phaseA_offset in  powerreactive   0   
phaseA  offset
BVARGAINin_powerreactive0_phaseB_scale  in  powerreactive   
0   phaseB  scale
BVAROS  in_powerreactive0_phaseB_offset in  powerreactive   0   
phaseB  offset
CVARGAINin_powerreactive0_phaseC_scale  in  powerreactive   
0   phaseC  scale
CVAROS  in_powerreactive0_phaseC_offset in  powerreactive   0   
phaseC  offset
AFWGAIN in_power0_phaseA_fundamental_scale  in  power   0   
phaseA_fundamental  scale
AFWATTOSin_power0_phaseA_fundamental_offset in  power   
0   phaseA_fundamental  offset
BFWGAIN in_power0_phaseB_fundamental_scale  in  power   0   
phaseB_fundamental  scale
BFWATTOSin_power0_phaseB_fundamental_offset in  power   
0   phaseB_fundamental  offset
CFWGAIN in_power0_phaseC_fundamental_scale  in  power   0   
phaseC_fundamental  scale
CFWATTOSin_power0_phaseC_fundamental_offset in  power   
0   phaseC_fundamental  offset
AFVARGAIN   in_powerreactive0_phaseA_fundamental_scale  in  
powerreactive   0   phaseA_fundamental  scale
AFVAROS in_powerreactive0_phaseA_fundamental_offset in  
powerreactive   0   phaseA_fundamental  offset
BFVARGAIN   in_powerreactive0_phaseB_fundamental_scale  in  
powerreactive   0   phaseB_fundamental  scale
BFVAROS in_powerreactive0_phaseB_fundamental_offset in  
powerreactive   0   phaseB_fundamental  offset
CFVARGAIN   in_powerreactive0_phaseC_fundamental_scale  in  
powerreactive   0   phaseC_fundamental  scale
CFVAROS in_powerreactive0_phaseC_fundamental_offset in  
powerreactive   0   phaseC_fundamental  offset
VATHR1  VATHR1  DT  
VATHR0  VATHR0  DT  
WTHR1   WTHR1   DT  
WTHR0   WTHR0   DT  
VARTHR1 VARTHR1 DT  
VARTHR0 VARTHR0 DT  
ReservedReserved
VANOLOADVANOLOADDT  
APNOLOADAPNOLOADDT  
VLEVEL  VLEVEL  DT  
DICOEFF DICOEFF DT  
HPFDIS  HPFDIS  DT

Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)

[John Syne]

> On Mar 24, 2018, at 8:02 AM, Jonathan Cameron  wrote:
> 
> On Mon, 19 Mar 2018 22:57:16 -0700
> John Syne  wrote:
> 
>> Hi Jonathan,
>> 
>> Thank you for all your hard work. Your feedback is really helpful. I’m 
>> surprised that no one from Analog Device has offered any suggestions.
>> 
> 
> Sadly those active in the mainline linux kernel from ADI are focused in
> other areas currently :(
Good point. I will reach out to Analog Devices and get a name of someone 
who is knowledgeable in this area. Perhaps Lars-Peters has a suggestion. 
>> Anyway, please see my comments inline below
>> 
>> Regards,
>> John
>> 
>> 
>> 
>> 
>> 
>>> On Mar 18, 2018, at 5:23 AM, Jonathan Cameron  wrote:
>>> 
>>> On Sat, 17 Mar 2018 23:11:45 -0700
>>> John Syne  wrote:
>>> 
 Hi Jonathan,  
>>> Hi John and All,
>>> 
>>> I'd love to get some additional input on this from anyone interested.
>>> There are a lot of weird and wonderful derived quantities in an energy
>>> meter and it seems we need to make some fundamental changes to support
>>> them - including potentially a userspace breaking change to the event
>>> code description.
>>> 
 
 Here is the complete list of registers for the ADE7878 which I copied from 
 the data sheet. I added a column “IIO Attribute” which I hope follows your 
 IIO ABI. Please make any changes you feel are incorrect. BTW, there are 
 several registers that cannot be generalized and are used purely for chip 
 configuration. I think we should add a new naming convention, namely 
 {register}_{}. Also, I see in the sys_bus_iio doc
> 
> 
> 
> 
 in_accel_x_peak_raw
 
 so shouldn’t the phase be represented as follows:
 
 in_current_A_scale  
>>> I'm still confused.  What does A represent here?  I assumed that was a wild
>>> card for the channel number before but clearly not.
>>> 
>>> Ah, you are labelling the 3 separate phases as A, B and C. Hmm.
>>> I guess they sort of look like axis, and sort of like independent channels.
>>> So could be indexed or done via modifiers depending on how you look at it.  
>> In metering terminology, the three phases are commonly referred to as RED, 
>> GREEN, BLUE or PhaseA, PhaseB, PhaseC and Neutral. Analog Devices uses the 
>> ABCN nomenclature so anyone using this driver will probably have referenced 
>> the Analog Devices datasheet so this terminology should be familiar. 
> Which is more commonly used in general?  We are designing what we hope
> will be a general interface and last thing we want is to have everyone
> not using ADI parts confused!  Personally not assigning 'colours' makes
> more sense to me.
I did a little research and here is the naming used by vendor:

ST Microelectronics: P1, P2, P3, N
http://www.st.com/content/ccc/resource/technical/document/application_note/5e/f4/22/4d/90/96/4c/c4/CD00153264.pdf/files/CD00153264.pdf/jcr:content/translations/en.CD00153264.pdf

Teridian: ABCN
https://www.mouser.com/ds/2/256/71M6513-71M6513H-22603.pdf

Maxim Integrated: ABCN
https://datasheets.maximintegrated.com/en/ds/78M6631.pdf

Microchip: ABCN
http://ww1.microchip.com/downloads/en/DeviceDoc/51643b.pdf

NXP: L1, L2, L3, N
https://www.nxp.com/support/developer-resources/reference-designs/kinetis-km3x-256-mcu-three-phase-metering-reference-design:RD-KM3x-256-3PH-METERING

Renesas: ABCN
https://www.renesas.com/en-us/solutions/home/metering/power-meter-three.html

So the most consistent would be ABCN


All vendor providing three phase energy metering ICs use the ABCN terminology. 
> 
> (btw please wrap any lines that don't need to be long to around 80 
> characters).
> 
>>> 
>>> Hmm. With neutral in there as well I guess we need to make them
>>> modifiers (but might change my mind later ;)
>>> 
>>> Particularly as we are using the the modifier for RMS under the previous
>>> plan... It appears we should treat that instead like we did for peak
>>> and do it as an additional info mask element.  The problem with doing that
>>> on a continuous measurement is that we can't treat it as a channel to
>>> be output through the buffered interface.  
>> I’ve changed the layout of the spreadsheet to breakout the Direction, Type, 
>> Index, Modifier and Info Mask to make sure there is no miss-understanding 
>> and will help identify all the items we need to add.
>> 
>> The ADE7878 channels that will use the buffer are IAWV, VAWV, IBWV, VBWV, 
>> ICWV, VCWV, INWV, AVA, BVA, CVA, AWATT, BWATT, CWATT, AVAR, BVAR, and CVAR. 
>> So I guess we have to add an index
> Probably a good idea anyway, we are sort of slowly moving away
> from index less channels.  The ABI always allowed index assignment
> and it makes for easier userspace code.
> 
>> 
>> Address Register IIO Attribute   
>> Dir TypeIndex   ModifierInfo Mask   R/W  
>>Bit Bit Length  TypeDefault 

Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)

[Jonathan Cameron]

On Mon, 19 Mar 2018 23:28:45 -0700
John Syne  wrote:

> Hi Jonathan,
> 
> I broke out the {Direction}_{Type}_{Index}_{Modifier}_{Info_Mask} into 
> separate columns to make sure I understand your instructions. Good way to 
> check the results. 
> 
> Probably easier to copy and paste this table into a spreadsheet. Let me know 
> if there is anything I got wrong. Thank you again for all your help.
Yeah, we need to shrink this if we do it again.

Can drop anything not related to ABI (so RW mask address etc and just have the
register name and the bits around ABI.

Note I mentioned the altvoltage stuff in the previous email, so we need
to think about whether that is useful to distinguish the instantaneous
measurements from the multi cycle AC ones.
Actually I think we are fine here as we explicitly describe all 
alt measurements as mav or rms which going to be handled in our
new computedvalue description.


> 
> Address   RegisterIIO Attribute   Device Tree or Code 
> Direction   TypeIndex   ModifierInfo Mask   R/W Bit 
> Length  Bit Length During CommunicationsTypeDefault Value   
> Description
> 0x4380AIGAIN  in_current0_phaseA_scalein  current 
> 0   phaseA  scale   R/W 24  32 ZPSE S   0x00Phase 
> A current gain adjust.
> 0x4381AVGAIN  in_voltage0_phaseA_scalein  voltage 
> 0   phaseA  scale   R/W 24  32 ZPSE S   0x00Phase 
> A voltage gain adjust.
> 0x4382BIGAIN  in_current0_phaseB_scalein  current 
> 0   phaseB  scale   R/W 24  32 ZPSE S   0x00Phase 
> B current gain adjust.
> 0x4383BVGAIN  in_voltage0_phaseB_scalein  voltage 
> 0   phaseB  scale   R/W 24  32 ZPSE S   0x00Phase 
> B voltage gain adjust.
> 0x4384CIGAIN  in_current0_phaseC_scalein  current 
> 0   phaseC  scale   R/W 24  32 ZPSE S   0x00Phase 
> C current gain adjust.
> 0x4385CVGAIN  in_voltage0_phaseC_scalein  voltage 
> 0   phaseC  scale   R/W 24  32 ZPSE S   0x00Phase 
> C voltage gain adjust.
> 0x4386NIGAIN  in_current0_neutral_scale   in  current 
> 0   neutral scale   R/W 24  32 ZPSE S   0x00
> Neutral current gain adjust (ADE7868 and ADE7878 only).
> 0x4387AIRMSOS in_current0_phaseA_rms_offset   in  current 
> 0   phaseA  offset  R/W 24  32 ZPSE S   0x00Phase 
> A current rms offset.
> 0x4388AVRMSOS in_voltage0_phaseA_rms_offset   in  voltage 
> 0   phaseA  offset  R/W 24  32 ZPSE S   0x00Phase 
> A voltage rms offset.
> 0x4389BIRMSOS in_current0_phaseB_rms_offset   in  current 
> 0   phaseB  offset  R/W 24  32 ZPSE S   0x00Phase 
> B current rms offset.
> 0x438ABVRMSOS in_voltage0_phaseB_rms_offset   in  voltage 
> 0   phaseB  offset  R/W 24  32 ZPSE S   0x00Phase 
> B voltage rms offset.
> 0x438BCIRMSOS in_current0_phaseC_rms_offset   in  current 
> 0   phaseC  offset  R/W 24  32 ZPSE S   0x00Phase 
> C current rms offset.
> 0x438CCVRMSOS in_voltage0_phaseC_rms_offset   in  voltage 
> 0   phaseC  offset  R/W 24  32 ZPSE S   0x00Phase 
> C voltage rms offset.
> 0x438DNIRMSOS in_current0_neutral_rms_offset  in  current 
> 0   neutral offset  R/W 24  32 ZPSE S   0x00
> Neutral current rms offset (ADE7868 and ADE7878 only).
> 0x438EAVAGAIN in_powerapparent0_phaseA_scale  in  
> powerapparent   0   phaseA  scale   R/W 24  32 ZPSE S   
> 0x00Phase A apparent power gain adjust.
> 0x438FBVAGAIN in_powerapparent0_phaseB_scale  in  
> powerapparent   0   phaseB  scale   R/W 24  32 ZPSE S   
> 0x00Phase B apparent power gain adjust.
> 0x4390CVAGAIN in_powerapparent0_phaseC_scale  in  
> powerapparent   0   phaseC  scale   R/W 24  32 ZPSE S   
> 0x00Phase C apparent power gain adjust.
> 0x4391AWGAIN  in_power0_phaseA_scale  in  power   0   
> phaseA  scale   R/W 24  32 ZPSE S   0x00Phase A total 
> active power gain adjust.
> 0x4392AWATTOS in_power0_phaseA_offset in  power   0   
> phaseA  offset  R/W 24  32 ZPSE S   0x00Phase A total 
> active power offset adjust.
> 0x4393BWGAIN  in_power0_phaseB_scale  in  power   0   
> phaseB  scale   R/W 24  32 ZPSE S   0x00Phase B total 

Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)

[Jonathan Cameron]

On Mon, 19 Mar 2018 22:57:16 -0700
John Syne  wrote:

> Hi Jonathan,
> 
> Thank you for all your hard work. Your feedback is really helpful. I’m 
> surprised that no one from Analog Device has offered any suggestions.
> 

Sadly those active in the mainline linux kernel from ADI are focused in
other areas currently :(
> Anyway, please see my comments inline below
> 
> Regards,
> John
> 
> 
> 
> 
> 
> > On Mar 18, 2018, at 5:23 AM, Jonathan Cameron  wrote:
> > 
> > On Sat, 17 Mar 2018 23:11:45 -0700
> > John Syne  wrote:
> >   
> >> Hi Jonathan,  
> > Hi John and All,
> > 
> > I'd love to get some additional input on this from anyone interested.
> > There are a lot of weird and wonderful derived quantities in an energy
> > meter and it seems we need to make some fundamental changes to support
> > them - including potentially a userspace breaking change to the event
> > code description.
> >   
> >> 
> >> Here is the complete list of registers for the ADE7878 which I copied from 
> >> the data sheet. I added a column “IIO Attribute” which I hope follows your 
> >> IIO ABI. Please make any changes you feel are incorrect. BTW, there are 
> >> several registers that cannot be generalized and are used purely for chip 
> >> configuration. I think we should add a new naming convention, namely 
> >> {register}_{}. Also, I see in the sys_bus_iio doc




> >> in_accel_x_peak_raw
> >> 
> >> so shouldn’t the phase be represented as follows:
> >> 
> >> in_current_A_scale  
> > I'm still confused.  What does A represent here?  I assumed that was a wild
> > card for the channel number before but clearly not.
> > 
> > Ah, you are labelling the 3 separate phases as A, B and C. Hmm.
> > I guess they sort of look like axis, and sort of like independent channels.
> > So could be indexed or done via modifiers depending on how you look at it.  
> In metering terminology, the three phases are commonly referred to as RED, 
> GREEN, BLUE or PhaseA, PhaseB, PhaseC and Neutral. Analog Devices uses the 
> ABCN nomenclature so anyone using this driver will probably have referenced 
> the Analog Devices datasheet so this terminology should be familiar. 
Which is more commonly used in general?  We are designing what we hope
will be a general interface and last thing we want is to have everyone
not using ADI parts confused!  Personally not assigning 'colours' makes
more sense to me.

(btw please wrap any lines that don't need to be long to around 80 characters).

> > 
> > Hmm. With neutral in there as well I guess we need to make them
> > modifiers (but might change my mind later ;)
> > 
> > Particularly as we are using the the modifier for RMS under the previous
> > plan... It appears we should treat that instead like we did for peak
> > and do it as an additional info mask element.  The problem with doing that
> > on a continuous measurement is that we can't treat it as a channel to
> > be output through the buffered interface.  
> I’ve changed the layout of the spreadsheet to breakout the Direction, Type, 
> Index, Modifier and Info Mask to make sure there is no miss-understanding and 
> will help identify all the items we need to add.
> 
> The ADE7878 channels that will use the buffer are IAWV, VAWV, IBWV, VBWV, 
> ICWV, VCWV, INWV, AVA, BVA, CVA, AWATT, BWATT, CWATT, AVAR, BVAR, and CVAR. 
> So I guess we have to add an index
Probably a good idea anyway, we are sort of slowly moving away
from index less channels.  The ABI always allowed index assignment
and it makes for easier userspace code.

> 
> Address Register  IIO Attribute   
> Dir TypeIndex   ModifierInfo Mask   R/W   
>   Bit Bit Length  TypeDefault Description 
>   
>   
>   Length  During Comm 
> Value   
> 0xE50CIAWVin_current0_phaseA_instantaneousin  
> current 0   phaseA  instantaneous   R   24
>   32 SE   S   N/A Instantaneous value of 
> Phase A current.

I'm unconvinced by "instantaneous".  That is the default assumption that you are
measuring current at this point in time.  I'm still confused on what this 
actually
is. Is it effectively the DC current? I.e. the wave form value for current? 
You answer this below.  They are DC measurements..

Which actually raises a point I'd forgotten.  We have an explicit type 
for altvoltage (defined for DDS devices as the waveform amplitude IIRC).
We should be consistent with that if possible.

So I think this should be.
in_current0_phaseA_raw
etc
A channel can be uniquely identified using index and modifier as as pair (if we 
add
the new field for computation applied 

Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)

[John Syne]

Hi Jonathan,

I broke out the {Direction}_{Type}_{Index}_{Modifier}_{Info_Mask} into separate 
columns to make sure I understand your instructions. Good way to check the 
results. 

Probably easier to copy and paste this table into a spreadsheet. Let me know if 
there is anything I got wrong. Thank you again for all your help.

Address RegisterIIO Attribute   Device Tree or Code Direction   
TypeIndex   ModifierInfo Mask   R/W Bit Length  Bit 
Length During CommunicationsTypeDefault Value   Description
0x4380  AIGAIN  in_current0_phaseA_scalein  current 0   
phaseA  scale   R/W 24  32 ZPSE S   0x00Phase A current 
gain adjust.
0x4381  AVGAIN  in_voltage0_phaseA_scalein  voltage 0   
phaseA  scale   R/W 24  32 ZPSE S   0x00Phase A voltage 
gain adjust.
0x4382  BIGAIN  in_current0_phaseB_scalein  current 0   
phaseB  scale   R/W 24  32 ZPSE S   0x00Phase B current 
gain adjust.
0x4383  BVGAIN  in_voltage0_phaseB_scalein  voltage 0   
phaseB  scale   R/W 24  32 ZPSE S   0x00Phase B voltage 
gain adjust.
0x4384  CIGAIN  in_current0_phaseC_scalein  current 0   
phaseC  scale   R/W 24  32 ZPSE S   0x00Phase C current 
gain adjust.
0x4385  CVGAIN  in_voltage0_phaseC_scalein  voltage 0   
phaseC  scale   R/W 24  32 ZPSE S   0x00Phase C voltage 
gain adjust.
0x4386  NIGAIN  in_current0_neutral_scale   in  current 0   
neutral scale   R/W 24  32 ZPSE S   0x00Neutral current 
gain adjust (ADE7868 and ADE7878 only).
0x4387  AIRMSOS in_current0_phaseA_rms_offset   in  current 0   
phaseA  offset  R/W 24  32 ZPSE S   0x00Phase A current 
rms offset.
0x4388  AVRMSOS in_voltage0_phaseA_rms_offset   in  voltage 0   
phaseA  offset  R/W 24  32 ZPSE S   0x00Phase A voltage 
rms offset.
0x4389  BIRMSOS in_current0_phaseB_rms_offset   in  current 0   
phaseB  offset  R/W 24  32 ZPSE S   0x00Phase B current 
rms offset.
0x438A  BVRMSOS in_voltage0_phaseB_rms_offset   in  voltage 0   
phaseB  offset  R/W 24  32 ZPSE S   0x00Phase B voltage 
rms offset.
0x438B  CIRMSOS in_current0_phaseC_rms_offset   in  current 0   
phaseC  offset  R/W 24  32 ZPSE S   0x00Phase C current 
rms offset.
0x438C  CVRMSOS in_voltage0_phaseC_rms_offset   in  voltage 0   
phaseC  offset  R/W 24  32 ZPSE S   0x00Phase C voltage 
rms offset.
0x438D  NIRMSOS in_current0_neutral_rms_offset  in  current 0   
neutral offset  R/W 24  32 ZPSE S   0x00Neutral current 
rms offset (ADE7868 and ADE7878 only).
0x438E  AVAGAIN in_powerapparent0_phaseA_scale  in  powerapparent   
0   phaseA  scale   R/W 24  32 ZPSE S   0x00Phase A 
apparent power gain adjust.
0x438F  BVAGAIN in_powerapparent0_phaseB_scale  in  powerapparent   
0   phaseB  scale   R/W 24  32 ZPSE S   0x00Phase B 
apparent power gain adjust.
0x4390  CVAGAIN in_powerapparent0_phaseC_scale  in  powerapparent   
0   phaseC  scale   R/W 24  32 ZPSE S   0x00Phase C 
apparent power gain adjust.
0x4391  AWGAIN  in_power0_phaseA_scale  in  power   0   phaseA  
scale   R/W 24  32 ZPSE S   0x00Phase A total active 
power gain adjust.
0x4392  AWATTOS in_power0_phaseA_offset in  power   0   phaseA  
offset  R/W 24  32 ZPSE S   0x00Phase A total active 
power offset adjust.
0x4393  BWGAIN  in_power0_phaseB_scale  in  power   0   phaseB  
scale   R/W 24  32 ZPSE S   0x00Phase B total active 
power gain adjust.
0x4394  BWATTOS in_power0_phaseB_offset in  power   0   phaseB  
offset  R/W 24  32 ZPSE S   0x00Phase B total active 
power offset adjust.
0x4395  CWGAIN  in_power0_PhaseC_scale  in  power   0   phaseC  
scale   R/W 24  32 ZPSE S   0x00Phase C total active 
power gain adjust.
0x4396  CWATTOS in_power0_phaseC_offset in  power   0   phaseC  
offset  R/W 24  32 ZPSE S   0x00Phase C total active 
power offset adjust.
0x4397  AVARGAINin_powerreactive0_phaseA_scale  in  
powerreactive   0   phaseA  scale   R/W 24  32 ZPSE S   
0x00Phase A total reactive power gain adjust (ADE7858, ADE7868, and 
ADE7878).
0x4398  AVAROS  in_powerreactive0_phaseA_offset in   

Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)

[John Syne]

Hi Jonathan,

Thank you for all your hard work. Your feedback is really helpful. I’m 
surprised that no one from Analog Device has offered any suggestions.

Anyway, please see my comments inline below

Regards,
John





> On Mar 18, 2018, at 5:23 AM, Jonathan Cameron  wrote:
> 
> On Sat, 17 Mar 2018 23:11:45 -0700
> John Syne  wrote:
> 
>> Hi Jonathan,
> Hi John and All,
> 
> I'd love to get some additional input on this from anyone interested.
> There are a lot of weird and wonderful derived quantities in an energy
> meter and it seems we need to make some fundamental changes to support
> them - including potentially a userspace breaking change to the event
> code description.
> 
>> 
>> Here is the complete list of registers for the ADE7878 which I copied from 
>> the data sheet. I added a column “IIO Attribute” which I hope follows your 
>> IIO ABI. Please make any changes you feel are incorrect. BTW, there are 
>> several registers that cannot be generalized and are used purely for chip 
>> configuration. I think we should add a new naming convention, namely 
>> {register}_{}. Also, I see in the sys_bus_iio doc
>> in_accel_x_peak_raw
>> 
>> so shouldn’t the phase be represented as follows:
>> 
>> in_current_A_scale
> I'm still confused.  What does A represent here?  I assumed that was a wild
> card for the channel number before but clearly not.
> 
> Ah, you are labelling the 3 separate phases as A, B and C. Hmm.
> I guess they sort of look like axis, and sort of like independent channels.
> So could be indexed or done via modifiers depending on how you look at it.
In metering terminology, the three phases are commonly referred to as RED, 
GREEN, BLUE or PhaseA, PhaseB, PhaseC and Neutral. Analog Devices uses the ABCN 
nomenclature so anyone using this driver will probably have referenced the 
Analog Devices datasheet so this terminology should be familiar. 
> 
> Hmm. With neutral in there as well I guess we need to make them
> modifiers (but might change my mind later ;)
> 
> Particularly as we are using the the modifier for RMS under the previous
> plan... It appears we should treat that instead like we did for peak
> and do it as an additional info mask element.  The problem with doing that
> on a continuous measurement is that we can't treat it as a channel to
> be output through the buffered interface.
I’ve changed the layout of the spreadsheet to breakout the Direction, Type, 
Index, Modifier and Info Mask to make sure there is no miss-understanding and 
will help identify all the items we need to add.

The ADE7878 channels that will use the buffer are IAWV, VAWV, IBWV, VBWV, ICWV, 
VCWV, INWV, AVA, BVA, CVA, AWATT, BWATT, CWATT, AVAR, BVAR, and CVAR. So I 
guess we have to add an index

Address RegisterIIO Attribute   
Dir TypeIndex   ModifierInfo Mask   R/W 
Bit Bit Length  TypeDefault Description 


Length  During Comm 
Value   
0xE50C  IAWVin_current0_phaseA_instantaneousin  current 
0   phaseA  instantaneous   R   24  32 SE   
S   N/A Instantaneous value of Phase A current.
0xE50D  IBWVin_current1_phaseB_instantaneousin  current 
1   phaseB  instantaneous   R   24  32 SE   
S   N/A Instantaneous value of Phase B current.
0xE50E  ICWVin_current2_phaseC_instantaneousin  current 
2   phaseC  instantaneous   R   24  32 SE   
S   N/A Instantaneous value of Phase C current.
0xE50F  INWVin_current3_phaseN_instantaneousin  current 
3   neutral instantaneous   R   24  32 SE   
S   N/A Instantaneous value of neutral current 
(ADE7868 and ADE7878 only).
0xE510  VAWVin_voltage4_phaseA_instantaneousin  voltage 
4   phaseA  instantaneous   R   24  32 SE   
S   N/A Instantaneous value of Phase A voltage.
0xE511  VBWVin_voltage5_phaseB_instantaneousin  voltage 
5   phaseB  instantaneous   R   24  32 SE   
S   N/A Instantaneous value of Phase B voltage.
0xE512  VCWVin_voltage6_phaseC_instantaneousin  voltage 
6   phaseC  instantaneous   R   24  32 SE   
S   N/A Instantaneous value of Phase C voltage.
0xE513  AWATT