| Message ID | 20180319045420.22046-10-cyril.bur@au1.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Coverity fixes | expand |
On 03/19/2018 05:54 AM, Cyril Bur wrote: > It is possible that fixing CID 209503 will fix these FIXMEs. I do not > know this code well at all and am in absolutely no position to test. > Removing the FIXME needs much more scrutiny!! When first support for the POWER8 DTS was added a couple of years ago, the trip bits always had some 'bogus' value, which raised useless warnings at the userspace level in hwmon. Is this fixed now ? What is 209503 about ? C. > CC: Cédric Le Goater <clg@fr.ibm.com> > CC: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com> > Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> > --- > hw/dts.c | 15 --------------- > 1 file changed, 15 deletions(-) > > diff --git a/hw/dts.c b/hw/dts.c > index 2c366699..5e9e5e39 100644 > --- a/hw/dts.c > +++ b/hw/dts.c > @@ -187,11 +187,6 @@ static int dts_read_core_temp_p8(uint32_t pir, struct dts *dts) > prlog(PR_TRACE, "DTS: Chip %x Core %x temp:%dC trip:%x\n", > chip_id, core, dts->temp, dts->trip); > > - /* > - * FIXME: The trip bits are always set ?! Just discard > - * them for the moment until we understand why. > - */ > - dts->trip = 0; > return 0; > } > > @@ -228,11 +223,6 @@ static int dts_read_core_temp_p9(uint32_t pir, struct dts *dts) > prlog(PR_TRACE, "DTS: Chip %x Core %x temp:%dC trip:%x\n", > chip_id, core, dts->temp, dts->trip); > > - /* > - * FIXME: The trip bits are always set ?! Just discard > - * them for the moment until we understand why. > - */ > - dts->trip = 0; > return 0; > } > > @@ -367,11 +357,6 @@ static int dts_read_mem_temp(uint32_t chip_id, struct dts *dts) > prlog(PR_TRACE, "DTS: Chip %x temp:%dC trip:%x\n", > chip_id, dts->temp, dts->trip); > > - /* > - * FIXME: The trip bits are always set ?! Just discard > - * them for the moment until we understand why. > - */ > - dts->trip = 0; > return 0; > } >
On Mon, 2018-03-19 at 09:06 +0100, Cédric Le Goater wrote: > On 03/19/2018 05:54 AM, Cyril Bur wrote: > > It is possible that fixing CID 209503 will fix these FIXMEs. I do not > > know this code well at all and am in absolutely no position to test. > > Removing the FIXME needs much more scrutiny!! > > When first support for the POWER8 DTS was added a couple of years ago, > the trip bits always had some 'bogus' value, which raised useless > warnings at the userspace level in hwmon. Is this fixed now ? > > What is 209503 about ? 209503 is about the dts parameter to these functions coming from uninitialised memory which may explain bits always being set. Sorry I probably should have cced you on the fix for 209503 as well, might have helped with context. Cyril > > C. > > > > CC: Cédric Le Goater <clg@fr.ibm.com> > > CC: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com> > > Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> > > --- > > hw/dts.c | 15 --------------- > > 1 file changed, 15 deletions(-) > > > > diff --git a/hw/dts.c b/hw/dts.c > > index 2c366699..5e9e5e39 100644 > > --- a/hw/dts.c > > +++ b/hw/dts.c > > @@ -187,11 +187,6 @@ static int dts_read_core_temp_p8(uint32_t pir, struct dts *dts) > > prlog(PR_TRACE, "DTS: Chip %x Core %x temp:%dC trip:%x\n", > > chip_id, core, dts->temp, dts->trip); > > > > - /* > > - * FIXME: The trip bits are always set ?! Just discard > > - * them for the moment until we understand why. > > - */ > > - dts->trip = 0; > > return 0; > > } > > > > @@ -228,11 +223,6 @@ static int dts_read_core_temp_p9(uint32_t pir, struct dts *dts) > > prlog(PR_TRACE, "DTS: Chip %x Core %x temp:%dC trip:%x\n", > > chip_id, core, dts->temp, dts->trip); > > > > - /* > > - * FIXME: The trip bits are always set ?! Just discard > > - * them for the moment until we understand why. > > - */ > > - dts->trip = 0; > > return 0; > > } > > > > @@ -367,11 +357,6 @@ static int dts_read_mem_temp(uint32_t chip_id, struct dts *dts) > > prlog(PR_TRACE, "DTS: Chip %x temp:%dC trip:%x\n", > > chip_id, dts->temp, dts->trip); > > > > - /* > > - * FIXME: The trip bits are always set ?! Just discard > > - * them for the moment until we understand why. > > - */ > > - dts->trip = 0; > > return 0; > > } > > > >
On 03/19/2018 09:12 AM, Cyril Bur wrote: > On Mon, 2018-03-19 at 09:06 +0100, Cédric Le Goater wrote: >> On 03/19/2018 05:54 AM, Cyril Bur wrote: >>> It is possible that fixing CID 209503 will fix these FIXMEs. I do not >>> know this code well at all and am in absolutely no position to test. >>> Removing the FIXME needs much more scrutiny!! >> >> When first support for the POWER8 DTS was added a couple of years ago, >> the trip bits always had some 'bogus' value, which raised useless >> warnings at the userspace level in hwmon. Is this fixed now ? >> >> What is 209503 about ? > > 209503 is about the dts parameter to these functions coming from > uninitialised memory which may explain bits always being set. hmm, I don't see where the dts could be uninitialized. Anyhow, you can check the raw DTS values from userspace with this command : getscom <0x10000000 | 0x50000 | (core_id << 24)> which returns something like 023f023f022f0000 on a palmetto. C.
diff --git a/hw/dts.c b/hw/dts.c index 2c366699..5e9e5e39 100644 --- a/hw/dts.c +++ b/hw/dts.c @@ -187,11 +187,6 @@ static int dts_read_core_temp_p8(uint32_t pir, struct dts *dts) prlog(PR_TRACE, "DTS: Chip %x Core %x temp:%dC trip:%x\n", chip_id, core, dts->temp, dts->trip); - /* - * FIXME: The trip bits are always set ?! Just discard - * them for the moment until we understand why. - */ - dts->trip = 0; return 0; } @@ -228,11 +223,6 @@ static int dts_read_core_temp_p9(uint32_t pir, struct dts *dts) prlog(PR_TRACE, "DTS: Chip %x Core %x temp:%dC trip:%x\n", chip_id, core, dts->temp, dts->trip); - /* - * FIXME: The trip bits are always set ?! Just discard - * them for the moment until we understand why. - */ - dts->trip = 0; return 0; } @@ -367,11 +357,6 @@ static int dts_read_mem_temp(uint32_t chip_id, struct dts *dts) prlog(PR_TRACE, "DTS: Chip %x temp:%dC trip:%x\n", chip_id, dts->temp, dts->trip); - /* - * FIXME: The trip bits are always set ?! Just discard - * them for the moment until we understand why. - */ - dts->trip = 0; return 0; }
It is possible that fixing CID 209503 will fix these FIXMEs. I do not know this code well at all and am in absolutely no position to test. Removing the FIXME needs much more scrutiny!! CC: Cédric Le Goater <clg@fr.ibm.com> CC: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com> Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> --- hw/dts.c | 15 --------------- 1 file changed, 15 deletions(-)