diff mbox series

powercap: intel_rapl_tpmi: Fix a sparse warning

Message ID	20230704050238.712223-1-rui.zhang@intel.com
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; From: Zhang Rui <rui.zhang@intel.com> To: linux-pm@vger.kernel.org, rafael.j.wysocki@intel.com Cc: daniel.lezcano@linaro.org, linux-kernel@vger.kernel.org, lkp@intel.com Subject: [PATCH] powercap: intel_rapl_tpmi: Fix a sparse warning Date: Tue, 4 Jul 2023 13:02:38 +0800 Message-Id: <20230704050238.712223-1-rui.zhang@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	powercap: intel_rapl_tpmi: Fix a sparse warning \| powercap: intel_rapl_tpmi: Fix a sparse warning

Commit Message

Zhang, Rui July 4, 2023, 5:02 a.m. UTC

  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@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202307031405.dy3druuy-lkp@intel.com/
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/powercap/intel_rapl_tpmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Rafael J. Wysocki July 11, 2023, 6:14 p.m. UTC | #1

On Tue, Jul 4, 2023 at 7:02 AM Zhang Rui <rui.zhang@intel.com> 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@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202307031405.dy3druuy-lkp@intel.com/
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  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);
>
>         /* Domain Parent bits are ignored for now */
> --

Applied as 6.5-rc material, thanks!

David Laight July 14, 2023, 1:33 p.m. UTC | #2

From: Rafael J. Wysocki
> Sent: 11 July 2023 19:14
...
> On Tue, Jul 4, 2023 at 7:02 AM Zhang Rui <rui.zhang@intel.com> 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@intel.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202307031405.dy3druuy-lkp@intel.com/
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > ---
> >  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?

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Rafael J. Wysocki July 14, 2023, 2:39 p.m. UTC | #3

On Fri, Jul 14, 2023 at 3:34 PM David Laight <David.Laight@aculab.com> wrote:
>
> From: Rafael J. Wysocki
> > Sent: 11 July 2023 19:14
> ...
> > On Tue, Jul 4, 2023 at 7:02 AM Zhang Rui <rui.zhang@intel.com> 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@intel.com>
> > > Closes: https://lore.kernel.org/oe-kbuild-all/202307031405.dy3druuy-lkp@intel.com/
> > > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > > ---
> > >  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?

Zhang, Rui July 17, 2023, 9:13 a.m. UTC | #4

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@aculab.com> wrote:
> > 
> > From: Rafael J. Wysocki
> > > Sent: 11 July 2023 19:14
> > ...
> > > On Tue, Jul 4, 2023 at 7:02 AM Zhang Rui <rui.zhang@intel.com>
> > > 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@intel.com>
> > > > Closes:
> > > > https://lore.kernel.org/oe-kbuild-all/202307031405.dy3druuy-lkp@intel.com/
> > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > > > ---
> > > >  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?

David Laight July 17, 2023, 9:31 a.m. UTC | #5

From: Zhang, Rui
> Sent: 17 July 2023 10:13
> 
> 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?

To avoid those issues (which seem different from the one here)
you could use a union of the __iomem address and the MSR number.

That ought to let you remove all the __iomem casts.

	David

> 
> 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@aculab.com> wrote:
> > >
> > > From: Rafael J. Wysocki
> > > > Sent: 11 July 2023 19:14
> > > ...
> > > > On Tue, Jul 4, 2023 at 7:02 AM Zhang Rui <rui.zhang@intel.com>
> > > > 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@intel.com>
> > > > > Closes:
> > > > > https://lore.kernel.org/oe-kbuild-all/202307031405.dy3druuy-lkp@intel.com/
> > > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > > > > ---
> > > > >  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?

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

diff mbox series

Patch

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);
 
 	/* Domain Parent bits are ignored for now */