* [PATCH v4 0/2] hwmon: scmi: Scale values to target desired HWMON units
@ 2019-05-08 17:00 ` Florian Fainelli
0 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2019-05-08 17:00 UTC (permalink / raw)
To: linux-kernel
Cc: bcm-kernel-feedback-list, Florian Fainelli, Sudeep Holla,
Jean Delvare, Guenter Roeck, linux-arm-kernel,
open list:HARDWARE MONITORING
Hi Sudeep, Guenter,
This patch series adds support for scaling SCMI sensor values read from
firmware. Sudeep, let me know if you think we should be treating scale
== 0 as a special value to preserve some firmware compatibility (not
that this would be desired).
Changes in v4:
- deal with overflow in the caller of __pow10() as suggested by Guenter
which makes us rework a bit how the value argument/return value are
passed
- don't harcode the latest power of 10 factor to be 18, just rely on
overflowing the u64 value instead
Changes in v3:
- add a local __pow10 function to scmi-hwmon.c while a plan to provide
a generic function is figured out.
- add check on power > 18 which would overflow a 64-bit unsigned integer
- use div64_u64() to properly divide a 64-bit quantity with an unsigned
64-bit quantity
Changes in v2:
- added a helper function in kernel.h: __pow10()
- made the scale in scmi_sensor_info an s8 type, added defines for
checking the sign bit and sign extending with a mask
- simplify computations in hwmon driver
Florian Fainelli (2):
firmware: arm_scmi: Fetch and store sensor scale
hwmon: scmi: Scale values to target desired HWMON units
drivers/firmware/arm_scmi/sensors.c | 6 ++++
drivers/hwmon/scmi-hwmon.c | 46 +++++++++++++++++++++++++++++
include/linux/scmi_protocol.h | 1 +
3 files changed, 53 insertions(+)
--
2.17.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 0/2] hwmon: scmi: Scale values to target desired HWMON units
@ 2019-05-08 17:00 ` Florian Fainelli
0 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2019-05-08 17:00 UTC (permalink / raw)
To: linux-kernel
Cc: open list:HARDWARE MONITORING, Jean Delvare, Florian Fainelli,
bcm-kernel-feedback-list, Guenter Roeck, Sudeep Holla,
linux-arm-kernel
Hi Sudeep, Guenter,
This patch series adds support for scaling SCMI sensor values read from
firmware. Sudeep, let me know if you think we should be treating scale
== 0 as a special value to preserve some firmware compatibility (not
that this would be desired).
Changes in v4:
- deal with overflow in the caller of __pow10() as suggested by Guenter
which makes us rework a bit how the value argument/return value are
passed
- don't harcode the latest power of 10 factor to be 18, just rely on
overflowing the u64 value instead
Changes in v3:
- add a local __pow10 function to scmi-hwmon.c while a plan to provide
a generic function is figured out.
- add check on power > 18 which would overflow a 64-bit unsigned integer
- use div64_u64() to properly divide a 64-bit quantity with an unsigned
64-bit quantity
Changes in v2:
- added a helper function in kernel.h: __pow10()
- made the scale in scmi_sensor_info an s8 type, added defines for
checking the sign bit and sign extending with a mask
- simplify computations in hwmon driver
Florian Fainelli (2):
firmware: arm_scmi: Fetch and store sensor scale
hwmon: scmi: Scale values to target desired HWMON units
drivers/firmware/arm_scmi/sensors.c | 6 ++++
drivers/hwmon/scmi-hwmon.c | 46 +++++++++++++++++++++++++++++
include/linux/scmi_protocol.h | 1 +
3 files changed, 53 insertions(+)
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 1/2] firmware: arm_scmi: Fetch and store sensor scale
2019-05-08 17:00 ` Florian Fainelli
@ 2019-05-08 17:00 ` Florian Fainelli
-1 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2019-05-08 17:00 UTC (permalink / raw)
To: linux-kernel
Cc: bcm-kernel-feedback-list, Florian Fainelli, Sudeep Holla,
Jean Delvare, Guenter Roeck, linux-arm-kernel,
open list:HARDWARE MONITORING
In preparation for dealing with scales within the SCMI HWMON driver,
fetch and store the sensor unit scale into the scmi_sensor_info
structure. In order to simplify computations for upper layer, take care
of sign extending the scale to a full 8-bit signed value.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/firmware/arm_scmi/sensors.c | 6 ++++++
include/linux/scmi_protocol.h | 1 +
2 files changed, 7 insertions(+)
diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c
index b53d5cc9c9f6..21353470a740 100644
--- a/drivers/firmware/arm_scmi/sensors.c
+++ b/drivers/firmware/arm_scmi/sensors.c
@@ -34,6 +34,8 @@ struct scmi_msg_resp_sensor_description {
__le32 attributes_high;
#define SENSOR_TYPE(x) ((x) & 0xff)
#define SENSOR_SCALE(x) (((x) >> 11) & 0x3f)
+#define SENSOR_SCALE_SIGN BIT(5)
+#define SENSOR_SCALE_EXTEND GENMASK(7, 6)
#define SENSOR_UPDATE_SCALE(x) (((x) >> 22) & 0x1f)
#define SENSOR_UPDATE_BASE(x) (((x) >> 27) & 0x1f)
u8 name[SCMI_MAX_STR_SIZE];
@@ -140,6 +142,10 @@ 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);
+ /* Sign extend to a full s8 */
+ if (s->scale & SENSOR_SCALE_SIGN)
+ s->scale |= SENSOR_SCALE_EXTEND;
strlcpy(s->name, buf->desc[cnt].name, SCMI_MAX_STR_SIZE);
}
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index 3105055c00a7..9ff2e9357e9a 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -144,6 +144,7 @@ struct scmi_power_ops {
struct scmi_sensor_info {
u32 id;
u8 type;
+ s8 scale;
char name[SCMI_MAX_STR_SIZE];
};
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 1/2] firmware: arm_scmi: Fetch and store sensor scale
@ 2019-05-08 17:00 ` Florian Fainelli
0 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2019-05-08 17:00 UTC (permalink / raw)
To: linux-kernel
Cc: open list:HARDWARE MONITORING, Jean Delvare, Florian Fainelli,
bcm-kernel-feedback-list, Guenter Roeck, Sudeep Holla,
linux-arm-kernel
In preparation for dealing with scales within the SCMI HWMON driver,
fetch and store the sensor unit scale into the scmi_sensor_info
structure. In order to simplify computations for upper layer, take care
of sign extending the scale to a full 8-bit signed value.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/firmware/arm_scmi/sensors.c | 6 ++++++
include/linux/scmi_protocol.h | 1 +
2 files changed, 7 insertions(+)
diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c
index b53d5cc9c9f6..21353470a740 100644
--- a/drivers/firmware/arm_scmi/sensors.c
+++ b/drivers/firmware/arm_scmi/sensors.c
@@ -34,6 +34,8 @@ struct scmi_msg_resp_sensor_description {
__le32 attributes_high;
#define SENSOR_TYPE(x) ((x) & 0xff)
#define SENSOR_SCALE(x) (((x) >> 11) & 0x3f)
+#define SENSOR_SCALE_SIGN BIT(5)
+#define SENSOR_SCALE_EXTEND GENMASK(7, 6)
#define SENSOR_UPDATE_SCALE(x) (((x) >> 22) & 0x1f)
#define SENSOR_UPDATE_BASE(x) (((x) >> 27) & 0x1f)
u8 name[SCMI_MAX_STR_SIZE];
@@ -140,6 +142,10 @@ 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);
+ /* Sign extend to a full s8 */
+ if (s->scale & SENSOR_SCALE_SIGN)
+ s->scale |= SENSOR_SCALE_EXTEND;
strlcpy(s->name, buf->desc[cnt].name, SCMI_MAX_STR_SIZE);
}
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index 3105055c00a7..9ff2e9357e9a 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -144,6 +144,7 @@ struct scmi_power_ops {
struct scmi_sensor_info {
u32 id;
u8 type;
+ s8 scale;
char name[SCMI_MAX_STR_SIZE];
};
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 2/2] hwmon: scmi: Scale values to target desired HWMON units
2019-05-08 17:00 ` Florian Fainelli
@ 2019-05-08 17:00 ` Florian Fainelli
-1 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2019-05-08 17:00 UTC (permalink / raw)
To: linux-kernel
Cc: bcm-kernel-feedback-list, Florian Fainelli, Sudeep Holla,
Jean Delvare, Guenter Roeck, linux-arm-kernel,
open list:HARDWARE MONITORING
If the SCMI firmware implementation is reporting values in a scale that
is different from the HWMON units, we need to scale up or down the value
according to how far appart they are.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/hwmon/scmi-hwmon.c | 46 ++++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/drivers/hwmon/scmi-hwmon.c b/drivers/hwmon/scmi-hwmon.c
index a80183a488c5..4399372e2131 100644
--- a/drivers/hwmon/scmi-hwmon.c
+++ b/drivers/hwmon/scmi-hwmon.c
@@ -7,6 +7,7 @@
*/
#include <linux/hwmon.h>
+#include <linux/limits.h>
#include <linux/module.h>
#include <linux/scmi_protocol.h>
#include <linux/slab.h>
@@ -18,6 +19,47 @@ struct scmi_sensors {
const struct scmi_sensor_info **info[hwmon_max];
};
+static inline u64 __pow10(u8 x)
+{
+ u64 r = 1;
+
+ while (x--)
+ r *= 10;
+
+ return r;
+}
+
+static int scmi_hwmon_scale(const struct scmi_sensor_info *sensor, u64 *value)
+{
+ s8 scale = sensor->scale;
+ u64 f;
+
+ switch (sensor->type) {
+ case TEMPERATURE_C:
+ case VOLTAGE:
+ case CURRENT:
+ scale += 3;
+ break;
+ case POWER:
+ case ENERGY:
+ scale += 6;
+ break;
+ default:
+ break;
+ }
+
+ f = __pow10(abs(scale));
+ if (f == U64_MAX)
+ return -E2BIG;
+
+ if (scale > 0)
+ *value *= f;
+ else
+ *value = div64_u64(*value, f);
+
+ return 0;
+}
+
static int scmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
u32 attr, int channel, long *val)
{
@@ -29,6 +71,10 @@ static int scmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
sensor = *(scmi_sensors->info[type] + channel);
ret = h->sensor_ops->reading_get(h, sensor->id, false, &value);
+ if (ret)
+ return ret;
+
+ ret = scmi_hwmon_scale(sensor, value);
if (!ret)
*val = value;
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 2/2] hwmon: scmi: Scale values to target desired HWMON units
@ 2019-05-08 17:00 ` Florian Fainelli
0 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2019-05-08 17:00 UTC (permalink / raw)
To: linux-kernel
Cc: open list:HARDWARE MONITORING, Jean Delvare, Florian Fainelli,
bcm-kernel-feedback-list, Guenter Roeck, Sudeep Holla,
linux-arm-kernel
If the SCMI firmware implementation is reporting values in a scale that
is different from the HWMON units, we need to scale up or down the value
according to how far appart they are.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/hwmon/scmi-hwmon.c | 46 ++++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/drivers/hwmon/scmi-hwmon.c b/drivers/hwmon/scmi-hwmon.c
index a80183a488c5..4399372e2131 100644
--- a/drivers/hwmon/scmi-hwmon.c
+++ b/drivers/hwmon/scmi-hwmon.c
@@ -7,6 +7,7 @@
*/
#include <linux/hwmon.h>
+#include <linux/limits.h>
#include <linux/module.h>
#include <linux/scmi_protocol.h>
#include <linux/slab.h>
@@ -18,6 +19,47 @@ struct scmi_sensors {
const struct scmi_sensor_info **info[hwmon_max];
};
+static inline u64 __pow10(u8 x)
+{
+ u64 r = 1;
+
+ while (x--)
+ r *= 10;
+
+ return r;
+}
+
+static int scmi_hwmon_scale(const struct scmi_sensor_info *sensor, u64 *value)
+{
+ s8 scale = sensor->scale;
+ u64 f;
+
+ switch (sensor->type) {
+ case TEMPERATURE_C:
+ case VOLTAGE:
+ case CURRENT:
+ scale += 3;
+ break;
+ case POWER:
+ case ENERGY:
+ scale += 6;
+ break;
+ default:
+ break;
+ }
+
+ f = __pow10(abs(scale));
+ if (f == U64_MAX)
+ return -E2BIG;
+
+ if (scale > 0)
+ *value *= f;
+ else
+ *value = div64_u64(*value, f);
+
+ return 0;
+}
+
static int scmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
u32 attr, int channel, long *val)
{
@@ -29,6 +71,10 @@ static int scmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
sensor = *(scmi_sensors->info[type] + channel);
ret = h->sensor_ops->reading_get(h, sensor->id, false, &value);
+ if (ret)
+ return ret;
+
+ ret = scmi_hwmon_scale(sensor, value);
if (!ret)
*val = value;
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v4 2/2] hwmon: scmi: Scale values to target desired HWMON units
2019-05-08 17:00 ` Florian Fainelli
@ 2019-05-08 18:32 ` Guenter Roeck
-1 siblings, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2019-05-08 18:32 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-kernel, bcm-kernel-feedback-list, Sudeep Holla,
Jean Delvare, linux-arm-kernel, open list:HARDWARE MONITORING
Hi Florian,
On Wed, May 08, 2019 at 10:00:35AM -0700, Florian Fainelli wrote:
> If the SCMI firmware implementation is reporting values in a scale that
> is different from the HWMON units, we need to scale up or down the value
> according to how far appart they are.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/hwmon/scmi-hwmon.c | 46 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 46 insertions(+)
>
> diff --git a/drivers/hwmon/scmi-hwmon.c b/drivers/hwmon/scmi-hwmon.c
> index a80183a488c5..4399372e2131 100644
> --- a/drivers/hwmon/scmi-hwmon.c
> +++ b/drivers/hwmon/scmi-hwmon.c
> @@ -7,6 +7,7 @@
> */
>
> #include <linux/hwmon.h>
> +#include <linux/limits.h>
> #include <linux/module.h>
> #include <linux/scmi_protocol.h>
> #include <linux/slab.h>
> @@ -18,6 +19,47 @@ struct scmi_sensors {
> const struct scmi_sensor_info **info[hwmon_max];
> };
>
> +static inline u64 __pow10(u8 x)
> +{
> + u64 r = 1;
> +
> + while (x--)
> + r *= 10;
> +
> + return r;
> +}
> +
> +static int scmi_hwmon_scale(const struct scmi_sensor_info *sensor, u64 *value)
> +{
> + s8 scale = sensor->scale;
> + u64 f;
> +
> + switch (sensor->type) {
> + case TEMPERATURE_C:
> + case VOLTAGE:
> + case CURRENT:
> + scale += 3;
> + break;
> + case POWER:
> + case ENERGY:
> + scale += 6;
> + break;
> + default:
> + break;
> + }
> +
> + f = __pow10(abs(scale));
> + if (f == U64_MAX)
> + return -E2BIG;
Unfortunately that is not how integer overflows work.
A test program with increasing values of scale reports:
0: 1
...
18: 1000000000000000000
19: 10000000000000000000
20: 7766279631452241920
21: 3875820019684212736
22: 1864712049423024128
23: 200376420520689664
24: 2003764205206896640
...
61: 11529215046068469760
62: 4611686018427387904
63: 9223372036854775808
64: 0
...
You'll have to check for abs(scale) > 19 if you want to report overflows.
Guenter
> +
> + if (scale > 0)
> + *value *= f;
> + else
> + *value = div64_u64(*value, f);
> +
> + return 0;
> +}
> +
> static int scmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> u32 attr, int channel, long *val)
> {
> @@ -29,6 +71,10 @@ static int scmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>
> sensor = *(scmi_sensors->info[type] + channel);
> ret = h->sensor_ops->reading_get(h, sensor->id, false, &value);
> + if (ret)
> + return ret;
> +
> + ret = scmi_hwmon_scale(sensor, value);
> if (!ret)
> *val = value;
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 2/2] hwmon: scmi: Scale values to target desired HWMON units
@ 2019-05-08 18:32 ` Guenter Roeck
0 siblings, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2019-05-08 18:32 UTC (permalink / raw)
To: Florian Fainelli
Cc: open list:HARDWARE MONITORING, Jean Delvare, linux-kernel,
bcm-kernel-feedback-list, Sudeep Holla, linux-arm-kernel
Hi Florian,
On Wed, May 08, 2019 at 10:00:35AM -0700, Florian Fainelli wrote:
> If the SCMI firmware implementation is reporting values in a scale that
> is different from the HWMON units, we need to scale up or down the value
> according to how far appart they are.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/hwmon/scmi-hwmon.c | 46 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 46 insertions(+)
>
> diff --git a/drivers/hwmon/scmi-hwmon.c b/drivers/hwmon/scmi-hwmon.c
> index a80183a488c5..4399372e2131 100644
> --- a/drivers/hwmon/scmi-hwmon.c
> +++ b/drivers/hwmon/scmi-hwmon.c
> @@ -7,6 +7,7 @@
> */
>
> #include <linux/hwmon.h>
> +#include <linux/limits.h>
> #include <linux/module.h>
> #include <linux/scmi_protocol.h>
> #include <linux/slab.h>
> @@ -18,6 +19,47 @@ struct scmi_sensors {
> const struct scmi_sensor_info **info[hwmon_max];
> };
>
> +static inline u64 __pow10(u8 x)
> +{
> + u64 r = 1;
> +
> + while (x--)
> + r *= 10;
> +
> + return r;
> +}
> +
> +static int scmi_hwmon_scale(const struct scmi_sensor_info *sensor, u64 *value)
> +{
> + s8 scale = sensor->scale;
> + u64 f;
> +
> + switch (sensor->type) {
> + case TEMPERATURE_C:
> + case VOLTAGE:
> + case CURRENT:
> + scale += 3;
> + break;
> + case POWER:
> + case ENERGY:
> + scale += 6;
> + break;
> + default:
> + break;
> + }
> +
> + f = __pow10(abs(scale));
> + if (f == U64_MAX)
> + return -E2BIG;
Unfortunately that is not how integer overflows work.
A test program with increasing values of scale reports:
0: 1
...
18: 1000000000000000000
19: 10000000000000000000
20: 7766279631452241920
21: 3875820019684212736
22: 1864712049423024128
23: 200376420520689664
24: 2003764205206896640
...
61: 11529215046068469760
62: 4611686018427387904
63: 9223372036854775808
64: 0
...
You'll have to check for abs(scale) > 19 if you want to report overflows.
Guenter
> +
> + if (scale > 0)
> + *value *= f;
> + else
> + *value = div64_u64(*value, f);
> +
> + return 0;
> +}
> +
> static int scmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> u32 attr, int channel, long *val)
> {
> @@ -29,6 +71,10 @@ static int scmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>
> sensor = *(scmi_sensors->info[type] + channel);
> ret = h->sensor_ops->reading_get(h, sensor->id, false, &value);
> + if (ret)
> + return ret;
> +
> + ret = scmi_hwmon_scale(sensor, value);
> if (!ret)
> *val = value;
>
> --
> 2.17.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/2] firmware: arm_scmi: Fetch and store sensor scale
2019-05-08 17:00 ` Florian Fainelli
@ 2019-05-08 18:33 ` Guenter Roeck
-1 siblings, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2019-05-08 18:33 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-kernel, bcm-kernel-feedback-list, Sudeep Holla,
Jean Delvare, linux-arm-kernel, open list:HARDWARE MONITORING
On Wed, May 08, 2019 at 10:00:34AM -0700, Florian Fainelli wrote:
> In preparation for dealing with scales within the SCMI HWMON driver,
> fetch and store the sensor unit scale into the scmi_sensor_info
> structure. In order to simplify computations for upper layer, take care
> of sign extending the scale to a full 8-bit signed value.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/firmware/arm_scmi/sensors.c | 6 ++++++
> include/linux/scmi_protocol.h | 1 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c
> index b53d5cc9c9f6..21353470a740 100644
> --- a/drivers/firmware/arm_scmi/sensors.c
> +++ b/drivers/firmware/arm_scmi/sensors.c
> @@ -34,6 +34,8 @@ struct scmi_msg_resp_sensor_description {
> __le32 attributes_high;
> #define SENSOR_TYPE(x) ((x) & 0xff)
> #define SENSOR_SCALE(x) (((x) >> 11) & 0x3f)
> +#define SENSOR_SCALE_SIGN BIT(5)
> +#define SENSOR_SCALE_EXTEND GENMASK(7, 6)
> #define SENSOR_UPDATE_SCALE(x) (((x) >> 22) & 0x1f)
> #define SENSOR_UPDATE_BASE(x) (((x) >> 27) & 0x1f)
> u8 name[SCMI_MAX_STR_SIZE];
> @@ -140,6 +142,10 @@ 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);
> + /* Sign extend to a full s8 */
> + if (s->scale & SENSOR_SCALE_SIGN)
> + s->scale |= SENSOR_SCALE_EXTEND;
> strlcpy(s->name, buf->desc[cnt].name, SCMI_MAX_STR_SIZE);
> }
>
> diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
> index 3105055c00a7..9ff2e9357e9a 100644
> --- a/include/linux/scmi_protocol.h
> +++ b/include/linux/scmi_protocol.h
> @@ -144,6 +144,7 @@ struct scmi_power_ops {
> struct scmi_sensor_info {
> u32 id;
> u8 type;
> + s8 scale;
> char name[SCMI_MAX_STR_SIZE];
> };
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/2] firmware: arm_scmi: Fetch and store sensor scale
@ 2019-05-08 18:33 ` Guenter Roeck
0 siblings, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2019-05-08 18:33 UTC (permalink / raw)
To: Florian Fainelli
Cc: open list:HARDWARE MONITORING, Jean Delvare, linux-kernel,
bcm-kernel-feedback-list, Sudeep Holla, linux-arm-kernel
On Wed, May 08, 2019 at 10:00:34AM -0700, Florian Fainelli wrote:
> In preparation for dealing with scales within the SCMI HWMON driver,
> fetch and store the sensor unit scale into the scmi_sensor_info
> structure. In order to simplify computations for upper layer, take care
> of sign extending the scale to a full 8-bit signed value.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/firmware/arm_scmi/sensors.c | 6 ++++++
> include/linux/scmi_protocol.h | 1 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c
> index b53d5cc9c9f6..21353470a740 100644
> --- a/drivers/firmware/arm_scmi/sensors.c
> +++ b/drivers/firmware/arm_scmi/sensors.c
> @@ -34,6 +34,8 @@ struct scmi_msg_resp_sensor_description {
> __le32 attributes_high;
> #define SENSOR_TYPE(x) ((x) & 0xff)
> #define SENSOR_SCALE(x) (((x) >> 11) & 0x3f)
> +#define SENSOR_SCALE_SIGN BIT(5)
> +#define SENSOR_SCALE_EXTEND GENMASK(7, 6)
> #define SENSOR_UPDATE_SCALE(x) (((x) >> 22) & 0x1f)
> #define SENSOR_UPDATE_BASE(x) (((x) >> 27) & 0x1f)
> u8 name[SCMI_MAX_STR_SIZE];
> @@ -140,6 +142,10 @@ 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);
> + /* Sign extend to a full s8 */
> + if (s->scale & SENSOR_SCALE_SIGN)
> + s->scale |= SENSOR_SCALE_EXTEND;
> strlcpy(s->name, buf->desc[cnt].name, SCMI_MAX_STR_SIZE);
> }
>
> diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
> index 3105055c00a7..9ff2e9357e9a 100644
> --- a/include/linux/scmi_protocol.h
> +++ b/include/linux/scmi_protocol.h
> @@ -144,6 +144,7 @@ struct scmi_power_ops {
> struct scmi_sensor_info {
> u32 id;
> u8 type;
> + s8 scale;
> char name[SCMI_MAX_STR_SIZE];
> };
>
> --
> 2.17.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 2/2] hwmon: scmi: Scale values to target desired HWMON units
2019-05-08 18:32 ` Guenter Roeck
@ 2019-05-08 18:34 ` Florian Fainelli
-1 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2019-05-08 18:34 UTC (permalink / raw)
To: Guenter Roeck, Florian Fainelli
Cc: linux-kernel, bcm-kernel-feedback-list, Sudeep Holla,
Jean Delvare, linux-arm-kernel, open list:HARDWARE MONITORING
On 5/8/19 11:32 AM, Guenter Roeck wrote:
> Hi Florian,
>
> On Wed, May 08, 2019 at 10:00:35AM -0700, Florian Fainelli wrote:
>> If the SCMI firmware implementation is reporting values in a scale that
>> is different from the HWMON units, we need to scale up or down the value
>> according to how far appart they are.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> drivers/hwmon/scmi-hwmon.c | 46 ++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 46 insertions(+)
>>
>> diff --git a/drivers/hwmon/scmi-hwmon.c b/drivers/hwmon/scmi-hwmon.c
>> index a80183a488c5..4399372e2131 100644
>> --- a/drivers/hwmon/scmi-hwmon.c
>> +++ b/drivers/hwmon/scmi-hwmon.c
>> @@ -7,6 +7,7 @@
>> */
>>
>> #include <linux/hwmon.h>
>> +#include <linux/limits.h>
>> #include <linux/module.h>
>> #include <linux/scmi_protocol.h>
>> #include <linux/slab.h>
>> @@ -18,6 +19,47 @@ struct scmi_sensors {
>> const struct scmi_sensor_info **info[hwmon_max];
>> };
>>
>> +static inline u64 __pow10(u8 x)
>> +{
>> + u64 r = 1;
>> +
>> + while (x--)
>> + r *= 10;
>> +
>> + return r;
>> +}
>> +
>> +static int scmi_hwmon_scale(const struct scmi_sensor_info *sensor, u64 *value)
>> +{
>> + s8 scale = sensor->scale;
>> + u64 f;
>> +
>> + switch (sensor->type) {
>> + case TEMPERATURE_C:
>> + case VOLTAGE:
>> + case CURRENT:
>> + scale += 3;
>> + break;
>> + case POWER:
>> + case ENERGY:
>> + scale += 6;
>> + break;
>> + default:
>> + break;
>> + }
>> +
>> + f = __pow10(abs(scale));
>> + if (f == U64_MAX)
>> + return -E2BIG;
>
> Unfortunately that is not how integer overflows work.
>
> A test program with increasing values of scale reports:
>
> 0: 1
> ...
> 18: 1000000000000000000
> 19: 10000000000000000000
> 20: 7766279631452241920
> 21: 3875820019684212736
> 22: 1864712049423024128
> 23: 200376420520689664
> 24: 2003764205206896640
> ...
> 61: 11529215046068469760
> 62: 4611686018427387904
> 63: 9223372036854775808
> 64: 0
> ...
>
> You'll have to check for abs(scale) > 19 if you want to report overflows.
Yes silly me, my test program was flawed, thanks for pointing out that.
You are okay with returning E2BIG when we overflow?
--
Florian
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 2/2] hwmon: scmi: Scale values to target desired HWMON units
@ 2019-05-08 18:34 ` Florian Fainelli
0 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2019-05-08 18:34 UTC (permalink / raw)
To: Guenter Roeck, Florian Fainelli
Cc: open list:HARDWARE MONITORING, Jean Delvare, linux-kernel,
bcm-kernel-feedback-list, Sudeep Holla, linux-arm-kernel
On 5/8/19 11:32 AM, Guenter Roeck wrote:
> Hi Florian,
>
> On Wed, May 08, 2019 at 10:00:35AM -0700, Florian Fainelli wrote:
>> If the SCMI firmware implementation is reporting values in a scale that
>> is different from the HWMON units, we need to scale up or down the value
>> according to how far appart they are.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> drivers/hwmon/scmi-hwmon.c | 46 ++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 46 insertions(+)
>>
>> diff --git a/drivers/hwmon/scmi-hwmon.c b/drivers/hwmon/scmi-hwmon.c
>> index a80183a488c5..4399372e2131 100644
>> --- a/drivers/hwmon/scmi-hwmon.c
>> +++ b/drivers/hwmon/scmi-hwmon.c
>> @@ -7,6 +7,7 @@
>> */
>>
>> #include <linux/hwmon.h>
>> +#include <linux/limits.h>
>> #include <linux/module.h>
>> #include <linux/scmi_protocol.h>
>> #include <linux/slab.h>
>> @@ -18,6 +19,47 @@ struct scmi_sensors {
>> const struct scmi_sensor_info **info[hwmon_max];
>> };
>>
>> +static inline u64 __pow10(u8 x)
>> +{
>> + u64 r = 1;
>> +
>> + while (x--)
>> + r *= 10;
>> +
>> + return r;
>> +}
>> +
>> +static int scmi_hwmon_scale(const struct scmi_sensor_info *sensor, u64 *value)
>> +{
>> + s8 scale = sensor->scale;
>> + u64 f;
>> +
>> + switch (sensor->type) {
>> + case TEMPERATURE_C:
>> + case VOLTAGE:
>> + case CURRENT:
>> + scale += 3;
>> + break;
>> + case POWER:
>> + case ENERGY:
>> + scale += 6;
>> + break;
>> + default:
>> + break;
>> + }
>> +
>> + f = __pow10(abs(scale));
>> + if (f == U64_MAX)
>> + return -E2BIG;
>
> Unfortunately that is not how integer overflows work.
>
> A test program with increasing values of scale reports:
>
> 0: 1
> ...
> 18: 1000000000000000000
> 19: 10000000000000000000
> 20: 7766279631452241920
> 21: 3875820019684212736
> 22: 1864712049423024128
> 23: 200376420520689664
> 24: 2003764205206896640
> ...
> 61: 11529215046068469760
> 62: 4611686018427387904
> 63: 9223372036854775808
> 64: 0
> ...
>
> You'll have to check for abs(scale) > 19 if you want to report overflows.
Yes silly me, my test program was flawed, thanks for pointing out that.
You are okay with returning E2BIG when we overflow?
--
Florian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 2/2] hwmon: scmi: Scale values to target desired HWMON units
2019-05-08 18:34 ` Florian Fainelli
@ 2019-05-08 19:40 ` Guenter Roeck
-1 siblings, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2019-05-08 19:40 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-kernel, bcm-kernel-feedback-list, Sudeep Holla,
Jean Delvare, linux-arm-kernel, open list:HARDWARE MONITORING
On Wed, May 08, 2019 at 11:34:50AM -0700, Florian Fainelli wrote:
> On 5/8/19 11:32 AM, Guenter Roeck wrote:
> > Hi Florian,
> >
> > On Wed, May 08, 2019 at 10:00:35AM -0700, Florian Fainelli wrote:
> >> If the SCMI firmware implementation is reporting values in a scale that
> >> is different from the HWMON units, we need to scale up or down the value
> >> according to how far appart they are.
> >>
> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> >> ---
> >> drivers/hwmon/scmi-hwmon.c | 46 ++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 46 insertions(+)
> >>
> >> diff --git a/drivers/hwmon/scmi-hwmon.c b/drivers/hwmon/scmi-hwmon.c
> >> index a80183a488c5..4399372e2131 100644
> >> --- a/drivers/hwmon/scmi-hwmon.c
> >> +++ b/drivers/hwmon/scmi-hwmon.c
> >> @@ -7,6 +7,7 @@
> >> */
> >>
> >> #include <linux/hwmon.h>
> >> +#include <linux/limits.h>
> >> #include <linux/module.h>
> >> #include <linux/scmi_protocol.h>
> >> #include <linux/slab.h>
> >> @@ -18,6 +19,47 @@ struct scmi_sensors {
> >> const struct scmi_sensor_info **info[hwmon_max];
> >> };
> >>
> >> +static inline u64 __pow10(u8 x)
> >> +{
> >> + u64 r = 1;
> >> +
> >> + while (x--)
> >> + r *= 10;
> >> +
> >> + return r;
> >> +}
> >> +
> >> +static int scmi_hwmon_scale(const struct scmi_sensor_info *sensor, u64 *value)
> >> +{
> >> + s8 scale = sensor->scale;
> >> + u64 f;
> >> +
> >> + switch (sensor->type) {
> >> + case TEMPERATURE_C:
> >> + case VOLTAGE:
> >> + case CURRENT:
> >> + scale += 3;
> >> + break;
> >> + case POWER:
> >> + case ENERGY:
> >> + scale += 6;
> >> + break;
> >> + default:
> >> + break;
> >> + }
> >> +
> >> + f = __pow10(abs(scale));
> >> + if (f == U64_MAX)
> >> + return -E2BIG;
> >
> > Unfortunately that is not how integer overflows work.
> >
> > A test program with increasing values of scale reports:
> >
> > 0: 1
> > ...
> > 18: 1000000000000000000
> > 19: 10000000000000000000
> > 20: 7766279631452241920
> > 21: 3875820019684212736
> > 22: 1864712049423024128
> > 23: 200376420520689664
> > 24: 2003764205206896640
> > ...
> > 61: 11529215046068469760
> > 62: 4611686018427387904
> > 63: 9223372036854775808
> > 64: 0
> > ...
> >
> > You'll have to check for abs(scale) > 19 if you want to report overflows.
>
> Yes silly me, my test program was flawed, thanks for pointing out that.
> You are okay with returning E2BIG when we overflow?
Yes.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 2/2] hwmon: scmi: Scale values to target desired HWMON units
@ 2019-05-08 19:40 ` Guenter Roeck
0 siblings, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2019-05-08 19:40 UTC (permalink / raw)
To: Florian Fainelli
Cc: open list:HARDWARE MONITORING, Jean Delvare, linux-kernel,
bcm-kernel-feedback-list, Sudeep Holla, linux-arm-kernel
On Wed, May 08, 2019 at 11:34:50AM -0700, Florian Fainelli wrote:
> On 5/8/19 11:32 AM, Guenter Roeck wrote:
> > Hi Florian,
> >
> > On Wed, May 08, 2019 at 10:00:35AM -0700, Florian Fainelli wrote:
> >> If the SCMI firmware implementation is reporting values in a scale that
> >> is different from the HWMON units, we need to scale up or down the value
> >> according to how far appart they are.
> >>
> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> >> ---
> >> drivers/hwmon/scmi-hwmon.c | 46 ++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 46 insertions(+)
> >>
> >> diff --git a/drivers/hwmon/scmi-hwmon.c b/drivers/hwmon/scmi-hwmon.c
> >> index a80183a488c5..4399372e2131 100644
> >> --- a/drivers/hwmon/scmi-hwmon.c
> >> +++ b/drivers/hwmon/scmi-hwmon.c
> >> @@ -7,6 +7,7 @@
> >> */
> >>
> >> #include <linux/hwmon.h>
> >> +#include <linux/limits.h>
> >> #include <linux/module.h>
> >> #include <linux/scmi_protocol.h>
> >> #include <linux/slab.h>
> >> @@ -18,6 +19,47 @@ struct scmi_sensors {
> >> const struct scmi_sensor_info **info[hwmon_max];
> >> };
> >>
> >> +static inline u64 __pow10(u8 x)
> >> +{
> >> + u64 r = 1;
> >> +
> >> + while (x--)
> >> + r *= 10;
> >> +
> >> + return r;
> >> +}
> >> +
> >> +static int scmi_hwmon_scale(const struct scmi_sensor_info *sensor, u64 *value)
> >> +{
> >> + s8 scale = sensor->scale;
> >> + u64 f;
> >> +
> >> + switch (sensor->type) {
> >> + case TEMPERATURE_C:
> >> + case VOLTAGE:
> >> + case CURRENT:
> >> + scale += 3;
> >> + break;
> >> + case POWER:
> >> + case ENERGY:
> >> + scale += 6;
> >> + break;
> >> + default:
> >> + break;
> >> + }
> >> +
> >> + f = __pow10(abs(scale));
> >> + if (f == U64_MAX)
> >> + return -E2BIG;
> >
> > Unfortunately that is not how integer overflows work.
> >
> > A test program with increasing values of scale reports:
> >
> > 0: 1
> > ...
> > 18: 1000000000000000000
> > 19: 10000000000000000000
> > 20: 7766279631452241920
> > 21: 3875820019684212736
> > 22: 1864712049423024128
> > 23: 200376420520689664
> > 24: 2003764205206896640
> > ...
> > 61: 11529215046068469760
> > 62: 4611686018427387904
> > 63: 9223372036854775808
> > 64: 0
> > ...
> >
> > You'll have to check for abs(scale) > 19 if you want to report overflows.
>
> Yes silly me, my test program was flawed, thanks for pointing out that.
> You are okay with returning E2BIG when we overflow?
Yes.
Thanks,
Guenter
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-05-08 19:40 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-08 17:00 [PATCH v4 0/2] hwmon: scmi: Scale values to target desired HWMON units Florian Fainelli
2019-05-08 17:00 ` Florian Fainelli
2019-05-08 17:00 ` [PATCH v4 1/2] firmware: arm_scmi: Fetch and store sensor scale Florian Fainelli
2019-05-08 17:00 ` Florian Fainelli
2019-05-08 18:33 ` Guenter Roeck
2019-05-08 18:33 ` Guenter Roeck
2019-05-08 17:00 ` [PATCH v4 2/2] hwmon: scmi: Scale values to target desired HWMON units Florian Fainelli
2019-05-08 17:00 ` Florian Fainelli
2019-05-08 18:32 ` Guenter Roeck
2019-05-08 18:32 ` Guenter Roeck
2019-05-08 18:34 ` Florian Fainelli
2019-05-08 18:34 ` Florian Fainelli
2019-05-08 19:40 ` Guenter Roeck
2019-05-08 19:40 ` Guenter Roeck
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.