Re: [U-Boot] [PATCH 2/3] fec_mxc: use phy_connect_by_mask

On 15/08/2012 22:40, Troy Kisky wrote:

Allow board config files to list a range of possible phy addresses, in case the exact phy address is not certain.

Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com

Hi Troy,

drivers/net/fec_mxc.c | 21 +++++++++++++++------ drivers/net/fec_mxc.h | 3 ++- 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c index fbfc842..4af4976 100644 --- a/drivers/net/fec_mxc.c +++ b/drivers/net/fec_mxc.c @@ -391,7 +391,7 @@ static void fec_eth_phy_config(struct eth_device *dev) struct fec_priv *fec = (struct fec_priv *)dev->priv; struct phy_device *phydev;

• phydev = phy_connect(fec->bus, fec->phy_id, dev,

• phydev = phy_connect_by_mask(fec->bus, fec->phy_mask, dev, PHY_INTERFACE_MODE_RGMII); if (phydev) { fec->phydev = phydev;

@@ -898,7 +898,8 @@ static int fec_recv(struct eth_device *dev) return len; }

-static int fec_probe(bd_t *bd, int dev_id, int phy_id, uint32_t base_addr) +static int fec_probe(bd_t *bd, int dev_id, unsigned phy_mask,

• uint32_t base_addr)
  

{ struct eth_device *edev; struct fec_priv *fec; @@ -958,8 +959,11 @@ static int fec_probe(bd_t *bd, int dev_id, int phy_id, uint32_t base_addr) sprintf(edev->name, "FEC%i", dev_id); fec->dev_id = dev_id; }

• fec->phy_id = phy_id;

+#ifdef CONFIG_PHYLIB

• fec->phy_mask = phy_mask;

+#else

• fec->phy_id = ffs(phy_mask) - 1;

+#endif bus = mdio_alloc(); if (!bus) { printf("mdio_alloc failed\n"); @@ -1008,9 +1012,14 @@ err1: int fecmxc_initialize(bd_t *bd) { int lout = 1; +#ifdef CONFIG_FEC_MXC_PHYMASK

• unsigned phy_mask = CONFIG_FEC_MXC_PHYMASK;

+#else

• unsigned phy_mask = 1 << CONFIG_FEC_MXC_PHYADDR;

+#endif

debug("eth_init: fec_probe(bd)\n");

• lout = fec_probe(bd, -1, CONFIG_FEC_MXC_PHYADDR, IMX_FEC_BASE);

• lout = fec_probe(bd, -1, phy_mask, IMX_FEC_BASE);

  return lout;

} @@ -1021,7 +1030,7 @@ int fecmxc_initialize_multi(bd_t *bd, int dev_id, int phy_id, uint32_t addr) int lout = 1;

debug("eth_init: fec_probe(bd, %i, %i) @ %08x\n", dev_id, phy_id, addr);

• lout = fec_probe(bd, dev_id, phy_id, addr);

• lout = fec_probe(bd, dev_id, 1 << phy_id, addr);

  return lout;

} diff --git a/drivers/net/fec_mxc.h b/drivers/net/fec_mxc.h index 852b2e0..a0f50af 100644 --- a/drivers/net/fec_mxc.h +++ b/drivers/net/fec_mxc.h @@ -268,11 +268,12 @@ struct fec_priv { bd_t *bd; uint8_t *tdb_ptr; int dev_id;

• int phy_id; struct mii_dev *bus;

#ifdef CONFIG_PHYLIB

• int phy_mask; struct phy_device *phydev;

#else

• int phy_id; int (*mii_postcall)(int);

#endif };

I generally agree with this patchset. My only concern is that we add a new CONFIG_ option without documentation. Can you add an entry into the README file (I know, a lot of CONFIG_ are missing, but we can at least avoid to add hidden features..).

Best regards, Stefano Babic