[U-Boot] 83xx, uec: adjust enet_interface settings on the fly.

[Andy Fleming]

On Thu, Jan 7, 2010 at 2:01 AM, Heiko Schocher <hs at denx.de> wrote:
> If using UCC as Ethernet Controller and type = FAST_ETH, it was
> not possible to switch between 10 and 100 MBit interfaces. This
> patch adds this for following interfaces:
>
> 10_MII
> 10_RMII
> 10_RGMII
> 100_MII
> 100_RMII
> 100_RGMII
>
> Signed-off-by: Heiko Schocher <hs at denx.de>
> ---
>  drivers/qe/uec.c |   73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 73 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/qe/uec.c b/drivers/qe/uec.c
> index db95ada..9851cc4 100644
> --- a/drivers/qe/uec.c
> +++ b/drivers/qe/uec.c
> @@ -65,6 +65,22 @@ static uec_info_t uec_info[] = {
>
>  #define MAXCONTROLLERS (8)
>
> +static char *enet_interface_name[] = {
> +       "10_MII",
> +       "10_RMII",
> +       "10_RGMII",
> +       "100_MII",
> +       "100_RMII",
> +       "100_RGMII",
> +       "1000_GMII",
> +       "1000_RGMII",
> +       "1000_RGMII_ID",
> +       "1000_RGMII_RXID",
> +       "1000_TBI",
> +       "1000_RTBI",
> +       "1000_SGMII"
> +};
> +
>  static struct eth_device *devlist[MAXCONTROLLERS];
>
>  u16 phy_read (struct uec_mii_info *mii_info, u16 regnum);
> @@ -497,6 +513,60 @@ bus_fail:
>        return err;
>  }
>
> +static  void adjust_fast_enet_interface(int speed, struct eth_device *dev)
> +{
> +       uec_private_t   *uec = (uec_private_t *)dev->priv;
> +       uec_t           *uec_regs;
> +       int             change = 0;
> +
> +       extern void change_phy_interface_mode(struct eth_device *dev,
> +                                        enet_interface_e mode);
> +       uec_regs = uec->uec_regs;
> +
> +       switch (speed) {
> +       case 100:
> +               switch (uec->uec_info->enet_interface) {
> +               case ENET_10_MII:
> +               case ENET_10_RMII:
> +               case ENET_10_RGMII:


I know you aren't responsible for this design choice, but I'd love it
if we stopped this sort of thing.  There's no sensible reason to unite
speed and interface type into one variable.  They are, in fact, two
nearly-independent variables.  If they directly corresponded to some
register value, it might make sense, but really they encourage a
rigidity of thought about a concept which is much more fluid.  The
speed will vary based on the link partner.  The interface is usually
hardwired by the board designer, and at most, modifiable at boot time
by DIP switches.


> +                       uec->uec_info->enet_interface += 3;


And then we end up with something *highly* fragile like this.  *3* has
no meaning, except in the very specific context of the particular
order in which the interfaces have been specified in this driver.  All
it takes is some architect deciding to add support for 10M SGMII, or
10M WHATEVERMII, and now everything breaks in a way that is not
immediately apparent....

Andy