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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 21 Apr 2021 12:37:07 +0300
From:   Tony Lindgren <tony@...mide.com>
To:     Daniel Lezcano <daniel.lezcano@...aro.org>
Cc:     Greg KH <gregkh@...uxfoundation.org>,
        Lin Ruizhe <linruizhe@...wei.com>, rui.zhang@...el.com,
        edubezval@...il.com, j-keerthy@...com, amitk@...nel.org,
        radhesh.fadnis@...com, eballetbo@...il.com,
        linux-pm@...r.kernel.org, linux-omap@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] thermal: ti-soc-thermal: Remove unused variable 'val'

* Daniel Lezcano <daniel.lezcano@...aro.org> [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@...wei.com>
> >> Fixes: b87ea759a4cc ("staging: omap-thermal: fix context restore function")
> >> Signed-off-by: Lin Ruizhe <linruizhe@...wei.com>
> >> ---
> >> 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@...mide.com>
Tested-by: Tony Lindgren <tony@...mide.com>