[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