Hi Daniel, Do I need to make more changes to this patch? Thanks -Lin Ruizhe -----邮件原件----- 发件人: Daniel Lezcano [mailto:daniel.lezcano@xxxxxxxxxx] 发送时间: 2021年4月21日 19:37 收件人: Tony Lindgren <tony@xxxxxxxxxxx> 抄送: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>; linruizhe <linruizhe@xxxxxxxxxx>; rui.zhang@xxxxxxxxx; edubezval@xxxxxxxxx; j-keerthy@xxxxxx; amitk@xxxxxxxxxx; eballetbo@xxxxxxxxx; linux-pm@xxxxxxxxxxxxxxx; linux-omap@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx 主题: Re: [PATCH v3] thermal: ti-soc-thermal: Remove unused variable 'val' Hi Tony, thanks for testing -- Daniel On 21/04/2021 11:37, Tony Lindgren wrote: > * Daniel Lezcano <daniel.lezcano@xxxxxxxxxx> [210421 09:07]: >> On 21/04/2021 10:49, Greg KH wrote: >>> On Wed, Apr 21, 2021 at 04:42:56PM +0800, Lin Ruizhe wrote: >>>> The variable 'val'in function ti_bandgap_restore_ctxt is the >>>> register value of read bandgap registers. This function is to >>>> restore the context. But there is no operation on the return value >>>> of this register, so this block is redundant. Hulk robot scans this >>>> warning.This commit remove the dead code. >>>> >>>> Reported-by: Hulk Robot <hulkci@xxxxxxxxxx> >>>> Fixes: b87ea759a4cc ("staging: omap-thermal: fix context restore >>>> function") >>>> Signed-off-by: Lin Ruizhe <linruizhe@xxxxxxxxxx> >>>> --- >>>> v3: >>>> -Add Fixes tag and more accurate commit message in this patch. >>>> v2: >>>> -As suggest remove the whole unuesed block in fuction >>>> ti_bandgap_restore_ctxt >>>> >>>> drivers/thermal/ti-soc-thermal/ti-bandgap.c | 4 ---- >>>> 1 file changed, 4 deletions(-) >>>> >>>> diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c >>>> b/drivers/thermal/ti-soc-thermal/ti-bandgap.c >>>> index d81af89166d2..684ffb645aa9 100644 >>>> --- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c >>>> +++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c >>>> @@ -1142,14 +1142,10 @@ static int ti_bandgap_restore_ctxt(struct ti_bandgap *bgp) >>>> for (i = 0; i < bgp->conf->sensor_count; i++) { >>>> struct temp_sensor_registers *tsr; >>>> struct temp_sensor_regval *rval; >>>> - u32 val = 0; >>>> >>>> rval = &bgp->regval[i]; >>>> tsr = bgp->conf->sensors[i].registers; >>>> >>>> - if (TI_BANDGAP_HAS(bgp, COUNTER)) >>>> - val = ti_bandgap_readl(bgp, tsr->bgap_counter); >>> >>> Are you sure that this hardware does not require this read to happen >>> in order for it to work properly? >> >> Yes, initially we had the same concern but we were unable to find >> anything specific in the history. The commit mentioned above removed >> the user of the 'val' code but without removing this block of code. >> >> When looking at the current code, it really looks like an oversight. > > Yes so it seems. > >> There is nothing in the commit's changelog referring to a need of >> reading the counter register but perhaps I'm wrong because I'm not >> sure to understand correctly the changelog. >> >>> Lots of hardware does need this, have you tested this? > > I just tested this on omap3 logicpd torpedo devkit that can do off > during idle and reading /sys/class/thermal/thermal_zone0/temp works. > So feel free to add: > > Reviewed-by: Tony Lindgren <tony@xxxxxxxxxxx> > Tested-by: Tony Lindgren <tony@xxxxxxxxxxx> Thanks for testing -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
