Re: [U-Boot] [PATCH 4/5] net: ftgmac100: add support for Aspeed SoC

Hi Cedric,

On 10 September 2018 at 07:21, Cédric Le Goater clg@kaod.org wrote:

The Faraday ftgmac100 MAC controllers as found on the Aspeed SoCs have some slight differences in the HW interface (End-Of-Rx/Tx-Ring bits). Also include the Aspeed clock enablement.

Signed-off-by: Cédric Le Goater clg@kaod.org

drivers/net/ftgmac100.h | 5 +++ drivers/net/ftgmac100.c | 72 +++++++++++++++++++++++++++++++++++++---- 2 files changed, 71 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ftgmac100.h b/drivers/net/ftgmac100.h index 9a789e4d5bee..b8f99ddf48bc 100644 --- a/drivers/net/ftgmac100.h +++ b/drivers/net/ftgmac100.h @@ -129,6 +129,11 @@ struct ftgmac100 { #define FTGMAC100_DMAFIFOS_RXDMA_REQ BIT(30) #define FTGMAC100_DMAFIFOS_TXDMA_REQ BIT(31)

+/*

• • Feature Register
• */

+#define FTGMAC100_REVR_NEW_MDIO BIT(31)

/*

• Receive buffer size register

*/ diff --git a/drivers/net/ftgmac100.c b/drivers/net/ftgmac100.c index 8d7bf5b9b351..3df48a82c1ad 100644 --- a/drivers/net/ftgmac100.c +++ b/drivers/net/ftgmac100.c @@ -27,6 +27,8 @@ /* PKTBUFSTX/PKTBUFSRX must both be power of 2 */ #define PKTBUFSTX 4

+#define FTGMAC100_ASPEED_NR_CLKS 2

struct ftgmac100_data { phys_addr_t iobase;

@@ -40,6 +42,11 @@ struct ftgmac100_data {

Comments on struct and members

    struct mii_dev *bus;
    u32 phy_mode;
    u32 max_speed;

•   struct clk clks[FTGMAC100_ASPEED_NR_CLKS];
  

•   u32 rxdes0_edorr_mask;
  

•   u32 txdes0_edotr_mask;
  

•   bool is_aspeed;
  

};

/* @@ -115,9 +122,15 @@ static int ftgmac100_mdio_write(struct mii_dev *bus, int phy_addr, int dev_addr,

static int ftgmac100_mdio_init(struct ftgmac100_data *priv, int dev_id) {

•   struct ftgmac100 *ftgmac100 = (struct ftgmac100 *)priv->iobase;
    struct mii_dev *bus;
    int ret;
  
  

•   if (priv->is_aspeed) {
  

Perhaps call this old_mdio_if ?

•           /* This driver supports the old MDIO interface */
  

•           clrbits_le32(&ftgmac100->revr, FTGMAC100_REVR_NEW_MDIO);
  

•   };
  

•   bus = mdio_alloc();
    if (!bus)
            return -ENOMEM;
  

@@ -246,13 +259,13 @@ static int ftgmac100_start(struct udevice *dev) priv->txdes[i].txdes3 = 0; priv->txdes[i].txdes0 = 0; }

•   priv->txdes[PKTBUFSTX - 1].txdes0 = FTGMAC100_TXDES0_EDOTR;
  

•   priv->txdes[PKTBUFSTX - 1].txdes0 = priv->txdes0_edotr_mask;
  
    for (i = 0; i < PKTBUFSRX; i++) {
            priv->rxdes[i].rxdes3 = (unsigned int)net_rx_packets[i];
            priv->rxdes[i].rxdes0 = 0;
    }
  

•   priv->rxdes[PKTBUFSRX - 1].rxdes0 = FTGMAC100_RXDES0_EDORR;
  

•   priv->rxdes[PKTBUFSRX - 1].rxdes0 = priv->rxdes0_edorr_mask;
  
    /* transmit ring */
    writel((u32)priv->txdes, &ftgmac100->txr_badr);
  

@@ -378,7 +391,7 @@ static int ftgmac100_send(struct udevice *dev, void *packet, int length) flush_dcache_range(data_start, data_end);

    /* only one descriptor on TXBUF */

•   curr_des->txdes0 &= FTGMAC100_TXDES0_EDOTR;
  

•   curr_des->txdes0 &= priv->txdes0_edotr_mask;
    curr_des->txdes0 |= FTGMAC100_TXDES0_FTS |
            FTGMAC100_TXDES0_LTS |
            FTGMAC100_TXDES0_TXBUF_SIZE(length) |
  

@@ -409,8 +422,11 @@ static int ftgmac100_write_hwaddr(struct udevice *dev)

static int ftgmac100_ofdata_to_platdata(struct udevice *dev) {

•   struct ftgmac100_data *priv = dev_get_priv(dev);
    struct eth_pdata *pdata = dev_get_platdata(dev);
    const char *phy_mode;
  

•   int ret;
  

•   int i;
  
    pdata->iobase = devfdt_get_addr(dev);
    pdata->phy_interface = -1;
  

@@ -424,13 +440,39 @@ static int ftgmac100_ofdata_to_platdata(struct udevice *dev)

    pdata->max_speed = dev_read_u32_default(dev, "max-speed", 0);

•   if (device_is_compatible(dev, "aspeed,ast2500-mac")) {
  

Should use dev_get_driver_data() here.

•           priv->rxdes0_edorr_mask = BIT(30);
  

•           priv->txdes0_edotr_mask = BIT(30);
  

•           priv->is_aspeed = true;
  

•   } else {
  

•           priv->rxdes0_edorr_mask = BIT(15);
  

•           priv->txdes0_edotr_mask = BIT(15);
  

•   }
  

•   if (priv->is_aspeed) {
  

•           for (i = 0; i < FTGMAC100_ASPEED_NR_CLKS; i++) {
  

•                   ret = clk_get_by_index(dev, i, &priv->clks[i]);
  

•                   if (ret) {
  

•                           dev_err(dev, "Failed to get clock: %d\n", ret);
  

•                           goto out_clk_free;
  

•                   }
  

•           }
  

clk_get_bulk() ?

•   }
  

•   return 0;
  

+out_clk_free:

•   while (--i >= 0)
  

•           clk_free(&priv->clks[i]);
  

•   return ret;
  

}

static int ftgmac100_probe(struct udevice *dev) { struct eth_pdata *pdata = dev_get_platdata(dev); struct ftgmac100_data *priv = dev_get_priv(dev);

•   int i;
    int ret;
  
    priv->iobase = pdata->iobase;
  

@@ -438,19 +480,33 @@ static int ftgmac100_probe(struct udevice *dev) priv->max_speed = pdata->max_speed; priv->phyaddr = 0;

•   if (priv->is_aspeed) {
  

Why does this depend on aspeed? Can the DT specify what clocks are needed and how many?

•           for (i = 0; i < FTGMAC100_ASPEED_NR_CLKS; i++) {
  

clk_get_bulk() ?

Should use device tree to get number of clocks.

•                   ret = clk_enable(&priv->clks[i]);
  

•                   if (ret) {
  

•                           dev_err(dev, "Failed to enable clock: %d\n",
  

•                                   ret);
  

•                           goto out_clk_release;
  

•                   }
  

•           }
  

•   }
  

•   ret = ftgmac100_mdio_init(priv, dev->seq);
    if (ret) {
            dev_err(dev, "Failed to initialize mdiobus: %d\n", ret);
  

•           goto out;
  

•           goto out_clk_release;
    }
  
    ret = ftgmac100_phy_init(priv, dev);
    if (ret) {
            dev_err(dev, "Failed to initialize PHY: %d\n", ret);
  

•           goto out;
  

•           goto out_clk_release;
    }
  
  

-out: +out_clk_release:

•   if (ret && priv->is_aspeed)
  

•           clk_release_all(priv->clks, FTGMAC100_ASPEED_NR_CLKS);
  

•   return ret;
  

}

@@ -462,6 +518,9 @@ static int ftgmac100_remove(struct udevice *dev) mdio_unregister(priv->bus); mdio_free(priv->bus);

•   if (priv->is_aspeed)
  

•           clk_release_all(priv->clks, FTGMAC100_ASPEED_NR_CLKS);
  

•   return 0;
  

}

@@ -476,6 +535,7 @@ static const struct eth_ops ftgmac100_ops = {

static const struct udevice_id ftgmac100_ids[] = { { .compatible = "faraday,ftgmac100" },

•   { .compatible = "aspeed,ast2500-mac" },
  

Need .data = ASPEED here, or something like that

    { }

};

-- 2.17.1

Regards, Simon