Hi Richard,

Richard Retanubun wrote:

Wolfgang Denk wrote:

<snip> > Can you please check the status of this patch: > http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/61758 > > Thanks. > > Best regards, > > Wolfgang Denk >

Hello,

Wofgang, I believe at the time Ben made the comment that my patch's usage of things like these:

-#if !(defined(CONFIG_EP8248) || defined(CONFIG_EP82XXM))

• volatile ioport_t *iop = ioport_addr ((immap_t *)

CONFIG_SYS_IMMR, MDIO_PORT); +#if !(defined(CONFIG_EP8248) || defined(CONFIG_EP82XXM)) \

• && !defined(CONFIG_MPC83XX)
• volatile gpio_n_t *iop = ioport_addr ((immap_t *)

CONFIG_SYS_IMMR, MDIO_PORT); +#elif defined(CONFIG_MPC83XX)

• volatile immap_t *im = (volatile immap_t *)CONFIG_SYS_IMMR;
• volatile qepio83xx_t *par_io = (volatile qepio83xx_t *)&im->qepio;

•    volatile gpio_n_t *iop = &(par_io->ioport[MDIO_PORT]);
  

  #endif

Is propagating bad practice, and I agreed. Since then, Luigi's deft implementation MDIO_DECLARE and MDC_DELCARE have eliminated this problem. (or at least move it away from Ben's vigilant eyes into the board specific header file).

Luigi did some good things here. I'm glad you're sync'ing up.

As a refresher, the declaration for which UEC uses the bitbang in the board header file is like this:

#define CONFIG_SYS_BITBANG_PHY_PORT(name) name,

/* Bit-bang SMI ports */ #define CONFIG_SYS_BITBANG_PHY_PORTS \ CONFIG_SYS_BITBANG_PHY_PORT("FSL UEC4") \ CONFIG_SYS_BITBANG_PHY_PORT("FSL UEC5")

similar to how CONFIG_SYS_FIXED_PHY_PORT is used.

And so, here is V2 of the patch, with less ugly board specific code and rebased to commit 2ff6922280025c1315c53fa2eb4ab33f0c9591de (Tue Jan 12 23:47:03 2010 +0100)

• Richard

====================================================================================================

From 46380bee3b8e63afff9d00729f38cb960e0d2bfb Mon Sep 17 00:00:00 2001 From: Richard Retanubun RichardRetanubun@RuggedCom.com Date: Wed, 17 Jun 2009 16:00:41 -0400 Subject: [PATCH] 83xx: UEC: Added support for bitBang MII driver access to PHYs

This patch enabled support for having PHYs on bitBang MII and uec MII operating at the same time. Modeled after the MPC8360ADS implementation and added the ability to specify which ethernet interfaces have bitbang SMI on the board header file.

Signed-off-by: Richard Retanubun RichardRetanubun@RuggedCom.com

drivers/net/phy/miiphybb.h | 26 +++++++++++++++++++++++ drivers/qe/uec.c | 6 +--- drivers/qe/uec_phy.c | 48 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 4 deletions(-) create mode 100644 drivers/net/phy/miiphybb.h

diff --git a/drivers/net/phy/miiphybb.h b/drivers/net/phy/miiphybb.h new file mode 100644 index 0000000..4fff40d --- /dev/null +++ b/drivers/net/phy/miiphybb.h @@ -0,0 +1,26 @@ +/*

• • Copyright (C) 2006-2009 RuggedCom, Inc.
• • Richard Retanubun richardretanubun@ruggedcom.com
• • This program is free software; you can redistribute it and/or
• • modify it under the terms of the GNU General Public License as
• • published by the Free Software Foundation; either version 2 of
• • the License, or (at your option) any later version.
• • This program is distributed in the hope that it will be useful,
• • but WITHOUT ANY WARRANTY; without even the implied warranty of
• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• • GNU General Public License for more details.
• • You should have received a copy of the GNU General Public License
• • along with this program; if not, write to the Free Software
• • Foundation, Inc., 59 Temple Place, Suite 330, Boston,
• • MA 02111-1307 USA
• */

+int bb_miiphy_read (char *devname, unsigned char addr,

•    unsigned char reg, unsigned short *value);
  

+int bb_miiphy_write (char *devname, unsigned char addr,

•    unsigned char reg, unsigned short value);
  

There's already BB-related stuff in include/miiphy.h. Why not put the prototypes there?

diff --git a/drivers/qe/uec.c b/drivers/qe/uec.c index db95ada..109edd9 100644 --- a/drivers/qe/uec.c +++ b/drivers/qe/uec.c @@ -579,8 +579,7 @@ static void phy_change(struct eth_device *dev) adjust_link(dev); }

-#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) \

• && !defined(BITBANGMII)

+#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) /*&& !defined(BITBANGMII)*/

What's the point of this comment?

/*

• Find a device index from the devlist by name

@@ -1372,8 +1371,7 @@ int uec_initialize(bd_t *bis, uec_info_t *uec_info) return err; }

-#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) \

• && !defined(BITBANGMII)

+#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) /* && !defined(CONFIG_BITBANGMII) */ miiphy_register(dev->name, uec_miiphy_read, uec_miiphy_write);

Same comment-related comment as above.

#endif

diff --git a/drivers/qe/uec_phy.c b/drivers/qe/uec_phy.c index 9715183..6fb3b4d 100644 --- a/drivers/qe/uec_phy.c +++ b/drivers/qe/uec_phy.c @@ -25,6 +25,7 @@ #include "uec.h" #include "uec_phy.h" #include "miiphy.h" +#include "../net/phy/miiphybb.h"

Please don't use relative includes. As I'm sure you've noticed, people around here are keen on massive file reorganization, and this sort of thing makes that more work. If you put your prototypes in miiphy.h it won't be a problem

#define ugphy_printk(format, arg...) \ printf(format "\n", ## arg) @@ -93,6 +94,27 @@ static const struct fixed_phy_port fixed_phy_port[] = { CONFIG_SYS_FIXED_PHY_PORTS /* defined in board configuration file */ };

+/*--------------------------------------------------------------------+

• • BitBang MII support for ethernet ports
• • Based from MPC8560ADS implementation
• *--------------------------------------------------------------------*/

+/*

• • Example board header file to define bitbang ethernet ports:
• • #define CONFIG_SYS_BITBANG_PHY_PORT(name) name,
• • #define CONFIG_SYS_BITBANG_PHY_PORTS

CONFIG_SYS_BITBANG_PHY_PORT("FSL UEC0")

Maybe you could come up with a shorter name?

+*/ +#ifndef CONFIG_SYS_BITBANG_PHY_PORTS +#define CONFIG_SYS_BITBANG_PHY_PORTS /* default is an empty array */ +#endif

+#if defined(CONFIG_BITBANGMII) +static const char *bitbang_phy_port[] = {

• CONFIG_SYS_BITBANG_PHY_PORTS /* defined in board configuration

file */ +}; +#endif /* CONFIG_BITBANGMII */

static void config_genmii_advert (struct uec_mii_info *mii_info); static void genmii_setup_forced (struct uec_mii_info *mii_info); static void genmii_restart_aneg (struct uec_mii_info *mii_info); @@ -113,6 +135,19 @@ void uec_write_phy_reg (struct eth_device *dev, int mii_id, int regnum, int valu enet_tbi_mii_reg_e mii_reg = (enet_tbi_mii_reg_e) regnum; u32 tmp_reg;

+#if defined(CONFIG_BITBANGMII)

• u8 i = 0;

Why is this a u8? I don't believe you save any space by making it less than an int, and probably invite compiler warnings with the comparison to ARRAY_SIZE()

• for (i = 0; i < ARRAY_SIZE(bitbang_phy_port); i++) {

•    if (strncmp(dev->name, bitbang_phy_port[i],
  

•        strlen(dev->name)) == 0) {
  

It's probably better to do sizeof(dev->name)

•        (void)bb_miiphy_write(NULL, mii_id, regnum, value);
  

What's the point of this cast?

•        return;
  

•    }
  

• }

+#endif /* CONFIG_BITBANGMII */

• ug_regs = ugeth->uec_mii_regs;

  /* Stop the MII management read cycle */

@@ -140,6 +175,19 @@ int uec_read_phy_reg (struct eth_device *dev, int mii_id, int regnum) u32 tmp_reg; u16 value;

+#if defined(CONFIG_BITBANGMII)

• u8 i = 0;
• for (i = 0; i < ARRAY_SIZE(bitbang_phy_port); i++) {

•    if (strncmp(dev->name, bitbang_phy_port[i],
  

•        strlen(dev->name)) == 0) {
  

•        (void)bb_miiphy_read(NULL, mii_id, regnum, &value);
  

•        return (value);
  

•    }
  

• }

Same comments as above.

+#endif /* CONFIG_BITBANGMII */

• ug_regs = ugeth->uec_mii_regs;

  /* Setting up the MII Mangement Address Register */

-- 1.6.5

thanks a lot, Ben

From 1f50506ad9a305c1c9dfc68aa70551166b44d3a0 Mon Sep 17 00:00:00 2001

From: Richard Retanubun RichardRetanubun@RuggedCom.com Date: Wed, 17 Jun 2009 16:00:41 -0400 Subject: [PATCH] 83xx: UEC: Added support for bitBang MII driver access to PHYs

This patch enabled support for having PHYs on bitBang MII and uec MII operating at the same time. Modeled after the MPC8360ADS implementation.

Added the ability to specify which ethernet interfaces have bitbang SMI on the board header file. --- Hi Ben,

Just wanted to follow up, I posted replies to your feedback for V1 of this patch, but did not hear from you about it. Now that the merge window is open again, I thought I should try again. So here is V2 with the changes you suggested.

Thanks

drivers/qe/uec.c | 6 ++---- drivers/qe/uec_phy.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/drivers/qe/uec.c b/drivers/qe/uec.c index 27dc500..ccbf27d 100644 --- a/drivers/qe/uec.c +++ b/drivers/qe/uec.c @@ -595,8 +595,7 @@ static void phy_change(struct eth_device *dev) adjust_link(dev); }

-#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) \ - && !defined(BITBANGMII) +#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII)

/* * Find a device index from the devlist by name @@ -1388,8 +1387,7 @@ int uec_initialize(bd_t *bis, uec_info_t *uec_info) return err; }

-#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) \ - && !defined(BITBANGMII) +#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) miiphy_register(dev->name, uec_miiphy_read, uec_miiphy_write); #endif

diff --git a/drivers/qe/uec_phy.c b/drivers/qe/uec_phy.c index c4214d9..a17fea4 100644 --- a/drivers/qe/uec_phy.c +++ b/drivers/qe/uec_phy.c @@ -93,6 +93,27 @@ static const struct fixed_phy_port fixed_phy_port[] = { CONFIG_SYS_FIXED_PHY_PORTS /* defined in board configuration file */ };

+/*--------------------------------------------------------------------+ + * BitBang MII support for ethernet ports + * + * Based from MPC8560ADS implementation + *--------------------------------------------------------------------*/ +/* + * Example board header file to define bitbang ethernet ports: + * + * #define CONFIG_SYS_BITBANG_PHY_PORT(name) name, + * #define CONFIG_SYS_BITBANG_PHY_PORTS CONFIG_SYS_BITBANG_PHY_PORT("FSL UEC0") +*/ +#ifndef CONFIG_SYS_BITBANG_PHY_PORTS +#define CONFIG_SYS_BITBANG_PHY_PORTS /* default is an empty array */ +#endif + +#if defined(CONFIG_BITBANGMII) +static const char *bitbang_phy_port[] = { + CONFIG_SYS_BITBANG_PHY_PORTS /* defined in board configuration file */ +}; +#endif /* CONFIG_BITBANGMII */ + static void config_genmii_advert (struct uec_mii_info *mii_info); static void genmii_setup_forced (struct uec_mii_info *mii_info); static void genmii_restart_aneg (struct uec_mii_info *mii_info); @@ -113,6 +134,19 @@ void uec_write_phy_reg (struct eth_device *dev, int mii_id, int regnum, int valu enet_tbi_mii_reg_e mii_reg = (enet_tbi_mii_reg_e) regnum; u32 tmp_reg;

+ +#if defined(CONFIG_BITBANGMII) + u32 i = 0; + + for (i = 0; i < ARRAY_SIZE(bitbang_phy_port); i++) { + if (strncmp(dev->name, bitbang_phy_port[i], + sizeof(dev->name)) == 0) { + (void)bb_miiphy_write(NULL, mii_id, regnum, value); + return; + } + } +#endif /* CONFIG_BITBANGMII */ + ug_regs = ugeth->uec_mii_regs;

/* Stop the MII management read cycle */ @@ -140,6 +174,19 @@ int uec_read_phy_reg (struct eth_device *dev, int mii_id, int regnum) u32 tmp_reg; u16 value;

+ +#if defined(CONFIG_BITBANGMII) + u32 i = 0; + + for (i = 0; i < ARRAY_SIZE(bitbang_phy_port); i++) { + if (strncmp(dev->name, bitbang_phy_port[i], + sizeof(dev->name)) == 0) { + (void)bb_miiphy_read(NULL, mii_id, regnum, &value); + return (value); + } + } +#endif /* CONFIG_BITBANGMII */ + ug_regs = ugeth->uec_mii_regs;

/* Setting up the MII Mangement Address Register */