Re: [PATCH] powercap: intel_rapl_tpmi: Fix a sparse warning

[Zhang, Rui]

Hi, David,

Thanks for pointing this out, after digging into the problem further,
I'm confused about how to address all the sparse warnings.

In the RAPL framework, u64 regs[RAPL_DOMAIN_MAX][RAPL_DOMAIN_REG_MAX]
is used to save the RAPL register addresses, because the register can
either be a MMIO register or MSR.

With sparse enabled, this becomes a problem for MMIO registers because
we will get a warning anyway when saving an __iomem address to u64
regs[][], right?

This problem is not new. The same warning is also there when compiling
drivers/thermal/intel/int340x_thermal/processor_thermal_rapl.c, which
also uses MMIO RAPL register.

Any idea how to fix this?

thanks,
rui

On Fri, 2023-07-14 at 16:39 +0200, Rafael J. Wysocki wrote:
> On Fri, Jul 14, 2023 at 3:34 PM David Laight
> <David.Laight@xxxxxxxxxx> wrote:
> > 
> > From: Rafael J. Wysocki
> > > Sent: 11 July 2023 19:14
> > ...
> > > On Tue, Jul 4, 2023 at 7:02 AM Zhang Rui <rui.zhang@xxxxxxxxx>
> > > wrote:
> > > > 
> > > > Fix a sparse warning in intel_rapl_tpmi driver.
> > > > 
> > > > ../drivers/powercap/intel_rapl_tpmi.c:141:41: sparse: warning:
> > > > incorrect type in initializer
> > > (different address spaces)
> > > > ../drivers/powercap/intel_rapl_tpmi.c:141:41: sparse:   
> > > > expected unsigned long long [usertype]
> > > *tpmi_rapl_regs
> > > > ../drivers/powercap/intel_rapl_tpmi.c:141:41: sparse:    got
> > > > void [noderef] __iomem *
> > > > 
> > > > Fixes: 9eef7f9da928 ("powercap: intel_rapl: Introduce RAPL TPMI
> > > > interface driver")
> > > > Reported-by: kernel test robot <lkp@xxxxxxxxx>
> > > > Closes:
> > > > https://lore.kernel.org/oe-kbuild-all/202307031405.dy3druuy-lkp@xxxxxxxxx/
> > > > Signed-off-by: Zhang Rui <rui.zhang@xxxxxxxxx>
> > > > ---
> > > >  drivers/powercap/intel_rapl_tpmi.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/powercap/intel_rapl_tpmi.c
> > > > b/drivers/powercap/intel_rapl_tpmi.c
> > > > index 4f4f13ded225..05f664a096ab 100644
> > > > --- a/drivers/powercap/intel_rapl_tpmi.c
> > > > +++ b/drivers/powercap/intel_rapl_tpmi.c
> > > > @@ -138,7 +138,7 @@ static int parse_one_domain(struct
> > > > tpmi_rapl_package *trp, u32 offset)
> > > >         enum tpmi_rapl_register reg_index;
> > > >         enum rapl_domain_reg_id reg_id;
> > > >         int tpmi_domain_size, tpmi_domain_flags;
> > > > -       u64 *tpmi_rapl_regs = trp->base + offset;
> > > > +       u64 *tpmi_rapl_regs = (u64 *)(trp->base + offset);
> > > >         u64 tpmi_domain_header = readq((void __iomem
> > > > *)tpmi_rapl_regs);
> > 
> > Isn't the correct fix to add __iomem to the definition of
> > tpmi_rapi_regs and also remove the cast from the following
> > readq() line?
> 
> Good point.
> 
> Rui, can you take another look at this, please?