Patchwork [net-next] gianfar: use more portable i/o accessors

login
register
mail settings
Submitter Kim Phillips
Date Jan. 11, 2013, 10:18 p.m.
Message ID <20130111161821.d62a0a60ee8d1005fb14b4d5@freescale.com>
Download mbox | patch
Permalink /patch/211424/
State Accepted
Delegated to: David Miller
Headers show

Comments

Kim Phillips - Jan. 11, 2013, 10:18 p.m.
in/out_be32 accessors are Power arch centric whereas
ioread/writebe32 are available in other arches.  Also, unlike
in/out_be32, ioread/writebe32 expect non-volatile address arguments.

Signed-off-by: Kim Phillips <kim.phillips@freescale.com>
---
 drivers/net/ethernet/freescale/gianfar.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
David Miller - Jan. 11, 2013, 11:59 p.m.
From: Kim Phillips <kim.phillips@freescale.com>
Date: Fri, 11 Jan 2013 16:18:21 -0600

> in/out_be32 accessors are Power arch centric whereas
> ioread/writebe32 are available in other arches.  Also, unlike
> in/out_be32, ioread/writebe32 expect non-volatile address arguments.
> 
> Signed-off-by: Kim Phillips <kim.phillips@freescale.com>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tabi Timur-B04825 - Jan. 12, 2013, 1:44 p.m.
On Fri, Jan 11, 2013 at 4:18 PM, Kim Phillips
<kim.phillips@freescale.com> wrote:
> in/out_be32 accessors are Power arch centric whereas
> ioread/writebe32 are available in other arches.  Also, unlike
> in/out_be32, ioread/writebe32 expect non-volatile address arguments.

I was under the impression that the "volatile" in in/out_be32() is so
that the functions can accept a volatile pointer, not that it expects
one.  Otherwise, if you pass in a volatile, you'll get a compiler
warning.

>
> Signed-off-by: Kim Phillips <kim.phillips@freescale.com>
> ---
>  drivers/net/ethernet/freescale/gianfar.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h
> index 1b6a67c..91bb2de 100644
> --- a/drivers/net/ethernet/freescale/gianfar.h
> +++ b/drivers/net/ethernet/freescale/gianfar.h
> @@ -1136,16 +1136,16 @@ static inline int gfar_has_errata(struct gfar_private *priv,
>         return priv->errata & err;
>  }
>
> -static inline u32 gfar_read(volatile unsigned __iomem *addr)
> +static inline u32 gfar_read(unsigned __iomem *addr)
>  {
>         u32 val;
> -       val = in_be32(addr);
> +       val = ioread32be(addr);
>         return val;
>  }

Can't we just get rid of these functions altogether?  Or at least, get
rid of the local variable?
Richard Cochran - Jan. 12, 2013, 3:57 p.m.
On Sat, Jan 12, 2013 at 01:44:43PM +0000, Tabi Timur-B04825 wrote:
> 
> Can't we just get rid of these functions altogether?

Since they are already in place, I would leave them there. Watching
the kernel development over time, every few years these IO access
idioms tend to change form, and having helper functions avoids huge
change sets when updating.

> Or at least, get
> rid of the local variable?

Okay, sure.

Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h
index 1b6a67c..91bb2de 100644
--- a/drivers/net/ethernet/freescale/gianfar.h
+++ b/drivers/net/ethernet/freescale/gianfar.h
@@ -1136,16 +1136,16 @@  static inline int gfar_has_errata(struct gfar_private *priv,
 	return priv->errata & err;
 }
 
-static inline u32 gfar_read(volatile unsigned __iomem *addr)
+static inline u32 gfar_read(unsigned __iomem *addr)
 {
 	u32 val;
-	val = in_be32(addr);
+	val = ioread32be(addr);
 	return val;
 }
 
-static inline void gfar_write(volatile unsigned __iomem *addr, u32 val)
+static inline void gfar_write(unsigned __iomem *addr, u32 val)
 {
-	out_be32(addr, val);
+	iowrite32be(val, addr);
 }
 
 static inline void gfar_write_filer(struct gfar_private *priv,