diff mbox

[U-Boot,15/28] net/memac_phy: reuse driver for little endian SoCs

Message ID 1426783559-26610-15-git-send-email-yorksun@freescale.com
State Superseded
Delegated to: York Sun
Headers show

Commit Message

York Sun March 19, 2015, 4:45 p.m. UTC 
From: Shaohui Xie <Shaohui.Xie@freescale.com>

The memac for PHY management on little endian SoCs is similar on big
endian SoCs, so we modify the driver by using I/O accessor function to
handle the endianness, so the driver can be reused on little endian
SoCs, we introduce CONFIG_SYS_MEMAC_LITTLE_ENDIAN for little endian
SoCs, if the CONFIG_SYS_MEMAC_LITTLE_ENDIAN is defined, the I/O access
is little endian, if not, the I/O access is big endian. Move fsl_memac.h
out of powerpc include.

Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
CC: Joe Hershberger <joe.hershberger@ni.com>
---
 arch/arm/include/asm/arch-fsl-lsch3/config.h      |    1 +
 drivers/net/Makefile                              |    1 +
 drivers/net/fm/eth.c                              |    2 +-
 drivers/net/fm/memac.c                            |    2 +-
 drivers/net/fm/memac_phy.c                        |   62 ++++++++++++++-------
 drivers/net/vsc9953.c                             |    2 +-
 {arch/powerpc/include/asm => include}/fsl_memac.h |    0
 7 files changed, 46 insertions(+), 24 deletions(-)
 rename {arch/powerpc/include/asm => include}/fsl_memac.h (100%)

Comments

Joe Hershberger March 19, 2015, 6:03 p.m. UTC | #1 
Hi Shaohui Xie,

On Thu, Mar 19, 2015 at 11:45 AM, York Sun <yorksun@freescale.com> wrote:
>
> From: Shaohui Xie <Shaohui.Xie@freescale.com>
>
> The memac for PHY management on little endian SoCs is similar on big
> endian SoCs, so we modify the driver by using I/O accessor function to
> handle the endianness, so the driver can be reused on little endian
> SoCs, we introduce CONFIG_SYS_MEMAC_LITTLE_ENDIAN for little endian
> SoCs, if the CONFIG_SYS_MEMAC_LITTLE_ENDIAN is defined, the I/O access
> is little endian, if not, the I/O access is big endian. Move fsl_memac.h
> out of powerpc include.
>
> Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
> CC: Joe Hershberger <joe.hershberger@ni.com>
> ---
>  arch/arm/include/asm/arch-fsl-lsch3/config.h      |    1 +
>  drivers/net/Makefile                              |    1 +
>  drivers/net/fm/eth.c                              |    2 +-
>  drivers/net/fm/memac.c                            |    2 +-
>  drivers/net/fm/memac_phy.c                        |   62
++++++++++++++-------
>  drivers/net/vsc9953.c                             |    2 +-
>  {arch/powerpc/include/asm => include}/fsl_memac.h |    0
>  7 files changed, 46 insertions(+), 24 deletions(-)
>  rename {arch/powerpc/include/asm => include}/fsl_memac.h (100%)
>
> diff --git a/arch/arm/include/asm/arch-fsl-lsch3/config.h
b/arch/arm/include/asm/arch-fsl-lsch3/config.h
> index 98db1ef..684c70f 100644
> --- a/arch/arm/include/asm/arch-fsl-lsch3/config.h
> +++ b/arch/arm/include/asm/arch-fsl-lsch3/config.h
> @@ -109,6 +109,7 @@
>
>  /* IFC */
>  #define CONFIG_SYS_FSL_IFC_LE
> +#define CONFIG_SYS_MEMAC_LITTLE_ENDIAN

It seems tedious to have to define this. Can't you just use the functions
available?

>  /* PCIe */
>  #define CONFIG_SYS_PCIE1_ADDR                  (CONFIG_SYS_IMMR +
0x2400000)
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> index 5497934..d871093 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -66,4 +66,5 @@ obj-$(CONFIG_XILINX_LL_TEMAC) += xilinx_ll_temac.o
xilinx_ll_temac_mdio.o \
>  obj-$(CONFIG_ZYNQ_GEM) += zynq_gem.o
>  obj-$(CONFIG_FSL_MC_ENET) += fsl-mc/
>  obj-$(CONFIG_FSL_MC_ENET) += ldpaa_eth/
> +obj-$(CONFIG_FSL_MEMAC) += fm/memac_phy.o
>  obj-$(CONFIG_VSC9953) += vsc9953.o
> diff --git a/drivers/net/fm/eth.c b/drivers/net/fm/eth.c
> index 1d1089d..a7a5c69 100644
> --- a/drivers/net/fm/eth.c
> +++ b/drivers/net/fm/eth.c
> @@ -15,7 +15,7 @@
>  #include <phy.h>
>  #include <asm/fsl_dtsec.h>
>  #include <asm/fsl_tgec.h>
> -#include <asm/fsl_memac.h>
> +#include <fsl_memac.h>
>
>  #include "fm.h"
>
> diff --git a/drivers/net/fm/memac.c b/drivers/net/fm/memac.c
> index 60e898c..81a64bf 100644
> --- a/drivers/net/fm/memac.c
> +++ b/drivers/net/fm/memac.c
> @@ -12,7 +12,7 @@
>  #include <phy.h>
>  #include <asm/types.h>
>  #include <asm/io.h>
> -#include <asm/fsl_memac.h>
> +#include <fsl_memac.h>
>
>  #include "fm.h"
>
> diff --git a/drivers/net/fm/memac_phy.c b/drivers/net/fm/memac_phy.c
> index a155d89..4ab78e6 100644
> --- a/drivers/net/fm/memac_phy.c
> +++ b/drivers/net/fm/memac_phy.c
> @@ -10,9 +10,28 @@
>  #include <miiphy.h>
>  #include <phy.h>
>  #include <asm/io.h>
> -#include <asm/fsl_memac.h>
> +#include <fsl_memac.h>
>  #include <fm_eth.h>
>
> +#ifdef CONFIG_SYS_MEMAC_LITTLE_ENDIAN

This can already be detected, right?

#if __BYTE_ORDER == __LITTLE_ENDIAN

> +#define memac_out_32(a, v)     out_le32(a, v)
> +#define memac_clrbits_32(a, v) clrbits_le32(a, v)
> +#define memac_setbits_32(a, v) setbits_le32(a, v)
> +#else
> +#define memac_out_32(a, v)     out_be32(a, v)
> +#define memac_clrbits_32(a, v) clrbits_be32(a, v)
> +#define memac_setbits_32(a, v) setbits_be32(a, v)
> +#endif
> +
> +static u32 memac_in_32(u32 *reg)
> +{
> +#ifdef CONFIG_SYS_MEMAC_LITTLE_ENDIAN
> +       return in_le32(reg);
> +#else
> +       return in_be32(reg);
> +#endif
> +}

Another option you have is to take the approach that you don't care the
endianness. Something like using the this type of pattern:

value = ntohl(in_be32(reg));
out_be32(reg, htonl(value));

> +
>  /*
>   * Write value to the PHY for this device to the register at regnum,
waiting
>   * until the write is done before it returns.  All PHY configuration has
to be
> @@ -28,31 +47,31 @@ int memac_mdio_write(struct mii_dev *bus, int
port_addr, int dev_addr,
>         if (dev_addr == MDIO_DEVAD_NONE) {
>                 c45 = 0; /* clause 22 */
>                 dev_addr = regnum & 0x1f;
> -               clrbits_be32(&regs->mdio_stat, MDIO_STAT_ENC);
> +               memac_clrbits_32(&regs->mdio_stat, MDIO_STAT_ENC);
>         } else
> -               setbits_be32(&regs->mdio_stat, MDIO_STAT_ENC);
> +               memac_setbits_32(&regs->mdio_stat, MDIO_STAT_ENC);
>
>         /* Wait till the bus is free */
> -       while ((in_be32(&regs->mdio_stat)) & MDIO_STAT_BSY)
> +       while ((memac_in_32(&regs->mdio_stat)) & MDIO_STAT_BSY)
>                 ;
>
>         /* Set the port and dev addr */
>         mdio_ctl = MDIO_CTL_PORT_ADDR(port_addr) |
MDIO_CTL_DEV_ADDR(dev_addr);
> -       out_be32(&regs->mdio_ctl, mdio_ctl);
> +       memac_out_32(&regs->mdio_ctl, mdio_ctl);
>
>         /* Set the register address */
>         if (c45)
> -               out_be32(&regs->mdio_addr, regnum & 0xffff);
> +               memac_out_32(&regs->mdio_addr, regnum & 0xffff);
>
>         /* Wait till the bus is free */
> -       while ((in_be32(&regs->mdio_stat)) & MDIO_STAT_BSY)
> +       while ((memac_in_32(&regs->mdio_stat)) & MDIO_STAT_BSY)
>                 ;
>
>         /* Write the value to the register */
> -       out_be32(&regs->mdio_data, MDIO_DATA(value));
> +       memac_out_32(&regs->mdio_data, MDIO_DATA(value));
>
>         /* Wait till the MDIO write is complete */
> -       while ((in_be32(&regs->mdio_data)) & MDIO_DATA_BSY)
> +       while ((memac_in_32(&regs->mdio_data)) & MDIO_DATA_BSY)
>                 ;
>
>         return 0;
> @@ -75,39 +94,39 @@ int memac_mdio_read(struct mii_dev *bus, int
port_addr, int dev_addr,
>                         return 0xffff;
>                 c45 = 0; /* clause 22 */
>                 dev_addr = regnum & 0x1f;
> -               clrbits_be32(&regs->mdio_stat, MDIO_STAT_ENC);
> +               memac_clrbits_32(&regs->mdio_stat, MDIO_STAT_ENC);
>         } else
> -               setbits_be32(&regs->mdio_stat, MDIO_STAT_ENC);
> +               memac_setbits_32(&regs->mdio_stat, MDIO_STAT_ENC);
>
>         /* Wait till the bus is free */
> -       while ((in_be32(&regs->mdio_stat)) & MDIO_STAT_BSY)
> +       while ((memac_in_32(&regs->mdio_stat)) & MDIO_STAT_BSY)
>                 ;
>
>         /* Set the Port and Device Addrs */
>         mdio_ctl = MDIO_CTL_PORT_ADDR(port_addr) |
MDIO_CTL_DEV_ADDR(dev_addr);
> -       out_be32(&regs->mdio_ctl, mdio_ctl);
> +       memac_out_32(&regs->mdio_ctl, mdio_ctl);
>
>         /* Set the register address */
>         if (c45)
> -               out_be32(&regs->mdio_addr, regnum & 0xffff);
> +               memac_out_32(&regs->mdio_addr, regnum & 0xffff);
>
>         /* Wait till the bus is free */
> -       while ((in_be32(&regs->mdio_stat)) & MDIO_STAT_BSY)
> +       while ((memac_in_32(&regs->mdio_stat)) & MDIO_STAT_BSY)
>                 ;
>
>         /* Initiate the read */
>         mdio_ctl |= MDIO_CTL_READ;
> -       out_be32(&regs->mdio_ctl, mdio_ctl);
> +       memac_out_32(&regs->mdio_ctl, mdio_ctl);
>
>         /* Wait till the MDIO write is complete */
> -       while ((in_be32(&regs->mdio_data)) & MDIO_DATA_BSY)
> +       while ((memac_in_32(&regs->mdio_data)) & MDIO_DATA_BSY)
>                 ;
>
>         /* Return all Fs if nothing was there */
> -       if (in_be32(&regs->mdio_stat) & MDIO_STAT_RD_ER)
> +       if (memac_in_32(&regs->mdio_stat) & MDIO_STAT_RD_ER)
>                 return 0xffff;
>
> -       return in_be32(&regs->mdio_data) & 0xffff;
> +       return memac_in_32(&regs->mdio_data) & 0xffff;
>  }
>
>  int memac_mdio_reset(struct mii_dev *bus)
> @@ -143,8 +162,9 @@ int fm_memac_mdio_init(bd_t *bis, struct
memac_mdio_info *info)
>          * like T2080QDS, this bit default is '0', which leads to MDIO
failure
>          * on XAUI PHY, so set this bit definitely.
>          */
> -       setbits_be32(&((struct memac_mdio_controller
*)info->regs)->mdio_stat,
> -                    MDIO_STAT_CLKDIV(258) | MDIO_STAT_NEG);
> +       memac_setbits_32(
> +               &((struct memac_mdio_controller *)info->regs)->mdio_stat,
> +               MDIO_STAT_CLKDIV(258) | MDIO_STAT_NEG);
>
>         return mdio_register(bus);
>  }
> diff --git a/drivers/net/vsc9953.c b/drivers/net/vsc9953.c
> index 9fc3c18..fed7358 100644
> --- a/drivers/net/vsc9953.c
> +++ b/drivers/net/vsc9953.c
> @@ -9,7 +9,7 @@
>  #include <asm/io.h>
>  #include <asm/fsl_serdes.h>
>  #include <fm_eth.h>
> -#include <asm/fsl_memac.h>
> +#include <fsl_memac.h>
>  #include <vsc9953.h>
>
>  static struct vsc9953_info vsc9953_l2sw = {
> diff --git a/arch/powerpc/include/asm/fsl_memac.h b/include/fsl_memac.h
> similarity index 100%
> rename from arch/powerpc/include/asm/fsl_memac.h
> rename to include/fsl_memac.h
> --
> 1.7.9.5
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
shaohui xie March 20, 2015, 3:06 a.m. UTC | #2 
Hello Joe,

Thank you for reviewing this patch!
Please see inline.

Best Regards,
Shaohui Xie

From: Joe Hershberger [mailto:joe.hershberger@gmail.com]

Sent: Friday, March 20, 2015 2:04 AM
To: Sun York-R58495
Cc: u-boot; Joe Hershberger; Xie Shaohui-B21989
Subject: Re: [U-Boot] [PATCH 15/28] net/memac_phy: reuse driver for little endian SoCs

Hi Shaohui Xie,

On Thu, Mar 19, 2015 at 11:45 AM, York Sun <yorksun@freescale.com<mailto:yorksun@freescale.com>> wrote:
>

> From: Shaohui Xie <Shaohui.Xie@freescale.com<mailto:Shaohui.Xie@freescale.com>>

>

> The memac for PHY management on little endian SoCs is similar on big

> endian SoCs, so we modify the driver by using I/O accessor function to

> handle the endianness, so the driver can be reused on little endian

> SoCs, we introduce CONFIG_SYS_MEMAC_LITTLE_ENDIAN for little endian

> SoCs, if the CONFIG_SYS_MEMAC_LITTLE_ENDIAN is defined, the I/O access

> is little endian, if not, the I/O access is big endian. Move fsl_memac.h

> out of powerpc include.

>

> Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com<mailto:Shaohui.Xie@freescale.com>>

> CC: Joe Hershberger <joe.hershberger@ni.com<mailto:joe.hershberger@ni.com>>

> ---

>  arch/arm/include/asm/arch-fsl-lsch3/config.h      |    1 +

>  drivers/net/Makefile                              |    1 +

>  drivers/net/fm/eth.c                              |    2 +-

>  drivers/net/fm/memac.c                            |    2 +-

>  drivers/net/fm/memac_phy.c                        |   62 ++++++++++++++-------

>  drivers/net/vsc9953.c                             |    2 +-

>  {arch/powerpc/include/asm => include}/fsl_memac.h |    0

>  7 files changed, 46 insertions(+), 24 deletions(-)

>  rename {arch/powerpc/include/asm => include}/fsl_memac.h (100%)

>

> diff --git a/arch/arm/include/asm/arch-fsl-lsch3/config.h b/arch/arm/include/asm/arch-fsl-lsch3/config.h

> index 98db1ef..684c70f 100644

> --- a/arch/arm/include/asm/arch-fsl-lsch3/config.h

> +++ b/arch/arm/include/asm/arch-fsl-lsch3/config.h

> @@ -109,6 +109,7 @@

>

>  /* IFC */

>  #define CONFIG_SYS_FSL_IFC_LE

> +#define CONFIG_SYS_MEMAC_LITTLE_ENDIAN


It seems tedious to have to define this. Can't you just use the functions available?
[S.H] To use a define is based on a concern that we cannot assume the I/O access of an IP share same endianness as the Soc(s), we cannot assume on little endian Soc the I/O access is little endian, on big endian Soc the I/O access is big endian, the I/O access could be little endian on big endian Soc and vice versa.


>  /* PCIe */

>  #define CONFIG_SYS_PCIE1_ADDR                  (CONFIG_SYS_IMMR + 0x2400000)

> diff --git a/drivers/net/Makefile b/drivers/net/Makefile

> index 5497934..d871093 100644

> --- a/drivers/net/Makefile

> +++ b/drivers/net/Makefile

> @@ -66,4 +66,5 @@ obj-$(CONFIG_XILINX_LL_TEMAC) += xilinx_ll_temac.o xilinx_ll_temac_mdio.o \

>  obj-$(CONFIG_ZYNQ_GEM) += zynq_gem.o

>  obj-$(CONFIG_FSL_MC_ENET) += fsl-mc/

>  obj-$(CONFIG_FSL_MC_ENET) += ldpaa_eth/

> +obj-$(CONFIG_FSL_MEMAC) += fm/memac_phy.o

>  obj-$(CONFIG_VSC9953) += vsc9953.o

> diff --git a/drivers/net/fm/eth.c b/drivers/net/fm/eth.c

> index 1d1089d..a7a5c69 100644

> --- a/drivers/net/fm/eth.c

> +++ b/drivers/net/fm/eth.c

> @@ -15,7 +15,7 @@

>  #include <phy.h>

>  #include <asm/fsl_dtsec.h>

>  #include <asm/fsl_tgec.h>

> -#include <asm/fsl_memac.h>

> +#include <fsl_memac.h>

>

>  #include "fm.h"

>

> diff --git a/drivers/net/fm/memac.c b/drivers/net/fm/memac.c

> index 60e898c..81a64bf 100644

> --- a/drivers/net/fm/memac.c

> +++ b/drivers/net/fm/memac.c

> @@ -12,7 +12,7 @@

>  #include <phy.h>

>  #include <asm/types.h>

>  #include <asm/io.h>

> -#include <asm/fsl_memac.h>

> +#include <fsl_memac.h>

>

>  #include "fm.h"

>

> diff --git a/drivers/net/fm/memac_phy.c b/drivers/net/fm/memac_phy.c

> index a155d89..4ab78e6 100644

> --- a/drivers/net/fm/memac_phy.c

> +++ b/drivers/net/fm/memac_phy.c

> @@ -10,9 +10,28 @@

>  #include <miiphy.h>

>  #include <phy.h>

>  #include <asm/io.h>

> -#include <asm/fsl_memac.h>

> +#include <fsl_memac.h>

>  #include <fm_eth.h>

>

> +#ifdef CONFIG_SYS_MEMAC_LITTLE_ENDIAN

This can already be detected, right?

#if __BYTE_ORDER == __LITTLE_ENDIAN
[S.H] The issue is the IP’s I/O access order on LS2 is different from big endian Soc(s).
> +#define memac_out_32(a, v)     out_le32(a, v)

> +#define memac_clrbits_32(a, v) clrbits_le32(a, v)

> +#define memac_setbits_32(a, v) setbits_le32(a, v)

> +#else

> +#define memac_out_32(a, v)     out_be32(a, v)

> +#define memac_clrbits_32(a, v) clrbits_be32(a, v)

> +#define memac_setbits_32(a, v) setbits_be32(a, v)

> +#endif

> +

> +static u32 memac_in_32(u32 *reg)

> +{

> +#ifdef CONFIG_SYS_MEMAC_LITTLE_ENDIAN

> +       return in_le32(reg);

> +#else

> +       return in_be32(reg);

> +#endif

> +}

Another option you have is to take the approach that you don't care the endianness. Something like using the this type of pattern:

value = ntohl(in_be32(reg));
out_be32(reg, htonl(value));
[S.H] same concern as above.

> +

>  /*

>   * Write value to the PHY for this device to the register at regnum, waiting

>   * until the write is done before it returns.  All PHY configuration has to be

> @@ -28,31 +47,31 @@ int memac_mdio_write(struct mii_dev *bus, int port_addr, int dev_addr,

>         if (dev_addr == MDIO_DEVAD_NONE) {

>                 c45 = 0; /* clause 22 */

>                 dev_addr = regnum & 0x1f;

> -               clrbits_be32(&regs->mdio_stat, MDIO_STAT_ENC);

> +               memac_clrbits_32(&regs->mdio_stat, MDIO_STAT_ENC);

>         } else

> -               setbits_be32(&regs->mdio_stat, MDIO_STAT_ENC);

> +               memac_setbits_32(&regs->mdio_stat, MDIO_STAT_ENC);

>

>         /* Wait till the bus is free */

> -       while ((in_be32(&regs->mdio_stat)) & MDIO_STAT_BSY)

> +       while ((memac_in_32(&regs->mdio_stat)) & MDIO_STAT_BSY)

>                 ;

>

>         /* Set the port and dev addr */

>         mdio_ctl = MDIO_CTL_PORT_ADDR(port_addr) | MDIO_CTL_DEV_ADDR(dev_addr);

> -       out_be32(&regs->mdio_ctl, mdio_ctl);

> +       memac_out_32(&regs->mdio_ctl, mdio_ctl);

>

>         /* Set the register address */

>         if (c45)

> -               out_be32(&regs->mdio_addr, regnum & 0xffff);

> +               memac_out_32(&regs->mdio_addr, regnum & 0xffff);

>

>         /* Wait till the bus is free */

> -       while ((in_be32(&regs->mdio_stat)) & MDIO_STAT_BSY)

> +       while ((memac_in_32(&regs->mdio_stat)) & MDIO_STAT_BSY)

>                 ;

>

>         /* Write the value to the register */

> -       out_be32(&regs->mdio_data, MDIO_DATA(value));

> +       memac_out_32(&regs->mdio_data, MDIO_DATA(value));

>

>         /* Wait till the MDIO write is complete */

> -       while ((in_be32(&regs->mdio_data)) & MDIO_DATA_BSY)

> +       while ((memac_in_32(&regs->mdio_data)) & MDIO_DATA_BSY)

>                 ;

>

>         return 0;

> @@ -75,39 +94,39 @@ int memac_mdio_read(struct mii_dev *bus, int port_addr, int dev_addr,

>                         return 0xffff;

>                 c45 = 0; /* clause 22 */

>                 dev_addr = regnum & 0x1f;

> -               clrbits_be32(&regs->mdio_stat, MDIO_STAT_ENC);

> +               memac_clrbits_32(&regs->mdio_stat, MDIO_STAT_ENC);

>         } else

> -               setbits_be32(&regs->mdio_stat, MDIO_STAT_ENC);

> +               memac_setbits_32(&regs->mdio_stat, MDIO_STAT_ENC);

>

>         /* Wait till the bus is free */

> -       while ((in_be32(&regs->mdio_stat)) & MDIO_STAT_BSY)

> +       while ((memac_in_32(&regs->mdio_stat)) & MDIO_STAT_BSY)

>                 ;

>

>         /* Set the Port and Device Addrs */

>         mdio_ctl = MDIO_CTL_PORT_ADDR(port_addr) | MDIO_CTL_DEV_ADDR(dev_addr);

> -       out_be32(&regs->mdio_ctl, mdio_ctl);

> +       memac_out_32(&regs->mdio_ctl, mdio_ctl);

>

>         /* Set the register address */

>         if (c45)

> -               out_be32(&regs->mdio_addr, regnum & 0xffff);

> +               memac_out_32(&regs->mdio_addr, regnum & 0xffff);

>

>         /* Wait till the bus is free */

> -       while ((in_be32(&regs->mdio_stat)) & MDIO_STAT_BSY)

> +       while ((memac_in_32(&regs->mdio_stat)) & MDIO_STAT_BSY)

>                 ;

>

>         /* Initiate the read */

>         mdio_ctl |= MDIO_CTL_READ;

> -       out_be32(&regs->mdio_ctl, mdio_ctl);

> +       memac_out_32(&regs->mdio_ctl, mdio_ctl);

>

>         /* Wait till the MDIO write is complete */

> -       while ((in_be32(&regs->mdio_data)) & MDIO_DATA_BSY)

> +       while ((memac_in_32(&regs->mdio_data)) & MDIO_DATA_BSY)

>                 ;

>

>         /* Return all Fs if nothing was there */

> -       if (in_be32(&regs->mdio_stat) & MDIO_STAT_RD_ER)

> +       if (memac_in_32(&regs->mdio_stat) & MDIO_STAT_RD_ER)

>                 return 0xffff;

>

> -       return in_be32(&regs->mdio_data) & 0xffff;

> +       return memac_in_32(&regs->mdio_data) & 0xffff;

>  }

>

>  int memac_mdio_reset(struct mii_dev *bus)

> @@ -143,8 +162,9 @@ int fm_memac_mdio_init(bd_t *bis, struct memac_mdio_info *info)

>          * like T2080QDS, this bit default is '0', which leads to MDIO failure

>          * on XAUI PHY, so set this bit definitely.

>          */

> -       setbits_be32(&((struct memac_mdio_controller *)info->regs)->mdio_stat,

> -                    MDIO_STAT_CLKDIV(258) | MDIO_STAT_NEG);

> +       memac_setbits_32(

> +               &((struct memac_mdio_controller *)info->regs)->mdio_stat,

> +               MDIO_STAT_CLKDIV(258) | MDIO_STAT_NEG);

>

>         return mdio_register(bus);

>  }

> diff --git a/drivers/net/vsc9953.c b/drivers/net/vsc9953.c

> index 9fc3c18..fed7358 100644

> --- a/drivers/net/vsc9953.c

> +++ b/drivers/net/vsc9953.c

> @@ -9,7 +9,7 @@

>  #include <asm/io.h>

>  #include <asm/fsl_serdes.h>

>  #include <fm_eth.h>

> -#include <asm/fsl_memac.h>

> +#include <fsl_memac.h>

>  #include <vsc9953.h>

>

>  static struct vsc9953_info vsc9953_l2sw = {

> diff --git a/arch/powerpc/include/asm/fsl_memac.h b/include/fsl_memac.h

> similarity index 100%

> rename from arch/powerpc/include/asm/fsl_memac.h

> rename to include/fsl_memac.h

> --

> 1.7.9.5

>

> _______________________________________________

> U-Boot mailing list

> U-Boot@lists.denx.de<mailto:U-Boot@lists.denx.de>

> http://lists.denx.de/mailman/listinfo/u-boot
Joe Hershberger March 20, 2015, 3:33 a.m. UTC | #3 
On Thu, Mar 19, 2015 at 10:06 PM, Shaohui Xie <Shaohui.Xie@freescale.com>
wrote:
>
> Hello Joe,
>
>
>
> Thank you for reviewing this patch!
>
> Please see inline.
>
>
>
> Best Regards,
>
> Shaohui Xie
>
>
>
> From: Joe Hershberger [mailto:joe.hershberger@gmail.com]
> Sent: Friday, March 20, 2015 2:04 AM
> To: Sun York-R58495
> Cc: u-boot; Joe Hershberger; Xie Shaohui-B21989
> Subject: Re: [U-Boot] [PATCH 15/28] net/memac_phy: reuse driver for
little endian SoCs
>
>
>
> Hi Shaohui Xie,
>
> On Thu, Mar 19, 2015 at 11:45 AM, York Sun <yorksun@freescale.com> wrote:
> >
> > From: Shaohui Xie <Shaohui.Xie@freescale.com>
> >
> > The memac for PHY management on little endian SoCs is similar on big
> > endian SoCs, so we modify the driver by using I/O accessor function to
> > handle the endianness, so the driver can be reused on little endian
> > SoCs, we introduce CONFIG_SYS_MEMAC_LITTLE_ENDIAN for little endian
> > SoCs, if the CONFIG_SYS_MEMAC_LITTLE_ENDIAN is defined, the I/O access
> > is little endian, if not, the I/O access is big endian. Move fsl_memac.h
> > out of powerpc include.
> >
> > Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
> > CC: Joe Hershberger <joe.hershberger@ni.com>
> > ---
> >  arch/arm/include/asm/arch-fsl-lsch3/config.h      |    1 +
> >  drivers/net/Makefile                              |    1 +
> >  drivers/net/fm/eth.c                              |    2 +-
> >  drivers/net/fm/memac.c                            |    2 +-
> >  drivers/net/fm/memac_phy.c                        |   62
++++++++++++++-------
> >  drivers/net/vsc9953.c                             |    2 +-
> >  {arch/powerpc/include/asm => include}/fsl_memac.h |    0
> >  7 files changed, 46 insertions(+), 24 deletions(-)
> >  rename {arch/powerpc/include/asm => include}/fsl_memac.h (100%)
> >
> > diff --git a/arch/arm/include/asm/arch-fsl-lsch3/config.h
b/arch/arm/include/asm/arch-fsl-lsch3/config.h
> > index 98db1ef..684c70f 100644
> > --- a/arch/arm/include/asm/arch-fsl-lsch3/config.h
> > +++ b/arch/arm/include/asm/arch-fsl-lsch3/config.h
> > @@ -109,6 +109,7 @@
> >
> >  /* IFC */
> >  #define CONFIG_SYS_FSL_IFC_LE
> > +#define CONFIG_SYS_MEMAC_LITTLE_ENDIAN
>
> It seems tedious to have to define this. Can't you just use the functions
available?
>
> [S.H] To use a define is based on a concern that we cannot assume the I/O
access of an IP share same endianness as the Soc(s), we cannot assume on
little endian Soc the I/O access is little endian, on big endian Soc the
I/O access is big endian, the I/O access could be little endian on big
endian Soc and vice versa.

You're saying that the IP is expected to be in different endianness? If
that is practically the case, then I'm fine with this patch. I just want to
ensure that it is not just speculative generality.

> >  /* PCIe */
> >  #define CONFIG_SYS_PCIE1_ADDR                  (CONFIG_SYS_IMMR +
0x2400000)
> > diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> > index 5497934..d871093 100644
> > --- a/drivers/net/Makefile
> > +++ b/drivers/net/Makefile
> > @@ -66,4 +66,5 @@ obj-$(CONFIG_XILINX_LL_TEMAC) += xilinx_ll_temac.o
xilinx_ll_temac_mdio.o \
> >  obj-$(CONFIG_ZYNQ_GEM) += zynq_gem.o
> >  obj-$(CONFIG_FSL_MC_ENET) += fsl-mc/
> >  obj-$(CONFIG_FSL_MC_ENET) += ldpaa_eth/
> > +obj-$(CONFIG_FSL_MEMAC) += fm/memac_phy.o
> >  obj-$(CONFIG_VSC9953) += vsc9953.o
> > diff --git a/drivers/net/fm/eth.c b/drivers/net/fm/eth.c
> > index 1d1089d..a7a5c69 100644
> > --- a/drivers/net/fm/eth.c
> > +++ b/drivers/net/fm/eth.c
> > @@ -15,7 +15,7 @@
> >  #include <phy.h>
> >  #include <asm/fsl_dtsec.h>
> >  #include <asm/fsl_tgec.h>
> > -#include <asm/fsl_memac.h>
> > +#include <fsl_memac.h>
> >
> >  #include "fm.h"
> >
> > diff --git a/drivers/net/fm/memac.c b/drivers/net/fm/memac.c
> > index 60e898c..81a64bf 100644
> > --- a/drivers/net/fm/memac.c
> > +++ b/drivers/net/fm/memac.c
> > @@ -12,7 +12,7 @@
> >  #include <phy.h>
> >  #include <asm/types.h>
> >  #include <asm/io.h>
> > -#include <asm/fsl_memac.h>
> > +#include <fsl_memac.h>
> >
> >  #include "fm.h"
> >
> > diff --git a/drivers/net/fm/memac_phy.c b/drivers/net/fm/memac_phy.c
> > index a155d89..4ab78e6 100644
> > --- a/drivers/net/fm/memac_phy.c
> > +++ b/drivers/net/fm/memac_phy.c
> > @@ -10,9 +10,28 @@
> >  #include <miiphy.h>
> >  #include <phy.h>
> >  #include <asm/io.h>
> > -#include <asm/fsl_memac.h>
> > +#include <fsl_memac.h>
> >  #include <fm_eth.h>
> >
> > +#ifdef CONFIG_SYS_MEMAC_LITTLE_ENDIAN
>
> This can already be detected, right?
>
>
>
> #if __BYTE_ORDER == __LITTLE_ENDIAN
>
> [S.H] The issue is the IP’s I/O access order on LS2 is different from big
endian Soc(s).
>
> > +#define memac_out_32(a, v)     out_le32(a, v)
> > +#define memac_clrbits_32(a, v) clrbits_le32(a, v)
> > +#define memac_setbits_32(a, v) setbits_le32(a, v)
> > +#else
> > +#define memac_out_32(a, v)     out_be32(a, v)
> > +#define memac_clrbits_32(a, v) clrbits_be32(a, v)
> > +#define memac_setbits_32(a, v) setbits_be32(a, v)
> > +#endif
> > +
> > +static u32 memac_in_32(u32 *reg)
> > +{
> > +#ifdef CONFIG_SYS_MEMAC_LITTLE_ENDIAN
> > +       return in_le32(reg);
> > +#else
> > +       return in_be32(reg);
> > +#endif
> > +}
>
> Another option you have is to take the approach that you don't care the
endianness. Something like using the this type of pattern:
>
>
>
> value = ntohl(in_be32(reg));
>
> out_be32(reg, htonl(value));
>
> [S.H] same concern as above.
>
>
>
> > +
> >  /*
> >   * Write value to the PHY for this device to the register at regnum,
waiting
> >   * until the write is done before it returns.  All PHY configuration
has to be
> > @@ -28,31 +47,31 @@ int memac_mdio_write(struct mii_dev *bus, int
port_addr, int dev_addr,
> >         if (dev_addr == MDIO_DEVAD_NONE) {
> >                 c45 = 0; /* clause 22 */
> >                 dev_addr = regnum & 0x1f;
> > -               clrbits_be32(&regs->mdio_stat, MDIO_STAT_ENC);
> > +               memac_clrbits_32(&regs->mdio_stat, MDIO_STAT_ENC);
> >         } else
> > -               setbits_be32(&regs->mdio_stat, MDIO_STAT_ENC);
> > +               memac_setbits_32(&regs->mdio_stat, MDIO_STAT_ENC);
> >
> >         /* Wait till the bus is free */
> > -       while ((in_be32(&regs->mdio_stat)) & MDIO_STAT_BSY)
> > +       while ((memac_in_32(&regs->mdio_stat)) & MDIO_STAT_BSY)
> >                 ;
> >
> >         /* Set the port and dev addr */
> >         mdio_ctl = MDIO_CTL_PORT_ADDR(port_addr) |
MDIO_CTL_DEV_ADDR(dev_addr);
> > -       out_be32(&regs->mdio_ctl, mdio_ctl);
> > +       memac_out_32(&regs->mdio_ctl, mdio_ctl);
> >
> >         /* Set the register address */
> >         if (c45)
> > -               out_be32(&regs->mdio_addr, regnum & 0xffff);
> > +               memac_out_32(&regs->mdio_addr, regnum & 0xffff);
> >
> >         /* Wait till the bus is free */
> > -       while ((in_be32(&regs->mdio_stat)) & MDIO_STAT_BSY)
> > +       while ((memac_in_32(&regs->mdio_stat)) & MDIO_STAT_BSY)
> >                 ;
> >
> >         /* Write the value to the register */
> > -       out_be32(&regs->mdio_data, MDIO_DATA(value));
> > +       memac_out_32(&regs->mdio_data, MDIO_DATA(value));
> >
> >         /* Wait till the MDIO write is complete */
> > -       while ((in_be32(&regs->mdio_data)) & MDIO_DATA_BSY)
> > +       while ((memac_in_32(&regs->mdio_data)) & MDIO_DATA_BSY)
> >                 ;
> >
> >         return 0;
> > @@ -75,39 +94,39 @@ int memac_mdio_read(struct mii_dev *bus, int
port_addr, int dev_addr,
> >                         return 0xffff;
> >                 c45 = 0; /* clause 22 */
> >                 dev_addr = regnum & 0x1f;
> > -               clrbits_be32(&regs->mdio_stat, MDIO_STAT_ENC);
> > +               memac_clrbits_32(&regs->mdio_stat, MDIO_STAT_ENC);
> >         } else
> > -               setbits_be32(&regs->mdio_stat, MDIO_STAT_ENC);
> > +               memac_setbits_32(&regs->mdio_stat, MDIO_STAT_ENC);
> >
> >         /* Wait till the bus is free */
> > -       while ((in_be32(&regs->mdio_stat)) & MDIO_STAT_BSY)
> > +       while ((memac_in_32(&regs->mdio_stat)) & MDIO_STAT_BSY)
> >                 ;
> >
> >         /* Set the Port and Device Addrs */
> >         mdio_ctl = MDIO_CTL_PORT_ADDR(port_addr) |
MDIO_CTL_DEV_ADDR(dev_addr);
> > -       out_be32(&regs->mdio_ctl, mdio_ctl);
> > +       memac_out_32(&regs->mdio_ctl, mdio_ctl);
> >
> >         /* Set the register address */
> >         if (c45)
> > -               out_be32(&regs->mdio_addr, regnum & 0xffff);
> > +               memac_out_32(&regs->mdio_addr, regnum & 0xffff);
> >
> >         /* Wait till the bus is free */
> > -       while ((in_be32(&regs->mdio_stat)) & MDIO_STAT_BSY)
> > +       while ((memac_in_32(&regs->mdio_stat)) & MDIO_STAT_BSY)
> >                 ;
> >
> >         /* Initiate the read */
> >         mdio_ctl |= MDIO_CTL_READ;
> > -       out_be32(&regs->mdio_ctl, mdio_ctl);
> > +       memac_out_32(&regs->mdio_ctl, mdio_ctl);
> >
> >         /* Wait till the MDIO write is complete */
> > -       while ((in_be32(&regs->mdio_data)) & MDIO_DATA_BSY)
> > +       while ((memac_in_32(&regs->mdio_data)) & MDIO_DATA_BSY)
> >                 ;
> >
> >         /* Return all Fs if nothing was there */
> > -       if (in_be32(&regs->mdio_stat) & MDIO_STAT_RD_ER)
> > +       if (memac_in_32(&regs->mdio_stat) & MDIO_STAT_RD_ER)
> >                 return 0xffff;
> >
> > -       return in_be32(&regs->mdio_data) & 0xffff;
> > +       return memac_in_32(&regs->mdio_data) & 0xffff;
> >  }
> >
> >  int memac_mdio_reset(struct mii_dev *bus)
> > @@ -143,8 +162,9 @@ int fm_memac_mdio_init(bd_t *bis, struct
memac_mdio_info *info)
> >          * like T2080QDS, this bit default is '0', which leads to MDIO
failure
> >          * on XAUI PHY, so set this bit definitely.
> >          */
> > -       setbits_be32(&((struct memac_mdio_controller
*)info->regs)->mdio_stat,
> > -                    MDIO_STAT_CLKDIV(258) | MDIO_STAT_NEG);
> > +       memac_setbits_32(
> > +               &((struct memac_mdio_controller
*)info->regs)->mdio_stat,
> > +               MDIO_STAT_CLKDIV(258) | MDIO_STAT_NEG);
> >
> >         return mdio_register(bus);
> >  }
> > diff --git a/drivers/net/vsc9953.c b/drivers/net/vsc9953.c
> > index 9fc3c18..fed7358 100644
> > --- a/drivers/net/vsc9953.c
> > +++ b/drivers/net/vsc9953.c
> > @@ -9,7 +9,7 @@
> >  #include <asm/io.h>
> >  #include <asm/fsl_serdes.h>
> >  #include <fm_eth.h>
> > -#include <asm/fsl_memac.h>
> > +#include <fsl_memac.h>
> >  #include <vsc9953.h>
> >
> >  static struct vsc9953_info vsc9953_l2sw = {
> > diff --git a/arch/powerpc/include/asm/fsl_memac.h b/include/fsl_memac.h
> > similarity index 100%
> > rename from arch/powerpc/include/asm/fsl_memac.h
> > rename to include/fsl_memac.h
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot@lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
shaohui xie March 20, 2015, 3:48 a.m. UTC | #4 
> >  /* IFC */
> >  #define CONFIG_SYS_FSL_IFC_LE
> > +#define CONFIG_SYS_MEMAC_LITTLE_ENDIAN
>
> It seems tedious to have to define this. Can't you just use the functions available?
>
> [S.H] To use a define is based on a concern that we cannot assume the I/O access of an IP share same endianness as the Soc(s), we cannot assume on little endian Soc the I/O access is little endian, on big endian Soc the I/O access is big endian, the I/O access could be little endian on big endian Soc and vice versa.

You're saying that the IP is expected to be in different endianness? If that is practically the case, then I'm fine with this patch. I just want to ensure that it is not just speculative generality.
[S.H] Yes. The IP is in different endianness, i.e. little endian on LS2, big endian on PowerPc Soc(s).
Thank you!
Shaohui
Joe Hershberger March 20, 2015, 3:58 a.m. UTC | #5 
On Thu, Mar 19, 2015 at 10:48 PM, Shaohui Xie <Shaohui.Xie@freescale.com>
wrote:
> > >  /* IFC */
> > >  #define CONFIG_SYS_FSL_IFC_LE
> > > +#define CONFIG_SYS_MEMAC_LITTLE_ENDIAN
> >
> > It seems tedious to have to define this. Can't you just use the
functions available?
> >
> > [S.H] To use a define is based on a concern that we cannot assume the
I/O access of an IP share same endianness as the Soc(s), we cannot assume
on little endian Soc the I/O access is little endian, on big endian Soc the
I/O access is big endian, the I/O access could be little endian on big
endian Soc and vice versa.
>
> You're saying that the IP is expected to be in different endianness? If
that is practically the case, then I'm fine with this patch. I just want to
ensure that it is not just speculative generality.
>
> [S.H] Yes. The IP is in different endianness, i.e. little endian on LS2,
big endian on PowerPc Soc(s).

OK. It seems that in both of your examples the IP endianness matches the
SoC endianness, which is why I recommended what I did in the first reply.

If you think that this is needed flexibility, then,

Acked-by: Joe Hershberger <joe.hershberger@ni.com>
diff mbox

Patch

diff --git a/arch/arm/include/asm/arch-fsl-lsch3/config.h b/arch/arm/include/asm/arch-fsl-lsch3/config.h
index 98db1ef..684c70f 100644
--- a/arch/arm/include/asm/arch-fsl-lsch3/config.h
+++ b/arch/arm/include/asm/arch-fsl-lsch3/config.h
@@ -109,6 +109,7 @@ 
 
 /* IFC */
 #define CONFIG_SYS_FSL_IFC_LE
+#define CONFIG_SYS_MEMAC_LITTLE_ENDIAN
 
 /* PCIe */
 #define CONFIG_SYS_PCIE1_ADDR			(CONFIG_SYS_IMMR + 0x2400000)
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 5497934..d871093 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -66,4 +66,5 @@  obj-$(CONFIG_XILINX_LL_TEMAC) += xilinx_ll_temac.o xilinx_ll_temac_mdio.o \
 obj-$(CONFIG_ZYNQ_GEM) += zynq_gem.o
 obj-$(CONFIG_FSL_MC_ENET) += fsl-mc/
 obj-$(CONFIG_FSL_MC_ENET) += ldpaa_eth/
+obj-$(CONFIG_FSL_MEMAC) += fm/memac_phy.o
 obj-$(CONFIG_VSC9953) += vsc9953.o
diff --git a/drivers/net/fm/eth.c b/drivers/net/fm/eth.c
index 1d1089d..a7a5c69 100644
--- a/drivers/net/fm/eth.c
+++ b/drivers/net/fm/eth.c
@@ -15,7 +15,7 @@ 
 #include <phy.h>
 #include <asm/fsl_dtsec.h>
 #include <asm/fsl_tgec.h>
-#include <asm/fsl_memac.h>
+#include <fsl_memac.h>
 
 #include "fm.h"
 
diff --git a/drivers/net/fm/memac.c b/drivers/net/fm/memac.c
index 60e898c..81a64bf 100644
--- a/drivers/net/fm/memac.c
+++ b/drivers/net/fm/memac.c
@@ -12,7 +12,7 @@ 
 #include <phy.h>
 #include <asm/types.h>
 #include <asm/io.h>
-#include <asm/fsl_memac.h>
+#include <fsl_memac.h>
 
 #include "fm.h"
 
diff --git a/drivers/net/fm/memac_phy.c b/drivers/net/fm/memac_phy.c
index a155d89..4ab78e6 100644
--- a/drivers/net/fm/memac_phy.c
+++ b/drivers/net/fm/memac_phy.c
@@ -10,9 +10,28 @@ 
 #include <miiphy.h>
 #include <phy.h>
 #include <asm/io.h>
-#include <asm/fsl_memac.h>
+#include <fsl_memac.h>
 #include <fm_eth.h>
 
+#ifdef CONFIG_SYS_MEMAC_LITTLE_ENDIAN
+#define memac_out_32(a, v)	out_le32(a, v)
+#define memac_clrbits_32(a, v)	clrbits_le32(a, v)
+#define memac_setbits_32(a, v)	setbits_le32(a, v)
+#else
+#define memac_out_32(a, v)	out_be32(a, v)
+#define memac_clrbits_32(a, v)	clrbits_be32(a, v)
+#define memac_setbits_32(a, v)	setbits_be32(a, v)
+#endif
+
+static u32 memac_in_32(u32 *reg)
+{
+#ifdef CONFIG_SYS_MEMAC_LITTLE_ENDIAN
+	return in_le32(reg);
+#else
+	return in_be32(reg);
+#endif
+}
+
 /*
  * Write value to the PHY for this device to the register at regnum, waiting
  * until the write is done before it returns.  All PHY configuration has to be
@@ -28,31 +47,31 @@  int memac_mdio_write(struct mii_dev *bus, int port_addr, int dev_addr,
 	if (dev_addr == MDIO_DEVAD_NONE) {
 		c45 = 0; /* clause 22 */
 		dev_addr = regnum & 0x1f;
-		clrbits_be32(&regs->mdio_stat, MDIO_STAT_ENC);
+		memac_clrbits_32(&regs->mdio_stat, MDIO_STAT_ENC);
 	} else
-		setbits_be32(&regs->mdio_stat, MDIO_STAT_ENC);
+		memac_setbits_32(&regs->mdio_stat, MDIO_STAT_ENC);
 
 	/* Wait till the bus is free */
-	while ((in_be32(&regs->mdio_stat)) & MDIO_STAT_BSY)
+	while ((memac_in_32(&regs->mdio_stat)) & MDIO_STAT_BSY)
 		;
 
 	/* Set the port and dev addr */
 	mdio_ctl = MDIO_CTL_PORT_ADDR(port_addr) | MDIO_CTL_DEV_ADDR(dev_addr);
-	out_be32(&regs->mdio_ctl, mdio_ctl);
+	memac_out_32(&regs->mdio_ctl, mdio_ctl);
 
 	/* Set the register address */
 	if (c45)
-		out_be32(&regs->mdio_addr, regnum & 0xffff);
+		memac_out_32(&regs->mdio_addr, regnum & 0xffff);
 
 	/* Wait till the bus is free */
-	while ((in_be32(&regs->mdio_stat)) & MDIO_STAT_BSY)
+	while ((memac_in_32(&regs->mdio_stat)) & MDIO_STAT_BSY)
 		;
 
 	/* Write the value to the register */
-	out_be32(&regs->mdio_data, MDIO_DATA(value));
+	memac_out_32(&regs->mdio_data, MDIO_DATA(value));
 
 	/* Wait till the MDIO write is complete */
-	while ((in_be32(&regs->mdio_data)) & MDIO_DATA_BSY)
+	while ((memac_in_32(&regs->mdio_data)) & MDIO_DATA_BSY)
 		;
 
 	return 0;
@@ -75,39 +94,39 @@  int memac_mdio_read(struct mii_dev *bus, int port_addr, int dev_addr,
 			return 0xffff;
 		c45 = 0; /* clause 22 */
 		dev_addr = regnum & 0x1f;
-		clrbits_be32(&regs->mdio_stat, MDIO_STAT_ENC);
+		memac_clrbits_32(&regs->mdio_stat, MDIO_STAT_ENC);
 	} else
-		setbits_be32(&regs->mdio_stat, MDIO_STAT_ENC);
+		memac_setbits_32(&regs->mdio_stat, MDIO_STAT_ENC);
 
 	/* Wait till the bus is free */
-	while ((in_be32(&regs->mdio_stat)) & MDIO_STAT_BSY)
+	while ((memac_in_32(&regs->mdio_stat)) & MDIO_STAT_BSY)
 		;
 
 	/* Set the Port and Device Addrs */
 	mdio_ctl = MDIO_CTL_PORT_ADDR(port_addr) | MDIO_CTL_DEV_ADDR(dev_addr);
-	out_be32(&regs->mdio_ctl, mdio_ctl);
+	memac_out_32(&regs->mdio_ctl, mdio_ctl);
 
 	/* Set the register address */
 	if (c45)
-		out_be32(&regs->mdio_addr, regnum & 0xffff);
+		memac_out_32(&regs->mdio_addr, regnum & 0xffff);
 
 	/* Wait till the bus is free */
-	while ((in_be32(&regs->mdio_stat)) & MDIO_STAT_BSY)
+	while ((memac_in_32(&regs->mdio_stat)) & MDIO_STAT_BSY)
 		;
 
 	/* Initiate the read */
 	mdio_ctl |= MDIO_CTL_READ;
-	out_be32(&regs->mdio_ctl, mdio_ctl);
+	memac_out_32(&regs->mdio_ctl, mdio_ctl);
 
 	/* Wait till the MDIO write is complete */
-	while ((in_be32(&regs->mdio_data)) & MDIO_DATA_BSY)
+	while ((memac_in_32(&regs->mdio_data)) & MDIO_DATA_BSY)
 		;
 
 	/* Return all Fs if nothing was there */
-	if (in_be32(&regs->mdio_stat) & MDIO_STAT_RD_ER)
+	if (memac_in_32(&regs->mdio_stat) & MDIO_STAT_RD_ER)
 		return 0xffff;
 
-	return in_be32(&regs->mdio_data) & 0xffff;
+	return memac_in_32(&regs->mdio_data) & 0xffff;
 }
 
 int memac_mdio_reset(struct mii_dev *bus)
@@ -143,8 +162,9 @@  int fm_memac_mdio_init(bd_t *bis, struct memac_mdio_info *info)
 	 * like T2080QDS, this bit default is '0', which leads to MDIO failure
 	 * on XAUI PHY, so set this bit definitely.
 	 */
-	setbits_be32(&((struct memac_mdio_controller *)info->regs)->mdio_stat,
-		     MDIO_STAT_CLKDIV(258) | MDIO_STAT_NEG);
+	memac_setbits_32(
+		&((struct memac_mdio_controller *)info->regs)->mdio_stat,
+		MDIO_STAT_CLKDIV(258) | MDIO_STAT_NEG);
 
 	return mdio_register(bus);
 }
diff --git a/drivers/net/vsc9953.c b/drivers/net/vsc9953.c
index 9fc3c18..fed7358 100644
--- a/drivers/net/vsc9953.c
+++ b/drivers/net/vsc9953.c
@@ -9,7 +9,7 @@ 
 #include <asm/io.h>
 #include <asm/fsl_serdes.h>
 #include <fm_eth.h>
-#include <asm/fsl_memac.h>
+#include <fsl_memac.h>
 #include <vsc9953.h>
 
 static struct vsc9953_info vsc9953_l2sw = {
diff --git a/arch/powerpc/include/asm/fsl_memac.h b/include/fsl_memac.h
similarity index 100%
rename from arch/powerpc/include/asm/fsl_memac.h
rename to include/fsl_memac.h