On 10/15/18 10:39 PM, Joe Hershberger wrote:
> On Wed, Oct 10, 2018 at 6:45 AM Cédric Le Goater <c...@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 <c...@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 <macp...@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.
For the printf below. later on in the pachset, the function is
completely removed, so it didn't seem important to introduce
more changes.
>
>> {
>> - 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.
The MDIO (patch 5) adds :
phydev = phy_connect(priv->bus, priv->phy_addr, dev, priv->phy_mode);
Nevetheless, I think we can improve the prototype of ftgmac100_phy_init()
to :
static int ftgmac100_phy_init(struct udevice *dev)
same for ftgmac100_mdio_init()
>
>> {
>> - 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.
I will propose the prototype changes in v4.
Thanks,
C.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
