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

[Stefano Babic <sbabic@denx.de>]

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

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================