* [PATCH] firmware: arm_scmi: fix bitfield definitions for SENSOR_DESC attributes
@ 2019-05-16 16:33 ` Sudeep Holla
0 siblings, 0 replies; 4+ messages in thread
From: Sudeep Holla @ 2019-05-16 16:33 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: Sudeep Holla, linux-kernel, Florian Fainelli
As per the SCMI specification the bitfields for SENSOR_DESC attributes
are as follows:
attributes_low [7:0] Number of trip points supported
attributes_high [15:11] The power-of-10 multiplier in 2's-complement
format that is applied to the sensor units
Looks like the code developed during the draft versions of the
specification slipped through and are wrong with respect to final
released version. Fix them by adjusting the bitfields appropriately.
Cc: Florian Fainelli <f.fainelli@gmail.com>
Fixes: 5179c523c1ea ("firmware: arm_scmi: add initial support for sensor protocol")
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/firmware/arm_scmi/sensors.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Hi Florian,
While testing your patches, I found this horrible/silly bug with bitfields
which initial made me think firmware is buggy but later found out driver
was buggy instead.
I updated your patch accordingly[1]
Regards,
Sudeep
[1] https://git.kernel.org/sudeep.holla/linux/h/for-next/scmi-updates
diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c
index b53d5cc9c9f6..c00287b5f2c2 100644
--- a/drivers/firmware/arm_scmi/sensors.c
+++ b/drivers/firmware/arm_scmi/sensors.c
@@ -30,10 +30,10 @@ struct scmi_msg_resp_sensor_description {
__le32 id;
__le32 attributes_low;
#define SUPPORTS_ASYNC_READ(x) ((x) & BIT(31))
-#define NUM_TRIP_POINTS(x) (((x) >> 4) & 0xff)
+#define NUM_TRIP_POINTS(x) ((x) & 0xff)
__le32 attributes_high;
#define SENSOR_TYPE(x) ((x) & 0xff)
-#define SENSOR_SCALE(x) (((x) >> 11) & 0x3f)
+#define SENSOR_SCALE(x) (((x) >> 11) & 0x1f)
#define SENSOR_UPDATE_SCALE(x) (((x) >> 22) & 0x1f)
#define SENSOR_UPDATE_BASE(x) (((x) >> 27) & 0x1f)
u8 name[SCMI_MAX_STR_SIZE];
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] firmware: arm_scmi: fix bitfield definitions for SENSOR_DESC attributes
@ 2019-05-16 16:33 ` Sudeep Holla
0 siblings, 0 replies; 4+ messages in thread
From: Sudeep Holla @ 2019-05-16 16:33 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: Florian Fainelli, linux-kernel, Sudeep Holla
As per the SCMI specification the bitfields for SENSOR_DESC attributes
are as follows:
attributes_low [7:0] Number of trip points supported
attributes_high [15:11] The power-of-10 multiplier in 2's-complement
format that is applied to the sensor units
Looks like the code developed during the draft versions of the
specification slipped through and are wrong with respect to final
released version. Fix them by adjusting the bitfields appropriately.
Cc: Florian Fainelli <f.fainelli@gmail.com>
Fixes: 5179c523c1ea ("firmware: arm_scmi: add initial support for sensor protocol")
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/firmware/arm_scmi/sensors.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Hi Florian,
While testing your patches, I found this horrible/silly bug with bitfields
which initial made me think firmware is buggy but later found out driver
was buggy instead.
I updated your patch accordingly[1]
Regards,
Sudeep
[1] https://git.kernel.org/sudeep.holla/linux/h/for-next/scmi-updates
diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c
index b53d5cc9c9f6..c00287b5f2c2 100644
--- a/drivers/firmware/arm_scmi/sensors.c
+++ b/drivers/firmware/arm_scmi/sensors.c
@@ -30,10 +30,10 @@ struct scmi_msg_resp_sensor_description {
__le32 id;
__le32 attributes_low;
#define SUPPORTS_ASYNC_READ(x) ((x) & BIT(31))
-#define NUM_TRIP_POINTS(x) (((x) >> 4) & 0xff)
+#define NUM_TRIP_POINTS(x) ((x) & 0xff)
__le32 attributes_high;
#define SENSOR_TYPE(x) ((x) & 0xff)
-#define SENSOR_SCALE(x) (((x) >> 11) & 0x3f)
+#define SENSOR_SCALE(x) (((x) >> 11) & 0x1f)
#define SENSOR_UPDATE_SCALE(x) (((x) >> 22) & 0x1f)
#define SENSOR_UPDATE_BASE(x) (((x) >> 27) & 0x1f)
u8 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] 4+ messages in thread
* Re: [PATCH] firmware: arm_scmi: fix bitfield definitions for SENSOR_DESC attributes
2019-05-16 16:33 ` Sudeep Holla
@ 2019-05-16 16:37 ` Florian Fainelli
-1 siblings, 0 replies; 4+ messages in thread
From: Florian Fainelli @ 2019-05-16 16:37 UTC (permalink / raw)
To: Sudeep Holla, linux-arm-kernel; +Cc: linux-kernel
On 5/16/19 9:33 AM, Sudeep Holla wrote:
> As per the SCMI specification the bitfields for SENSOR_DESC attributes
> are as follows:
> attributes_low [7:0] Number of trip points supported
> attributes_high [15:11] The power-of-10 multiplier in 2's-complement
> format that is applied to the sensor units
>
> Looks like the code developed during the draft versions of the
> specification slipped through and are wrong with respect to final
> released version. Fix them by adjusting the bitfields appropriately.
>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Fixes: 5179c523c1ea ("firmware: arm_scmi: add initial support for sensor protocol")
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/firmware/arm_scmi/sensors.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> Hi Florian,
>
> While testing your patches, I found this horrible/silly bug with bitfields
> which initial made me think firmware is buggy but later found out driver
> was buggy instead.
>
> I updated your patch accordingly[1]
Looks good to me, thanks for fixing that up!
>
> Regards,
> Sudeep
>
> [1] https://git.kernel.org/sudeep.holla/linux/h/for-next/scmi-updates
>
> diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c
> index b53d5cc9c9f6..c00287b5f2c2 100644
> --- a/drivers/firmware/arm_scmi/sensors.c
> +++ b/drivers/firmware/arm_scmi/sensors.c
> @@ -30,10 +30,10 @@ struct scmi_msg_resp_sensor_description {
> __le32 id;
> __le32 attributes_low;
> #define SUPPORTS_ASYNC_READ(x) ((x) & BIT(31))
> -#define NUM_TRIP_POINTS(x) (((x) >> 4) & 0xff)
> +#define NUM_TRIP_POINTS(x) ((x) & 0xff)
> __le32 attributes_high;
> #define SENSOR_TYPE(x) ((x) & 0xff)
> -#define SENSOR_SCALE(x) (((x) >> 11) & 0x3f)
> +#define SENSOR_SCALE(x) (((x) >> 11) & 0x1f)
> #define SENSOR_UPDATE_SCALE(x) (((x) >> 22) & 0x1f)
> #define SENSOR_UPDATE_BASE(x) (((x) >> 27) & 0x1f)
> u8 name[SCMI_MAX_STR_SIZE];
>
--
Florian
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] firmware: arm_scmi: fix bitfield definitions for SENSOR_DESC attributes
@ 2019-05-16 16:37 ` Florian Fainelli
0 siblings, 0 replies; 4+ messages in thread
From: Florian Fainelli @ 2019-05-16 16:37 UTC (permalink / raw)
To: Sudeep Holla, linux-arm-kernel; +Cc: linux-kernel
On 5/16/19 9:33 AM, Sudeep Holla wrote:
> As per the SCMI specification the bitfields for SENSOR_DESC attributes
> are as follows:
> attributes_low [7:0] Number of trip points supported
> attributes_high [15:11] The power-of-10 multiplier in 2's-complement
> format that is applied to the sensor units
>
> Looks like the code developed during the draft versions of the
> specification slipped through and are wrong with respect to final
> released version. Fix them by adjusting the bitfields appropriately.
>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Fixes: 5179c523c1ea ("firmware: arm_scmi: add initial support for sensor protocol")
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/firmware/arm_scmi/sensors.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> Hi Florian,
>
> While testing your patches, I found this horrible/silly bug with bitfields
> which initial made me think firmware is buggy but later found out driver
> was buggy instead.
>
> I updated your patch accordingly[1]
Looks good to me, thanks for fixing that up!
>
> Regards,
> Sudeep
>
> [1] https://git.kernel.org/sudeep.holla/linux/h/for-next/scmi-updates
>
> diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c
> index b53d5cc9c9f6..c00287b5f2c2 100644
> --- a/drivers/firmware/arm_scmi/sensors.c
> +++ b/drivers/firmware/arm_scmi/sensors.c
> @@ -30,10 +30,10 @@ struct scmi_msg_resp_sensor_description {
> __le32 id;
> __le32 attributes_low;
> #define SUPPORTS_ASYNC_READ(x) ((x) & BIT(31))
> -#define NUM_TRIP_POINTS(x) (((x) >> 4) & 0xff)
> +#define NUM_TRIP_POINTS(x) ((x) & 0xff)
> __le32 attributes_high;
> #define SENSOR_TYPE(x) ((x) & 0xff)
> -#define SENSOR_SCALE(x) (((x) >> 11) & 0x3f)
> +#define SENSOR_SCALE(x) (((x) >> 11) & 0x1f)
> #define SENSOR_UPDATE_SCALE(x) (((x) >> 22) & 0x1f)
> #define SENSOR_UPDATE_BASE(x) (((x) >> 27) & 0x1f)
> u8 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-05-16 16:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-16 16:33 [PATCH] firmware: arm_scmi: fix bitfield definitions for SENSOR_DESC attributes Sudeep Holla
2019-05-16 16:33 ` Sudeep Holla
2019-05-16 16:37 ` Florian Fainelli
2019-05-16 16:37 ` Florian Fainelli
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.