The AST2700 is the 7th generation SoC from Aspeed, featuring three GPIO
controllers that are support 64-bit DMA capability.
Adding features is shown in the following list.
1.Support 64-bit DMA
  Add the high address (63:32) registers for description address and the
  description field for packet buffer with high address part.
  These registers and fields in legacy Aspeed SoC are reserved.
  This 64-bit DMA changing has verified on legacy Aspeed Soc, like
  AST2600.
2.Set RMII pin strap in AST2700 compitable
  Use bit 20 of MAC 0x50 to represent the pin strap of AST2700 RMII and
  RGMII. Set to 1 is RMII pin, otherwise is RGMII.
  This bis is also reserved in legacy Aspeed SoC.

Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 62 +++++++++++++++++-------
 drivers/net/ethernet/faraday/ftgmac100.h | 10 ++++
 2 files changed, 55 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 17ec35e75a65..0f4f161d0a90 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -9,6 +9,7 @@
 #define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
 
 #include <linux/clk.h>
+#include <linux/reset.h>
 #include <linux/dma-mapping.h>
 #include <linux/etherdevice.h>
 #include <linux/ethtool.h>
@@ -19,6 +20,7 @@
 #include <linux/of.h>
 #include <linux/of_mdio.h>
 #include <linux/phy.h>
+#include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 #include <linux/property.h>
 #include <linux/crc32.h>
@@ -98,6 +100,7 @@ struct ftgmac100 {
 	struct work_struct reset_task;
 	struct mii_bus *mii_bus;
 	struct clk *clk;
+	struct reset_control *rst;
 
 	/* AST2500/AST2600 RMII ref clock gate */
 	struct clk *rclk;
@@ -265,10 +268,12 @@ static void ftgmac100_init_hw(struct ftgmac100 *priv)
 	iowrite32(reg, priv->base + FTGMAC100_OFFSET_ISR);
 
 	/* Setup RX ring buffer base */
-	iowrite32(priv->rxdes_dma, priv->base + FTGMAC100_OFFSET_RXR_BADR);
+	iowrite32(lower_32_bits(priv->rxdes_dma), priv->base + FTGMAC100_OFFSET_RXR_BADR);
+	iowrite32(upper_32_bits(priv->rxdes_dma), priv->base + FTGMAC100_OFFSET_RXR_BADDR_HIGH);
 
 	/* Setup TX ring buffer base */
-	iowrite32(priv->txdes_dma, priv->base + FTGMAC100_OFFSET_NPTXR_BADR);
+	iowrite32(lower_32_bits(priv->txdes_dma), priv->base + FTGMAC100_OFFSET_NPTXR_BADR);
+	iowrite32(upper_32_bits(priv->txdes_dma), priv->base + FTGMAC100_OFFSET_TXR_BADDR_HIGH);
 
 	/* Configure RX buffer size */
 	iowrite32(FTGMAC100_RBSR_SIZE(RX_BUF_SIZE),
@@ -349,6 +354,10 @@ static void ftgmac100_start_hw(struct ftgmac100 *priv)
 	if (priv->netdev->features & NETIF_F_HW_VLAN_CTAG_RX)
 		maccr |= FTGMAC100_MACCR_RM_VLAN;
 
+	if (of_device_is_compatible(priv->dev->of_node, "aspeed,ast2700-mac") &&
+	    priv->netdev->phydev->interface == PHY_INTERFACE_MODE_RMII)
+		maccr |= FTGMAC100_MACCR_RMII_ENABLE;
+
 	/* Hit the HW */
 	iowrite32(maccr, priv->base + FTGMAC100_OFFSET_MACCR);
 }
@@ -425,7 +434,8 @@ static int ftgmac100_alloc_rx_buf(struct ftgmac100 *priv, unsigned int entry,
 	priv->rx_skbs[entry] = skb;
 
 	/* Store DMA address into RX desc */
-	rxdes->rxdes3 = cpu_to_le32(map);
+	rxdes->rxdes2 = FIELD_PREP(FTGMAC100_RXDES2_RXBUF_BADR_HI, upper_32_bits(map));
+	rxdes->rxdes3 = lower_32_bits(map);
 
 	/* Ensure the above is ordered vs clearing the OWN bit */
 	dma_wmb();
@@ -551,7 +561,7 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 				       csum_vlan & 0xffff);
 
 	/* Tear down DMA mapping, do necessary cache management */
-	map = le32_to_cpu(rxdes->rxdes3);
+	map = le32_to_cpu(rxdes->rxdes3) | ((rxdes->rxdes2 & FTGMAC100_RXDES2_RXBUF_BADR_HI) << 16);
 
 #if defined(CONFIG_ARM) && !defined(CONFIG_ARM_DMA_USE_IOMMU)
 	/* When we don't have an iommu, we can save cycles by not
@@ -563,7 +573,6 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 	dma_unmap_single(priv->dev, map, RX_BUF_SIZE, DMA_FROM_DEVICE);
 #endif
 
-
 	/* Resplenish rx ring */
 	ftgmac100_alloc_rx_buf(priv, pointer, rxdes, GFP_ATOMIC);
 	priv->rx_pointer = ftgmac100_next_rx_pointer(priv, pointer);
@@ -628,7 +637,9 @@ static void ftgmac100_free_tx_packet(struct ftgmac100 *priv,
 				     struct ftgmac100_txdes *txdes,
 				     u32 ctl_stat)
 {
-	dma_addr_t map = le32_to_cpu(txdes->txdes3);
+	dma_addr_t map = le32_to_cpu(txdes->txdes3) |
+			 ((txdes->txdes2 & FTGMAC100_TXDES2_TXBUF_BADR_HI) << 16);
+
 	size_t len;
 
 	if (ctl_stat & FTGMAC100_TXDES0_FTS) {
@@ -784,7 +795,8 @@ static netdev_tx_t ftgmac100_hard_start_xmit(struct sk_buff *skb,
 	f_ctl_stat |= FTGMAC100_TXDES0_FTS;
 	if (nfrags == 0)
 		f_ctl_stat |= FTGMAC100_TXDES0_LTS;
-	txdes->txdes3 = cpu_to_le32(map);
+	txdes->txdes2 = FIELD_PREP(FTGMAC100_TXDES2_TXBUF_BADR_HI, upper_32_bits((ulong)map));
+	txdes->txdes3 = lower_32_bits(map);
 	txdes->txdes1 = cpu_to_le32(csum_vlan);
 
 	/* Next descriptor */
@@ -812,7 +824,9 @@ static netdev_tx_t ftgmac100_hard_start_xmit(struct sk_buff *skb,
 			ctl_stat |= FTGMAC100_TXDES0_LTS;
 		txdes->txdes0 = cpu_to_le32(ctl_stat);
 		txdes->txdes1 = 0;
-		txdes->txdes3 = cpu_to_le32(map);
+		txdes->txdes2 =
+			FIELD_PREP(FTGMAC100_TXDES2_TXBUF_BADR_HI, upper_32_bits((ulong)map));
+		txdes->txdes3 = lower_32_bits(map);
 
 		/* Next one */
 		pointer = ftgmac100_next_tx_pointer(priv, pointer);
@@ -887,7 +901,8 @@ static void ftgmac100_free_buffers(struct ftgmac100 *priv)
 	for (i = 0; i < priv->rx_q_entries; i++) {
 		struct ftgmac100_rxdes *rxdes = &priv->rxdes[i];
 		struct sk_buff *skb = priv->rx_skbs[i];
-		dma_addr_t map = le32_to_cpu(rxdes->rxdes3);
+		dma_addr_t map = le32_to_cpu(rxdes->rxdes3) |
+				 ((rxdes->rxdes2 & FTGMAC100_RXDES2_RXBUF_BADR_HI) << 16);
 
 		if (!skb)
 			continue;
@@ -986,7 +1001,9 @@ static void ftgmac100_init_rings(struct ftgmac100 *priv)
 	for (i = 0; i < priv->rx_q_entries; i++) {
 		rxdes = &priv->rxdes[i];
 		rxdes->rxdes0 = 0;
-		rxdes->rxdes3 = cpu_to_le32(priv->rx_scratch_dma);
+		rxdes->rxdes2 = FIELD_PREP(FTGMAC100_RXDES2_RXBUF_BADR_HI,
+					   upper_32_bits(priv->rx_scratch_dma));
+		rxdes->rxdes3 = lower_32_bits(priv->rx_scratch_dma);
 	}
 	/* Mark the end of the ring */
 	rxdes->rxdes0 |= cpu_to_le32(priv->rxdes0_edorr_mask);
@@ -1249,7 +1266,6 @@ static int ftgmac100_poll(struct napi_struct *napi, int budget)
 		more = ftgmac100_rx_packet(priv, &work_done);
 	} while (more && work_done < budget);
 
-
 	/* The interrupt is telling us to kick the MAC back to life
 	 * after an RX overflow
 	 */
@@ -1339,7 +1355,6 @@ static void ftgmac100_reset(struct ftgmac100 *priv)
 	if (priv->mii_bus)
 		mutex_lock(&priv->mii_bus->mdio_lock);
 
-
 	/* Check if the interface is still up */
 	if (!netif_running(netdev))
 		goto bail;
@@ -1438,7 +1453,6 @@ static void ftgmac100_adjust_link(struct net_device *netdev)
 
 	if (netdev->phydev)
 		mutex_lock(&netdev->phydev->lock);
-
 }
 
 static int ftgmac100_mii_probe(struct net_device *netdev)
@@ -1882,7 +1896,8 @@ static int ftgmac100_probe(struct platform_device *pdev)
 	np = pdev->dev.of_node;
 	if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac") ||
 		   of_device_is_compatible(np, "aspeed,ast2500-mac") ||
-		   of_device_is_compatible(np, "aspeed,ast2600-mac"))) {
+		   of_device_is_compatible(np, "aspeed,ast2600-mac") ||
+		   of_device_is_compatible(np, "aspeed,ast2700-mac"))) {
 		priv->rxdes0_edorr_mask = BIT(30);
 		priv->txdes0_edotr_mask = BIT(30);
 		priv->is_aspeed = true;
@@ -1965,16 +1980,27 @@ static int ftgmac100_probe(struct platform_device *pdev)
 			dev_err(priv->dev, "MII probe failed!\n");
 			goto err_ncsi_dev;
 		}
-
 	}
 
 	if (priv->is_aspeed) {
+		struct reset_control *rst;
+
 		err = ftgmac100_setup_clk(priv);
 		if (err)
 			goto err_phy_connect;
 
-		/* Disable ast2600 problematic HW arbitration */
-		if (of_device_is_compatible(np, "aspeed,ast2600-mac"))
+		rst = devm_reset_control_get_optional(priv->dev, NULL);
+		if (IS_ERR(rst))
+			goto err_register_netdev;
+
+		priv->rst = rst;
+		err = reset_control_assert(priv->rst);
+		mdelay(10);
+		err = reset_control_deassert(priv->rst);
+
+		/* Disable some aspeed platform problematic HW arbitration */
+		if (of_device_is_compatible(np, "aspeed,ast2600-mac") ||
+		    of_device_is_compatible(np, "aspeed,ast2700-mac"))
 			iowrite32(FTGMAC100_TM_DEFAULT,
 				  priv->base + FTGMAC100_OFFSET_TM);
 	}
@@ -2010,6 +2036,8 @@ static int ftgmac100_probe(struct platform_device *pdev)
 		goto err_register_netdev;
 	}
 
+	dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
+
 	netdev_info(netdev, "irq %d, mapped at %p\n", netdev->irq, priv->base);
 
 	return 0;
diff --git a/drivers/net/ethernet/faraday/ftgmac100.h b/drivers/net/ethernet/faraday/ftgmac100.h
index 4968f6f0bdbc..ac39b864a80c 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.h
+++ b/drivers/net/ethernet/faraday/ftgmac100.h
@@ -57,6 +57,13 @@
 #define FTGMAC100_OFFSET_RX_RUNT	0xc0
 #define FTGMAC100_OFFSET_RX_CRCER_FTL	0xc4
 #define FTGMAC100_OFFSET_RX_COL_LOST	0xc8
+/* reserved 0xcc - 0x174 */
+#define FTGMAC100_OFFSET_TXR_BADDR_LOW		0x178	/* ast2700 */
+#define FTGMAC100_OFFSET_TXR_BADDR_HIGH		0x17c	/* ast2700 */
+#define FTGMAC100_OFFSET_HPTXR_BADDR_LOW	0x180	/* ast2700 */
+#define FTGMAC100_OFFSET_HPTXR_BADDR_HIGH	0x184	/* ast2700 */
+#define FTGMAC100_OFFSET_RXR_BADDR_LOW		0x188	/* ast2700 */
+#define FTGMAC100_OFFSET_RXR_BADDR_HIGH		0x18C	/* ast2700 */
 
 /*
  * Interrupt status register & interrupt enable register
@@ -166,6 +173,7 @@
 #define FTGMAC100_MACCR_RX_MULTIPKT	(1 << 16)
 #define FTGMAC100_MACCR_RX_BROADPKT	(1 << 17)
 #define FTGMAC100_MACCR_DISCARD_CRCERR	(1 << 18)
+#define FTGMAC100_MACCR_RMII_ENABLE	(1 << 20) /* defined in ast2700 */
 #define FTGMAC100_MACCR_FAST_MODE	(1 << 19)
 #define FTGMAC100_MACCR_SW_RST		(1 << 31)
 
@@ -225,6 +233,7 @@ struct ftgmac100_txdes {
 #define FTGMAC100_TXDES1_TX2FIC		(1 << 30)
 #define FTGMAC100_TXDES1_TXIC		(1 << 31)
 
+#define FTGMAC100_TXDES2_TXBUF_BADR_HI	GENMASK(18, 16)
 /*
  * Receive descriptor, aligned to 16 bytes
  */
@@ -271,4 +280,5 @@ struct ftgmac100_rxdes {
 #define FTGMAC100_RXDES1_UDP_CHKSUM_ERR	(1 << 26)
 #define FTGMAC100_RXDES1_IP_CHKSUM_ERR	(1 << 27)
 
+#define FTGMAC100_RXDES2_RXBUF_BADR_HI	GENMASK(18, 16)
 #endif /* __FTGMAC100_H */
-- 
2.25.1

On Thu, Nov 07, 2024 at 07:15:00PM +0800, Jacky Chou wrote:
> The AST2700 is the 7th generation SoC from Aspeed, featuring three GPIO
> controllers that are support 64-bit DMA capability.
> Adding features is shown in the following list.
> 1.Support 64-bit DMA
>   Add the high address (63:32) registers for description address and the
>   description field for packet buffer with high address part.
>   These registers and fields in legacy Aspeed SoC are reserved.
>   This 64-bit DMA changing has verified on legacy Aspeed Soc, like
>   AST2600.
> 2.Set RMII pin strap in AST2700 compitable
>   Use bit 20 of MAC 0x50 to represent the pin strap of AST2700 RMII and
>   RGMII. Set to 1 is RMII pin, otherwise is RGMII.
>   This bis is also reserved in legacy Aspeed SoC.
> 
> Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>

...

> @@ -1965,16 +1980,27 @@ static int ftgmac100_probe(struct platform_device *pdev)
>  			dev_err(priv->dev, "MII probe failed!\n");
>  			goto err_ncsi_dev;
>  		}
> -
>  	}
>  
>  	if (priv->is_aspeed) {
> +		struct reset_control *rst;
> +
>  		err = ftgmac100_setup_clk(priv);
>  		if (err)
>  			goto err_phy_connect;
>  
> -		/* Disable ast2600 problematic HW arbitration */
> -		if (of_device_is_compatible(np, "aspeed,ast2600-mac"))
> +		rst = devm_reset_control_get_optional(priv->dev, NULL);
> +		if (IS_ERR(rst))

Hi Jacky,

Should err be set to ERR_PTR(rst) here so that value is returned by
the function?

> +			goto err_register_netdev;
> +
> +		priv->rst = rst;
> +		err = reset_control_assert(priv->rst);
> +		mdelay(10);
> +		err = reset_control_deassert(priv->rst);
> +
> +		/* Disable some aspeed platform problematic HW arbitration */
> +		if (of_device_is_compatible(np, "aspeed,ast2600-mac") ||
> +		    of_device_is_compatible(np, "aspeed,ast2700-mac"))
>  			iowrite32(FTGMAC100_TM_DEFAULT,
>  				  priv->base + FTGMAC100_OFFSET_TM);
>  	}

>  	/* Setup RX ring buffer base */
> -	iowrite32(priv->rxdes_dma, priv->base + FTGMAC100_OFFSET_RXR_BADR);
> +	iowrite32(lower_32_bits(priv->rxdes_dma), priv->base + FTGMAC100_OFFSET_RXR_BADR);
> +	iowrite32(upper_32_bits(priv->rxdes_dma), priv->base + FTGMAC100_OFFSET_RXR_BADDR_HIGH);

This appears to write to registers which older generations do not
have. Is this safe? Is it defined in the datasheet what happens when
you write to reserved registers?

>  	/* Store DMA address into RX desc */
> -	rxdes->rxdes3 = cpu_to_le32(map);
> +	rxdes->rxdes2 = FIELD_PREP(FTGMAC100_RXDES2_RXBUF_BADR_HI, upper_32_bits(map));
> +	rxdes->rxdes3 = lower_32_bits(map);

Maybe update the comment:
        unsigned int    rxdes3; /* not used by HW */

Also, should its type be changed to __le32 ?

> -	map = le32_to_cpu(rxdes->rxdes3);
> +	map = le32_to_cpu(rxdes->rxdes3) | ((rxdes->rxdes2 & FTGMAC100_RXDES2_RXBUF_BADR_HI) << 16);

Is this safe? You have to assume older generation of devices will
return 42 in rxdes3, since it is not used by the hardware.

>  	/* Mark the end of the ring */
>  	rxdes->rxdes0 |= cpu_to_le32(priv->rxdes0_edorr_mask);
> @@ -1249,7 +1266,6 @@ static int ftgmac100_poll(struct napi_struct *napi, int budget)
>  		more = ftgmac100_rx_packet(priv, &work_done);
>  	} while (more && work_done < budget);
>  
> -
>  	/* The interrupt is telling us to kick the MAC back to life
>  	 * after an RX overflow
>  	 */
> @@ -1339,7 +1355,6 @@ static void ftgmac100_reset(struct ftgmac100 *priv)
>  	if (priv->mii_bus)
>  		mutex_lock(&priv->mii_bus->mdio_lock);
>  
> -
>  	/* Check if the interface is still up */
>  	if (!netif_running(netdev))
>  		goto bail;
> @@ -1438,7 +1453,6 @@ static void ftgmac100_adjust_link(struct net_device *netdev)
>  
>  	if (netdev->phydev)
>  		mutex_lock(&netdev->phydev->lock);
> -
>  }

There are quite a few whitespace changes like this in this
patch. Please remove them. If they are important, put them into a
patch of there own.

> +	dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));

This can fail, you should check the return value.

	Andrew

Hi,

On Thu, 7 Nov 2024 19:15:00 +0800
Jacky Chou <jacky_chou@aspeedtech.com> wrote:

> The AST2700 is the 7th generation SoC from Aspeed, featuring three GPIO
> controllers that are support 64-bit DMA capability.
> Adding features is shown in the following list.
> 1.Support 64-bit DMA
>   Add the high address (63:32) registers for description address and the
>   description field for packet buffer with high address part.
>   These registers and fields in legacy Aspeed SoC are reserved.
>   This 64-bit DMA changing has verified on legacy Aspeed Soc, like
>   AST2600.

Maybe each of these features should be in a dedicated patch ?

> 2.Set RMII pin strap in AST2700 compitable
				  compatible

>   Use bit 20 of MAC 0x50 to represent the pin strap of AST2700 RMII and
>   RGMII. Set to 1 is RMII pin, otherwise is RGMII.
>   This bis is also reserved in legacy Aspeed SoC.
> Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>

[...]

> @@ -349,6 +354,10 @@ static void ftgmac100_start_hw(struct ftgmac100 *priv)
>  	if (priv->netdev->features & NETIF_F_HW_VLAN_CTAG_RX)
>  		maccr |= FTGMAC100_MACCR_RM_VLAN;
>  
> +	if (of_device_is_compatible(priv->dev->of_node, "aspeed,ast2700-mac") &&
> +	    priv->netdev->phydev->interface == PHY_INTERFACE_MODE_RMII)
> +		maccr |= FTGMAC100_MACCR_RMII_ENABLE;

The driver code takes the assumption that netdev->phydev might be NULL,
I think you should therefore add an extra check here as well before
getting the interface mode.

Thanks,

Maxime