All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jean Delvare <jdelvare@suse.de>
To: Tiezhu Yang <yangtiezhu@loongson.cn>
Cc: kernel test robot <lkp@intel.com>,
	kbuild-all@lists.01.org, linux-kernel@vger.kernel.org,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Subject: Re: drivers/firmware/dmi_scan.c:151:9: sparse: sparse: incorrect type in argument 1 (different address spaces)
Date: Fri, 30 Jul 2021 14:59:53 +0200	[thread overview]
Message-ID: <20210730145953.43308750@endymion> (raw)
In-Reply-To: <20210729164533.48fed217@endymion>

On Thu, 29 Jul 2021 16:45:33 +0200, Jean Delvare wrote:
> On Mon, 26 Jul 2021 20:53:42 +0800, Tiezhu Yang wrote:
> > I think the following change can make the above warning silent:
> > 
> > diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> > index f191a1f..9e254d9 100644
> > --- a/drivers/firmware/dmi_scan.c
> > +++ b/drivers/firmware/dmi_scan.c
> > @@ -148,7 +148,7 @@ static int __init dmi_walk_early(void 
> > (*decode)(const struct dmi_header *,
> > 
> >          add_device_randomness(buf, dmi_len);
> > 
> > -       dmi_early_unmap(buf, orig_dmi_len);
> > +       dmi_early_unmap((u8 __iomem *)buf, orig_dmi_len);
> >          return 0;
> >   }
> > 
> > If it is OK, I can send a patch later.  
> 
> Explicit pointer casting is almost always the wrong way to make
> warnings go away. I can't confirm because I'm not able to get sparse to
> work at the moment, but more likely the correct fix would be something
> along the lines of:
> 
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -137,7 +137,7 @@ static phys_addr_t dmi_base;
>  static int __init dmi_walk_early(void (*decode)(const struct dmi_header *,
>  		void *))
>  {
> -	u8 *buf;
> +	u8 __iomem *buf;
>  	u32 orig_dmi_len = dmi_len;
>  
>  	buf = dmi_early_remap(dmi_base, orig_dmi_len);
> @@ -754,7 +754,7 @@ static BIN_ATTR(DMI, S_IRUSR, raw_table_
>  static int __init dmi_init(void)
>  {
>  	struct kobject *tables_kobj;
> -	u8 *dmi_table;
> +	u8 __iomem *dmi_table;
>  	int ret = -ENOMEM;
>  
>  	if (!dmi_available)
> @@ -1101,7 +1101,7 @@ EXPORT_SYMBOL(dmi_get_bios_year);
>  int dmi_walk(void (*decode)(const struct dmi_header *, void *),
>  	     void *private_data)
>  {
> -	u8 *buf;
> +	u8 __iomem *buf;
>  
>  	if (!dmi_available)
>  		return -ENXIO;
> 
> (Note sure why there are only 2 sparse warnings when the same issue is
> present 3 times in the file.)
> 

I took a deeper look at the code. There's currently no easy way to fix
these sparse warnings because different architectures have different
prototypes for dmi_early_remap() and dmi_early_unmap(). Some have
__iomem and some do not. So, for the time being, fixing warnings on
some architectures would introduce new warnings on other architectures.

I'd rather leave the code as is until this is sorted out (if it can be).

-- 
Jean Delvare
SUSE L3 Support

WARNING: multiple messages have this Message-ID (diff)
From: Jean Delvare <jdelvare@suse.de>
To: kbuild-all@lists.01.org
Subject: Re: drivers/firmware/dmi_scan.c:151:9: sparse: sparse: incorrect type in argument 1 (different address spaces)
Date: Fri, 30 Jul 2021 14:59:53 +0200	[thread overview]
Message-ID: <20210730145953.43308750@endymion> (raw)
In-Reply-To: <20210729164533.48fed217@endymion>

[-- Attachment #1: Type: text/plain, Size: 2417 bytes --]

On Thu, 29 Jul 2021 16:45:33 +0200, Jean Delvare wrote:
> On Mon, 26 Jul 2021 20:53:42 +0800, Tiezhu Yang wrote:
> > I think the following change can make the above warning silent:
> > 
> > diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> > index f191a1f..9e254d9 100644
> > --- a/drivers/firmware/dmi_scan.c
> > +++ b/drivers/firmware/dmi_scan.c
> > @@ -148,7 +148,7 @@ static int __init dmi_walk_early(void 
> > (*decode)(const struct dmi_header *,
> > 
> >          add_device_randomness(buf, dmi_len);
> > 
> > -       dmi_early_unmap(buf, orig_dmi_len);
> > +       dmi_early_unmap((u8 __iomem *)buf, orig_dmi_len);
> >          return 0;
> >   }
> > 
> > If it is OK, I can send a patch later.  
> 
> Explicit pointer casting is almost always the wrong way to make
> warnings go away. I can't confirm because I'm not able to get sparse to
> work at the moment, but more likely the correct fix would be something
> along the lines of:
> 
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -137,7 +137,7 @@ static phys_addr_t dmi_base;
>  static int __init dmi_walk_early(void (*decode)(const struct dmi_header *,
>  		void *))
>  {
> -	u8 *buf;
> +	u8 __iomem *buf;
>  	u32 orig_dmi_len = dmi_len;
>  
>  	buf = dmi_early_remap(dmi_base, orig_dmi_len);
> @@ -754,7 +754,7 @@ static BIN_ATTR(DMI, S_IRUSR, raw_table_
>  static int __init dmi_init(void)
>  {
>  	struct kobject *tables_kobj;
> -	u8 *dmi_table;
> +	u8 __iomem *dmi_table;
>  	int ret = -ENOMEM;
>  
>  	if (!dmi_available)
> @@ -1101,7 +1101,7 @@ EXPORT_SYMBOL(dmi_get_bios_year);
>  int dmi_walk(void (*decode)(const struct dmi_header *, void *),
>  	     void *private_data)
>  {
> -	u8 *buf;
> +	u8 __iomem *buf;
>  
>  	if (!dmi_available)
>  		return -ENXIO;
> 
> (Note sure why there are only 2 sparse warnings when the same issue is
> present 3 times in the file.)
> 

I took a deeper look at the code. There's currently no easy way to fix
these sparse warnings because different architectures have different
prototypes for dmi_early_remap() and dmi_early_unmap(). Some have
__iomem and some do not. So, for the time being, fixing warnings on
some architectures would introduce new warnings on other architectures.

I'd rather leave the code as is until this is sorted out (if it can be).

-- 
Jean Delvare
SUSE L3 Support

  reply	other threads:[~2021-07-30 12:59 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-24 14:31 drivers/firmware/dmi_scan.c:151:9: sparse: sparse: incorrect type in argument 1 (different address spaces) kernel test robot
2021-07-24 14:31 ` kernel test robot
2021-07-26 12:53 ` Tiezhu Yang
2021-07-26 12:53   ` Tiezhu Yang
2021-07-29 14:45   ` Jean Delvare
2021-07-29 14:45     ` Jean Delvare
2021-07-30 12:59     ` Jean Delvare [this message]
2021-07-30 12:59       ` Jean Delvare

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210730145953.43308750@endymion \
    --to=jdelvare@suse.de \
    --cc=kbuild-all@lists.01.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lkp@intel.com \
    --cc=tsbogend@alpha.franken.de \
    --cc=yangtiezhu@loongson.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.