[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1ead013c-56ef-4f11-afb9-2b11e0de7eb2@baylibre.com>
Date: Fri, 11 Jul 2025 14:23:14 -0500
From: David Lechner <dlechner@...libre.com>
To: Antoniu Miclaus <antoniu.miclaus@...log.com>, jic23@...nel.org,
robh@...nel.org, conor+dt@...nel.org, devicetree@...r.kernel.org,
linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] iio: add power and energy measurement modifiers
On 7/11/25 8:02 AM, Antoniu Miclaus wrote:
> Add new IIO modifiers to support power and energy measurement devices:
>
> Power modifiers:
> - IIO_MOD_ACTIVE: Real power consumed by the load
> - IIO_MOD_REACTIVE: Power that oscillates between source and load
> - IIO_MOD_APPARENT: Magnitude of complex power
These make sense a modifiers since they are components of a single
measured value.
> - IIO_MOD_FUND_REACTIVE: Reactive power at fundamental frequency
This one seems like there should just be a separate channel
with IIO_POWER + IIO_MOD_REACTIVE since it is measuring a different
value.
> - IIO_MOD_FACTOR: Power factor (ratio of active to apparent power)
Power factor seems like it should be a IIO_CHAN_INFO_ rather than
IIO_MOD_. It is also unitless, so doesn't make sense to be part
of power_raw which would imply that it shuold be converted to Watts.
>
> Energy modifiers:
> - IIO_MOD_ACTIVE_ACCUM: Accumulated active energy
> - IIO_MOD_APPARENT_ACCUM: Accumulated apparent energy
> - IIO_MOD_REACTIVE_ACCUM: Accumulated reactive energy
As below, this one seems like there should be a separate
energy channel for accumulated energy.
>
> Signal quality modifiers:
> - IIO_MOD_RMS: Root Mean Square value
Suprised we don't have something like this already. altvoltageY isn't
clear about if the value is peak-to-peak or RMS.
> - IIO_MOD_SWELL: Voltage swell detection
> - IIO_MOD_DIP: Voltage dip (sag) detection
These sound like events, not modifiers.
>
> These modifiers enable proper representation of power measurement
> devices like energy meters and power analyzers.
>
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@...log.com>
> ---
> Documentation/ABI/testing/sysfs-bus-iio | 19 +++++++++++++++++++
> drivers/iio/industrialio-core.c | 11 +++++++++++
> include/uapi/linux/iio/types.h | 11 +++++++++++
> 3 files changed, 41 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index 3bc386995fb6..d5c227c03589 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -143,6 +143,9 @@ What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_raw
> What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_supply_raw
> What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_i_raw
> What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_q_raw
> +What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_rms_raw
This should be on altvoltage, not voltage.
Also, the exisiting i and q are wrong for the same reason.
> +What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_swell_raw
> +What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_dip_raw
> KernelVersion: 2.6.35
> Contact: linux-iio@...r.kernel.org
> Description:
> @@ -158,6 +161,7 @@ Description:
> component of the signal while the 'q' channel contains the quadrature
> component.
>
> +
> What: /sys/bus/iio/devices/iio:deviceX/in_voltageY-voltageZ_raw
> KernelVersion: 2.6.35
> Contact: linux-iio@...r.kernel.org
> @@ -170,6 +174,11 @@ Description:
> of scale and offset are millivolts.
>
> What: /sys/bus/iio/devices/iio:deviceX/in_powerY_raw
> +What: /sys/bus/iio/devices/iio:deviceX/in_powerY_active_raw
> +What: /sys/bus/iio/devices/iio:deviceX/in_powerY_reactive_raw
> +What: /sys/bus/iio/devices/iio:deviceX/in_powerY_apparent_raw
> +What: /sys/bus/iio/devices/iio:deviceX/in_powerY_fund_reactive_raw
> +What: /sys/bus/iio/devices/iio:deviceX/in_powerY_factor_raw
As above, power factor doesn't have units of watts so doesn't belong here.
> KernelVersion: 4.5
> Contact: linux-iio@...r.kernel.org
> Description:
> @@ -178,6 +187,7 @@ Description:
> unique to allow association with event codes. Units after
> application of scale and offset are milliwatts.
>
> +
> What: /sys/bus/iio/devices/iio:deviceX/in_capacitanceY_raw
> KernelVersion: 3.2
> Contact: linux-iio@...r.kernel.org
> @@ -1593,6 +1603,12 @@ Description:
>
> What: /sys/.../iio:deviceX/in_energy_input
> What: /sys/.../iio:deviceX/in_energy_raw
> +What: /sys/.../iio:deviceX/in_energyY_active_raw
> +What: /sys/.../iio:deviceX/in_energyY_reactive_raw
> +What: /sys/.../iio:deviceX/in_energyY_apparent_raw
> +What: /sys/.../iio:deviceX/in_energyY_active_accum_raw
> +What: /sys/.../iio:deviceX/in_energyY_reactive_accum_raw
> +What: /sys/.../iio:deviceX/in_energyY_apparent_accum_raw
I think the accumulated would just be a separate channel, not a modifier.
> KernelVersion: 4.0
> Contact: linux-iio@...r.kernel.org
> Description:
> @@ -1600,6 +1616,7 @@ Description:
> device (e.g.: human activity sensors report energy burnt by the
> user). Units after application of scale are Joules.
>
> +
Stray blank line.
> What: /sys/.../iio:deviceX/in_distance_input
> What: /sys/.../iio:deviceX/in_distance_raw
> KernelVersion: 4.0
> @@ -1718,6 +1735,7 @@ What: /sys/bus/iio/devices/iio:deviceX/in_currentY_raw
> What: /sys/bus/iio/devices/iio:deviceX/in_currentY_supply_raw
> What: /sys/bus/iio/devices/iio:deviceX/in_currentY_i_raw
> What: /sys/bus/iio/devices/iio:deviceX/in_currentY_q_raw
> +What: /sys/bus/iio/devices/iio:deviceX/in_currentY_rms_raw
Interesting that we don't have altcurrent like we do altvoltage.
And there don't appeary to be any users of i and q modifiers on current
so that can be dropped.
> KernelVersion: 3.17
> Contact: linux-iio@...r.kernel.org
> Description:
> @@ -1733,6 +1751,7 @@ Description:
> component of the signal while the 'q' channel contains the quadrature
> component.
>
> +
Stray blank line.
> What: /sys/.../iio:deviceX/in_energy_en
> What: /sys/.../iio:deviceX/in_distance_en
> What: /sys/.../iio:deviceX/in_velocity_sqrt(x^2+y^2+z^2)_en
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index f13c3aa470d7..daf486cbe0bd 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -152,6 +152,17 @@ static const char * const iio_modifier_names[] = {
> [IIO_MOD_PITCH] = "pitch",
> [IIO_MOD_YAW] = "yaw",
> [IIO_MOD_ROLL] = "roll",
> + [IIO_MOD_RMS] = "rms",
> + [IIO_MOD_ACTIVE] = "active",
> + [IIO_MOD_REACTIVE] = "reactive",
> + [IIO_MOD_APPARENT] = "apparent",
> + [IIO_MOD_FUND_REACTIVE] = "fund_reactive",
> + [IIO_MOD_FACTOR] = "factor",
> + [IIO_MOD_ACTIVE_ACCUM] = "active_accum",
> + [IIO_MOD_APPARENT_ACCUM] = "apparent_accum",
> + [IIO_MOD_REACTIVE_ACCUM] = "reactive_accum",
If we end up keeping any of the two-word modifiers, the actual string
needs to omit the "_". The readability isn't so great, but it makes it
much easier to machine parse if we can assume the modifier is always
"oneword".
> + [IIO_MOD_SWELL] = "swell",
> + [IIO_MOD_DIP] = "dip",
> };
>
> /* relies on pairs of these shared then separate */
> diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
> index 3eb0821af7a4..9e05bbddcbe2 100644
> --- a/include/uapi/linux/iio/types.h
> +++ b/include/uapi/linux/iio/types.h
> @@ -108,6 +108,17 @@ enum iio_modifier {
> IIO_MOD_ROLL,
> IIO_MOD_LIGHT_UVA,
> IIO_MOD_LIGHT_UVB,
> + IIO_MOD_RMS,
> + IIO_MOD_ACTIVE,
> + IIO_MOD_REACTIVE,
> + IIO_MOD_APPARENT,
> + IIO_MOD_FUND_REACTIVE,
> + IIO_MOD_FACTOR,
> + IIO_MOD_ACTIVE_ACCUM,
> + IIO_MOD_APPARENT_ACCUM,
> + IIO_MOD_REACTIVE_ACCUM,
> + IIO_MOD_SWELL,
> + IIO_MOD_DIP,
> };
>
> enum iio_event_type {
Powered by blists - more mailing lists