gpio-pch: cleanup __iomem annotation usage

PATCH Superseded Németh Márton 4 participants 9 messages Jan 15, 2012 – Jan 18, 2012

Subsystems (2)

GPIO Intel GPIO drivers

lore

Share & tools

lore

Németh Márton

author

2012-01-15 10:33 UTC #0 lore raw

PATCH 1/2 gpio-pch: cleanup __iomem annotation usage

From: Márton Németh <[email protected]>

The __iomem annotation is to be used together with pointers used as iowrite32() parameter. For more details see [1] and [2].

This patch will remove the following sparse warnings ("make C=1"):
* warning: incorrect type in assignment (different address spaces)
* warning: incorrect type in argument 1 (different address spaces)
* warning: incorrect type in argument 2 (different address spaces)

References:
[1] A new I/O memory access mechanism (Sep 15, 2004)

    http://lwn.net/Articles/102232/

[2] Being more anal about iospace accesses (Sep 15, 2004)

    http://lwn.net/Articles/102240/

Signed-off-by Márton Németh <[email protected]>

---
 drivers/gpio/gpio-pch.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/gpio/gpio-pch.c +1 -1

index f060329..9061b55 100644
@@ -231,7 +231,7 @@ static void pch_gpio_setup(struct pch_gpio *chip)
 static int pch_irq_type(struct irq_data *d, unsigned int type)
 {
 	u32 im;
-	u32 *im_reg;
+	u32 __iomem *im_reg;
 	u32 ien;
 	u32 im_pos;
 	int ch;

Signature · 1 line

1.7.2.5

Linus Walleij

2012-01-17 08:14 UTC #1 lore raw

2012/1/15 Németh Márton <[email protected]>:

Quoted 4 lines

From: Márton Németh <[email protected]>
The __iomem annotation is to be used together with pointers used as iowrite32() parameter. For more details see [1] and [2].

(...)

Quoted 2 lines

- u32 *im_reg;
+ u32 __iomem *im_reg;

Does it work to just say void __iomem *im_reg?

We usually don't type addresses.

Yours,
Linus Walleij

Németh Márton

author

2012-01-17 08:33 UTC #2 lore raw

Hi Linux,

Quoted 6 lines · Linus Walleij

2012/1/15 Németh Márton <[email protected]>:
Quoted 4 lines
From: Márton Németh <[email protected]>
The __iomem annotation is to be used together with pointers used as iowrite32() parameter. For more details see [1] and [2].

Quoted 7 lines

(...)
drivers/gpio/gpio-pch.c static void pch_gpio_setup(struct pch_gpio *chip) · line 234
Quoted 2 lines
drivers/gpio/gpio-pch.c +1 -1
-       u32 *im_reg;
+       u32 __iomem *im_reg;
Does it work to just say void __iomem *im_reg?
We usually don't type addresses.

You are right, the correct one should be "void __iomem *im_reg;" .

Should I resend a patch for this?

Márton Németh

Linus Walleij

2012-01-17 09:01 UTC #3 lore raw

2012/1/17 Németh Márton <[email protected]>:

Quoted 1 line

Hi Linux,

:-)

Common mistake, as are our brother Kubys who also
appear on the lists sometimes...

Quoted 3 lines

You are right, the correct one should be "void __iomem *im_reg;" .
Should I resend a patch for this?

Yep just send it with subject [PATCH 1/2 v2] atleast that's what I usually do.

Yours,
Linus Walleij

Németh Márton

author

2012-01-17 20:44 UTC #4 lore raw

PATCH v2 1/2 gpio-pch: cleanup __iomem annotation usage

From: Márton Németh <[email protected]>

The __iomem annotation is to be used together with pointers used as iowrite32() parameter. For more details see [1] and [2].

References:
[1] A new I/O memory access mechanism (Sep 15, 2004)

    http://lwn.net/Articles/102232/

[2] Being more anal about iospace accesses (Sep 15, 2004)

    http://lwn.net/Articles/102240/

Signed-off-by Márton Németh <[email protected]>

Cc Grant Likely <[email protected]>

Cc Linus Walleij <[email protected]>

---
 drivers/gpio/gpio-pch.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/gpio/gpio-pch.c +1 -1

index f060329..8f101b3 100644
@@ -231,7 +231,7 @@ static void pch_gpio_setup(struct pch_gpio *chip)
 static int pch_irq_type(struct irq_data *d, unsigned int type)
 {
 	u32 im;
-	u32 *im_reg;
+	void __iomem *im_reg;
 	u32 ien;
 	u32 im_pos;
 	int ch;

Signature · 1 line

1.7.2.5

Grant Likely

2012-01-17 22:50 UTC #5 lore raw

In reply to Linus Walleij

2012/1/17 Linus Walleij <[email protected]>:

Quoted 9 lines

2012/1/17 Németh Márton <[email protected]>:
Quoted 1 line
Hi Linux,
:-)
Common mistake, as are our brother Kubys who also
appear on the lists sometimes...

Quoted 6 lines

Quoted 3 lines
You are right, the correct one should be "void __iomem *im_reg;" .
Should I resend a patch for this?
Yep just send it with subject [PATCH 1/2 v2] atleast that's what I usually do.

Actually, I've already applied it. Send a follow-on patch. It is logically a separate change anyway since the existing code already uses u32.

Németh Márton

author

2012-01-18 06:06 UTC #6 lore raw

PATCH] use void __iomem * pointers for ioread32()/iowrite32()

From: Márton Németh <[email protected]>

The address pointer of ioread32()/iowrite32() functions are of type "void __iomem *". Use matching variable type.

For details on __iomem see [1] and [2].

References:
[1] A new I/O memory access mechanism (Sep 15, 2004)

    http://lwn.net/Articles/102232/

[2] Being more anal about iospace accesses (Sep 15, 2004)

    http://lwn.net/Articles/102240/

Signed-off-by Márton Németh <[email protected]>

Cc Grant Likely <[email protected]>

Cc Linus Walleij <[email protected]>

---
 drivers/gpio/gpio-pch.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/gpio/gpio-pch.c +1 -1

index 68fa55e..84b7f28 100644
@@ -231,7 +231,7 @@ static void pch_gpio_setup(struct pch_gpio *chip)
 static int pch_irq_type(struct irq_data *d, unsigned int type)
 {
 	u32 im;
-	u32 __iomem *im_reg;
+	void __iomem *im_reg;
 	u32 ien;
 	u32 im_pos;
 	int ch;

Signature · 1 line

1.7.2.5

Al Viro

2012-01-18 06:18 UTC #7 lore raw

Re: PATCH] use void __iomem * pointers for ioread32()/iowrite32()

Quoted 10 lines · N?meth M?rton

From: M?rton N?meth <[email protected]>
The address pointer of ioread32()/iowrite32() functions are of type "void __iomem *". Use matching variable type.
drivers/gpio/gpio-pch.c +0 -0
index 68fa55e..84b7f28 100644

drivers/gpio/gpio-pch.c static void pch_gpio_setup(struct pch_gpio *chip) · line 234

Quoted 5 lines

drivers/gpio/gpio-pch.c +1 -1

@@ -231,7 +231,7 @@ static void pch_gpio_setup(struct pch_gpio *chip)
 static int pch_irq_type(struct irq_data *d, unsigned int type)
 {
 	u32 im;
-	u32 __iomem *im_reg;
+	void __iomem *im_reg;

NAK. Please, learn what "function with pointer to (possibly qualified) void" means. It's fairly basic C, so any (post-K&R) textbook would do.

Grant Likely

2012-01-18 21:07 UTC #8 lore raw

Re: PATCH] use void __iomem * pointers for ioread32()/iowrite32()

Quoted 6 lines · Al Viro

Quoted 4 lines · N?meth M?rton
From: M?rton N?meth <[email protected]>
The address pointer of ioread32()/iowrite32() functions are of type "void __iomem *". Use matching variable type.

Quoted 13 lines

drivers/gpio/gpio-pch.c static void pch_gpio_setup(struct pch_gpio *chip) · line 234
Quoted 10 lines
drivers/gpio/gpio-pch.c +1 -1
index 68fa55e..84b7f28 100644
@@ -231,7 +231,7 @@ static void pch_gpio_setup(struct pch_gpio *chip)
 static int pch_irq_type(struct irq_data *d, unsigned int type)
 {
 	u32 im;
-	u32 __iomem *im_reg;
+	void __iomem *im_reg;
NAK. Please, learn what "function with pointer to (possibly qualified) void" means. It's fairly basic C, so any (post-K&R) textbook would do.

Okay, ignoring.