hwmon: peci: add energy sensor to peci-cpupower

Add support for energy consumption of CPU package sensor.
Energy is reported in micro Joules and exposed under
energyN_input file.

Signed-off-by: Olender, Agata <agata.olender@intel.com>

4 files changed, 342 insertions, 150 deletions

diff --git a/Documentation/hwmon/peci-cpupower.rst b/Documentation/hwmon/peci-cpupower.rst
index 1e1f4e0fd395..d3b6a666f076 100644
--- a/Documentation/hwmon/peci-cpupower.rst
+++ b/Documentation/hwmon/peci-cpupower.rst
@@ -38,13 +38,17 @@ Description
 -----------
 
 This driver implements a generic PECI hwmon feature which provides
-average power consumption readings of the CPU package based on energy counter.
+average power and energy consumption readings of the CPU package based on
+energy counter.
 
 Power values are average power since last measure given in milli Watt and
 will be measurable only when the target CPU is powered on.
 
+Energy values are energy consumption in micro Joules.
+
 Driver provides current package power limit, maximal (TDP) and minimal power
 setting as well.
+
 All needed processor registers are accessible using the PECI Client Command
 Suite via the processor PECI client.
 
@@ -56,4 +60,6 @@ power1_average		Provides average power since last read in milli Watt.
 power1_cap		Provides current package power limit 1 (PPL1).
 power1_cap_max		Provides maximal (TDP) package power setting.
 power1_cap_min		Provides minimal package power setting.
+energy1_label		Provides string "cpu energy".
+energy1_input		Provides energy consumption in micro Joules.
 ======================= =======================================================
diff --git a/drivers/hwmon/peci-cpupower.c b/drivers/hwmon/peci-cpupower.c
index d0a3af70f589..5a30b565fbb8 100644
--- a/drivers/hwmon/peci-cpupower.c
+++ b/drivers/hwmon/peci-cpupower.c
@@ -9,25 +9,51 @@
 #include <linux/platform_device.h>
 #include "peci-hwmon.h"
 
-#define PECI_CPUPOWER_CHANNEL_COUNT   1 /* Supported channels number */
+enum PECI_CPUPOWER_POWER_CONFIG_TYPES {
+	PECI_CPUPOWER_CONFIG_TYPE_POWER = 0,
+	PECI_CPUPOWER_CONFIG_TYPE_ENERGY,
+	PECI_CPUPOWER_CONFIG_TYPES_COUNT,
+};
+
+#define PECI_CPUPOWER_POWER_CHANNEL_COUNT	1 /* Supported channels number */
+#define PECI_CPUPOWER_ENERGY_CHANNEL_COUNT	1 /* Supported channels number */
 
-#define PECI_CPUPOWER_SENSOR_COUNT 4 /* Supported sensors/readings number */
+#define PECI_CPUPOWER_POWER_SENSOR_COUNT	4 /* Supported sensors number */
+#define PECI_CPUPOWER_ENERGY_SENSOR_COUNT	1 /* Supported sensors number */
 
 struct peci_cpupower {
 	struct device *dev;
 	struct peci_client_manager *mgr;
 	char name[PECI_NAME_SIZE];
-	u32 power_config[PECI_CPUPOWER_CHANNEL_COUNT + 1];
+	u32 power_config[PECI_CPUPOWER_POWER_CHANNEL_COUNT + 1];
+	u32 energy_config[PECI_CPUPOWER_ENERGY_CHANNEL_COUNT + 1];
 	u32 config_idx;
+
 	struct hwmon_channel_info power_info;
-	const struct hwmon_channel_info *info[2];
+	struct hwmon_channel_info energy_info;
+	const struct hwmon_channel_info *info[PECI_CPUPOWER_CONFIG_TYPES_COUNT + 1];
 	struct hwmon_chip_info chip;
 
 	struct peci_sensor_data
-		sensor_data_list[PECI_CPUPOWER_CHANNEL_COUNT]
-				[PECI_CPUPOWER_SENSOR_COUNT];
+		power_sensor_data_list[PECI_CPUPOWER_POWER_CHANNEL_COUNT]
+				      [PECI_CPUPOWER_POWER_SENSOR_COUNT];
+	struct peci_sensor_data
+		energy_sensor_data_list[PECI_CPUPOWER_ENERGY_CHANNEL_COUNT]
+				       [PECI_CPUPOWER_ENERGY_SENSOR_COUNT];
+
+	/*
+	 * Not exposed to any sensor directly - just used to limit PECI
+	 * communication for energy/power_average which are derived from the
+	 * same underlying data
+	 */
+	struct peci_sensor_data energy_cache;
+
+	/*
+	 * Not exposed to any sensor directly - just used to store previous raw
+	 * energy counter value that is required to calculate average power
+	 */
+	struct peci_sensor_data power_sensor_prev_energy;
 
-	s32 avg_power_val;
 	union peci_pkg_power_sku_unit units;
 	bool units_valid;
 
@@ -36,8 +62,9 @@ struct peci_cpupower {
 	bool ppl_time_windows_valid;
 };
 
-static const char *peci_cpupower_labels[PECI_CPUPOWER_CHANNEL_COUNT] = {
+static const char *peci_cpupower_labels[PECI_CPUPOWER_CONFIG_TYPES_COUNT] = {
 	"cpu power",
+	"cpu energy",
 };
 
 /**
@@ -71,54 +98,86 @@ peci_cpupower_read_cpu_pkg_pwr_lim_low(struct peci_client_manager *peci_mgr,
 }
 
 static int
-peci_cpupower_get_average_power(void *ctx, struct peci_sensor_conf *sensor_conf,
-				struct peci_sensor_data *sensor_data,
-				s32 *val)
+peci_cpupower_get_energy_counter(struct peci_cpupower *priv,
+				 struct peci_sensor_data *sensor_data,
+				 ulong update_interval)
+{
+	int ret;
+
+	/* Skip energy counter read if the interval time not elapsed */
+	if (!peci_sensor_need_update_with_time(sensor_data,
+					       update_interval)) {
+		dev_dbg(priv->dev, "skip reading peci\n");
+		return 0;
+	}
+
+	ret = peci_pcs_read(priv->mgr, PECI_MBX_INDEX_ENERGY_COUNTER,
+			    PECI_PKG_ID_CPU_PACKAGE, &sensor_data->value);
+	if (ret) {
+		dev_dbg(priv->dev, "not able to read package energy\n");
+		return ret;
+	}
+
+	peci_sensor_mark_updated(sensor_data);
+
+	dev_dbg(priv->dev,
+		"energy counter updated %duJ, jif %lu, HZ is %d jiffies\n",
+		sensor_data->value, sensor_data->last_updated, HZ);
+
+	return ret;
+}
+
+static int
+peci_cpupower_get_average_power(void *ctx,
+				struct peci_sensor_conf *sensor_conf,
+				struct peci_sensor_data *sensor_data)
 {
 	struct peci_cpupower *priv = (struct peci_cpupower *)ctx;
-	u32 energy_cnt;
-	ulong jif;
 	int ret;
 
 	if (!peci_sensor_need_update_with_time(sensor_data,
 					       sensor_conf->update_interval)) {
-		*val = priv->avg_power_val;
 		dev_dbg(priv->dev,
-			"skip reading peci, average power %dmW\n", *val);
+			"skip generating new power value %dmW jif %lu\n",
+			sensor_data->value, jiffies);
 		return 0;
 	}
 
+	ret = peci_cpupower_get_energy_counter(priv, &priv->energy_cache,
+					       sensor_conf->update_interval);
+	if (ret) {
+		dev_dbg(priv->dev, "cannot update energy counter\n");
+		return ret;
+	}
+
 	ret = peci_pcs_get_units(priv->mgr, &priv->units, &priv->units_valid);
 	if (ret) {
 		dev_dbg(priv->dev, "not able to read units\n");
 		return ret;
 	}
 
-	jif = jiffies;
-	ret = peci_pcs_read(priv->mgr, PECI_MBX_INDEX_ENERGY_COUNTER,
-			    PECI_PKG_ID_CPU_PACKAGE, &energy_cnt);
-
+	ret = peci_pcs_calc_pwr_from_eng(priv->dev,
+					 &priv->power_sensor_prev_energy,
+					 &priv->energy_cache,
+					 priv->units.bits.eng_unit,
+					 &sensor_data->value);
 	if (ret) {
-		dev_dbg(priv->dev, "not able to read package energy\n");
+		dev_dbg(priv->dev, "power calculation failed\n");
 		return ret;
 	}
 
-	ret = peci_pcs_calc_pwr_from_eng(priv->dev, sensor_data, energy_cnt,
-					 priv->units.bits.eng_unit, val);
-
-	priv->avg_power_val = *val;
-	peci_sensor_mark_updated_with_time(sensor_data, jif);
+	peci_sensor_mark_updated_with_time(sensor_data,
+					   priv->energy_cache.last_updated);
 
 	dev_dbg(priv->dev, "average power %dmW, jif %lu, HZ is %d jiffies\n",
-		*val, jif, HZ);
+		sensor_data->value, sensor_data->last_updated, HZ);
 
 	return ret;
 }
 
 static int
 peci_cpupower_get_power_limit(void *ctx, struct peci_sensor_conf *sensor_conf,
-			      struct peci_sensor_data *sensor_data,
-			      s32 *val)
+			      struct peci_sensor_data *sensor_data)
 {
 	struct peci_cpupower *priv = (struct peci_cpupower *)ctx;
 	union peci_package_power_limit_low power_limit;
@@ -127,9 +186,8 @@ peci_cpupower_get_power_limit(void *ctx, struct peci_sensor_conf *sensor_conf,
 
 	if (!peci_sensor_need_update_with_time(sensor_data,
 					       sensor_conf->update_interval)) {
-		*val = sensor_data->value;
 		dev_dbg(priv->dev, "skip reading peci, power limit %dmW\n",
-			*val);
+			sensor_data->value);
 		return 0;
 	}
 
@@ -146,14 +204,14 @@ peci_cpupower_get_power_limit(void *ctx, struct peci_sensor_conf *sensor_conf,
 		return ret;
 	}
 
-	*val = peci_pcs_xn_to_munits(power_limit.bits.pwr_lim_1,
-				     priv->units.bits.pwr_unit);
+	sensor_data->value = peci_pcs_xn_to_munits(power_limit.bits.pwr_lim_1,
+						   priv->units.bits.pwr_unit);
 
-	sensor_data->value = *val;
 	peci_sensor_mark_updated_with_time(sensor_data, jif);
 
 	dev_dbg(priv->dev, "raw power limit %u, unit %u, power limit %d\n",
-		power_limit.bits.pwr_lim_1, priv->units.bits.pwr_unit, *val);
+		power_limit.bits.pwr_lim_1, priv->units.bits.pwr_unit,
+		sensor_data->value);
 
 	return ret;
 }
@@ -249,8 +307,7 @@ peci_cpupower_set_power_limit(void *ctx, struct peci_sensor_conf *sensor_conf,
 
 static int
 peci_cpupower_read_max_power(void *ctx, struct peci_sensor_conf *sensor_conf,
-			     struct peci_sensor_data *sensor_data,
-			     s32 *val)
+			     struct peci_sensor_data *sensor_data)
 {
 	struct peci_cpupower *priv = (struct peci_cpupower *)ctx;
 	union peci_package_power_info_low power_info;
@@ -259,8 +316,8 @@ peci_cpupower_read_max_power(void *ctx, struct peci_sensor_conf *sensor_conf,
 
 	if (!peci_sensor_need_update_with_time(sensor_data,
 					       sensor_conf->update_interval)) {
-		*val = sensor_data->value;
-		dev_dbg(priv->dev, "skip reading peci, max power %dmW\n", *val);
+		dev_dbg(priv->dev, "skip reading peci, max power %dmW\n",
+			sensor_data->value);
 		return 0;
 	}
 
@@ -277,22 +334,21 @@ peci_cpupower_read_max_power(void *ctx, struct peci_sensor_conf *sensor_conf,
 		return ret;
 	}
 
-	*val = peci_pcs_xn_to_munits(power_info.bits.pkg_tdp,
-				     priv->units.bits.pwr_unit);
+	sensor_data->value = peci_pcs_xn_to_munits(power_info.bits.pkg_tdp,
+						   priv->units.bits.pwr_unit);
 
-	sensor_data->value = *val;
 	peci_sensor_mark_updated_with_time(sensor_data, jif);
 
 	dev_dbg(priv->dev, "raw max power %u, unit %u, max power %dmW\n",
-		power_info.bits.pkg_tdp, priv->units.bits.pwr_unit, *val);
+		power_info.bits.pkg_tdp, priv->units.bits.pwr_unit,
+		sensor_data->value);
 
 	return ret;
 }
 
 static int
 peci_cpupower_read_min_power(void *ctx, struct peci_sensor_conf *sensor_conf,
-			     struct peci_sensor_data *sensor_data,
-			     s32 *val)
+			     struct peci_sensor_data *sensor_data)
 {
 	struct peci_cpupower *priv = (struct peci_cpupower *)ctx;
 	union peci_package_power_info_low power_info;
@@ -301,9 +357,8 @@ peci_cpupower_read_min_power(void *ctx, struct peci_sensor_conf *sensor_conf,
 
 	if (!peci_sensor_need_update_with_time(sensor_data,
 					       sensor_conf->update_interval)) {
-		*val = sensor_data->value;
 		dev_dbg(priv->dev, "skip reading peci, min power %dmW\n",
-			*val);
+			sensor_data->value);
 		return 0;
 	}
 
@@ -320,20 +375,60 @@ peci_cpupower_read_min_power(void *ctx, struct peci_sensor_conf *sensor_conf,
 		return ret;
 	}
 
-	*val = peci_pcs_xn_to_munits(power_info.bits.pkg_min_pwr,
-				     priv->units.bits.pwr_unit);
-
-	sensor_data->value = *val;
+	sensor_data->value = peci_pcs_xn_to_munits(power_info.bits.pkg_min_pwr,
+						   priv->units.bits.pwr_unit);
 	peci_sensor_mark_updated_with_time(sensor_data, jif);
 
 	dev_dbg(priv->dev, "raw min power %u, unit %u, min power %dmW\n",
-		power_info.bits.pkg_min_pwr, priv->units.bits.pwr_unit, *val);
+		power_info.bits.pkg_min_pwr, priv->units.bits.pwr_unit,
+		sensor_data->value);
 
 	return ret;
 }
 
+static int
+peci_cpupower_read_energy(void *ctx, struct peci_sensor_conf *sensor_conf,
+			  struct peci_sensor_data *sensor_data)
+{
+	struct peci_cpupower *priv = (struct peci_cpupower *)ctx;
+	int ret;
+
+	if (!peci_sensor_need_update_with_time(sensor_data,
+					       sensor_conf->update_interval)) {
+		dev_dbg(priv->dev,
+			"skip generating new energy value %duJ jif %lu\n",
+			sensor_data->value, jiffies);
+		return 0;
+	}
+
+	ret = peci_cpupower_get_energy_counter(priv, &priv->energy_cache,
+					       sensor_conf->update_interval);
+	if (ret) {
+		dev_dbg(priv->dev, "cannot update energy counter\n");
+		return ret;
+	}
+
+	ret = peci_pcs_get_units(priv->mgr, &priv->units, &priv->units_valid);
+	if (ret) {
+		dev_dbg(priv->dev, "not able to read units\n");
+		return ret;
+	}
+
+	/* Energy consumed in microjoules */
+	sensor_data->value = (u32)peci_pcs_xn_to_uunits(priv->energy_cache.value,
+							priv->units.bits.eng_unit);
+	peci_sensor_mark_updated_with_time(sensor_data,
+					   priv->energy_cache.last_updated);
+
+	dev_dbg(priv->dev, "energy %duJ, jif %lu, HZ is %d jiffies\n",
+		sensor_data->value, sensor_data->last_updated, HZ);
+
+	return 0;
+}
+
 static struct peci_sensor_conf
-peci_cpupower_cfg[PECI_CPUPOWER_CHANNEL_COUNT][PECI_CPUPOWER_SENSOR_COUNT] = {
+peci_cpupower_power_cfg[PECI_CPUPOWER_POWER_CHANNEL_COUNT]
+		       [PECI_CPUPOWER_POWER_SENSOR_COUNT] = {
 	/* Channel 0  - Power */
 	{
 		{
@@ -367,17 +462,52 @@ peci_cpupower_cfg[PECI_CPUPOWER_CHANNEL_COUNT][PECI_CPUPOWER_SENSOR_COUNT] = {
 	},
 };
 
+static struct peci_sensor_conf
+peci_cpupower_energy_cfg[PECI_CPUPOWER_ENERGY_CHANNEL_COUNT]
+		[PECI_CPUPOWER_ENERGY_SENSOR_COUNT] = {
+	/* Channel 0  - Energy */
+	{
+		{
+			.attribute = hwmon_energy_input,
+			.config = HWMON_E_INPUT,
+			.update_interval = UPDATE_INTERVAL_100MS,
+			.read = peci_cpupower_read_energy,
+			.write = NULL,
+		},
+	}
+};
+
+static bool
+peci_cpupower_is_channel_valid(enum hwmon_sensor_types type,
+			       int channel)
+{
+	if ((type == hwmon_power && channel < PECI_CPUPOWER_POWER_CHANNEL_COUNT) ||
+	    (type == hwmon_energy && channel < PECI_CPUPOWER_ENERGY_CHANNEL_COUNT))
+		return true;
+
+	return false;
+}
+
 static int
 peci_cpupower_read_string(struct device *dev, enum hwmon_sensor_types type,
 			  u32 attr, int channel, const char **str)
 {
-	if (attr != hwmon_power_label || channel >= PECI_CPUPOWER_CHANNEL_COUNT)
+	if (!str)
+		return -EINVAL;
+
+	if (!peci_cpupower_is_channel_valid(type, channel))
 		return -EOPNOTSUPP;
 
-	if (str)
-		*str = peci_cpupower_labels[channel];
-	else
-		return -EINVAL;
+	switch (attr) {
+	case hwmon_power_label:
+		*str = peci_cpupower_labels[PECI_CPUPOWER_CONFIG_TYPE_POWER];
+		break;
+	case hwmon_energy_label:
+		*str = peci_cpupower_labels[PECI_CPUPOWER_CONFIG_TYPE_ENERGY];
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
 
 	return 0;
 }
@@ -394,22 +524,35 @@ peci_cpupower_read(struct device *dev, enum hwmon_sensor_types type,
 	if (!priv || !val)
 		return -EINVAL;
 
-	if (channel >= PECI_CPUPOWER_CHANNEL_COUNT)
+	if (!peci_cpupower_is_channel_valid(type, channel))
 		return -EOPNOTSUPP;
 
-	ret = peci_sensor_get_ctx(attr, peci_cpupower_cfg[channel],
-				  &sensor_conf, priv->sensor_data_list[channel],
-				  &sensor_data,
-				  ARRAY_SIZE(peci_cpupower_cfg[channel]));
+	switch (type) {
+	case hwmon_power:
+		ret = peci_sensor_get_ctx(attr, peci_cpupower_power_cfg[channel],
+					  &sensor_conf,
+					  priv->power_sensor_data_list[channel],
+					  &sensor_data,
+					  ARRAY_SIZE(peci_cpupower_power_cfg[channel]));
+		break;
+	case hwmon_energy:
+		ret = peci_sensor_get_ctx(attr, peci_cpupower_energy_cfg[channel],
+					  &sensor_conf,
+					  priv->energy_sensor_data_list[channel],
+					  &sensor_data,
+					  ARRAY_SIZE(peci_cpupower_energy_cfg[channel]));
+		break;
+	default:
+		ret = -EOPNOTSUPP;
+	}
+
 	if (ret)
 		return ret;
 
 	if (sensor_conf->read) {
-		s32 tmp;
-
-		ret = sensor_conf->read(priv, sensor_conf, sensor_data, &tmp);
+		ret = sensor_conf->read(priv, sensor_conf, sensor_data);
 		if (!ret)
-			*val = (long)tmp;
+			*val = (long)sensor_data->value;
 	} else {
 		ret = -EOPNOTSUPP;
 	}
@@ -429,13 +572,28 @@ peci_cpupower_write(struct device *dev, enum hwmon_sensor_types type,
 	if (!priv)
 		return -EINVAL;
 
-	if (channel >= PECI_CPUPOWER_CHANNEL_COUNT)
+	if (!peci_cpupower_is_channel_valid(type, channel))
 		return -EOPNOTSUPP;
 
-	ret = peci_sensor_get_ctx(attr, peci_cpupower_cfg[channel],
-				  &sensor_conf, priv->sensor_data_list[channel],
-				  &sensor_data,
-				  ARRAY_SIZE(peci_cpupower_cfg[channel]));
+	switch (type) {
+	case hwmon_power:
+		ret = peci_sensor_get_ctx(attr, peci_cpupower_power_cfg[channel],
+					  &sensor_conf,
+					  priv->power_sensor_data_list[channel],
+					  &sensor_data,
+					  ARRAY_SIZE(peci_cpupower_power_cfg[channel]));
+		break;
+	case hwmon_energy:
+		ret = peci_sensor_get_ctx(attr, peci_cpupower_energy_cfg[channel],
+					  &sensor_conf,
+					  priv->energy_sensor_data_list[channel],
+					  &sensor_data,
+					  ARRAY_SIZE(peci_cpupower_energy_cfg[channel]));
+		break;
+	default:
+		ret = -EOPNOTSUPP;
+	}
+
 	if (ret)
 		return ret;
 
@@ -457,15 +615,27 @@ peci_cpupower_is_visible(const void *data, enum hwmon_sensor_types type,
 	umode_t mode = 0;
 	int ret;
 
-	if (channel >= PECI_CPUPOWER_CHANNEL_COUNT)
+	if (!peci_cpupower_is_channel_valid(type, channel))
 		return mode;
 
-	if (attr == hwmon_power_label)
+	if (attr == hwmon_power_label || attr == hwmon_energy_label)
 		return 0444;
 
-	ret = peci_sensor_get_ctx(attr, peci_cpupower_cfg[channel],
-				  &sensor_conf, NULL, NULL,
-				  ARRAY_SIZE(peci_cpupower_cfg[channel]));
+	switch (type) {
+	case hwmon_power:
+		ret = peci_sensor_get_ctx(attr, peci_cpupower_power_cfg[channel],
+					  &sensor_conf, NULL, NULL,
+					  ARRAY_SIZE(peci_cpupower_power_cfg[channel]));
+		break;
+	case hwmon_energy:
+		ret = peci_sensor_get_ctx(attr, peci_cpupower_energy_cfg[channel],
+					  &sensor_conf, NULL, NULL,
+					  ARRAY_SIZE(peci_cpupower_energy_cfg[channel]));
+		break;
+	default:
+		return mode;
+	}
+
 	if (!ret) {
 		if (sensor_conf->read)
 			mode |= 0444;
@@ -506,18 +676,26 @@ static int peci_cpupower_probe(struct platform_device *pdev)
 	snprintf(priv->name, PECI_NAME_SIZE, "peci_cpupower.cpu%d",
 		 mgr->client->addr - PECI_BASE_ADDR);
 
-	priv->power_config[priv->config_idx] = HWMON_P_LABEL |
-		peci_sensor_get_config(peci_cpupower_cfg[priv->config_idx],
-				       ARRAY_SIZE(peci_cpupower_cfg
-						  [priv->config_idx]));
-	priv->config_idx++;
+	priv->power_config[0] = HWMON_P_LABEL |
+		peci_sensor_get_config(peci_cpupower_power_cfg[0],
+				       ARRAY_SIZE(peci_cpupower_power_cfg[0]));
 
-	priv->chip.ops = &peci_cpupower_ops;
-	priv->chip.info = priv->info;
-	priv->info[0] = &priv->power_info;
+	priv->energy_config[0] = HWMON_E_LABEL |
+		peci_sensor_get_config(peci_cpupower_energy_cfg[0],
+				       ARRAY_SIZE(peci_cpupower_energy_cfg[0]));
 
+	priv->info[priv->config_idx] = &priv->power_info;
 	priv->power_info.type = hwmon_power;
 	priv->power_info.config = priv->power_config;
+	priv->config_idx++;
+
+	priv->info[priv->config_idx] = &priv->energy_info;
+	priv->energy_info.type = hwmon_energy;
+	priv->energy_info.config = priv->energy_config;
+	priv->config_idx++;
+
+	priv->chip.ops = &peci_cpupower_ops;
+	priv->chip.info = priv->info;
 
 	hwmon_dev = devm_hwmon_device_register_with_info(priv->dev, priv->name,
 							 priv, &priv->chip,
diff --git a/drivers/hwmon/peci-dimmpower.c b/drivers/hwmon/peci-dimmpower.c
index 69205d8714bc..c0adeb7e8f65 100644
--- a/drivers/hwmon/peci-dimmpower.c
+++ b/drivers/hwmon/peci-dimmpower.c
@@ -27,7 +27,12 @@ struct peci_dimmpower {
 		sensor_data_list[PECI_DIMMPOWER_CHANNEL_COUNT]
 				[PECI_DIMMPOWER_SENSOR_COUNT];
 
-	s32 avg_power_val;
+	/*
+	 * Not exposed to any sensor directly - just used to store previous raw
+	 * energy counter value that is required to calculate average power
+	 */
+	struct peci_sensor_data power_sensor_prev_energy;
+
 	union peci_pkg_power_sku_unit units;
 	bool units_valid;
 
@@ -56,19 +61,16 @@ peci_dimmpower_read_dram_power_limit(struct peci_client_manager *peci_mgr,
 
 static int
 peci_dimmpower_get_avg_power(void *ctx, struct peci_sensor_conf *sensor_conf,
-			     struct peci_sensor_data *sensor_data,
-			     s32 *val)
+			     struct peci_sensor_data *sensor_data)
 {
 	struct peci_dimmpower *priv = (struct peci_dimmpower *)ctx;
-	u32 energy_cnt;
-	ulong jif;
+	struct peci_sensor_data energy;
 	int ret;
 
 	if (!peci_sensor_need_update_with_time(sensor_data,
 					       sensor_conf->update_interval)) {
-		*val = priv->avg_power_val;
 		dev_dbg(priv->dev, "skip reading peci, average power %dmW\n",
-			*val);
+			sensor_data->value);
 		return 0;
 	}
 
@@ -78,30 +80,30 @@ peci_dimmpower_get_avg_power(void *ctx, struct peci_sensor_conf *sensor_conf,
 		return ret;
 	}
 
-	jif = jiffies;
 	ret = peci_pcs_read(priv->mgr, PECI_MBX_INDEX_ENERGY_STATUS,
-			    PECI_PKG_ID_DIMM, &energy_cnt);
+			    PECI_PKG_ID_DIMM, &energy.value);
 	if (ret) {
 		dev_dbg(priv->dev, "not able to read energy\n");
 		return ret;
 	}
 
-	ret = peci_pcs_calc_pwr_from_eng(priv->dev, sensor_data, energy_cnt,
-					 priv->units.bits.eng_unit, val);
+	energy.last_updated = jiffies;
 
-	priv->avg_power_val = *val;
-	peci_sensor_mark_updated_with_time(sensor_data, jif);
+	ret = peci_pcs_calc_pwr_from_eng(priv->dev,
+					 &priv->power_sensor_prev_energy,
+					 &energy, priv->units.bits.eng_unit,
+					 &sensor_data->value);
+	peci_sensor_mark_updated_with_time(sensor_data, energy.last_updated);
 
 	dev_dbg(priv->dev, "average power %dmW, jif %lu, HZ is %d jiffies\n",
-		*val, jif, HZ);
+		sensor_data->value, energy.last_updated, HZ);
 
 	return ret;
 }
 
 static int
 peci_dimmpower_get_power_limit(void *ctx, struct peci_sensor_conf *sensor_conf,
-			       struct peci_sensor_data *sensor_data,
-			       s32 *val)
+			       struct peci_sensor_data *sensor_data)
 {
 	struct peci_dimmpower *priv = (struct peci_dimmpower *)ctx;
 	union peci_dram_power_limit power_limit;
@@ -110,9 +112,8 @@ peci_dimmpower_get_power_limit(void *ctx, struct peci_sensor_conf *sensor_conf,
 
 	if (!peci_sensor_need_update_with_time(sensor_data,
 					       sensor_conf->update_interval)) {
-		*val = sensor_data->value;
 		dev_dbg(priv->dev, "skip reading peci, power limit %dmW\n",
-			*val);
+			sensor_data->value);
 		return 0;
 	}
 
@@ -129,14 +130,13 @@ peci_dimmpower_get_power_limit(void *ctx, struct peci_sensor_conf *sensor_conf,
 		return ret;
 	}
 
-	*val = peci_pcs_xn_to_munits(power_limit.bits.pp_pwr_lim,
-				     priv->units.bits.pwr_unit);
-
-	sensor_data->value = *val;
+	sensor_data->value = peci_pcs_xn_to_munits(power_limit.bits.pp_pwr_lim,
+						   priv->units.bits.pwr_unit);
 	peci_sensor_mark_updated_with_time(sensor_data, jif);
 
 	dev_dbg(priv->dev, "raw power limit %u, unit %u, power limit %d\n",
-		power_limit.bits.pp_pwr_lim, priv->units.bits.pwr_unit, *val);
+		power_limit.bits.pp_pwr_lim, priv->units.bits.pwr_unit,
+		sensor_data->value);
 
 	return ret;
 }
@@ -197,8 +197,7 @@ peci_dimmpower_set_power_limit(void *ctx, struct peci_sensor_conf *sensor_conf,
 
 static int
 peci_dimmpower_read_max_power(void *ctx, struct peci_sensor_conf *sensor_conf,
-			      struct peci_sensor_data *sensor_data,
-			      s32 *val)
+			      struct peci_sensor_data *sensor_data)
 {
 	struct peci_dimmpower *priv = (struct peci_dimmpower *)ctx;
 	union peci_dram_power_info_high power_info;
@@ -207,9 +206,8 @@ peci_dimmpower_read_max_power(void *ctx, struct peci_sensor_conf *sensor_conf,
 
 	if (!peci_sensor_need_update_with_time(sensor_data,
 					       sensor_conf->update_interval)) {
-		*val = sensor_data->value;
 		dev_dbg(priv->dev, "skip reading peci, max power %dmW\n",
-			*val);
+			sensor_data->value);
 		return 0;
 	}
 
@@ -227,22 +225,20 @@ peci_dimmpower_read_max_power(void *ctx, struct peci_sensor_conf *sensor_conf,
 		return ret;
 	}
 
-	*val = peci_pcs_xn_to_munits(power_info.bits.max_pwr,
-				     priv->units.bits.pwr_unit);
-
-	sensor_data->value = *val;
+	sensor_data->value = peci_pcs_xn_to_munits(power_info.bits.max_pwr,
+						   priv->units.bits.pwr_unit);
 	peci_sensor_mark_updated_with_time(sensor_data, jif);
 
 	dev_dbg(priv->dev, "raw max power %u, unit %u, max power %dmW\n",
-		power_info.bits.max_pwr, priv->units.bits.pwr_unit, *val);
+		power_info.bits.max_pwr, priv->units.bits.pwr_unit,
+		sensor_data->value);
 
 	return ret;
 }
 
 static int
 peci_dimmpower_read_min_power(void *ctx, struct peci_sensor_conf *sensor_conf,
-			      struct peci_sensor_data *sensor_data,
-			      s32 *val)
+			      struct peci_sensor_data *sensor_data)
 {
 	struct peci_dimmpower *priv = (struct peci_dimmpower *)ctx;
 	union peci_dram_power_info_low power_info;
@@ -251,9 +247,8 @@ peci_dimmpower_read_min_power(void *ctx, struct peci_sensor_conf *sensor_conf,
 
 	if (!peci_sensor_need_update_with_time(sensor_data,
 					       sensor_conf->update_interval)) {
-		*val = sensor_data->value;
 		dev_dbg(priv->dev, "skip reading peci, min power %dmW\n",
-			*val);
+			sensor_data->value);
 		return 0;
 	}
 
@@ -271,14 +266,13 @@ peci_dimmpower_read_min_power(void *ctx, struct peci_sensor_conf *sensor_conf,
 		return ret;
 	}
 
-	*val = peci_pcs_xn_to_munits(power_info.bits.min_pwr,
-				     priv->units.bits.pwr_unit);
-
-	sensor_data->value = *val;
+	sensor_data->value = peci_pcs_xn_to_munits(power_info.bits.min_pwr,
+						   priv->units.bits.pwr_unit);
 	peci_sensor_mark_updated_with_time(sensor_data, jif);
 
 	dev_dbg(priv->dev, "raw min power %u, unit %u, min power %dmW\n",
-		power_info.bits.min_pwr, priv->units.bits.pwr_unit, *val);
+		power_info.bits.min_pwr, priv->units.bits.pwr_unit,
+		sensor_data->value);
 
 	return ret;
 }
@@ -358,11 +352,9 @@ peci_dimmpower_read(struct device *dev, enum hwmon_sensor_types type,
 		return ret;
 
 	if (sensor_conf->read) {
-		s32 tmp;
-
-		ret = sensor_conf->read(priv, sensor_conf, sensor_data, &tmp);
+		ret = sensor_conf->read(priv, sensor_conf, sensor_data);
 		if (!ret)
-			*val = (long)tmp;
+			*val = (long)sensor_data->value;
 	} else {
 		ret = -EOPNOTSUPP;
 	}
diff --git a/drivers/hwmon/peci-hwmon.h b/drivers/hwmon/peci-hwmon.h
index 8b0dd11a42fe..010d000db31e 100644
--- a/drivers/hwmon/peci-hwmon.h
+++ b/drivers/hwmon/peci-hwmon.h
@@ -106,8 +106,7 @@ struct peci_sensor_conf {
 	const ulong update_interval;
 
 	int (*const read)(void *priv, struct peci_sensor_conf *sensor_conf,
-			  struct peci_sensor_data *sensor_data,
-			  s32 *val);
+			  struct peci_sensor_data *sensor_data);
 	int (*const write)(void *priv, struct peci_sensor_conf *sensor_conf,
 			   struct peci_sensor_data *sensor_data,
 			   s32 val);
@@ -361,6 +360,23 @@ union peci_dram_power_limit {
 static_assert(sizeof(union peci_dram_power_limit) == PECI_PCS_REGISTER_SIZE);
 
 /**
+ * peci_pcs_xn_to_uunits - function converting value in units in x.N format to
+ * micro units (microjoules, microseconds, microdegrees) in regular format
+ * @x_n_value: Value in units in x.n format
+ * @n: n factor for x.n format
+
+ *
+ * Return: value in micro units (microjoules, microseconds, microdegrees)
+ * in regular format
+ */
+static inline u64 peci_pcs_xn_to_uunits(u32 x_n_value, u8 n)
+{
+	u64 mx_n_value = (u64)x_n_value * 1000000uLL;
+
+	return mx_n_value >> n;
+}
+
+/**
  * peci_pcs_xn_to_munits - function converting value in units in x.N format to
  * milli units (millijoules, milliseconds, millidegrees) in regular format
  * @x_n_value: Value in units in x.n format
@@ -372,9 +388,8 @@ static_assert(sizeof(union peci_dram_power_limit) == PECI_PCS_REGISTER_SIZE);
  */
 static inline u64 peci_pcs_xn_to_munits(u32 x_n_value, u8 n)
 {
-	/* Convert value in units in x.n format to milli units in x.n format */
 	u64 mx_n_value = (u64)x_n_value * 1000uLL;
-	/* Convert x.n format to regular format */
+
 	return mx_n_value >> n;
 }
 
@@ -453,10 +468,10 @@ static inline int peci_pcs_write(struct peci_client_manager *peci_mgr, u8 index,
 
 /**
  * peci_pcs_calc_pwr_from_eng - calculate power (in milliwatts) based on
- * energy reading
+ * two energy readings
  * @dev: Device handle
- * @energy: Energy reading context
- * @energy_cnt: Raw energy reading
+ * @prev_energy: Previous energy reading context with raw energy counter value
+ * @energy: Current energy reading context with raw energy counter value
  * @unit: Calculation factor
  * @power_val_in_mW: Pointer to the variable calculation result is going to
  * be put
@@ -466,38 +481,38 @@ static inline int peci_pcs_write(struct peci_client_manager *peci_mgr, u8 index,
  *	-EAGAIN if calculation is skipped.
  */
 static inline int peci_pcs_calc_pwr_from_eng(struct device *dev,
-					     struct peci_sensor_data *sensor,
-					     u32 energy_cnt, u32 unit,
+					     struct peci_sensor_data *prev_energy,
+					     struct peci_sensor_data *energy,
+					     u32 unit,
 					     s32 *power_in_mW)
 {
-	ulong jif = jiffies;
 	ulong elapsed;
 	int ret;
 
-	if (!dev || !sensor || !power_in_mW)
+	if (!dev || !prev_energy || !energy || !power_in_mW)
 		return -EINVAL;
 
-	elapsed = jif - sensor->last_updated;
+	elapsed = energy->last_updated - prev_energy->last_updated;
 
 	dev_dbg(dev, "raw energy before %u, raw energy now %u, unit %u, jiffies elapsed %lu\n",
-		sensor->value, energy_cnt, unit, elapsed);
+		prev_energy->value, energy->value, unit, elapsed);
 
 	/*
 	 * Don't calculate average power for first counter read  last counter
 	 * read was more than 60 minutes ago (jiffies did not wrap and power
 	 * calculation does not overflow or underflow).
 	 */
-	if (sensor->last_updated > 0 && elapsed < (HZ * 3600)) {
+	if (prev_energy->last_updated > 0 && elapsed < (HZ * 3600) && elapsed) {
 		u32 energy_consumed;
 		u64 energy_consumed_in_mJ;
 		u64 energy_by_jiffies;
 
 		/* Take care here about energy counter rollover */
-		if (energy_cnt >= (u32)(sensor->value))
-			energy_consumed = energy_cnt - (u32)(sensor->value);
+		if ((u32)(energy->value) >= (u32)(prev_energy->value))
+			energy_consumed = (u32)(energy->value) - (u32)(prev_energy->value);
 		else
-			energy_consumed = (U32_MAX - (u32)(sensor->value)) +
-					energy_cnt + 1u;
+			energy_consumed = (U32_MAX - (u32)(prev_energy->value)) +
+					(u32)(energy->value) + 1u;
 
 		/* Calculate the energy here */
 		energy_consumed_in_mJ =
@@ -522,8 +537,9 @@ static inline int peci_pcs_calc_pwr_from_eng(struct device *dev,
 		ret = -EAGAIN;
 	}
 
-	/* Update sensor context */
-	sensor->value = energy_cnt;
+	/* Update previous energy sensor context with current value */
+	prev_energy->value = energy->value;
+	peci_sensor_mark_updated_with_time(prev_energy, energy->last_updated);
 
 	return ret;
 }