[U-Boot] [PATCH v3 03/13] net: ftgmac100: convert to driver model

Joe Hershberger joe.hershberger at ni.com
Mon Oct 15 20:40:00 UTC 2018


On Wed, Oct 10, 2018 at 6:45 AM Cédric Le Goater <clg at kaod.org> wrote:
>
> The driver is based on the previous one and the code is only adapted
> to fit the driver model. The support for the Faraday ftgmac100
> controller is the same with MAC and MDIO bus support for RGMII/RMII
> modes.
>
> Configuration is updated to enable compile again. At this stage, the
> driver compiles but is not yet functional.
>
> Signed-off-by: Cédric Le Goater <clg at kaod.org>
> ---
>  include/netdev.h        |   1 -
>  drivers/net/ftgmac100.c | 223 +++++++++++++++++++++++-----------------
>  drivers/net/Kconfig     |  26 +++++
>  3 files changed, 157 insertions(+), 93 deletions(-)
>
> diff --git a/include/netdev.h b/include/netdev.h
> index 55001625fb92..0a1a3a2d8da2 100644
> --- a/include/netdev.h
> +++ b/include/netdev.h
> @@ -43,7 +43,6 @@ int ethoc_initialize(u8 dev_num, int base_addr);
>  int fec_initialize (bd_t *bis);
>  int fecmxc_initialize(bd_t *bis);
>  int fecmxc_initialize_multi(bd_t *bis, int dev_id, int phy_id, uint32_t addr);
> -int ftgmac100_initialize(bd_t *bits);
>  int ftmac100_initialize(bd_t *bits);
>  int ftmac110_initialize(bd_t *bits);
>  void gt6426x_eth_initialize(bd_t *bis);
> diff --git a/drivers/net/ftgmac100.c b/drivers/net/ftgmac100.c
> index c996f5f4a167..67a7c73503c5 100644
> --- a/drivers/net/ftgmac100.c
> +++ b/drivers/net/ftgmac100.c
> @@ -7,15 +7,16 @@
>   *
>   * (C) Copyright 2010 Andes Technology
>   * Macpaul Lin <macpaul at andestech.com>
> + *
> + * Copyright (C) 2018, IBM Corporation.
>   */
>
> -#include <config.h>
> -#include <common.h>
> +#include <dm.h>
> +#include <miiphy.h>
>  #include <malloc.h>
>  #include <net.h>
> -#include <asm/io.h>
> +#include <linux/io.h>
>  #include <asm/dma-mapping.h>
> -#include <linux/mii.h>
>
>  #include "ftgmac100.h"
>
> @@ -28,7 +29,19 @@
>  /* PKTBUFSTX/PKTBUFSRX must both be power of 2 */
>  #define PKTBUFSTX      4       /* must be power of 2 */
>
> +/**
> + * struct ftgmac100_data - private data for the FTGMAC100 driver
> + *
> + * @iobase: The base address of the hardware registers
> + * @txdes: The array of transmit descriptors
> + * @rxdes: The array of receive descriptors
> + * @tx_index: Transmit descriptor index in @txdes
> + * @rx_index: Receive descriptor index in @rxdes
> + * @phy_addr: The PHY interface address to use
> + */
>  struct ftgmac100_data {
> +       struct ftgmac100 *iobase;
> +
>         ulong txdes_dma;
>         struct ftgmac100_txdes *txdes;
>         ulong rxdes_dma;
> @@ -41,10 +54,10 @@ struct ftgmac100_data {
>  /*
>   * struct mii_bus functions
>   */
> -static int ftgmac100_mdiobus_read(struct eth_device *dev, int phy_addr,
> -       int regnum)
> +static int ftgmac100_mdiobus_read(struct ftgmac100_data *priv, int phy_addr,
> +                                 int regnum)
>  {
> -       struct ftgmac100 *ftgmac100 = (struct ftgmac100 *)dev->iobase;
> +       struct ftgmac100 *ftgmac100 = priv->iobase;
>         int phycr;
>         int i;
>
> @@ -76,10 +89,10 @@ static int ftgmac100_mdiobus_read(struct eth_device *dev, int phy_addr,
>         return -1;
>  }
>
> -static int ftgmac100_mdiobus_write(struct eth_device *dev, int phy_addr,
> -       int regnum, u16 value)
> +static int ftgmac100_mdiobus_write(struct ftgmac100_data *priv, int phy_addr,
> +                                  int regnum, u16 value)
>  {
> -       struct ftgmac100 *ftgmac100 = (struct ftgmac100 *)dev->iobase;
> +       struct ftgmac100 *ftgmac100 = priv->iobase;
>         int phycr;
>         int data;
>         int i;
> @@ -114,9 +127,10 @@ static int ftgmac100_mdiobus_write(struct eth_device *dev, int phy_addr,
>         return -1;
>  }
>
> -int ftgmac100_phy_read(struct eth_device *dev, int addr, int reg, u16 *value)
> +int ftgmac100_phy_read(struct ftgmac100_data *priv, int addr, int reg,
> +                      u16 *value)
>  {
> -       *value = ftgmac100_mdiobus_read(dev , addr, reg);
> +       *value = ftgmac100_mdiobus_read(priv, addr, reg);
>
>         if (*value == -1)
>                 return -1;
> @@ -124,31 +138,31 @@ int ftgmac100_phy_read(struct eth_device *dev, int addr, int reg, u16 *value)
>         return 0;
>  }
>
> -int  ftgmac100_phy_write(struct eth_device *dev, int addr, int reg, u16 value)
> +int ftgmac100_phy_write(struct ftgmac100_data *priv, int addr, int reg,
> +                       u16 value)
>  {
> -       if (ftgmac100_mdiobus_write(dev, addr, reg, value) == -1)
> +       if (ftgmac100_mdiobus_write(priv, addr, reg, value) == -1)
>                 return -1;
>
>         return 0;
>  }
>
> -static int ftgmac100_phy_reset(struct eth_device *dev)
> +static int ftgmac100_phy_reset(struct ftgmac100_data *priv, struct udevice *dev)

Why does this function get a dev parameter? Seems unused.

>  {
> -       struct ftgmac100_data *priv = dev->priv;
>         int i;
>         u16 status, adv;
>
>         adv = ADVERTISE_CSMA | ADVERTISE_ALL;
>
> -       ftgmac100_phy_write(dev, priv->phy_addr, MII_ADVERTISE, adv);
> +       ftgmac100_phy_write(priv, priv->phy_addr, MII_ADVERTISE, adv);
>
>         printf("%s: Starting autonegotiation...\n", dev->name);

Seems like a useless print. Remove dev?

>
> -       ftgmac100_phy_write(dev, priv->phy_addr,
> -               MII_BMCR, (BMCR_ANENABLE | BMCR_ANRESTART));
> +       ftgmac100_phy_write(priv, priv->phy_addr,
> +                           MII_BMCR, (BMCR_ANENABLE | BMCR_ANRESTART));
>
>         for (i = 0; i < 100000 / 100; i++) {
> -               ftgmac100_phy_read(dev, priv->phy_addr, MII_BMSR, &status);
> +               ftgmac100_phy_read(priv, priv->phy_addr, MII_BMSR, &status);
>
>                 if (status & BMSR_ANEGCOMPLETE)
>                         break;
> @@ -166,19 +180,17 @@ static int ftgmac100_phy_reset(struct eth_device *dev)
>         return 1;
>  }
>
> -static int ftgmac100_phy_init(struct eth_device *dev)
> +static int ftgmac100_phy_init(struct ftgmac100_data *priv, struct udevice *dev)

Same here, why dev parameter? Only passed to reset.

>  {
> -       struct ftgmac100_data *priv = dev->priv;
> -
>         int phy_addr;
>         u16 phy_id, status, adv, lpa, stat_ge;
>         int media, speed, duplex;
>         int i;
>
>         /* Check if the PHY is up to snuff... */
> -       for (phy_addr = 0; phy_addr < CONFIG_PHY_MAX_ADDR; phy_addr++) {
> +       for (phy_addr = 0; phy_addr < PHY_MAX_ADDR; phy_addr++) {
>
> -               ftgmac100_phy_read(dev, phy_addr, MII_PHYSID1, &phy_id);
> +               ftgmac100_phy_read(priv, phy_addr, MII_PHYSID1, &phy_id);
>
>                 /*
>                  * When it is unable to found PHY,
> @@ -197,15 +209,15 @@ static int ftgmac100_phy_init(struct eth_device *dev)
>                 return 0;
>         }

[ ... ]

Looks good otherwise.


More information about the U-Boot mailing list