From: Florian Fainelli <f.fainelli@gmail.com> To: Sudeep Holla <sudeep.holla@arm.com>, linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH] firmware: arm_scmi: fix bitfield definitions for SENSOR_DESC attributes Date: Thu, 16 May 2019 09:37:52 -0700 [thread overview] Message-ID: <8843182d-aea4-6a75-caca-6b48de594f30@gmail.com> (raw) In-Reply-To: <20190516163315.18505-1-sudeep.holla@arm.com> 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
WARNING: multiple messages have this Message-ID (diff)
From: Florian Fainelli <f.fainelli@gmail.com> To: Sudeep Holla <sudeep.holla@arm.com>, linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH] firmware: arm_scmi: fix bitfield definitions for SENSOR_DESC attributes Date: Thu, 16 May 2019 09:37:52 -0700 [thread overview] Message-ID: <8843182d-aea4-6a75-caca-6b48de594f30@gmail.com> (raw) In-Reply-To: <20190516163315.18505-1-sudeep.holla@arm.com> 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
next prev parent reply other threads:[~2019-05-16 16:38 UTC|newest] Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top 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 [this message] 2019-05-16 16:37 ` Florian Fainelli
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=8843182d-aea4-6a75-caca-6b48de594f30@gmail.com \ --to=f.fainelli@gmail.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --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.