[PATCH] net: ftgmac100: Add Aspeed AST2700 support

[PATCH] net: ftgmac100: Add Aspeed AST2700 support

[Jacky Chou]

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(-)

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 */
-- 
2.25.1

Re: [PATCH] net: ftgmac100: Add Aspeed AST2700 support

[Leo Liang]

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>

Re: [PATCH] net: ftgmac100: Add Aspeed AST2700 support

[Joel Stanley]

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);

回覆: [PATCH] net: ftgmac100: Add Aspeed AST2700 support

[Jacky Chou]

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