diff mbox series

[03/12] nvmem: mtk-efuse: fix different address space warnings of sparse

Message ID 20171009132641.27169-4-srinivas.kandagatla@linaro.org
State New
Headers show
Series nvmem: patches set-1 for v4.15 | expand

Commit Message

Srinivas Kandagatla Oct. 9, 2017, 1:26 p.m. UTC
From: Masahiro Yamada <yamada.masahiro@socionext.com>


Fix the following sparse warnings:

drivers/nvmem/mtk-efuse.c:24:30: warning: incorrect type in initializer (different address spaces)
drivers/nvmem/mtk-efuse.c:24:30:    expected void [noderef] <asn:2>*base
drivers/nvmem/mtk-efuse.c:24:30:    got void *context
drivers/nvmem/mtk-efuse.c:37:30: warning: incorrect type in initializer (different address spaces)
drivers/nvmem/mtk-efuse.c:37:30:    expected void [noderef] <asn:2>*base
drivers/nvmem/mtk-efuse.c:37:30:    got void *context
drivers/nvmem/mtk-efuse.c:69:23: warning: incorrect type in assignment (different address spaces)
drivers/nvmem/mtk-efuse.c:69:23:    expected void *priv
drivers/nvmem/mtk-efuse.c:69:23:    got void [noderef] <asn:2>*[assigned] base

The type of nvmem_config->priv is (void *), so sparse complains
about assignment of the base address with (void __iomem *) type.

Even if we cast it out, sparse still warns:
warning: cast removes address space of expression

Of course, we can shut up the sparse by marking __force, but a more
correct way is to put the base address into driver private data.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

---
 drivers/nvmem/mtk-efuse.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

-- 
2.11.0

Comments

Greg KH Oct. 20, 2017, 1:34 p.m. UTC | #1
On Mon, Oct 09, 2017 at 03:26:32PM +0200, srinivas.kandagatla@linaro.org wrote:
> From: Masahiro Yamada <yamada.masahiro@socionext.com>

> 

> Fix the following sparse warnings:

> 

> drivers/nvmem/mtk-efuse.c:24:30: warning: incorrect type in initializer (different address spaces)

> drivers/nvmem/mtk-efuse.c:24:30:    expected void [noderef] <asn:2>*base

> drivers/nvmem/mtk-efuse.c:24:30:    got void *context

> drivers/nvmem/mtk-efuse.c:37:30: warning: incorrect type in initializer (different address spaces)

> drivers/nvmem/mtk-efuse.c:37:30:    expected void [noderef] <asn:2>*base

> drivers/nvmem/mtk-efuse.c:37:30:    got void *context

> drivers/nvmem/mtk-efuse.c:69:23: warning: incorrect type in assignment (different address spaces)

> drivers/nvmem/mtk-efuse.c:69:23:    expected void *priv

> drivers/nvmem/mtk-efuse.c:69:23:    got void [noderef] <asn:2>*[assigned] base

> 

> The type of nvmem_config->priv is (void *), so sparse complains

> about assignment of the base address with (void __iomem *) type.

> 

> Even if we cast it out, sparse still warns:

> warning: cast removes address space of expression

> 

> Of course, we can shut up the sparse by marking __force, but a more

> correct way is to put the base address into driver private data.


Ick, no, really?

That's horrid.  Do the __force cast as that is what you are really doing
here, and it is good to see the bad use of it (hint, perhaps the api
needs to be fixed up so you do not have to do that???)



> 

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

> ---

>  drivers/nvmem/mtk-efuse.c | 26 +++++++++++++++++---------

>  1 file changed, 17 insertions(+), 9 deletions(-)

> 

> diff --git a/drivers/nvmem/mtk-efuse.c b/drivers/nvmem/mtk-efuse.c

> index fa7a0f66b37e..c4058b598703 100644

> --- a/drivers/nvmem/mtk-efuse.c

> +++ b/drivers/nvmem/mtk-efuse.c

> @@ -18,15 +18,19 @@

>  #include <linux/nvmem-provider.h>

>  #include <linux/platform_device.h>

>  

> +struct mtk_efuse_priv {

> +	void __iomem *base;

> +};

> +

>  static int mtk_reg_read(void *context,



Make the read call take a __iomem *, not a random void *.  That should
solve this issue, right?

thanks,

greg k-h
Masahiro Yamada Oct. 20, 2017, 1:55 p.m. UTC | #2
2017-10-20 22:34 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
> On Mon, Oct 09, 2017 at 03:26:32PM +0200, srinivas.kandagatla@linaro.org wrote:

>> From: Masahiro Yamada <yamada.masahiro@socionext.com>

>>

>> Fix the following sparse warnings:

>>

>> drivers/nvmem/mtk-efuse.c:24:30: warning: incorrect type in initializer (different address spaces)

>> drivers/nvmem/mtk-efuse.c:24:30:    expected void [noderef] <asn:2>*base

>> drivers/nvmem/mtk-efuse.c:24:30:    got void *context

>> drivers/nvmem/mtk-efuse.c:37:30: warning: incorrect type in initializer (different address spaces)

>> drivers/nvmem/mtk-efuse.c:37:30:    expected void [noderef] <asn:2>*base

>> drivers/nvmem/mtk-efuse.c:37:30:    got void *context

>> drivers/nvmem/mtk-efuse.c:69:23: warning: incorrect type in assignment (different address spaces)

>> drivers/nvmem/mtk-efuse.c:69:23:    expected void *priv

>> drivers/nvmem/mtk-efuse.c:69:23:    got void [noderef] <asn:2>*[assigned] base

>>

>> The type of nvmem_config->priv is (void *), so sparse complains

>> about assignment of the base address with (void __iomem *) type.

>>

>> Even if we cast it out, sparse still warns:

>> warning: cast removes address space of expression

>>

>> Of course, we can shut up the sparse by marking __force, but a more

>> correct way is to put the base address into driver private data.

>

> Ick, no, really?

>

> That's horrid.  Do the __force cast as that is what you are really doing

> here, and it is good to see the bad use of it (hint, perhaps the api

> needs to be fixed up so you do not have to do that???)



Other drivers take private data from the "void *context".

In this driver, the priv just happens to have one member.
What's the matter?




>> diff --git a/drivers/nvmem/mtk-efuse.c b/drivers/nvmem/mtk-efuse.c

>> index fa7a0f66b37e..c4058b598703 100644

>> --- a/drivers/nvmem/mtk-efuse.c

>> +++ b/drivers/nvmem/mtk-efuse.c

>> @@ -18,15 +18,19 @@

>>  #include <linux/nvmem-provider.h>

>>  #include <linux/platform_device.h>

>>

>> +struct mtk_efuse_priv {

>> +     void __iomem *base;

>> +};

>> +

>>  static int mtk_reg_read(void *context,

>

>

> Make the read call take a __iomem *, not a random void *.  That should

> solve this issue, right?

>


NACK.

See drivers/nvmem/imx-iim.c

The ->reg_read hook needs more private data than __iomem *.



-- 
Best Regards
Masahiro Yamada
Greg KH Oct. 20, 2017, 2:49 p.m. UTC | #3
On Fri, Oct 20, 2017 at 10:55:04PM +0900, Masahiro Yamada wrote:
> 2017-10-20 22:34 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:

> > On Mon, Oct 09, 2017 at 03:26:32PM +0200, srinivas.kandagatla@linaro.org wrote:

> >> From: Masahiro Yamada <yamada.masahiro@socionext.com>

> >>

> >> Fix the following sparse warnings:

> >>

> >> drivers/nvmem/mtk-efuse.c:24:30: warning: incorrect type in initializer (different address spaces)

> >> drivers/nvmem/mtk-efuse.c:24:30:    expected void [noderef] <asn:2>*base

> >> drivers/nvmem/mtk-efuse.c:24:30:    got void *context

> >> drivers/nvmem/mtk-efuse.c:37:30: warning: incorrect type in initializer (different address spaces)

> >> drivers/nvmem/mtk-efuse.c:37:30:    expected void [noderef] <asn:2>*base

> >> drivers/nvmem/mtk-efuse.c:37:30:    got void *context

> >> drivers/nvmem/mtk-efuse.c:69:23: warning: incorrect type in assignment (different address spaces)

> >> drivers/nvmem/mtk-efuse.c:69:23:    expected void *priv

> >> drivers/nvmem/mtk-efuse.c:69:23:    got void [noderef] <asn:2>*[assigned] base

> >>

> >> The type of nvmem_config->priv is (void *), so sparse complains

> >> about assignment of the base address with (void __iomem *) type.

> >>

> >> Even if we cast it out, sparse still warns:

> >> warning: cast removes address space of expression

> >>

> >> Of course, we can shut up the sparse by marking __force, but a more

> >> correct way is to put the base address into driver private data.

> >

> > Ick, no, really?

> >

> > That's horrid.  Do the __force cast as that is what you are really doing

> > here, and it is good to see the bad use of it (hint, perhaps the api

> > needs to be fixed up so you do not have to do that???)

> 

> 

> Other drivers take private data from the "void *context".

> 

> In this driver, the priv just happens to have one member.

> What's the matter?

> 

> 

> 

> 

> >> diff --git a/drivers/nvmem/mtk-efuse.c b/drivers/nvmem/mtk-efuse.c

> >> index fa7a0f66b37e..c4058b598703 100644

> >> --- a/drivers/nvmem/mtk-efuse.c

> >> +++ b/drivers/nvmem/mtk-efuse.c

> >> @@ -18,15 +18,19 @@

> >>  #include <linux/nvmem-provider.h>

> >>  #include <linux/platform_device.h>

> >>

> >> +struct mtk_efuse_priv {

> >> +     void __iomem *base;

> >> +};

> >> +

> >>  static int mtk_reg_read(void *context,

> >

> >

> > Make the read call take a __iomem *, not a random void *.  That should

> > solve this issue, right?

> >

> 

> NACK.

> 

> See drivers/nvmem/imx-iim.c

> 

> The ->reg_read hook needs more private data than __iomem *.


Ok, that makes a bit more sense, but it still feels a bit of a hack,
don't you agree?

thanks,

greg k-h
Masahiro Yamada Oct. 20, 2017, 4:08 p.m. UTC | #4
2017-10-20 23:49 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
> On Fri, Oct 20, 2017 at 10:55:04PM +0900, Masahiro Yamada wrote:

>> 2017-10-20 22:34 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:

>> > On Mon, Oct 09, 2017 at 03:26:32PM +0200, srinivas.kandagatla@linaro.org wrote:

>> >> From: Masahiro Yamada <yamada.masahiro@socionext.com>

>> >>

>> >> Fix the following sparse warnings:

>> >>

>> >> drivers/nvmem/mtk-efuse.c:24:30: warning: incorrect type in initializer (different address spaces)

>> >> drivers/nvmem/mtk-efuse.c:24:30:    expected void [noderef] <asn:2>*base

>> >> drivers/nvmem/mtk-efuse.c:24:30:    got void *context

>> >> drivers/nvmem/mtk-efuse.c:37:30: warning: incorrect type in initializer (different address spaces)

>> >> drivers/nvmem/mtk-efuse.c:37:30:    expected void [noderef] <asn:2>*base

>> >> drivers/nvmem/mtk-efuse.c:37:30:    got void *context

>> >> drivers/nvmem/mtk-efuse.c:69:23: warning: incorrect type in assignment (different address spaces)

>> >> drivers/nvmem/mtk-efuse.c:69:23:    expected void *priv

>> >> drivers/nvmem/mtk-efuse.c:69:23:    got void [noderef] <asn:2>*[assigned] base

>> >>

>> >> The type of nvmem_config->priv is (void *), so sparse complains

>> >> about assignment of the base address with (void __iomem *) type.

>> >>

>> >> Even if we cast it out, sparse still warns:

>> >> warning: cast removes address space of expression

>> >>

>> >> Of course, we can shut up the sparse by marking __force, but a more

>> >> correct way is to put the base address into driver private data.

>> >

>> > Ick, no, really?

>> >

>> > That's horrid.  Do the __force cast as that is what you are really doing

>> > here, and it is good to see the bad use of it (hint, perhaps the api

>> > needs to be fixed up so you do not have to do that???)

>>

>>

>> Other drivers take private data from the "void *context".

>>

>> In this driver, the priv just happens to have one member.

>> What's the matter?

>>

>>

>>

>>

>> >> diff --git a/drivers/nvmem/mtk-efuse.c b/drivers/nvmem/mtk-efuse.c

>> >> index fa7a0f66b37e..c4058b598703 100644

>> >> --- a/drivers/nvmem/mtk-efuse.c

>> >> +++ b/drivers/nvmem/mtk-efuse.c

>> >> @@ -18,15 +18,19 @@

>> >>  #include <linux/nvmem-provider.h>

>> >>  #include <linux/platform_device.h>

>> >>

>> >> +struct mtk_efuse_priv {

>> >> +     void __iomem *base;

>> >> +};

>> >> +

>> >>  static int mtk_reg_read(void *context,

>> >

>> >

>> > Make the read call take a __iomem *, not a random void *.  That should

>> > solve this issue, right?

>> >

>>

>> NACK.

>>

>> See drivers/nvmem/imx-iim.c

>>

>> The ->reg_read hook needs more private data than __iomem *.

>

> Ok, that makes a bit more sense, but it still feels a bit of a hack,

> don't you agree?



But, I have no choice but __force.

I disagree with using __force to hide things.

In my opinion, __force should be used only where it is really legitimate.

This driver is not the case, as we can fix it
by writing correct code.


-- 
Best Regards
Masahiro Yamada
diff mbox series

Patch

diff --git a/drivers/nvmem/mtk-efuse.c b/drivers/nvmem/mtk-efuse.c
index fa7a0f66b37e..c4058b598703 100644
--- a/drivers/nvmem/mtk-efuse.c
+++ b/drivers/nvmem/mtk-efuse.c
@@ -18,15 +18,19 @@ 
 #include <linux/nvmem-provider.h>
 #include <linux/platform_device.h>
 
+struct mtk_efuse_priv {
+	void __iomem *base;
+};
+
 static int mtk_reg_read(void *context,
 			unsigned int reg, void *_val, size_t bytes)
 {
-	void __iomem *base = context;
+	struct mtk_efuse_priv *priv = context;
 	u32 *val = _val;
 	int i = 0, words = bytes / 4;
 
 	while (words--)
-		*val++ = readl(base + reg + (i++ * 4));
+		*val++ = readl(priv->base + reg + (i++ * 4));
 
 	return 0;
 }
@@ -34,12 +38,12 @@  static int mtk_reg_read(void *context,
 static int mtk_reg_write(void *context,
 			 unsigned int reg, void *_val, size_t bytes)
 {
-	void __iomem *base = context;
+	struct mtk_efuse_priv *priv = context;
 	u32 *val = _val;
 	int i = 0, words = bytes / 4;
 
 	while (words--)
-		writel(*val++, base + reg + (i++ * 4));
+		writel(*val++, priv->base + reg + (i++ * 4));
 
 	return 0;
 }
@@ -50,19 +54,23 @@  static int mtk_efuse_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct nvmem_device *nvmem;
 	struct nvmem_config econfig = {};
-	void __iomem *base;
+	struct mtk_efuse_priv *priv;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	base = devm_ioremap_resource(dev, res);
-	if (IS_ERR(base))
-		return PTR_ERR(base);
+	priv->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
 
 	econfig.stride = 4;
 	econfig.word_size = 4;
 	econfig.reg_read = mtk_reg_read;
 	econfig.reg_write = mtk_reg_write;
 	econfig.size = resource_size(res);
-	econfig.priv = base;
+	econfig.priv = priv;
 	econfig.dev = dev;
 	econfig.owner = THIS_MODULE;
 	nvmem = nvmem_register(&econfig);