* [PATCH] firmware: arm_scmi: Remove set but not used variable 'val' @ 2019-11-10 10:30 Zheng Yongjun 2019-11-11 15:49 ` Robin Murphy 2019-11-11 16:36 ` [PATCH] firmware: arm_scmi: Fix doorbell ring logic for !CONFIG_64BIT Sudeep Holla 0 siblings, 2 replies; 4+ messages in thread From: Zheng Yongjun @ 2019-11-10 10:30 UTC (permalink / raw) To: sudeep.holla, linux-arm-kernel; +Cc: linux-kernel, zhengyongjun3, Hulk Robot Fixes gcc '-Wunused-but-set-variable' warning: drivers/firmware/arm_scmi/perf.c: In function scmi_perf_fc_ring_db: drivers/firmware/arm_scmi/perf.c:323:7: warning: variable val set but not used [-Wunused-but-set-variable] val is never used, so remove it. Reported-by: Hulk Robot <hulkci@huawei.com> Signed-off-by: Zheng Yongjun <zhengyongjun3@huawei.com> --- drivers/firmware/arm_scmi/perf.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c index 4a8012e3cb8c..efa98d2ee045 100644 --- a/drivers/firmware/arm_scmi/perf.c +++ b/drivers/firmware/arm_scmi/perf.c @@ -319,10 +319,8 @@ static void scmi_perf_fc_ring_db(struct scmi_fc_db_info *db) SCMI_PERF_FC_RING_DB(64); #else { - u64 val = 0; - if (db->mask) - val = ioread64_hi_lo(db->addr) & db->mask; + ioread64_hi_lo(db->addr) & db->mask; iowrite64_hi_lo(db->set, db->addr); } #endif -- 2.23.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] firmware: arm_scmi: Remove set but not used variable 'val' 2019-11-10 10:30 [PATCH] firmware: arm_scmi: Remove set but not used variable 'val' Zheng Yongjun @ 2019-11-11 15:49 ` Robin Murphy 2019-11-11 16:04 ` Sudeep Holla 2019-11-11 16:36 ` [PATCH] firmware: arm_scmi: Fix doorbell ring logic for !CONFIG_64BIT Sudeep Holla 1 sibling, 1 reply; 4+ messages in thread From: Robin Murphy @ 2019-11-11 15:49 UTC (permalink / raw) To: Zheng Yongjun, sudeep.holla, linux-arm-kernel; +Cc: linux-kernel, Hulk Robot On 10/11/2019 10:30, Zheng Yongjun wrote: > Fixes gcc '-Wunused-but-set-variable' warning: > > drivers/firmware/arm_scmi/perf.c: In function scmi_perf_fc_ring_db: > drivers/firmware/arm_scmi/perf.c:323:7: warning: variable val set but not used [-Wunused-but-set-variable] > > val is never used, so remove it. > > Reported-by: Hulk Robot <hulkci@huawei.com> > Signed-off-by: Zheng Yongjun <zhengyongjun3@huawei.com> > --- > drivers/firmware/arm_scmi/perf.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c > index 4a8012e3cb8c..efa98d2ee045 100644 > --- a/drivers/firmware/arm_scmi/perf.c > +++ b/drivers/firmware/arm_scmi/perf.c > @@ -319,10 +319,8 @@ static void scmi_perf_fc_ring_db(struct scmi_fc_db_info *db) > SCMI_PERF_FC_RING_DB(64); > #else > { > - u64 val = 0; > - > if (db->mask) > - val = ioread64_hi_lo(db->addr) & db->mask; > + ioread64_hi_lo(db->addr) & db->mask; > iowrite64_hi_lo(db->set, db->addr); FWIW, compared to the SCMI_PERF_FC_RING_DB() macro, this looks like the wrong "fix". Robin. > } > #endif > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] firmware: arm_scmi: Remove set but not used variable 'val' 2019-11-11 15:49 ` Robin Murphy @ 2019-11-11 16:04 ` Sudeep Holla 0 siblings, 0 replies; 4+ messages in thread From: Sudeep Holla @ 2019-11-11 16:04 UTC (permalink / raw) To: Robin Murphy, Zheng Yongjun; +Cc: linux-arm-kernel, linux-kernel, Hulk Robot On Mon, Nov 11, 2019 at 03:49:55PM +0000, Robin Murphy wrote: > On 10/11/2019 10:30, Zheng Yongjun wrote: > > Fixes gcc '-Wunused-but-set-variable' warning: > > > > drivers/firmware/arm_scmi/perf.c: In function scmi_perf_fc_ring_db: > > drivers/firmware/arm_scmi/perf.c:323:7: warning: variable val set but not used [-Wunused-but-set-variable] > > > > val is never used, so remove it. > > > > Reported-by: Hulk Robot <hulkci@huawei.com> > > Signed-off-by: Zheng Yongjun <zhengyongjun3@huawei.com> > > --- > > drivers/firmware/arm_scmi/perf.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c > > index 4a8012e3cb8c..efa98d2ee045 100644 > > --- a/drivers/firmware/arm_scmi/perf.c > > +++ b/drivers/firmware/arm_scmi/perf.c > > @@ -319,10 +319,8 @@ static void scmi_perf_fc_ring_db(struct scmi_fc_db_info *db) > > SCMI_PERF_FC_RING_DB(64); > > #else > > { > > - u64 val = 0; > > - > > if (db->mask) > > - val = ioread64_hi_lo(db->addr) & db->mask; > > + ioread64_hi_lo(db->addr) & db->mask; > > iowrite64_hi_lo(db->set, db->addr); > > FWIW, compared to the SCMI_PERF_FC_RING_DB() macro, this looks like the > wrong "fix". > Yes, no idea how I didn't spot this earlier. That could be because this was just added to fix 32-bit build and wasn't tested. The below patch should fix the warning and also fixes the real bug. Regards, Sudeep diff --git i/drivers/firmware/arm_scmi/perf.c w/drivers/firmware/arm_scmi/perf.c index 4a8012e3cb8c..601af4edad5e 100644 --- i/drivers/firmware/arm_scmi/perf.c +++ w/drivers/firmware/arm_scmi/perf.c @@ -323,7 +323,7 @@ static void scmi_perf_fc_ring_db(struct scmi_fc_db_info *db) if (db->mask) val = ioread64_hi_lo(db->addr) & db->mask; - iowrite64_hi_lo(db->set, db->addr); + iowrite64_hi_lo(db->set | val, db->addr); } #endif } ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] firmware: arm_scmi: Fix doorbell ring logic for !CONFIG_64BIT 2019-11-10 10:30 [PATCH] firmware: arm_scmi: Remove set but not used variable 'val' Zheng Yongjun 2019-11-11 15:49 ` Robin Murphy @ 2019-11-11 16:36 ` Sudeep Holla 1 sibling, 0 replies; 4+ messages in thread From: Sudeep Holla @ 2019-11-11 16:36 UTC (permalink / raw) To: linux-kernel; +Cc: Sudeep Holla, linux-arm-kernel, hulkci, zhengyongjun3 The logic to ring the scmi performance fastchannel ignores the value read from the doorbell register in case of !CONFIG_64BIT. This bug also shows up as warning with '-Wunused-but-set-variable' gcc flag: drivers/firmware/arm_scmi/perf.c: In function scmi_perf_fc_ring_db: drivers/firmware/arm_scmi/perf.c:323:7: warning: variable val set but not used [-Wunused-but-set-variable] Fix the same by aligning the logic with CONFIG_64BIT as used in the macro SCMI_PERF_FC_RING_DB(). Fixes: 823839571d76 ("firmware: arm_scmi: Make use SCMI v2.0 fastchannel for performance protocol") Reported-by: Hulk Robot <hulkci@huawei.com> Reported-by: Zheng Yongjun <zhengyongjun3@huawei.com> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- drivers/firmware/arm_scmi/perf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Hi Zheng, Thanks for the bug report, but the fix is incorrect as discussed. This is proper fix, let me know if this fixes the gcc warning you found. Regards, Sudeep diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c index 4a8012e3cb8c..601af4edad5e 100644 --- a/drivers/firmware/arm_scmi/perf.c +++ b/drivers/firmware/arm_scmi/perf.c @@ -323,7 +323,7 @@ static void scmi_perf_fc_ring_db(struct scmi_fc_db_info *db) if (db->mask) val = ioread64_hi_lo(db->addr) & db->mask; - iowrite64_hi_lo(db->set, db->addr); + iowrite64_hi_lo(db->set | val, db->addr); } #endif } -- 2.17.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-11-11 16:36 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-11-10 10:30 [PATCH] firmware: arm_scmi: Remove set but not used variable 'val' Zheng Yongjun 2019-11-11 15:49 ` Robin Murphy 2019-11-11 16:04 ` Sudeep Holla 2019-11-11 16:36 ` [PATCH] firmware: arm_scmi: Fix doorbell ring logic for !CONFIG_64BIT Sudeep Holla
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox