From: Florian Fainelli <f.fainelli@gmail.com> To: Sudeep Holla <sudeep.holla@arm.com>, Guenter Roeck <linux@roeck-us.net>, linux-hwmon@vger.kernel.org, "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" <linux-arm-kernel@lists.infradead.org> Subject: SCMI sensor reads unit scaling Date: Sat, 20 Apr 2019 09:43:48 -0700 [thread overview] Message-ID: <9207bc01-e0c6-ef73-6a24-6deda7b789c5@gmail.com> (raw) Hi Sudeep, Guenter, The current SCMI hwmon support does not seem to make use of the sensor scale/unit as defined in the sensor replies (or the update scale for that matter). I came up with the patch below which gets the job done, but I am worried about possibly breaking people's SCMI deployments and sensors reading because they may have intentionally or not already decided to return a value which is scaled the way Linux's hwmon expect it, and may, or may not have populated a valid unit number in the sensor reply. Ideally we should probably do two conversions: - from within scmi_sensor_reading_get(), scale the value as indicated by the reply - from within scmi_hwmon_read(), take that scaled value and apply the necessary conversion expected by Linux's HWMON conventions (e.g.: reporting voltage as mV values) What do you think? --- From: Florian Fainelli <f.fainelli@gmail.com> Subject: firmware: arm_scmi: Support sensor scaling The SCMI sensor management protocol includes the following provision: The power-of-10 multiplier in two’s-complement format that is applied to the sensor unit specified by the SensorType field. Add support for scaling the value returned based on what is provided by the firmware. This requires us to be able to look up a sensor identifier to its backing scmi_sensor_info structure and apply the necessary scale. Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> --- drivers/firmware/arm_scmi/sensors.c | 37 ++++++++++++++++++++++++++++++++++++- include/linux/scmi_protocol.h | 1 + 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c index bbb469f..2eccc6a 100644 --- a/drivers/firmware/arm_scmi/sensors.c +++ b/drivers/firmware/arm_scmi/sensors.c @@ -33,7 +33,8 @@ struct scmi_msg_resp_sensor_description { #define NUM_TRIP_POINTS(x) (((x) >> 4) & 0xff) __le32 attributes_high; #define SENSOR_TYPE(x) ((x) & 0xff) -#define SENSOR_SCALE(x) (((x) >> 11) & 0x3f) +#define SENSOR_SCALE_MASK 0x3f +#define SENSOR_SCALE(x) (((x) >> 11) & SENSOR_SCALE_MASK) #define SENSOR_UPDATE_SCALE(x) (((x) >> 22) & 0x1f) #define SENSOR_UPDATE_BASE(x) (((x) >> 27) & 0x1f) u8 name[SCMI_MAX_STR_SIZE]; @@ -72,6 +73,35 @@ struct sensors_info { struct scmi_sensor_info *sensors; }; +static +struct scmi_sensor_info *scmi_sensor_info_from_id(struct sensors_info *si, + u32 sensor_id) +{ + struct scmi_sensor_info *s = NULL; + int i; + + for (i = 0; i < si->num_sensors; i++) { + s = &si->sensors[i]; + if (s->id == sensor_id) + return s; + } + + return s; +} + +static void inline scmi_scale_value(u8 scale, u64 *value) +{ + unsigned int i; + + if (scale & ((SENSOR_SCALE_MASK + 1) >> 1)) { + for (i = 0; i < (~scale) + 1; i++) + *value /= 10; + } else { + for (i = 0; i < scale; i++) + *value *= 10; + } +} + static int scmi_sensor_attributes_get(const struct scmi_handle *handle, struct sensors_info *si) { @@ -140,6 +170,7 @@ static int scmi_sensor_description_get(const struct scmi_handle *handle, s = &si->sensors[desc_index + cnt]; s->id = le32_to_cpu(buf->desc[cnt].id); s->type = SENSOR_TYPE(attrh); + s->scale = SENSOR_SCALE(attrh); memcpy(s->name, buf->desc[cnt].name, SCMI_MAX_STR_SIZE); } @@ -205,6 +236,8 @@ static int scmi_sensor_trip_point_set(const struct scmi_handle *handle, static int scmi_sensor_reading_get(const struct scmi_handle *handle, u32 sensor_id, bool async, u64 *value) { + struct sensors_info *si = handle->sensor_priv; + struct scmi_sensor_info *s = scmi_sensor_info_from_id(si, sensor_id); int ret; struct scmi_xfer *t; struct scmi_msg_sensor_reading_get *sensor; @@ -225,6 +258,8 @@ static int scmi_sensor_reading_get(const struct scmi_handle *handle, *value = le32_to_cpu(*pval); *value |= (u64)le32_to_cpu(*(pval + 1)) << 32; + if (s && s->scale) + scmi_scale_value(s->scale, value); } scmi_one_xfer_put(handle, t); diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h index b458c87..782b53e 100644 --- a/include/linux/scmi_protocol.h +++ b/include/linux/scmi_protocol.h @@ -140,6 +140,7 @@ struct scmi_power_ops { struct scmi_sensor_info { u32 id; u8 type; + u8 scale; char name[SCMI_MAX_STR_SIZE]; }; -- -- Florian
WARNING: multiple messages have this Message-ID (diff)
From: Florian Fainelli <f.fainelli@gmail.com> To: Sudeep Holla <sudeep.holla@arm.com>, Guenter Roeck <linux@roeck-us.net>, linux-hwmon@vger.kernel.org, "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" <linux-arm-kernel@lists.infradead.org> Subject: SCMI sensor reads unit scaling Date: Sat, 20 Apr 2019 09:43:48 -0700 [thread overview] Message-ID: <9207bc01-e0c6-ef73-6a24-6deda7b789c5@gmail.com> (raw) Hi Sudeep, Guenter, The current SCMI hwmon support does not seem to make use of the sensor scale/unit as defined in the sensor replies (or the update scale for that matter). I came up with the patch below which gets the job done, but I am worried about possibly breaking people's SCMI deployments and sensors reading because they may have intentionally or not already decided to return a value which is scaled the way Linux's hwmon expect it, and may, or may not have populated a valid unit number in the sensor reply. Ideally we should probably do two conversions: - from within scmi_sensor_reading_get(), scale the value as indicated by the reply - from within scmi_hwmon_read(), take that scaled value and apply the necessary conversion expected by Linux's HWMON conventions (e.g.: reporting voltage as mV values) What do you think? --- From: Florian Fainelli <f.fainelli@gmail.com> Subject: firmware: arm_scmi: Support sensor scaling The SCMI sensor management protocol includes the following provision: The power-of-10 multiplier in two’s-complement format that is applied to the sensor unit specified by the SensorType field. Add support for scaling the value returned based on what is provided by the firmware. This requires us to be able to look up a sensor identifier to its backing scmi_sensor_info structure and apply the necessary scale. Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> --- drivers/firmware/arm_scmi/sensors.c | 37 ++++++++++++++++++++++++++++++++++++- include/linux/scmi_protocol.h | 1 + 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c index bbb469f..2eccc6a 100644 --- a/drivers/firmware/arm_scmi/sensors.c +++ b/drivers/firmware/arm_scmi/sensors.c @@ -33,7 +33,8 @@ struct scmi_msg_resp_sensor_description { #define NUM_TRIP_POINTS(x) (((x) >> 4) & 0xff) __le32 attributes_high; #define SENSOR_TYPE(x) ((x) & 0xff) -#define SENSOR_SCALE(x) (((x) >> 11) & 0x3f) +#define SENSOR_SCALE_MASK 0x3f +#define SENSOR_SCALE(x) (((x) >> 11) & SENSOR_SCALE_MASK) #define SENSOR_UPDATE_SCALE(x) (((x) >> 22) & 0x1f) #define SENSOR_UPDATE_BASE(x) (((x) >> 27) & 0x1f) u8 name[SCMI_MAX_STR_SIZE]; @@ -72,6 +73,35 @@ struct sensors_info { struct scmi_sensor_info *sensors; }; +static +struct scmi_sensor_info *scmi_sensor_info_from_id(struct sensors_info *si, + u32 sensor_id) +{ + struct scmi_sensor_info *s = NULL; + int i; + + for (i = 0; i < si->num_sensors; i++) { + s = &si->sensors[i]; + if (s->id == sensor_id) + return s; + } + + return s; +} + +static void inline scmi_scale_value(u8 scale, u64 *value) +{ + unsigned int i; + + if (scale & ((SENSOR_SCALE_MASK + 1) >> 1)) { + for (i = 0; i < (~scale) + 1; i++) + *value /= 10; + } else { + for (i = 0; i < scale; i++) + *value *= 10; + } +} + static int scmi_sensor_attributes_get(const struct scmi_handle *handle, struct sensors_info *si) { @@ -140,6 +170,7 @@ static int scmi_sensor_description_get(const struct scmi_handle *handle, s = &si->sensors[desc_index + cnt]; s->id = le32_to_cpu(buf->desc[cnt].id); s->type = SENSOR_TYPE(attrh); + s->scale = SENSOR_SCALE(attrh); memcpy(s->name, buf->desc[cnt].name, SCMI_MAX_STR_SIZE); } @@ -205,6 +236,8 @@ static int scmi_sensor_trip_point_set(const struct scmi_handle *handle, static int scmi_sensor_reading_get(const struct scmi_handle *handle, u32 sensor_id, bool async, u64 *value) { + struct sensors_info *si = handle->sensor_priv; + struct scmi_sensor_info *s = scmi_sensor_info_from_id(si, sensor_id); int ret; struct scmi_xfer *t; struct scmi_msg_sensor_reading_get *sensor; @@ -225,6 +258,8 @@ static int scmi_sensor_reading_get(const struct scmi_handle *handle, *value = le32_to_cpu(*pval); *value |= (u64)le32_to_cpu(*(pval + 1)) << 32; + if (s && s->scale) + scmi_scale_value(s->scale, value); } scmi_one_xfer_put(handle, t); diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h index b458c87..782b53e 100644 --- a/include/linux/scmi_protocol.h +++ b/include/linux/scmi_protocol.h @@ -140,6 +140,7 @@ struct scmi_power_ops { struct scmi_sensor_info { u32 id; u8 type; + u8 scale; char name[SCMI_MAX_STR_SIZE]; }; -- -- Florian _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next reply other threads:[~2019-04-20 16:43 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-04-20 16:43 Florian Fainelli [this message] 2019-04-20 16:43 ` SCMI sensor reads unit scaling Florian Fainelli 2019-04-20 17:31 ` Guenter Roeck 2019-04-20 17:31 ` Guenter Roeck 2019-04-22 18:31 ` Florian Fainelli 2019-04-22 18:31 ` Florian Fainelli 2019-04-22 19:29 ` Guenter Roeck 2019-04-22 19:29 ` Guenter Roeck 2019-04-23 13:06 ` Sudeep Holla 2019-04-23 13:06 ` Sudeep Holla
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=9207bc01-e0c6-ef73-6a24-6deda7b789c5@gmail.com \ --to=f.fainelli@gmail.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-hwmon@vger.kernel.org \ --cc=linux@roeck-us.net \ --cc=sudeep.holla@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.