| Message ID | 20240708060718.186075-1-jacky_chou@aspeedtech.com |
|---|---|
| State | Accepted |
| Delegated to: | Andes |
| Headers | show |
| Series | net: ftgmac100: Add Aspeed AST2700 support | expand |
On Mon, Jul 08, 2024 at 02:07:18PM +0800, Jacky Chou wrote: > Add support of Aspeed AST2700 SoC. AST2700 is based on ARM64 so modify > the DMA address related code to fit both ARM and ARM64. Besides, the > RMII/RGMII mode control register is moved from SCU500 to MAC50 so > initialize the register in ftgmac100_start correspondingly. > > Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com> > --- > drivers/net/ftgmac100.c | 66 +++++++++++++++++++++++++++++++++-------- > drivers/net/ftgmac100.h | 12 ++++++++ > 2 files changed, 65 insertions(+), 13 deletions(-) Acked-by: Leo Yu-Chi Liang <ycliang@andestech.com>
Hi Jacky, On Mon, 8 Jul 2024 at 15:37, Jacky Chou <jacky_chou@aspeedtech.com> wrote: > > Add support of Aspeed AST2700 SoC. AST2700 is based on ARM64 so modify > the DMA address related code to fit both ARM and ARM64. Besides, the > RMII/RGMII mode control register is moved from SCU500 to MAC50 so > initialize the register in ftgmac100_start correspondingly. It looks like these changes will affect the older platforms. Have you tested on an ast2600? Is it okay to be writing to the new memory locations on the old platforms? If this is the case, please mention it in the commit message. > diff --git a/drivers/net/ftgmac100.c b/drivers/net/ftgmac100.c > index 9fb8c12838..a909a0b309 100644 > --- a/drivers/net/ftgmac100.c > +++ b/drivers/net/ftgmac100.c > @@ -321,6 +333,7 @@ static int ftgmac100_start(struct udevice *dev) > struct eth_pdata *plat = dev_get_plat(dev); > struct ftgmac100_data *priv = dev_get_priv(dev); > struct ftgmac100 *ftgmac100 = priv->iobase; > + union ftgmac100_dma_addr dma_addr = {.hi = 0, .lo = 0}; You always set dma_addr before using it, so this zero-init is unnecessary. > @@ -352,7 +366,14 @@ static int ftgmac100_start(struct udevice *dev) > flush_dcache_range(start, end); > > for (i = 0; i < PKTBUFSRX; i++) { > - priv->rxdes[i].rxdes3 = (unsigned int)net_rx_packets[i]; > + unsigned int ip_align = 0; > + > + dma_addr.addr = (dma_addr_t)net_rx_packets[i]; > + priv->rxdes[i].rxdes2 = FIELD_PREP(FTGMAC100_RXDES2_RXBUF_BADR_HI, dma_addr.hi); > + /* For IP alignment */ > + if ((dma_addr.lo & (PKTALIGN - 1)) == 0) > + ip_align = 2; > + priv->rxdes[i].rxdes3 = dma_addr.lo + ip_align; Please explain why the old code didn't need alignment added, and the new version does. > @@ -394,6 +419,10 @@ static int ftgmac100_start(struct udevice *dev) > FTGMAC100_MACCR_RX_RUNT | > FTGMAC100_MACCR_RX_BROADPKT; > > + if (priv->is_ast2700 && (priv->phydev->interface == PHY_INTERFACE_MODE_RMII || > + priv->phydev->interface == PHY_INTERFACE_MODE_NCSI)) Use device_is_compatible(dev, "aspeed,ast2700-mac") instead of adding is_ast2700. > + maccr |= FTGMAC100_MACCR_RMII_ENABLE; > + > writel(maccr, &ftgmac100->maccr); > > ret = phy_startup(phydev); > @@ -443,9 +472,11 @@ static int ftgmac100_recv(struct udevice *dev, int flags, uchar **packetp) > ulong des_start = ((ulong)curr_des) & ~(ARCH_DMA_MINALIGN - 1); > ulong des_end = des_start + > roundup(sizeof(*curr_des), ARCH_DMA_MINALIGN); > - ulong data_start = curr_des->rxdes3; > + union ftgmac100_dma_addr data_start = { .lo = 0, .hi = 0 }; Same here. > ulong data_end; > > + data_start.hi = FIELD_GET(FTGMAC100_RXDES2_RXBUF_BADR_HI, curr_des->rxdes2); > + data_start.lo = curr_des->rxdes3; > invalidate_dcache_range(des_start, des_end);
Hello Joel, Thanks for your reply. > > > > Add support of Aspeed AST2700 SoC. AST2700 is based on ARM64 so > > modify the DMA address related code to fit both ARM and ARM64. > > Besides, the RMII/RGMII mode control register is moved from SCU500 to > > MAC50 so initialize the register in ftgmac100_start correspondingly. > > It looks like these changes will affect the older platforms. Have you tested on > an ast2600? Yes. We have verified on AST2600 and AST2700. These changes do not affect. > > Is it okay to be writing to the new memory locations on the old platforms? If > this is the case, please mention it in the commit message. Yes. I will refine the commit message to add more information in the next version. > > > diff --git a/drivers/net/ftgmac100.c b/drivers/net/ftgmac100.c index > > 9fb8c12838..a909a0b309 100644 > > --- a/drivers/net/ftgmac100.c > > +++ b/drivers/net/ftgmac100.c > > > @@ -321,6 +333,7 @@ static int ftgmac100_start(struct udevice *dev) > > struct eth_pdata *plat = dev_get_plat(dev); > > struct ftgmac100_data *priv = dev_get_priv(dev); > > struct ftgmac100 *ftgmac100 = priv->iobase; > > + union ftgmac100_dma_addr dma_addr = {.hi = 0, .lo = 0}; > > You always set dma_addr before using it, so this zero-init is unnecessary. Agree. I checked this part again. It does not need to initialize the variable to zero. I will remove it in the next version. Thanks for your kindly reminder. > > > @@ -352,7 +366,14 @@ static int ftgmac100_start(struct udevice *dev) > > flush_dcache_range(start, end); > > > > for (i = 0; i < PKTBUFSRX; i++) { > > - priv->rxdes[i].rxdes3 = (unsigned int)net_rx_packets[i]; > > + unsigned int ip_align = 0; > > + > > + dma_addr.addr = (dma_addr_t)net_rx_packets[i]; > > + priv->rxdes[i].rxdes2 = > FIELD_PREP(FTGMAC100_RXDES2_RXBUF_BADR_HI, dma_addr.hi); > > + /* For IP alignment */ > > + if ((dma_addr.lo & (PKTALIGN - 1)) == 0) > > + ip_align = 2; > > + priv->rxdes[i].rxdes3 = dma_addr.lo + ip_align; > > Please explain why the old code didn't need alignment added, and the new > version does. Sorry. I forgot the driver no longer need to support IP alignmen. IP alignment was originally supported because the NCSI PHY driver would crash. I have fixed the NCSI PHY driver and have submitted fixed patch to U-boot. And it has accepted. Therefore, I will remove this part in the next version. > > > @@ -394,6 +419,10 @@ static int ftgmac100_start(struct udevice *dev) > > FTGMAC100_MACCR_RX_RUNT | > > FTGMAC100_MACCR_RX_BROADPKT; > > > > + if (priv->is_ast2700 && (priv->phydev->interface == > PHY_INTERFACE_MODE_RMII || > > + priv->phydev->interface == > > + PHY_INTERFACE_MODE_NCSI)) > > Use device_is_compatible(dev, "aspeed,ast2700-mac") instead of adding > is_ast2700. Agree. I will adjust this code using device_is_compatible() in the next version. > > > + maccr |= FTGMAC100_MACCR_RMII_ENABLE; > > + > > writel(maccr, &ftgmac100->maccr); > > > > ret = phy_startup(phydev); > > @@ -443,9 +472,11 @@ static int ftgmac100_recv(struct udevice *dev, int > flags, uchar **packetp) > > ulong des_start = ((ulong)curr_des) & ~(ARCH_DMA_MINALIGN - > 1); > > ulong des_end = des_start + > > roundup(sizeof(*curr_des), ARCH_DMA_MINALIGN); > > - ulong data_start = curr_des->rxdes3; > > + union ftgmac100_dma_addr data_start = { .lo = 0, .hi = 0 }; > > Same here. I will remove it in the next version. Thanks for your kindly reminder. > > > ulong data_end; > > > > + data_start.hi = FIELD_GET(FTGMAC100_RXDES2_RXBUF_BADR_HI, > curr_des->rxdes2); > > + data_start.lo = curr_des->rxdes3; > > invalidate_dcache_range(des_start, des_end); Thanks, Jacky
diff --git a/drivers/net/ftgmac100.c b/drivers/net/ftgmac100.c index 9fb8c12838..a909a0b309 100644 --- a/drivers/net/ftgmac100.c +++ b/drivers/net/ftgmac100.c @@ -27,6 +27,7 @@ #include <linux/io.h> #include <linux/iopoll.h> #include <linux/printk.h> +#include <linux/bitfield.h> #include "ftgmac100.h" @@ -58,6 +59,15 @@ enum ftgmac100_model { FTGMAC100_MODEL_FARADAY, FTGMAC100_MODEL_ASPEED, + FTGMAC100_MODEL_ASPEED_AST2700, +}; + +union ftgmac100_dma_addr { + dma_addr_t addr; + struct { + u32 lo; + u32 hi; + }; }; /** @@ -97,6 +107,8 @@ struct ftgmac100_data { /* End of RX/TX ring buffer bits. Depend on model */ u32 rxdes0_edorr_mask; u32 txdes0_edotr_mask; + + bool is_ast2700; }; /* @@ -321,6 +333,7 @@ static int ftgmac100_start(struct udevice *dev) struct eth_pdata *plat = dev_get_plat(dev); struct ftgmac100_data *priv = dev_get_priv(dev); struct ftgmac100 *ftgmac100 = priv->iobase; + union ftgmac100_dma_addr dma_addr = {.hi = 0, .lo = 0}; struct phy_device *phydev = priv->phydev; unsigned int maccr, dblac, desc_size; ulong start, end; @@ -342,6 +355,7 @@ static int ftgmac100_start(struct udevice *dev) priv->rx_index = 0; for (i = 0; i < PKTBUFSTX; i++) { + priv->txdes[i].txdes2 = 0; priv->txdes[i].txdes3 = 0; priv->txdes[i].txdes0 = 0; } @@ -352,7 +366,14 @@ static int ftgmac100_start(struct udevice *dev) flush_dcache_range(start, end); for (i = 0; i < PKTBUFSRX; i++) { - priv->rxdes[i].rxdes3 = (unsigned int)net_rx_packets[i]; + unsigned int ip_align = 0; + + dma_addr.addr = (dma_addr_t)net_rx_packets[i]; + priv->rxdes[i].rxdes2 = FIELD_PREP(FTGMAC100_RXDES2_RXBUF_BADR_HI, dma_addr.hi); + /* For IP alignment */ + if ((dma_addr.lo & (PKTALIGN - 1)) == 0) + ip_align = 2; + priv->rxdes[i].rxdes3 = dma_addr.lo + ip_align; priv->rxdes[i].rxdes0 = 0; } priv->rxdes[PKTBUFSRX - 1].rxdes0 = priv->rxdes0_edorr_mask; @@ -362,10 +383,14 @@ static int ftgmac100_start(struct udevice *dev) flush_dcache_range(start, end); /* transmit ring */ - writel((u32)priv->txdes, &ftgmac100->txr_badr); + dma_addr.addr = (dma_addr_t)priv->txdes; + writel(dma_addr.lo, &ftgmac100->txr_badr); + writel(dma_addr.hi, &ftgmac100->txr_badr_hi); /* receive ring */ - writel((u32)priv->rxdes, &ftgmac100->rxr_badr); + dma_addr.addr = (dma_addr_t)priv->rxdes; + writel(dma_addr.lo, &ftgmac100->rxr_badr); + writel(dma_addr.hi, &ftgmac100->rxr_badr_hi); /* Configure TX/RX decsriptor size * This size is calculated based on cache line. @@ -394,6 +419,10 @@ static int ftgmac100_start(struct udevice *dev) FTGMAC100_MACCR_RX_RUNT | FTGMAC100_MACCR_RX_BROADPKT; + if (priv->is_ast2700 && (priv->phydev->interface == PHY_INTERFACE_MODE_RMII || + priv->phydev->interface == PHY_INTERFACE_MODE_NCSI)) + maccr |= FTGMAC100_MACCR_RMII_ENABLE; + writel(maccr, &ftgmac100->maccr); ret = phy_startup(phydev); @@ -443,9 +472,11 @@ static int ftgmac100_recv(struct udevice *dev, int flags, uchar **packetp) ulong des_start = ((ulong)curr_des) & ~(ARCH_DMA_MINALIGN - 1); ulong des_end = des_start + roundup(sizeof(*curr_des), ARCH_DMA_MINALIGN); - ulong data_start = curr_des->rxdes3; + union ftgmac100_dma_addr data_start = { .lo = 0, .hi = 0 }; ulong data_end; + data_start.hi = FIELD_GET(FTGMAC100_RXDES2_RXBUF_BADR_HI, curr_des->rxdes2); + data_start.lo = curr_des->rxdes3; invalidate_dcache_range(des_start, des_end); if (!(curr_des->rxdes0 & FTGMAC100_RXDES0_RXPKT_RDY)) @@ -465,9 +496,9 @@ static int ftgmac100_recv(struct udevice *dev, int flags, uchar **packetp) __func__, priv->rx_index, rxlen); /* Invalidate received data */ - data_end = data_start + roundup(rxlen, ARCH_DMA_MINALIGN); - invalidate_dcache_range(data_start, data_end); - *packetp = (uchar *)data_start; + data_end = data_start.addr + roundup(rxlen, ARCH_DMA_MINALIGN); + invalidate_dcache_range(data_start.addr, data_end); + *packetp = (uchar *)data_start.addr; return rxlen; } @@ -493,6 +524,7 @@ static int ftgmac100_send(struct udevice *dev, void *packet, int length) struct ftgmac100_data *priv = dev_get_priv(dev); struct ftgmac100 *ftgmac100 = priv->iobase; struct ftgmac100_txdes *curr_des = &priv->txdes[priv->tx_index]; + union ftgmac100_dma_addr dma_addr; ulong des_start = ((ulong)curr_des) & ~(ARCH_DMA_MINALIGN - 1); ulong des_end = des_start + roundup(sizeof(*curr_des), ARCH_DMA_MINALIGN); @@ -511,10 +543,12 @@ static int ftgmac100_send(struct udevice *dev, void *packet, int length) length = (length < ETH_ZLEN) ? ETH_ZLEN : length; - curr_des->txdes3 = (unsigned int)packet; + dma_addr.addr = (dma_addr_t)packet; + curr_des->txdes2 = FIELD_PREP(FTGMAC100_TXDES2_TXBUF_BADR_HI, dma_addr.hi); + curr_des->txdes3 = dma_addr.lo; /* Flush data to be sent */ - data_start = curr_des->txdes3; + data_start = (ulong)dma_addr.addr; data_end = data_start + roundup(length, ARCH_DMA_MINALIGN); flush_dcache_range(data_start, data_end); @@ -577,6 +611,11 @@ static int ftgmac100_of_to_plat(struct udevice *dev) if (dev_get_driver_data(dev) == FTGMAC100_MODEL_ASPEED) { priv->rxdes0_edorr_mask = BIT(30); priv->txdes0_edotr_mask = BIT(30); + priv->is_ast2700 = false; + } else if (dev_get_driver_data(dev) == FTGMAC100_MODEL_ASPEED_AST2700) { + priv->rxdes0_edorr_mask = BIT(30); + priv->txdes0_edotr_mask = BIT(30); + priv->is_ast2700 = true; } else { priv->rxdes0_edorr_mask = BIT(15); priv->txdes0_edotr_mask = BIT(15); @@ -667,10 +706,11 @@ static const struct eth_ops ftgmac100_ops = { }; static const struct udevice_id ftgmac100_ids[] = { - { .compatible = "faraday,ftgmac100", .data = FTGMAC100_MODEL_FARADAY }, - { .compatible = "aspeed,ast2500-mac", .data = FTGMAC100_MODEL_ASPEED }, - { .compatible = "aspeed,ast2600-mac", .data = FTGMAC100_MODEL_ASPEED }, - { } + { .compatible = "faraday,ftgmac100", .data = FTGMAC100_MODEL_FARADAY }, + { .compatible = "aspeed,ast2500-mac", .data = FTGMAC100_MODEL_ASPEED }, + { .compatible = "aspeed,ast2600-mac", .data = FTGMAC100_MODEL_ASPEED }, + { .compatible = "aspeed,ast2700-mac", .data = FTGMAC100_MODEL_ASPEED_AST2700 }, + {} }; U_BOOT_DRIVER(ftgmac100) = { diff --git a/drivers/net/ftgmac100.h b/drivers/net/ftgmac100.h index 37c029aded..c38b57c954 100644 --- a/drivers/net/ftgmac100.h +++ b/drivers/net/ftgmac100.h @@ -66,6 +66,13 @@ struct ftgmac100 { unsigned int rx_runt; /* 0xc0 */ unsigned int rx_crcer_ftl; /* 0xc4 */ unsigned int rx_col_lost; /* 0xc8 */ + unsigned int reserved[43]; /* 0xcc - 0x174 */ + unsigned int txr_badr_lo; /* 0x178, defined in ast2700 */ + unsigned int txr_badr_hi; /* 0x17c, defined in ast2700 */ + unsigned int hptxr_badr_lo; /* 0x180, defined in ast2700 */ + unsigned int hptxr_badr_hi; /* 0x184, defined in ast2700 */ + unsigned int rxr_badr_lo; /* 0x188, defined in ast2700 */ + unsigned int rxr_badr_hi; /* 0x18c, defined in ast2700 */ }; /* @@ -158,6 +165,7 @@ struct ftgmac100 { #define FTGMAC100_MACCR_RX_BROADPKT BIT(17) #define FTGMAC100_MACCR_DISCARD_CRCERR BIT(18) #define FTGMAC100_MACCR_FAST_MODE BIT(19) +#define FTGMAC100_MACCR_RMII_ENABLE BIT(20) /* defined in ast2700 */ #define FTGMAC100_MACCR_SW_RST BIT(31) /* @@ -202,6 +210,8 @@ struct ftgmac100_txdes { #define FTGMAC100_TXDES1_TX2FIC BIT(30) #define FTGMAC100_TXDES1_TXIC BIT(31) +#define FTGMAC100_TXDES2_TXBUF_BADR_HI GENMASK(18, 16) + /* * Receive descriptor, aligned to 16 bytes */ @@ -241,4 +251,6 @@ struct ftgmac100_rxdes { #define FTGMAC100_RXDES1_UDP_CHKSUM_ERR BIT(26) #define FTGMAC100_RXDES1_IP_CHKSUM_ERR BIT(27) +#define FTGMAC100_RXDES2_RXBUF_BADR_HI GENMASK(18, 16) + #endif /* __FTGMAC100_H */
Add support of Aspeed AST2700 SoC. AST2700 is based on ARM64 so modify the DMA address related code to fit both ARM and ARM64. Besides, the RMII/RGMII mode control register is moved from SCU500 to MAC50 so initialize the register in ftgmac100_start correspondingly. Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com> --- drivers/net/ftgmac100.c | 66 +++++++++++++++++++++++++++++++++-------- drivers/net/ftgmac100.h | 12 ++++++++ 2 files changed, 65 insertions(+), 13 deletions(-)