From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-oa1-f48.google.com (mail-oa1-f48.google.com [209.85.160.48]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1ACD51DE4E0 for ; Fri, 11 Jul 2025 19:23:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.160.48 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1752261800; cv=none; b=IZmVPNzYG1JtGJV78hkbr7y17Y3YsuwzF2gPCnBMx6gGtQQsvU49tp30iigWWkDUbG6X52V/8GjXYlBxnuga4DMG3HtnfZeZi8/HnT+pNC+rWUJ1qOmYIU3O5WEiOU0c9v+ZAjs8uN86f6Ar5AcmVphFPnfP+Jyc/tPr34x6pDE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1752261800; c=relaxed/simple; bh=CT8j4G71nN8Lnzk6rXI/vvQAsHcH1biWkLypZw72nnM=; h=Message-ID:Date:MIME-Version:Subject:To:References:From: In-Reply-To:Content-Type; b=XghvS27R7iSAT3O3/C5CY5Vr88z9Vd0uAfdvkGc5RxW0XW8OBkTVfIY5qG2UWNtuI/miH1MmvRO96gfx+6YcwQag+JotFuuX9iky1z6sXa/gF8D86tP3gl7XeTIPbgNY2uYuYzWUciR5mB+ok9T73NQWOrw9SUckep/Q4C0So2g= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=baylibre.com; spf=pass smtp.mailfrom=baylibre.com; dkim=pass (2048-bit key) header.d=baylibre-com.20230601.gappssmtp.com header.i=@baylibre-com.20230601.gappssmtp.com header.b=SO/yxq0/; arc=none smtp.client-ip=209.85.160.48 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=baylibre.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=baylibre.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=baylibre-com.20230601.gappssmtp.com header.i=@baylibre-com.20230601.gappssmtp.com header.b="SO/yxq0/" Received: by mail-oa1-f48.google.com with SMTP id 586e51a60fabf-2c6ed7efb1dso1534643fac.2 for ; Fri, 11 Jul 2025 12:23:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20230601.gappssmtp.com; s=20230601; t=1752261796; x=1752866596; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id:from :to:cc:subject:date:message-id:reply-to; bh=xbQ4gjhcKf1FV36kcI6WczoSoNZh07OMTR5d0NMFKgo=; b=SO/yxq0/FRnxLsWSB0dHlPrVIoH7rvokZ0CzCMznRyEWj/b/i7hfgImcY8V+8euTmT nS2YBQ4Jj6ZtPo6j+NDOVsJPq057qNI4i0hE27Z+KYexi57+kWDB4XWm1VCY1qCOKNCy 49YFfbKEqkbQ0lVaU2LfZaRreGmGMQRNpGZJ32vNbSIVM78IesYYQvncddXFXyOJWPgU 4yF6g71RYm2BhiYL8O7xWxOs+DkAzJdwe/KfnwmsYSD7vY/Qhmi+BiokalCeSzRruX9x NDVIZ+59ypj2R15B3vJF1BM/CJgofmiiVsOlDy7MiQrYwjA8Ir3AVQoBNlm8bHgq0mei 1DMQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1752261796; x=1752866596; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=xbQ4gjhcKf1FV36kcI6WczoSoNZh07OMTR5d0NMFKgo=; b=Q77ClOfvo3rEyRcILN3r0CpiB9ywlTPu5SkwsNkKjr7R6PVA1mFZUIJVFEQYccZxnM H19b+d3n2nXg8rB9KaFMTXuWc3grxO2T6y6ZAkFXe6hBQCGZU9TLgpKwLow/Itdhsi+a AUjW1Q6+QngWCfNgzsZLmdUryhnuDg3VHjEMB9CSjSorPQj0Rd8OxNZFkwkSg2nmNye2 HFtkwElYkK34GFfa+woOk6ihOha9qDUZtGfjJf9LFnXqj0jgxfvJcE57ddi6RZGXqLX/ srJuOCX573xTYQloo9Pz/NnEa4H40eouywmM0j/8jn4pJfitGWDpFJAUj4ugftxSmiUh UfEA== X-Forwarded-Encrypted: i=1; AJvYcCWgY+9fMNvLL1jZNtOyQrEe8bzkKpXMlaAG0HtMOGNp98vYyu/c4GT26TVM44ItXoInwecJRLu4GKnuVn8=@vger.kernel.org X-Gm-Message-State: AOJu0YyvDHkDRZL3aHwbaau1qWpopfnq/X2GBarqUOr0xXGjH3gvK29r 0k3fMkgSVkG3qXYPyzMHKQo3jJ1BRbaOABtGMsvP1gR4RohTc5VWdlF5BIg++smU/Mo= X-Gm-Gg: ASbGncuK1WPheNXn978fms2eqNlFHa8bAcppuJX6k9B0Dtbpp+VIkrqFIOa7JywA0Uq 6R/LSX7QR78bSxC0W/lXU0u+mHOLEfoyETlCx1O8UmRJUGtX8OBlo7JUk4kfF3q4RjWpYNIpkD9 sQWHX74o4h+8tT3mTgYvwGkSNBpIg31JrXjXAkVIIbXnLQYg2hZdKBEcxpVptTK2Ll9VY36JHFx rHOlWVsZN45sn2oB65HbMhsvWPE6M6J4/zKeKHNNem+dLyvhhS9qNDxglBCdS6j/+Ozarbr1YXS vJbZd8bK6slmzPk8orJpvKoEhZl60PnxDc9qB9fosw93MccScD5Vj7BVnx4NFinGulAJtm/3F+j AN4UZ7aWlpiZ0sHcuvSV17Yd3XQByM3c9BY0H9HjVBpvsbXUt/LiryV635mE0rxVti2Sv14Sj5O rrv090K/+71g== X-Google-Smtp-Source: AGHT+IHJJG1pjfMfHbXCiP2mYhrjtaG+ds4ATps3yE0wgdEBzivun4Aw8g8ipsQFhpaLwTypjmt8jw== X-Received: by 2002:a05:6871:3518:b0:29e:69a9:8311 with SMTP id 586e51a60fabf-2ff27099808mr3221078fac.36.1752261796031; Fri, 11 Jul 2025 12:23:16 -0700 (PDT) Received: from ?IPV6:2600:8803:e7e4:1d00:4601:15f9:b923:d487? ([2600:8803:e7e4:1d00:4601:15f9:b923:d487]) by smtp.gmail.com with ESMTPSA id 586e51a60fabf-2ff116d3550sm846917fac.34.2025.07.11.12.23.14 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 11 Jul 2025 12:23:14 -0700 (PDT) Message-ID: <1ead013c-56ef-4f11-afb9-2b11e0de7eb2@baylibre.com> Date: Fri, 11 Jul 2025 14:23:14 -0500 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 1/3] iio: add power and energy measurement modifiers To: Antoniu Miclaus , jic23@kernel.org, robh@kernel.org, conor+dt@kernel.org, devicetree@vger.kernel.org, linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org References: <20250711130241.159143-1-antoniu.miclaus@analog.com> <20250711130241.159143-2-antoniu.miclaus@analog.com> Content-Language: en-US From: David Lechner In-Reply-To: <20250711130241.159143-2-antoniu.miclaus@analog.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit 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 > --- > 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@vger.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@vger.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@vger.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@vger.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@vger.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@vger.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 {