Re: [PATCH v3] thermal: ti-soc-thermal: Remove unused variable 'val'

[Tony Lindgren]

* 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>