[net-next v2 0/7] Add Aspeed G7 FTGMAC100 support

[net-next v2 0/7] Add Aspeed G7 FTGMAC100 support

[Jacky Chou]

The Aspeed 7th generation SoC features three FTGMAC100.
The main difference from the previous generation is that the
FTGMAC100 adds support for 64-bit DMA capability. Another change
is that the RMII/RGMII pin strap configuration is changed to be set
in the bit 20 fo register 0x50.

Jacky Chou (7):
  dt-bindings: net: ftgmac100: support for AST2700
  net: faraday: Add ARM64 in FTGMAC100 for AST2700
  net: ftgmac100: Add reset toggling for Aspeed SOCs
  net: ftgmac100: Add support for AST2700
  net: ftgmac100: add pin strap configuration for AST2700
  net: ftgmac100: Add 64-bit DMA support for AST2700
  net: ftgmac100: remove extra newline symbols

 .../bindings/net/faraday,ftgmac100.yaml       |  3 +-
 drivers/net/ethernet/faraday/Kconfig          |  5 +-
 drivers/net/ethernet/faraday/ftgmac100.c      | 80 +++++++++++++++----
 drivers/net/ethernet/faraday/ftgmac100.h      | 10 +++
 4 files changed, 78 insertions(+), 20 deletions(-)

---
 v2:
  - Separate old patch to multiple patch
  - Add more commit information in all patches
  - Add error handling in ftgmac100.
-- 
2.25.1

[net-next v2 1/7] dt-bindings: net: ftgmac100: support for AST2700

[Jacky Chou]

The AST2700 is the 7th generation SoC from Aspeed.
Add compatible support for AST2700 in yaml.

Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
---
 Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
index 9bcbacb6640d..fffe5c51daa9 100644
--- a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
+++ b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
@@ -21,6 +21,7 @@ properties:
               - aspeed,ast2400-mac
               - aspeed,ast2500-mac
               - aspeed,ast2600-mac
+              - aspeed,ast2700-mac
           - const: faraday,ftgmac100
 
   reg:
@@ -33,7 +34,7 @@ properties:
     minItems: 1
     items:
       - description: MAC IP clock
-      - description: RMII RCLK gate for AST2500/2600
+      - description: RMII RCLK gate for AST2500/2600/2700
 
   clock-names:
     minItems: 1
-- 
2.25.1

[net-next v2 2/7] net: faraday: Add ARM64 in FTGMAC100 for AST2700

[Jacky Chou]

AST2700 is a ARM 64-bit SoC and ftgmac100 adds support 64-bit
DMA capability in AST2700.

Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
---
 drivers/net/ethernet/faraday/Kconfig | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/faraday/Kconfig b/drivers/net/ethernet/faraday/Kconfig
index c699bd6bcbb9..d5a088d88c3d 100644
--- a/drivers/net/ethernet/faraday/Kconfig
+++ b/drivers/net/ethernet/faraday/Kconfig
@@ -6,7 +6,7 @@
 config NET_VENDOR_FARADAY
 	bool "Faraday devices"
 	default y
-	depends on ARM || COMPILE_TEST
+	depends on ARM || ARM64 || COMPILE_TEST
 	help
 	  If you have a network (Ethernet) card belonging to this class, say Y.
 
@@ -28,8 +28,7 @@ config FTMAC100
 
 config FTGMAC100
 	tristate "Faraday FTGMAC100 Gigabit Ethernet support"
-	depends on ARM || COMPILE_TEST
-	depends on !64BIT || BROKEN
+	depends on ARM || ARM64 || COMPILE_TEST
 	select PHYLIB
 	select MDIO_ASPEED if MACH_ASPEED_G6
 	select CRC32
-- 
2.25.1

[net-next v2 3/7] net: ftgmac100: Add reset toggling for Aspeed SOCs

[Jacky Chou]

Toggle the SCU reset before hardware initialization.

Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 17ec35e75a65..cae23b712a6d 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>
@@ -98,6 +99,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;
@@ -1969,10 +1971,29 @@ static int ftgmac100_probe(struct platform_device *pdev)
 	}
 
 	if (priv->is_aspeed) {
+		struct reset_control *rst;
+
 		err = ftgmac100_setup_clk(priv);
 		if (err)
 			goto err_phy_connect;
 
+		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);
+		if (err) {
+			dev_err(priv->dev, "Failed to reset mac (%d)\n", err);
+			goto err_register_netdev;
+		}
+		usleep_range(10000, 20000);
+		err = reset_control_deassert(priv->rst);
+		if (err) {
+			dev_err(priv->dev, "Failed to deassert mac reset (%d)\n", err);
+			goto err_register_netdev;
+		}
+
 		/* Disable ast2600 problematic HW arbitration */
 		if (of_device_is_compatible(np, "aspeed,ast2600-mac"))
 			iowrite32(FTGMAC100_TM_DEFAULT,
-- 
2.25.1

[net-next v2 4/7] net: ftgmac100: Add support for AST2700

[Jacky Chou]

Add comptaible for AST2700 and set dma mask to 64-bit.
The ftgmac100 on AST2700 supports 64-bit DMA.

Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index cae23b712a6d..5b277285666a 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1884,7 +1884,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;
@@ -1967,7 +1968,6 @@ static int ftgmac100_probe(struct platform_device *pdev)
 			dev_err(priv->dev, "MII probe failed!\n");
 			goto err_ncsi_dev;
 		}
-
 	}
 
 	if (priv->is_aspeed) {
@@ -1995,9 +1995,18 @@ static int ftgmac100_probe(struct platform_device *pdev)
 		}
 
 		/* Disable ast2600 problematic HW arbitration */
-		if (of_device_is_compatible(np, "aspeed,ast2600-mac"))
+		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);
+
+		if (of_device_is_compatible(np, "aspeed,ast2700-mac")) {
+			err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
+			if (err) {
+				dev_err(&pdev->dev, "64-bit DMA enable failed\n");
+				goto err_register_netdev;
+			}
+		}
 	}
 
 	/* Default ring sizes */
-- 
2.25.1

[net-next v2 5/7] net: ftgmac100: add pin strap configuration for AST2700

[Jacky Chou]

On AST2700, the RMII/RGMII pin strap is configured by setting
MAC 0x50 bit 20. Set to 0 is RGMII and set to 1 is RMII.
Use compatible property to distinguish different generations for
configuration.

Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 5 +++++
 drivers/net/ethernet/faraday/ftgmac100.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 5b277285666a..801fbc89ab09 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -323,6 +323,7 @@ static void ftgmac100_init_hw(struct ftgmac100 *priv)
 static void ftgmac100_start_hw(struct ftgmac100 *priv)
 {
 	u32 maccr = ioread32(priv->base + FTGMAC100_OFFSET_MACCR);
+	struct phy_device *phydev = priv->netdev->phydev;
 
 	/* Keep the original GMAC and FAST bits */
 	maccr &= (FTGMAC100_MACCR_FAST_MODE | FTGMAC100_MACCR_GIGA_MODE);
@@ -351,6 +352,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") &&
+	    phydev && phydev->interface == PHY_INTERFACE_MODE_RMII)
+		maccr |= FTGMAC100_MACCR_RMII_ENABLE;
+
 	/* Hit the HW */
 	iowrite32(maccr, priv->base + FTGMAC100_OFFSET_MACCR);
 }
diff --git a/drivers/net/ethernet/faraday/ftgmac100.h b/drivers/net/ethernet/faraday/ftgmac100.h
index 4968f6f0bdbc..c87aa7d7f14c 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.h
+++ b/drivers/net/ethernet/faraday/ftgmac100.h
@@ -166,6 +166,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)
 
-- 
2.25.1

[net-next v2 6/7] net: ftgmac100: Add 64-bit DMA support for AST2700

[Jacky Chou]

In 7th generation of Aspeed, the ftgmac100 supports 64-bit
DMA operations because it is 64-bit SoC.
Add the high address base registers for TX/RX description and
add the high address base fields for packet buffer in TX/RX description.
These added registers and fields are reserved in older generations and
they will not affect older hardware for accessing read/write
operations to them.
We have verified these patches in older generations, link AST2600.
Run iperf3, tftp, ping and dhcp etc. to confirm that it works normally
on the new generation and older generation.

In TX/RX description, software will reuse the packet buffer address
fields to construct the dma information to unmap these dma region.
Filling in these high address fields in tx/rxdesc2 is software, and the
hardware just use it to do DMA operation and it does not modify these
fields.
And, in older generation, the software will assign to ZERO to the
fields that added in 7th gereration. Because these older SoCs are 32-bit
platforms, the high address will always be ZERO.

Therefore, avoid using a lot of conditions to set the physical address
on old and new generations.

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

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 801fbc89ab09..f3fbe241edb0 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -267,10 +267,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),
@@ -432,7 +434,9 @@ 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 = cpu_to_le32(FIELD_PREP(FTGMAC100_RXDES2_RXBUF_BADR_HI,
+					       upper_32_bits(map)));
+	rxdes->rxdes3 = cpu_to_le32(lower_32_bits(map));
 
 	/* Ensure the above is ordered vs clearing the OWN bit */
 	dma_wmb();
@@ -558,7 +562,8 @@ 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) |
+	      ((le32_to_cpu(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
@@ -635,9 +640,12 @@ 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;
 	size_t len;
 
+	map = le32_to_cpu(txdes->txdes3) |
+	      ((le32_to_cpu(txdes->txdes2) & FTGMAC100_TXDES2_TXBUF_BADR_HI) << 16);
+
 	if (ctl_stat & FTGMAC100_TXDES0_FTS) {
 		len = skb_headlen(skb);
 		dma_unmap_single(priv->dev, map, len, DMA_TO_DEVICE);
@@ -791,7 +799,9 @@ 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 = cpu_to_le32(FIELD_PREP(FTGMAC100_TXDES2_TXBUF_BADR_HI,
+					       upper_32_bits((ulong)map)));
+	txdes->txdes3 = cpu_to_le32(lower_32_bits(map));
 	txdes->txdes1 = cpu_to_le32(csum_vlan);
 
 	/* Next descriptor */
@@ -819,7 +829,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 = cpu_to_le32(FIELD_PREP(FTGMAC100_TXDES2_TXBUF_BADR_HI,
+						       upper_32_bits((ulong)map)));
+		txdes->txdes3 = cpu_to_le32(lower_32_bits(map));
 
 		/* Next one */
 		pointer = ftgmac100_next_tx_pointer(priv, pointer);
@@ -894,7 +906,10 @@ 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;
+
+		map = le32_to_cpu(rxdes->rxdes3) |
+		      ((le32_to_cpu(rxdes->rxdes2) & FTGMAC100_RXDES2_RXBUF_BADR_HI) << 16);
 
 		if (!skb)
 			continue;
@@ -993,7 +1008,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 =	cpu_to_le32(FIELD_PREP(FTGMAC100_RXDES2_RXBUF_BADR_HI,
+						       upper_32_bits(priv->rx_scratch_dma)));
+		rxdes->rxdes3 =	cpu_to_le32(lower_32_bits(priv->rx_scratch_dma));
 	}
 	/* Mark the end of the ring */
 	rxdes->rxdes0 |= cpu_to_le32(priv->rxdes0_edorr_mask);
diff --git a/drivers/net/ethernet/faraday/ftgmac100.h b/drivers/net/ethernet/faraday/ftgmac100.h
index c87aa7d7f14c..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
@@ -226,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
  */
@@ -272,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

[net-next v2 7/7] net: ftgmac100: remove extra newline symbols

[Jacky Chou]

Remove some unnecessary newline symbols in code.

Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index f3fbe241edb0..0ba41c59072a 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -575,7 +575,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);
@@ -1273,7 +1272,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
 	 */
@@ -1363,7 +1361,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;
@@ -1462,7 +1459,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)
-- 
2.25.1

Re: [net-next v2 3/7] net: ftgmac100: Add reset toggling for Aspeed SOCs

[Arnd Bergmann]

On Mon, Nov 18, 2024, at 07:02, Jacky Chou wrote:
> @@ -1969,10 +1971,29 @@ static int ftgmac100_probe(struct platform_device *pdev)
>  	}
> 
>  	if (priv->is_aspeed) {
> +		struct reset_control *rst;
> +
>  		err = ftgmac100_setup_clk(priv);
>  		if (err)
>  			goto err_phy_connect;
> 
> +		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);

Since that reset line is optional, how about making it
part of the normal probe procedure, not just the if(aspeed)
section? It seems this does nothing for older devices but
may help for future ones regardless of the SoC family.

      Arnd

Re: [net-next v2 5/7] net: ftgmac100: add pin strap configuration for AST2700

[Arnd Bergmann]

On Mon, Nov 18, 2024, at 07:02, Jacky Chou wrote:
> @@ -351,6 +352,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") 
> &&
> +	    phydev && phydev->interface == PHY_INTERFACE_MODE_RMII)
> +		maccr |= FTGMAC100_MACCR_RMII_ENABLE;
> +
>  	/* Hit the HW */

Is there a way to probe the presence of 64-bit addressing from
hardware registers? That would be nicer than triggering it from
the compatible string, given that any future SoC is likely
also 64-bit.

      Arnd

回覆: [net-next v2 3/7] net: ftgmac100: Add reset toggling for Aspeed SOCs

[Jacky Chou]

Hi Arnd,

Thank you for you reply.

> >  	}
> >
> >  	if (priv->is_aspeed) {
> > +		struct reset_control *rst;
> > +
> >  		err = ftgmac100_setup_clk(priv);
> >  		if (err)
> >  			goto err_phy_connect;
> >
> > +		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);
> 
> Since that reset line is optional, how about making it part of the normal probe
> procedure, not just the if(aspeed) section? It seems this does nothing for older
> devices but may help for future ones regardless of the SoC family.

Agree.
Because it is optional, even if reset line does not exist on other SoCs, it will 
not affect the behavior of the code.
Thank you for pointing this out.
I will adjust it to normal probe procedure in next version.
Or I will separate it from Aspeed AST2700 support series.

Jacky

回覆: [net-next v2 5/7] net: ftgmac100: add pin strap configuration for AST2700

[Jacky Chou]

Hi Arnd,

Thank you for your reply.

> > @@ -351,6 +352,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")
> > &&
> > +	    phydev && phydev->interface == PHY_INTERFACE_MODE_RMII)
> > +		maccr |= FTGMAC100_MACCR_RMII_ENABLE;
> > +
> >  	/* Hit the HW */
> 
> Is there a way to probe the presence of 64-bit addressing from hardware
> registers? That would be nicer than triggering it from the compatible string,
> given that any future SoC is likely also 64-bit.

There is no register indicated about 64-bit address support in the ftgmac100 
of Aspeed 7th generation. Therefore, we use the compatible to configure pin strap 
and DMA mask.

Thanks,
Jacky

Re: [net-next v2 5/7] net: ftgmac100: add pin strap configuration for AST2700

[Arnd Bergmann]

On Mon, Nov 18, 2024, at 08:51, Jacky Chou wrote:
 
>> Is there a way to probe the presence of 64-bit addressing from hardware
>> registers? That would be nicer than triggering it from the compatible string,
>> given that any future SoC is likely also 64-bit.

I just realized I replied to the wrong email, I meant to send
my question as a reply to patch 4/7. The patch for the pin strap
looks fine.

> There is no register indicated about 64-bit address support in the 
> ftgmac100 of Aspeed 7th generation. Therefore, we use the compatible
> to configure pin strap and DMA mask.

Later in the series you just unconditionally write the 64-bit
address, so it appears that the ftgmac100 can actually do
64-bti addressing all along, and this doesn't have to be
conditional at all, the call to dma_set_mask_and_coherent()
only tells the kernel that the device can do it, which should
work on all of them. Since the other devices won't have a
larger "dma-ranges" configuration in DT, and no RAM above
32-bit addressing, it should have no effect.

Just make that part in patch 5 unconditional.

     Arnd

回覆: [net-next v2 5/7] net: ftgmac100: add pin strap configuration for AST2700

[Jacky Chou]

> >> Is there a way to probe the presence of 64-bit addressing from
> >> hardware registers? That would be nicer than triggering it from the
> >> compatible string, given that any future SoC is likely also 64-bit.
> 
> I just realized I replied to the wrong email, I meant to send my question as a
> reply to patch 4/7. The patch for the pin strap looks fine.
> 
> > There is no register indicated about 64-bit address support in the
> > ftgmac100 of Aspeed 7th generation. Therefore, we use the compatible
> > to configure pin strap and DMA mask.
> 
> Later in the series you just unconditionally write the 64-bit address, so it
> appears that the ftgmac100 can actually do 64-bti addressing all along, and
> this doesn't have to be conditional at all, the call to
> dma_set_mask_and_coherent() only tells the kernel that the device can do it,
> which should work on all of them. Since the other devices won't have a larger
> "dma-ranges" configuration in DT, and no RAM above 32-bit addressing, it
> should have no effect.
> 
> Just make that part in patch 5 unconditional.

Agree, I got your point.
We tried to add less codes to compatible the older generations.
I will adjust it to normal probe procedure in next version, not use compatible to call 
dma_set_mask_and_coherent().
Thank you for your kind reminder.

Thanks,
Jacky

Re: [net-next v2 3/7] net: ftgmac100: Add reset toggling for Aspeed SOCs

[Philipp Zabel]

On Mo, 2024-11-18 at 14:02 +0800, Jacky Chou wrote:
> Toggle the SCU reset before hardware initialization.
> 
> Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
> ---
>  drivers/net/ethernet/faraday/ftgmac100.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index 17ec35e75a65..cae23b712a6d 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>
> @@ -98,6 +99,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;
> @@ -1969,10 +1971,29 @@ static int ftgmac100_probe(struct platform_device *pdev)
>  	}
>  
>  	if (priv->is_aspeed) {
> +		struct reset_control *rst;
> +
>  		err = ftgmac100_setup_clk(priv);
>  		if (err)
>  			goto err_phy_connect;
>  
> +		rst = devm_reset_control_get_optional(priv->dev, NULL);

Please use devm_reset_control_get_optional_exclusive() directly.


regards
Philipp

回覆: [net-next v2 3/7] net: ftgmac100: Add reset toggling for Aspeed SOCs

[Jacky Chou]

Hi Philipp

Thank you for your reply.

> > Toggle the SCU reset before hardware initialization.
> >
> > Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
> > ---
> >  drivers/net/ethernet/faraday/ftgmac100.c | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/faraday/ftgmac100.c
> b/drivers/net/ethernet/faraday/ftgmac100.c
> > index 17ec35e75a65..cae23b712a6d 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>
> > @@ -98,6 +99,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;
> > @@ -1969,10 +1971,29 @@ static int ftgmac100_probe(struct
> platform_device *pdev)
> >  	}
> >
> >  	if (priv->is_aspeed) {
> > +		struct reset_control *rst;
> > +
> >  		err = ftgmac100_setup_clk(priv);
> >  		if (err)
> >  			goto err_phy_connect;
> >
> > +		rst = devm_reset_control_get_optional(priv->dev, NULL);
> 
> Please use devm_reset_control_get_optional_exclusive() directly.

Got it.

Thanks,
Jacky

Re: [net-next v2 1/7] dt-bindings: net: ftgmac100: support for AST2700

[Krzysztof Kozlowski]

On 18/11/2024 07:02, Jacky Chou wrote:
> The AST2700 is the 7th generation SoC from Aspeed.
> Add compatible support for AST2700 in yaml.
> 
> Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>

Please use standard email subjects, so with the PATCH keyword in the
title. `git format-patch -vX` helps here to create proper versioned
patches. Another useful tool is b4. Skipping the PATCH keyword makes
filtering of emails more difficult thus making the review process less
convenient.

For net next it is PATCH net-next

<form letter>
This is a friendly reminder during the review process.

It looks like you received a tag and forgot to add it.

If you do not know the process, here is a short explanation:
Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.

https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577

If a tag was not added on purpose, please state why and what changed.
</form letter>

I am not going to do the work twice.

Best regards,
Krzysztof

回覆: [net-next v2 1/7] dt-bindings: net: ftgmac100: support for AST2700

[Jacky Chou]

Hi Krzysztof,

Thank you for your reply and kind reminder.

> > The AST2700 is the 7th generation SoC from Aspeed.
> > Add compatible support for AST2700 in yaml.
> >
> > Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
> 
> Please use standard email subjects, so with the PATCH keyword in the title. `git
> format-patch -vX` helps here to create proper versioned patches. Another
> useful tool is b4. Skipping the PATCH keyword makes filtering of emails more
> difficult thus making the review process less convenient.
> 
> For net next it is PATCH net-next
> 
> <form letter>
> This is a friendly reminder during the review process.
> 
> It looks like you received a tag and forgot to add it.
> 
> If you do not know the process, here is a short explanation:
> Please add Acked-by/Reviewed-by/Tested-by tags when posting new versions,
> under or above your Signed-off-by tag. Tag is "received", when provided in a
> message replied to you on the mailing list. Tools like b4 can help here. However,
> there's no need to repost patches *only* to add the tags. The upstream
> maintainer will do that for tags received on the version they apply.
> 
> https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submi
> tting-patches.rst#L577
> 
> If a tag was not added on purpose, please state why and what changed.
> </form letter>
> 
> I am not going to do the work twice.

I will notice these and adjust in next version.

Thanks,
Jacky