[PATCH 00/10] Add Gigabit Ethernet driver support

[PATCH 00/10] Add Gigabit Ethernet driver support

[Biju Das]

The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC are
similar to the R-Car Ethernet AVB IP.

The Gigabit Ethernet IP consists of Ethernet controller (E-MAC), Internal
TCP/IP Offload Engine (TOE)  and Dedicated Direct memory access controller
(DMAC).

With a few changes in the driver we can support both IPs.

This patch series is in preparation for adding Gigabit ethernet driver support to RZ/G2L SoC.

The number of patches after incorporatng RFC review comments is 19.
So splitting the patches into 2 patchsets (10 + 9).

The series is the first patchset which aims to add RZ/G2L SoC with
compatible strings, E-MAC and D-MAC initialization.

Second patchset basically fillup all the stubs for the full Gigabit
Ethernet functionality.

RFC->V1:
 * Added Rb tags of patch#1 and patch #9
 * Renamed "rgeth" to gbeth
 * Renamed the variable "no_ptp_cfg_active" with "gptp" and
   "ptp_cfg_active" with "ccc_gac
 * Handled NC queue only for R-Car.
 * Removed RIC3 initialization from DMAC init, as it is 
   same as reset value.
 * moved stubs function to patch#4.
 * Added tsrq variable instead of multi_tsrq feature bit.
 * moved CSR0 initialization from E-MAC init to later patch.
 * started using ravb_modify for initializing link registers.

Ref:-
https://lore.kernel.org/linux-renesas-soc/20210923140813.13541-1-biju.das.jz@bp.renesas.com/T/#m5c007b42d6c334de7b2224f2b219f52efc712fe9


Biju Das (10):
  ravb: Rename "ravb_set_features_rx_csum" function to
    "ravb_set_features_rcar"
  ravb: Rename "no_ptp_cfg_active" and "ptp_cfg_active" variables
  ravb: Add nc_queue to struct ravb_hw_info
  ravb: Add support for RZ/G2L SoC
  ravb: Initialize GbEthernet DMAC
  ravb: Exclude gPTP feature support for RZ/G2L
  ravb: Add tsrq to struct ravb_hw_info
  ravb: Add magic_pkt to struct ravb_hw_info
  ravb: Add half_duplex to struct ravb_hw_info
  ravb: Initialize GbEthernet E-MAC

 drivers/net/ethernet/renesas/ravb.h      |  39 +-
 drivers/net/ethernet/renesas/ravb_main.c | 452 +++++++++++++++++------
 2 files changed, 362 insertions(+), 129 deletions(-)

-- 
2.17.1

[PATCH 01/10] ravb: Rename "ravb_set_features_rx_csum" function to "ravb_set_features_rcar"

[Biju Das]

Rename "ravb_set_features_rx_csum" function to "ravb_set_features_rcar" and
replace the function pointer "set_rx_csum_feature" with "set_feature".

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Suggested-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
---
RFC->V1:
 * Added Rb tags of Sergei
---
 drivers/net/ethernet/renesas/ravb.h      |  2 +-
 drivers/net/ethernet/renesas/ravb_main.c | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 47c5377e4f42..7363abae6e59 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -985,7 +985,7 @@ struct ravb_hw_info {
 	void *(*alloc_rx_desc)(struct net_device *ndev, int q);
 	bool (*receive)(struct net_device *ndev, int *quota, int q);
 	void (*set_rate)(struct net_device *ndev);
-	int (*set_rx_csum_feature)(struct net_device *ndev, netdev_features_t features);
+	int (*set_feature)(struct net_device *ndev, netdev_features_t features);
 	void (*dmac_init)(struct net_device *ndev);
 	void (*emac_init)(struct net_device *ndev);
 	const char (*gstrings_stats)[ETH_GSTRING_LEN];
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 0f85f2d97b18..8f2358caef34 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1918,8 +1918,8 @@ static void ravb_set_rx_csum(struct net_device *ndev, bool enable)
 	spin_unlock_irqrestore(&priv->lock, flags);
 }
 
-static int ravb_set_features_rx_csum(struct net_device *ndev,
-				     netdev_features_t features)
+static int ravb_set_features_rcar(struct net_device *ndev,
+				  netdev_features_t features)
 {
 	netdev_features_t changed = ndev->features ^ features;
 
@@ -1937,7 +1937,7 @@ static int ravb_set_features(struct net_device *ndev,
 	struct ravb_private *priv = netdev_priv(ndev);
 	const struct ravb_hw_info *info = priv->info;
 
-	return info->set_rx_csum_feature(ndev, features);
+	return info->set_feature(ndev, features);
 }
 
 static const struct net_device_ops ravb_netdev_ops = {
@@ -2006,7 +2006,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.alloc_rx_desc = ravb_alloc_rx_desc,
 	.receive = ravb_rcar_rx,
 	.set_rate = ravb_set_rate,
-	.set_rx_csum_feature = ravb_set_features_rx_csum,
+	.set_feature = ravb_set_features_rcar,
 	.dmac_init = ravb_rcar_dmac_init,
 	.emac_init = ravb_rcar_emac_init,
 	.gstrings_stats = ravb_gstrings_stats,
@@ -2027,7 +2027,7 @@ static const struct ravb_hw_info ravb_gen2_hw_info = {
 	.alloc_rx_desc = ravb_alloc_rx_desc,
 	.receive = ravb_rcar_rx,
 	.set_rate = ravb_set_rate,
-	.set_rx_csum_feature = ravb_set_features_rx_csum,
+	.set_feature = ravb_set_features_rcar,
 	.dmac_init = ravb_rcar_dmac_init,
 	.emac_init = ravb_rcar_emac_init,
 	.gstrings_stats = ravb_gstrings_stats,
-- 
2.17.1

[PATCH 02/10] ravb: Rename "no_ptp_cfg_active" and "ptp_cfg_active" variables

[Biju Das]

Rename the variable "no_ptp_cfg_active" with "gptp" and
"ptp_cfg_active" with "ccc_gac" to match the HW features.

There is no functional change.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Suggested-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFc->v1:
 * Renamed the variable "no_ptp_cfg_active" with "gptp" and
   "ptp_cfg_active" with "ccc_gac
---
 drivers/net/ethernet/renesas/ravb.h      |  4 ++--
 drivers/net/ethernet/renesas/ravb_main.c | 26 ++++++++++++------------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 7363abae6e59..a33fbcb4aac3 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -1000,8 +1000,8 @@ struct ravb_hw_info {
 	unsigned internal_delay:1;	/* AVB-DMAC has internal delays */
 	unsigned tx_counters:1;		/* E-MAC has TX counters */
 	unsigned multi_irqs:1;		/* AVB-DMAC and E-MAC has multiple irqs */
-	unsigned no_ptp_cfg_active:1;	/* AVB-DMAC does not support gPTP active in config mode */
-	unsigned ptp_cfg_active:1;	/* AVB-DMAC has gPTP support active in config mode */
+	unsigned gptp:1;		/* AVB-DMAC has gPTP support */
+	unsigned ccc_gac:1;		/* AVB-DMAC has gPTP support active in config mode */
 };
 
 struct ravb_private {
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 8f2358caef34..dc7654abfe55 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1274,7 +1274,7 @@ static int ravb_set_ringparam(struct net_device *ndev,
 	if (netif_running(ndev)) {
 		netif_device_detach(ndev);
 		/* Stop PTP Clock driver */
-		if (info->no_ptp_cfg_active)
+		if (info->gptp)
 			ravb_ptp_stop(ndev);
 		/* Wait for DMA stopping */
 		error = ravb_stop_dma(ndev);
@@ -1306,7 +1306,7 @@ static int ravb_set_ringparam(struct net_device *ndev,
 		ravb_emac_init(ndev);
 
 		/* Initialise PTP Clock driver */
-		if (info->no_ptp_cfg_active)
+		if (info->gptp)
 			ravb_ptp_init(ndev, priv->pdev);
 
 		netif_device_attach(ndev);
@@ -1446,7 +1446,7 @@ static int ravb_open(struct net_device *ndev)
 	ravb_emac_init(ndev);
 
 	/* Initialise PTP Clock driver */
-	if (info->no_ptp_cfg_active)
+	if (info->gptp)
 		ravb_ptp_init(ndev, priv->pdev);
 
 	netif_tx_start_all_queues(ndev);
@@ -1460,7 +1460,7 @@ static int ravb_open(struct net_device *ndev)
 
 out_ptp_stop:
 	/* Stop PTP Clock driver */
-	if (info->no_ptp_cfg_active)
+	if (info->gptp)
 		ravb_ptp_stop(ndev);
 out_free_irq_nc_tx:
 	if (!info->multi_irqs)
@@ -1508,7 +1508,7 @@ static void ravb_tx_timeout_work(struct work_struct *work)
 	netif_tx_stop_all_queues(ndev);
 
 	/* Stop PTP Clock driver */
-	if (info->no_ptp_cfg_active)
+	if (info->gptp)
 		ravb_ptp_stop(ndev);
 
 	/* Wait for DMA stopping */
@@ -1543,7 +1543,7 @@ static void ravb_tx_timeout_work(struct work_struct *work)
 
 out:
 	/* Initialise PTP Clock driver */
-	if (info->no_ptp_cfg_active)
+	if (info->gptp)
 		ravb_ptp_init(ndev, priv->pdev);
 
 	netif_tx_start_all_queues(ndev);
@@ -1752,7 +1752,7 @@ static int ravb_close(struct net_device *ndev)
 	ravb_write(ndev, 0, TIC);
 
 	/* Stop PTP Clock driver */
-	if (info->no_ptp_cfg_active)
+	if (info->gptp)
 		ravb_ptp_stop(ndev);
 
 	/* Set the config mode to stop the AVB-DMAC's processes */
@@ -2018,7 +2018,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.internal_delay = 1,
 	.tx_counters = 1,
 	.multi_irqs = 1,
-	.ptp_cfg_active = 1,
+	.ccc_gac = 1,
 };
 
 static const struct ravb_hw_info ravb_gen2_hw_info = {
@@ -2037,7 +2037,7 @@ static const struct ravb_hw_info ravb_gen2_hw_info = {
 	.stats_len = ARRAY_SIZE(ravb_gstrings_stats),
 	.max_rx_len = RX_BUF_SZ + RAVB_ALIGN - 1,
 	.aligned_tx = 1,
-	.no_ptp_cfg_active = 1,
+	.gptp = 1,
 };
 
 static const struct of_device_id ravb_match_table[] = {
@@ -2080,7 +2080,7 @@ static void ravb_set_config_mode(struct net_device *ndev)
 	struct ravb_private *priv = netdev_priv(ndev);
 	const struct ravb_hw_info *info = priv->info;
 
-	if (info->no_ptp_cfg_active) {
+	if (info->gptp) {
 		ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_CONFIG);
 		/* Set CSEL value */
 		ravb_modify(ndev, CCC, CCC_CSEL, CCC_CSEL_HPB);
@@ -2301,7 +2301,7 @@ static int ravb_probe(struct platform_device *pdev)
 	INIT_LIST_HEAD(&priv->ts_skb_list);
 
 	/* Initialise PTP Clock driver */
-	if (info->ptp_cfg_active)
+	if (info->ccc_gac)
 		ravb_ptp_init(ndev, pdev);
 
 	/* Debug message level */
@@ -2349,7 +2349,7 @@ static int ravb_probe(struct platform_device *pdev)
 			  priv->desc_bat_dma);
 
 	/* Stop PTP Clock driver */
-	if (info->ptp_cfg_active)
+	if (info->ccc_gac)
 		ravb_ptp_stop(ndev);
 out_disable_refclk:
 	clk_disable_unprepare(priv->refclk);
@@ -2369,7 +2369,7 @@ static int ravb_remove(struct platform_device *pdev)
 	const struct ravb_hw_info *info = priv->info;
 
 	/* Stop PTP Clock driver */
-	if (info->ptp_cfg_active)
+	if (info->ccc_gac)
 		ravb_ptp_stop(ndev);
 
 	clk_disable_unprepare(priv->refclk);
-- 
2.17.1

[PATCH 03/10] ravb: Add nc_queue to struct ravb_hw_info

[Biju Das]

R-Car supports network control queue whereas RZ/G2L does not support
it. Add nc_queue to struct ravb_hw_info, so that NC queue is handled
only by R-Car.

This patch also renames ravb_rcar_dmac_init to ravb_dmac_init_rcar
to be consistent with the naming convention used in sh_eth driver.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v1:
 * Handled NC queue only for R-Car.
---
 drivers/net/ethernet/renesas/ravb.h      |   3 +-
 drivers/net/ethernet/renesas/ravb_main.c | 140 +++++++++++++++--------
 2 files changed, 94 insertions(+), 49 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index a33fbcb4aac3..c91e93e5590f 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -986,7 +986,7 @@ struct ravb_hw_info {
 	bool (*receive)(struct net_device *ndev, int *quota, int q);
 	void (*set_rate)(struct net_device *ndev);
 	int (*set_feature)(struct net_device *ndev, netdev_features_t features);
-	void (*dmac_init)(struct net_device *ndev);
+	int (*dmac_init)(struct net_device *ndev);
 	void (*emac_init)(struct net_device *ndev);
 	const char (*gstrings_stats)[ETH_GSTRING_LEN];
 	size_t gstrings_size;
@@ -1002,6 +1002,7 @@ struct ravb_hw_info {
 	unsigned multi_irqs:1;		/* AVB-DMAC and E-MAC has multiple irqs */
 	unsigned gptp:1;		/* AVB-DMAC has gPTP support */
 	unsigned ccc_gac:1;		/* AVB-DMAC has gPTP support active in config mode */
+	unsigned nc_queue:1;		/* AVB-DMAC has NC queue */
 };
 
 struct ravb_private {
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index dc7654abfe55..8bf13586e90a 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -461,10 +461,24 @@ static void ravb_emac_init(struct net_device *ndev)
 	info->emac_init(ndev);
 }
 
-static void ravb_rcar_dmac_init(struct net_device *ndev)
+static int ravb_dmac_init_rcar(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
 	const struct ravb_hw_info *info = priv->info;
+	int error;
+
+	error = ravb_ring_init(ndev, RAVB_BE);
+	if (error)
+		return error;
+	error = ravb_ring_init(ndev, RAVB_NC);
+	if (error) {
+		ravb_ring_free(ndev, RAVB_BE);
+		return error;
+	}
+
+	/* Descriptor format */
+	ravb_ring_format(ndev, RAVB_BE);
+	ravb_ring_format(ndev, RAVB_NC);
 
 	/* Set AVB RX */
 	ravb_write(ndev,
@@ -491,6 +505,8 @@ static void ravb_rcar_dmac_init(struct net_device *ndev)
 	ravb_write(ndev, RIC2_QFE0 | RIC2_QFE1 | RIC2_RFFE, RIC2);
 	/* Frame transmitted, timestamp FIFO updated */
 	ravb_write(ndev, TIC_FTE0 | TIC_FTE1 | TIC_TFUE, TIC);
+
+	return 0;
 }
 
 /* Device init function for Ethernet AVB */
@@ -505,20 +521,9 @@ static int ravb_dmac_init(struct net_device *ndev)
 	if (error)
 		return error;
 
-	error = ravb_ring_init(ndev, RAVB_BE);
+	error = info->dmac_init(ndev);
 	if (error)
 		return error;
-	error = ravb_ring_init(ndev, RAVB_NC);
-	if (error) {
-		ravb_ring_free(ndev, RAVB_BE);
-		return error;
-	}
-
-	/* Descriptor format */
-	ravb_ring_format(ndev, RAVB_BE);
-	ravb_ring_format(ndev, RAVB_NC);
-
-	info->dmac_init(ndev);
 
 	/* Setting the control will start the AVB-DMAC process. */
 	ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_OPERATION);
@@ -859,6 +864,7 @@ static irqreturn_t ravb_interrupt(int irq, void *dev_id)
 {
 	struct net_device *ndev = dev_id;
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
 	irqreturn_t result = IRQ_NONE;
 	u32 iss;
 
@@ -875,8 +881,13 @@ static irqreturn_t ravb_interrupt(int irq, void *dev_id)
 			result = IRQ_HANDLED;
 
 		/* Network control and best effort queue RX/TX */
-		for (q = RAVB_NC; q >= RAVB_BE; q--) {
-			if (ravb_queue_interrupt(ndev, q))
+		if (info->nc_queue) {
+			for (q = RAVB_NC; q >= RAVB_BE; q--) {
+				if (ravb_queue_interrupt(ndev, q))
+					result = IRQ_HANDLED;
+			}
+		} else {
+			if (ravb_queue_interrupt(ndev, RAVB_BE))
 				result = IRQ_HANDLED;
 		}
 	}
@@ -1000,7 +1011,8 @@ static int ravb_poll(struct napi_struct *napi, int budget)
 
 	/* Receive error message handling */
 	priv->rx_over_errors =  priv->stats[RAVB_BE].rx_over_errors;
-	priv->rx_over_errors += priv->stats[RAVB_NC].rx_over_errors;
+	if (info->nc_queue)
+		priv->rx_over_errors += priv->stats[RAVB_NC].rx_over_errors;
 	if (priv->rx_over_errors != ndev->stats.rx_over_errors)
 		ndev->stats.rx_over_errors = priv->rx_over_errors;
 	if (priv->rx_fifo_errors != ndev->stats.rx_fifo_errors)
@@ -1208,11 +1220,14 @@ static void ravb_get_ethtool_stats(struct net_device *ndev,
 				   struct ethtool_stats *estats, u64 *data)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
+	int num_rx_q;
 	int i = 0;
 	int q;
 
+	num_rx_q = info->nc_queue ? NUM_RX_QUEUE : 1;
 	/* Device-specific stats */
-	for (q = RAVB_BE; q < NUM_RX_QUEUE; q++) {
+	for (q = RAVB_BE; q < num_rx_q; q++) {
 		struct net_device_stats *stats = &priv->stats[q];
 
 		data[i++] = priv->cur_rx[q];
@@ -1287,7 +1302,8 @@ static int ravb_set_ringparam(struct net_device *ndev,
 
 		/* Free all the skb's in the RX queue and the DMA buffers. */
 		ravb_ring_free(ndev, RAVB_BE);
-		ravb_ring_free(ndev, RAVB_NC);
+		if (info->nc_queue)
+			ravb_ring_free(ndev, RAVB_NC);
 	}
 
 	/* Set new parameters */
@@ -1403,7 +1419,8 @@ static int ravb_open(struct net_device *ndev)
 	int error;
 
 	napi_enable(&priv->napi[RAVB_BE]);
-	napi_enable(&priv->napi[RAVB_NC]);
+	if (info->nc_queue)
+		napi_enable(&priv->napi[RAVB_NC]);
 
 	if (!info->multi_irqs) {
 		error = request_irq(ndev->irq, ravb_interrupt, IRQF_SHARED,
@@ -1477,7 +1494,8 @@ static int ravb_open(struct net_device *ndev)
 out_free_irq:
 	free_irq(ndev->irq, ndev);
 out_napi_off:
-	napi_disable(&priv->napi[RAVB_NC]);
+	if (info->nc_queue)
+		napi_disable(&priv->napi[RAVB_NC]);
 	napi_disable(&priv->napi[RAVB_BE]);
 	return error;
 }
@@ -1526,7 +1544,8 @@ static void ravb_tx_timeout_work(struct work_struct *work)
 	}
 
 	ravb_ring_free(ndev, RAVB_BE);
-	ravb_ring_free(ndev, RAVB_NC);
+	if (info->nc_queue)
+		ravb_ring_free(ndev, RAVB_NC);
 
 	/* Device init */
 	error = ravb_dmac_init(ndev);
@@ -1698,28 +1717,38 @@ static struct net_device_stats *ravb_get_stats(struct net_device *ndev)
 
 	nstats = &ndev->stats;
 	stats0 = &priv->stats[RAVB_BE];
-	stats1 = &priv->stats[RAVB_NC];
 
 	if (info->tx_counters) {
 		nstats->tx_dropped += ravb_read(ndev, TROCR);
 		ravb_write(ndev, 0, TROCR);	/* (write clear) */
 	}
 
-	nstats->rx_packets = stats0->rx_packets + stats1->rx_packets;
-	nstats->tx_packets = stats0->tx_packets + stats1->tx_packets;
-	nstats->rx_bytes = stats0->rx_bytes + stats1->rx_bytes;
-	nstats->tx_bytes = stats0->tx_bytes + stats1->tx_bytes;
-	nstats->multicast = stats0->multicast + stats1->multicast;
-	nstats->rx_errors = stats0->rx_errors + stats1->rx_errors;
-	nstats->rx_crc_errors = stats0->rx_crc_errors + stats1->rx_crc_errors;
-	nstats->rx_frame_errors =
-		stats0->rx_frame_errors + stats1->rx_frame_errors;
-	nstats->rx_length_errors =
-		stats0->rx_length_errors + stats1->rx_length_errors;
-	nstats->rx_missed_errors =
-		stats0->rx_missed_errors + stats1->rx_missed_errors;
-	nstats->rx_over_errors =
-		stats0->rx_over_errors + stats1->rx_over_errors;
+	nstats->rx_packets = stats0->rx_packets;
+	nstats->tx_packets = stats0->tx_packets;
+	nstats->rx_bytes = stats0->rx_bytes;
+	nstats->tx_bytes = stats0->tx_bytes;
+	nstats->multicast = stats0->multicast;
+	nstats->rx_errors = stats0->rx_errors;
+	nstats->rx_crc_errors = stats0->rx_crc_errors;
+	nstats->rx_frame_errors = stats0->rx_frame_errors;
+	nstats->rx_length_errors = stats0->rx_length_errors;
+	nstats->rx_missed_errors = stats0->rx_missed_errors;
+	nstats->rx_over_errors = stats0->rx_over_errors;
+	if (info->nc_queue) {
+		stats1 = &priv->stats[RAVB_NC];
+
+		nstats->rx_packets += stats1->rx_packets;
+		nstats->tx_packets += stats1->tx_packets;
+		nstats->rx_bytes += stats1->rx_bytes;
+		nstats->tx_bytes += stats1->tx_bytes;
+		nstats->multicast += stats1->multicast;
+		nstats->rx_errors += stats1->rx_errors;
+		nstats->rx_crc_errors += stats1->rx_crc_errors;
+		nstats->rx_frame_errors += stats1->rx_frame_errors;
+		nstats->rx_length_errors += stats1->rx_length_errors;
+		nstats->rx_missed_errors += stats1->rx_missed_errors;
+		nstats->rx_over_errors += stats1->rx_over_errors;
+	}
 
 	return nstats;
 }
@@ -1784,12 +1813,14 @@ static int ravb_close(struct net_device *ndev)
 	}
 	free_irq(ndev->irq, ndev);
 
-	napi_disable(&priv->napi[RAVB_NC]);
+	if (info->nc_queue)
+		napi_disable(&priv->napi[RAVB_NC]);
 	napi_disable(&priv->napi[RAVB_BE]);
 
 	/* Free all the skb's in the RX queue and the DMA buffers. */
 	ravb_ring_free(ndev, RAVB_BE);
-	ravb_ring_free(ndev, RAVB_NC);
+	if (info->nc_queue)
+		ravb_ring_free(ndev, RAVB_NC);
 
 	return 0;
 }
@@ -2007,7 +2038,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.receive = ravb_rcar_rx,
 	.set_rate = ravb_set_rate,
 	.set_feature = ravb_set_features_rcar,
-	.dmac_init = ravb_rcar_dmac_init,
+	.dmac_init = ravb_dmac_init_rcar,
 	.emac_init = ravb_rcar_emac_init,
 	.gstrings_stats = ravb_gstrings_stats,
 	.gstrings_size = sizeof(ravb_gstrings_stats),
@@ -2019,6 +2050,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.tx_counters = 1,
 	.multi_irqs = 1,
 	.ccc_gac = 1,
+	.nc_queue = 1,
 };
 
 static const struct ravb_hw_info ravb_gen2_hw_info = {
@@ -2028,7 +2060,7 @@ static const struct ravb_hw_info ravb_gen2_hw_info = {
 	.receive = ravb_rcar_rx,
 	.set_rate = ravb_set_rate,
 	.set_feature = ravb_set_features_rcar,
-	.dmac_init = ravb_rcar_dmac_init,
+	.dmac_init = ravb_dmac_init_rcar,
 	.emac_init = ravb_rcar_emac_init,
 	.gstrings_stats = ravb_gstrings_stats,
 	.gstrings_size = sizeof(ravb_gstrings_stats),
@@ -2038,6 +2070,7 @@ static const struct ravb_hw_info ravb_gen2_hw_info = {
 	.max_rx_len = RX_BUF_SZ + RAVB_ALIGN - 1,
 	.aligned_tx = 1,
 	.gptp = 1,
+	.nc_queue = 1,
 };
 
 static const struct of_device_id ravb_match_table[] = {
@@ -2192,8 +2225,11 @@ static int ravb_probe(struct platform_device *pdev)
 	priv->pdev = pdev;
 	priv->num_tx_ring[RAVB_BE] = BE_TX_RING_SIZE;
 	priv->num_rx_ring[RAVB_BE] = BE_RX_RING_SIZE;
-	priv->num_tx_ring[RAVB_NC] = NC_TX_RING_SIZE;
-	priv->num_rx_ring[RAVB_NC] = NC_RX_RING_SIZE;
+	if (info->nc_queue) {
+		priv->num_tx_ring[RAVB_NC] = NC_TX_RING_SIZE;
+		priv->num_rx_ring[RAVB_NC] = NC_RX_RING_SIZE;
+	}
+
 	priv->addr = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
 	if (IS_ERR(priv->addr)) {
 		error = PTR_ERR(priv->addr);
@@ -2323,7 +2359,8 @@ static int ravb_probe(struct platform_device *pdev)
 	}
 
 	netif_napi_add(ndev, &priv->napi[RAVB_BE], ravb_poll, 64);
-	netif_napi_add(ndev, &priv->napi[RAVB_NC], ravb_poll, 64);
+	if (info->nc_queue)
+		netif_napi_add(ndev, &priv->napi[RAVB_NC], ravb_poll, 64);
 
 	/* Network device register */
 	error = register_netdev(ndev);
@@ -2341,7 +2378,9 @@ static int ravb_probe(struct platform_device *pdev)
 	return 0;
 
 out_napi_del:
-	netif_napi_del(&priv->napi[RAVB_NC]);
+	if (info->nc_queue)
+		netif_napi_del(&priv->napi[RAVB_NC]);
+
 	netif_napi_del(&priv->napi[RAVB_BE]);
 	ravb_mdio_release(priv);
 out_dma_free:
@@ -2380,7 +2419,8 @@ static int ravb_remove(struct platform_device *pdev)
 	ravb_write(ndev, CCC_OPC_RESET, CCC);
 	pm_runtime_put_sync(&pdev->dev);
 	unregister_netdev(ndev);
-	netif_napi_del(&priv->napi[RAVB_NC]);
+	if (info->nc_queue)
+		netif_napi_del(&priv->napi[RAVB_NC]);
 	netif_napi_del(&priv->napi[RAVB_BE]);
 	ravb_mdio_release(priv);
 	pm_runtime_disable(&pdev->dev);
@@ -2394,6 +2434,7 @@ static int ravb_remove(struct platform_device *pdev)
 static int ravb_wol_setup(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
 
 	/* Disable interrupts by clearing the interrupt masks. */
 	ravb_write(ndev, 0, RIC0);
@@ -2402,7 +2443,8 @@ static int ravb_wol_setup(struct net_device *ndev)
 
 	/* Only allow ECI interrupts */
 	synchronize_irq(priv->emac_irq);
-	napi_disable(&priv->napi[RAVB_NC]);
+	if (info->nc_queue)
+		napi_disable(&priv->napi[RAVB_NC]);
 	napi_disable(&priv->napi[RAVB_BE]);
 	ravb_write(ndev, ECSIPR_MPDIP, ECSIPR);
 
@@ -2415,9 +2457,11 @@ static int ravb_wol_setup(struct net_device *ndev)
 static int ravb_wol_restore(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
 	int ret;
 
-	napi_enable(&priv->napi[RAVB_NC]);
+	if (info->nc_queue)
+		napi_enable(&priv->napi[RAVB_NC]);
 	napi_enable(&priv->napi[RAVB_BE]);
 
 	/* Disable MagicPacket */
-- 
2.17.1

[PATCH 04/10] ravb: Add support for RZ/G2L SoC

[Biju Das]

RZ/G2L SoC has Gigabit Ethernet IP consisting of Ethernet controller
(E-MAC), Internal TCP/IP Offload Engine (TOE) and Dedicated Direct
memory access controller (DMAC).

This patch adds compatible string for RZ/G2L and fills up the
ravb_hw_info struct. Function stubs are added which will be used by
gbeth_hw_info and will be filled incrementally.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v1:
 * Added compatible string for RZ/G2L.
 * Added feature bits max_rx_len, aligned_tx and tx_counters for RZ/G2L.
---
 drivers/net/ethernet/renesas/ravb.h      |  2 +
 drivers/net/ethernet/renesas/ravb_main.c | 62 ++++++++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index c91e93e5590f..f6398fdcead2 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -956,6 +956,8 @@ enum RAVB_QUEUE {
 
 #define RX_BUF_SZ	(2048 - ETH_FCS_LEN + sizeof(__sum16))
 
+#define GBETH_RX_BUFF_MAX 8192
+
 struct ravb_tstamp_skb {
 	struct list_head list;
 	struct sk_buff *skb;
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 8bf13586e90a..dc817b4d95a1 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -83,6 +83,11 @@ static int ravb_config(struct net_device *ndev)
 	return error;
 }
 
+static void ravb_set_rate_gbeth(struct net_device *ndev)
+{
+	/* Place holder */
+}
+
 static void ravb_set_rate(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
@@ -217,6 +222,11 @@ static int ravb_tx_free(struct net_device *ndev, int q, bool free_txed_only)
 	return free_num;
 }
 
+static void ravb_rx_ring_free_gbeth(struct net_device *ndev, int q)
+{
+	/* Place holder */
+}
+
 static void ravb_rx_ring_free(struct net_device *ndev, int q)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
@@ -283,6 +293,11 @@ static void ravb_ring_free(struct net_device *ndev, int q)
 	priv->tx_skb[q] = NULL;
 }
 
+static void ravb_rx_ring_format_gbeth(struct net_device *ndev, int q)
+{
+	/* Place holder */
+}
+
 static void ravb_rx_ring_format(struct net_device *ndev, int q)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
@@ -356,6 +371,12 @@ static void ravb_ring_format(struct net_device *ndev, int q)
 	desc->dptr = cpu_to_le32((u32)priv->tx_desc_dma[q]);
 }
 
+static void *ravb_alloc_rx_desc_gbeth(struct net_device *ndev, int q)
+{
+	/* Place holder */
+	return NULL;
+}
+
 static void *ravb_alloc_rx_desc(struct net_device *ndev, int q)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
@@ -426,6 +447,11 @@ static int ravb_ring_init(struct net_device *ndev, int q)
 	return -ENOMEM;
 }
 
+static void ravb_emac_init_gbeth(struct net_device *ndev)
+{
+	/* Place holder */
+}
+
 static void ravb_rcar_emac_init(struct net_device *ndev)
 {
 	/* Receive frame limit set register */
@@ -461,6 +487,12 @@ static void ravb_emac_init(struct net_device *ndev)
 	info->emac_init(ndev);
 }
 
+static int ravb_dmac_init_gbeth(struct net_device *ndev)
+{
+	/* Place holder */
+	return 0;
+}
+
 static int ravb_dmac_init_rcar(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
@@ -584,6 +616,14 @@ static void ravb_rx_csum(struct sk_buff *skb)
 	skb_trim(skb, skb->len - sizeof(__sum16));
 }
 
+/* Packet receive function for Gigabit Ethernet */
+static bool ravb_rx_gbeth(struct net_device *ndev, int *quota, int q)
+{
+	/* Place holder */
+	return true;
+}
+
+/* Packet receive function for Ethernet AVB */
 static bool ravb_rcar_rx(struct net_device *ndev, int *quota, int q)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
@@ -1949,6 +1989,13 @@ static void ravb_set_rx_csum(struct net_device *ndev, bool enable)
 	spin_unlock_irqrestore(&priv->lock, flags);
 }
 
+static int ravb_set_features_gbeth(struct net_device *ndev,
+				   netdev_features_t features)
+{
+	/* Place holder */
+	return 0;
+}
+
 static int ravb_set_features_rcar(struct net_device *ndev,
 				  netdev_features_t features)
 {
@@ -2073,12 +2120,27 @@ static const struct ravb_hw_info ravb_gen2_hw_info = {
 	.nc_queue = 1,
 };
 
+static const struct ravb_hw_info gbeth_hw_info = {
+	.rx_ring_free = ravb_rx_ring_free_gbeth,
+	.rx_ring_format = ravb_rx_ring_format_gbeth,
+	.alloc_rx_desc = ravb_alloc_rx_desc_gbeth,
+	.receive = ravb_rx_gbeth,
+	.set_rate = ravb_set_rate_gbeth,
+	.set_feature = ravb_set_features_gbeth,
+	.dmac_init = ravb_dmac_init_gbeth,
+	.emac_init = ravb_emac_init_gbeth,
+	.max_rx_len = GBETH_RX_BUFF_MAX + RAVB_ALIGN - 1,
+	.aligned_tx = 1,
+	.tx_counters = 1,
+};
+
 static const struct of_device_id ravb_match_table[] = {
 	{ .compatible = "renesas,etheravb-r8a7790", .data = &ravb_gen2_hw_info },
 	{ .compatible = "renesas,etheravb-r8a7794", .data = &ravb_gen2_hw_info },
 	{ .compatible = "renesas,etheravb-rcar-gen2", .data = &ravb_gen2_hw_info },
 	{ .compatible = "renesas,etheravb-r8a7795", .data = &ravb_gen3_hw_info },
 	{ .compatible = "renesas,etheravb-rcar-gen3", .data = &ravb_gen3_hw_info },
+	{ .compatible = "renesas,rzg2l-gbeth", .data = &gbeth_hw_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ravb_match_table);
-- 
2.17.1

[PATCH 05/10] ravb: Initialize GbEthernet DMAC

[Biju Das]

Initialize GbEthernet DMAC found on RZ/G2L SoC.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v1:
 * Removed RIC3 initialization from DMAC init, as it is 
   same as reset value.
 * moved stubs function to earlier patches.
 * renamed "rgeth" with "gbeth"
---
 drivers/net/ethernet/renesas/ravb.h      |  3 ++-
 drivers/net/ethernet/renesas/ravb_main.c | 30 +++++++++++++++++++++++-
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index f6398fdcead2..9cd3a15743b4 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -81,6 +81,7 @@ enum ravb_reg {
 	RQC3	= 0x00A0,
 	RQC4	= 0x00A4,
 	RPC	= 0x00B0,
+	RTC	= 0x00B4,	/* R-Car Gen3 and RZ/G2L only */
 	UFCW	= 0x00BC,
 	UFCS	= 0x00C0,
 	UFCV0	= 0x00C4,
@@ -193,7 +194,7 @@ enum ravb_reg {
 	GECMR	= 0x05b0,
 	MAHR	= 0x05c0,
 	MALR	= 0x05c8,
-	TROCR	= 0x0700,	/* R-Car Gen3 only */
+	TROCR	= 0x0700,	/* R-Car Gen3 and RZ/G2L only */
 	CEFCR	= 0x0740,
 	FRECR	= 0x0748,
 	TSFRCR	= 0x0750,
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index dc817b4d95a1..5790a9332e7b 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -489,7 +489,35 @@ static void ravb_emac_init(struct net_device *ndev)
 
 static int ravb_dmac_init_gbeth(struct net_device *ndev)
 {
-	/* Place holder */
+	int error;
+
+	error = ravb_ring_init(ndev, RAVB_BE);
+	if (error)
+		return error;
+
+	/* Descriptor format */
+	ravb_ring_format(ndev, RAVB_BE);
+
+	/* Set AVB RX */
+	ravb_write(ndev, 0x60000000, RCR);
+
+	/* Set Max Frame Length (RTC) */
+	ravb_write(ndev, 0x7ffc0000 | GBETH_RX_BUFF_MAX, RTC);
+
+	/* Set FIFO size */
+	ravb_write(ndev, 0x00222200, TGC);
+
+	ravb_write(ndev, 0, TCCR);
+
+	/* Frame receive */
+	ravb_write(ndev, RIC0_FRE0, RIC0);
+	/* Disable FIFO full warning */
+	ravb_write(ndev, 0x0, RIC1);
+	/* Receive FIFO full error, descriptor empty */
+	ravb_write(ndev, RIC2_QFE0 | RIC2_RFFE, RIC2);
+
+	ravb_write(ndev, TIC_FTE0, TIC);
+
 	return 0;
 }
 
-- 
2.17.1

[PATCH 06/10] ravb: Exclude gPTP feature support for RZ/G2L

[Biju Das]

R-Car supports gPTP feature whereas RZ/G2L does not support it.
This patch excludes gtp feature support for RZ/G2L.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v1:
 * handled timestamps only for gptp cases.
 * Removed ptp check from irq handler
---
 drivers/net/ethernet/renesas/ravb_main.c | 85 ++++++++++++++----------
 1 file changed, 49 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 5790a9332e7b..ac141a491ca2 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1403,6 +1403,7 @@ static int ravb_get_ts_info(struct net_device *ndev,
 			    struct ethtool_ts_info *info)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *hw_info = priv->info;
 
 	info->so_timestamping =
 		SOF_TIMESTAMPING_TX_SOFTWARE |
@@ -1416,7 +1417,8 @@ static int ravb_get_ts_info(struct net_device *ndev,
 		(1 << HWTSTAMP_FILTER_NONE) |
 		(1 << HWTSTAMP_FILTER_PTP_V2_L2_EVENT) |
 		(1 << HWTSTAMP_FILTER_ALL);
-	info->phc_index = ptp_clock_index(priv->ptp.clock);
+	if (hw_info->gptp || hw_info->ccc_gac)
+		info->phc_index = ptp_clock_index(priv->ptp.clock);
 
 	return 0;
 }
@@ -1640,6 +1642,7 @@ static void ravb_tx_timeout_work(struct work_struct *work)
 static netdev_tx_t ravb_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
 	unsigned int num_tx_desc = priv->num_tx_desc;
 	u16 q = skb_get_queue_mapping(skb);
 	struct ravb_tstamp_skb *ts_skb;
@@ -1716,28 +1719,30 @@ static netdev_tx_t ravb_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 	desc->dptr = cpu_to_le32(dma_addr);
 
 	/* TX timestamp required */
-	if (q == RAVB_NC) {
-		ts_skb = kmalloc(sizeof(*ts_skb), GFP_ATOMIC);
-		if (!ts_skb) {
-			if (num_tx_desc > 1) {
-				desc--;
-				dma_unmap_single(ndev->dev.parent, dma_addr,
-						 len, DMA_TO_DEVICE);
+	if (info->gptp || info->ccc_gac) {
+		if (q == RAVB_NC) {
+			ts_skb = kmalloc(sizeof(*ts_skb), GFP_ATOMIC);
+			if (!ts_skb) {
+				if (num_tx_desc > 1) {
+					desc--;
+					dma_unmap_single(ndev->dev.parent, dma_addr,
+							 len, DMA_TO_DEVICE);
+				}
+				goto unmap;
 			}
-			goto unmap;
+			ts_skb->skb = skb_get(skb);
+			ts_skb->tag = priv->ts_skb_tag++;
+			priv->ts_skb_tag &= 0x3ff;
+			list_add_tail(&ts_skb->list, &priv->ts_skb_list);
+
+			/* TAG and timestamp required flag */
+			skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
+			desc->tagh_tsr = (ts_skb->tag >> 4) | TX_TSR;
+			desc->ds_tagl |= cpu_to_le16(ts_skb->tag << 12);
 		}
-		ts_skb->skb = skb_get(skb);
-		ts_skb->tag = priv->ts_skb_tag++;
-		priv->ts_skb_tag &= 0x3ff;
-		list_add_tail(&ts_skb->list, &priv->ts_skb_list);
 
-		/* TAG and timestamp required flag */
-		skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
-		desc->tagh_tsr = (ts_skb->tag >> 4) | TX_TSR;
-		desc->ds_tagl |= cpu_to_le16(ts_skb->tag << 12);
+		skb_tx_timestamp(skb);
 	}
-
-	skb_tx_timestamp(skb);
 	/* Descriptor type must be set after all the above writes */
 	dma_wmb();
 	if (num_tx_desc > 1) {
@@ -1858,10 +1863,12 @@ static int ravb_close(struct net_device *ndev)
 			   "device will be stopped after h/w processes are done.\n");
 
 	/* Clear the timestamp list */
-	list_for_each_entry_safe(ts_skb, ts_skb2, &priv->ts_skb_list, list) {
-		list_del(&ts_skb->list);
-		kfree_skb(ts_skb->skb);
-		kfree(ts_skb);
+	if (info->gptp || info->ccc_gac) {
+		list_for_each_entry_safe(ts_skb, ts_skb2, &priv->ts_skb_list, list) {
+			list_del(&ts_skb->list);
+			kfree_skb(ts_skb->skb);
+			kfree(ts_skb);
+		}
 	}
 
 	/* PHY disconnect */
@@ -2207,9 +2214,11 @@ static void ravb_set_config_mode(struct net_device *ndev)
 		ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_CONFIG);
 		/* Set CSEL value */
 		ravb_modify(ndev, CCC, CCC_CSEL, CCC_CSEL_HPB);
-	} else {
+	} else if (info->ccc_gac) {
 		ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_CONFIG |
 			    CCC_GAC | CCC_CSEL_HPB);
+	} else {
+		ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_CONFIG);
 	}
 }
 
@@ -2395,13 +2404,15 @@ static int ravb_probe(struct platform_device *pdev)
 	/* Set AVB config mode */
 	ravb_set_config_mode(ndev);
 
-	/* Set GTI value */
-	error = ravb_set_gti(ndev);
-	if (error)
-		goto out_disable_refclk;
+	if (info->gptp || info->ccc_gac) {
+		/* Set GTI value */
+		error = ravb_set_gti(ndev);
+		if (error)
+			goto out_disable_refclk;
 
-	/* Request GTI loading */
-	ravb_modify(ndev, GCCR, GCCR_LTI, GCCR_LTI);
+		/* Request GTI loading */
+		ravb_modify(ndev, GCCR, GCCR_LTI, GCCR_LTI);
+	}
 
 	if (info->internal_delay) {
 		ravb_parse_delay_mode(np, ndev);
@@ -2602,13 +2613,15 @@ static int __maybe_unused ravb_resume(struct device *dev)
 	/* Set AVB config mode */
 	ravb_set_config_mode(ndev);
 
-	/* Set GTI value */
-	ret = ravb_set_gti(ndev);
-	if (ret)
-		return ret;
+	if (info->gptp || info->ccc_gac) {
+		/* Set GTI value */
+		ret = ravb_set_gti(ndev);
+		if (ret)
+			return ret;
 
-	/* Request GTI loading */
-	ravb_modify(ndev, GCCR, GCCR_LTI, GCCR_LTI);
+		/* Request GTI loading */
+		ravb_modify(ndev, GCCR, GCCR_LTI, GCCR_LTI);
+	}
 
 	if (info->internal_delay)
 		ravb_set_delay_mode(ndev);
-- 
2.17.1

[PATCH 07/10] ravb: Add tsrq to struct ravb_hw_info

[Biju Das]

R-Car AVB-DMAC has 4 Transmit start request queues, whereas
RZ/G2L has only 1 Transmit start request queue.

Add a tsrq variable to struct ravb_hw_info to handle this
difference.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v1:
 * Added tsrq variable instead of multi_tsrq feature bit.
---
 drivers/net/ethernet/renesas/ravb.h      | 1 +
 drivers/net/ethernet/renesas/ravb_main.c | 9 +++++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 9cd3a15743b4..c586070193ef 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -997,6 +997,7 @@ struct ravb_hw_info {
 	netdev_features_t net_features;
 	int stats_len;
 	size_t max_rx_len;
+	u32 tsrq;
 	unsigned aligned_tx: 1;
 
 	/* hardware features */
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index ac141a491ca2..4784832bd93c 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -790,11 +790,13 @@ static void ravb_rcv_snd_enable(struct net_device *ndev)
 /* function for waiting dma process finished */
 static int ravb_stop_dma(struct net_device *ndev)
 {
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
 	int error;
 
 	/* Wait for stopping the hardware TX process */
-	error = ravb_wait(ndev, TCCR,
-			  TCCR_TSRQ0 | TCCR_TSRQ1 | TCCR_TSRQ2 | TCCR_TSRQ3, 0);
+	error = ravb_wait(ndev, TCCR, info->tsrq, 0);
+
 	if (error)
 		return error;
 
@@ -2128,6 +2130,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.net_features = NETIF_F_RXCSUM,
 	.stats_len = ARRAY_SIZE(ravb_gstrings_stats),
 	.max_rx_len = RX_BUF_SZ + RAVB_ALIGN - 1,
+	.tsrq = TCCR_TSRQ0 | TCCR_TSRQ1 | TCCR_TSRQ2 | TCCR_TSRQ3,
 	.internal_delay = 1,
 	.tx_counters = 1,
 	.multi_irqs = 1,
@@ -2150,6 +2153,7 @@ static const struct ravb_hw_info ravb_gen2_hw_info = {
 	.net_features = NETIF_F_RXCSUM,
 	.stats_len = ARRAY_SIZE(ravb_gstrings_stats),
 	.max_rx_len = RX_BUF_SZ + RAVB_ALIGN - 1,
+	.tsrq = TCCR_TSRQ0 | TCCR_TSRQ1 | TCCR_TSRQ2 | TCCR_TSRQ3,
 	.aligned_tx = 1,
 	.gptp = 1,
 	.nc_queue = 1,
@@ -2165,6 +2169,7 @@ static const struct ravb_hw_info gbeth_hw_info = {
 	.dmac_init = ravb_dmac_init_gbeth,
 	.emac_init = ravb_emac_init_gbeth,
 	.max_rx_len = GBETH_RX_BUFF_MAX + RAVB_ALIGN - 1,
+	.tsrq = TCCR_TSRQ0,
 	.aligned_tx = 1,
 	.tx_counters = 1,
 };
-- 
2.17.1

[PATCH 08/10] ravb: Add magic_pkt to struct ravb_hw_info

[Biju Das]

E-MAC on R-Car supports magic packet detection, whereas RZ/G2L
does not support this feature. Add magic_pkt to struct ravb_hw_info
and enable this feature only for R-Car.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v1:
 * Removed the feature checking from interrupt handler.
---
 drivers/net/ethernet/renesas/ravb.h      | 1 +
 drivers/net/ethernet/renesas/ravb_main.c | 5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index c586070193ef..f0d9b7bc8c20 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -1007,6 +1007,7 @@ struct ravb_hw_info {
 	unsigned gptp:1;		/* AVB-DMAC has gPTP support */
 	unsigned ccc_gac:1;		/* AVB-DMAC has gPTP support active in config mode */
 	unsigned nc_queue:1;		/* AVB-DMAC has NC queue */
+	unsigned magic_pkt:1;		/* E-MAC supports magic packet detection */
 };
 
 struct ravb_private {
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 4784832bd93c..6fb11d4cfc57 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1436,8 +1436,9 @@ static void ravb_get_wol(struct net_device *ndev, struct ethtool_wolinfo *wol)
 static int ravb_set_wol(struct net_device *ndev, struct ethtool_wolinfo *wol)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
 
-	if (wol->wolopts & ~WAKE_MAGIC)
+	if (!info->magic_pkt || (wol->wolopts & ~WAKE_MAGIC))
 		return -EOPNOTSUPP;
 
 	priv->wol_enabled = !!(wol->wolopts & WAKE_MAGIC);
@@ -2136,6 +2137,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.multi_irqs = 1,
 	.ccc_gac = 1,
 	.nc_queue = 1,
+	.magic_pkt = 1,
 };
 
 static const struct ravb_hw_info ravb_gen2_hw_info = {
@@ -2157,6 +2159,7 @@ static const struct ravb_hw_info ravb_gen2_hw_info = {
 	.aligned_tx = 1,
 	.gptp = 1,
 	.nc_queue = 1,
+	.magic_pkt = 1,
 };
 
 static const struct ravb_hw_info gbeth_hw_info = {
-- 
2.17.1

[PATCH 09/10] ravb: Add half_duplex to struct ravb_hw_info

[Biju Das]

RZ/G2L supports half duplex mode.
Add a half_duplex hw feature bit to struct ravb_hw_info for
supporting half duplex mode for RZ/G2L.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
---
RFC->v1:
 * Added Sergei's Rb tag.
---
 drivers/net/ethernet/renesas/ravb.h      |  3 ++
 drivers/net/ethernet/renesas/ravb_main.c | 36 ++++++++++++++++++------
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index f0d9b7bc8c20..d83d3b4f3f5f 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -1008,6 +1008,7 @@ struct ravb_hw_info {
 	unsigned ccc_gac:1;		/* AVB-DMAC has gPTP support active in config mode */
 	unsigned nc_queue:1;		/* AVB-DMAC has NC queue */
 	unsigned magic_pkt:1;		/* E-MAC supports magic packet detection */
+	unsigned half_duplex:1;		/* E-MAC supports half duplex mode */
 };
 
 struct ravb_private {
@@ -1062,6 +1063,8 @@ struct ravb_private {
 	unsigned rgmii_override:1;	/* Deprecated rgmii-*id behavior */
 	unsigned int num_tx_desc;	/* TX descriptors per packet */
 
+	int duplex;
+
 	const struct ravb_hw_info *info;
 	struct reset_control *rstc;
 };
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 6fb11d4cfc57..3e694738e683 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1091,6 +1091,13 @@ static int ravb_poll(struct napi_struct *napi, int budget)
 	return budget - quota;
 }
 
+static void ravb_set_duplex_gbeth(struct net_device *ndev)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+
+	ravb_modify(ndev, ECMR, ECMR_DM, priv->duplex > 0 ? ECMR_DM : 0);
+}
+
 /* PHY state control function */
 static void ravb_adjust_link(struct net_device *ndev)
 {
@@ -1107,6 +1114,12 @@ static void ravb_adjust_link(struct net_device *ndev)
 		ravb_rcv_snd_disable(ndev);
 
 	if (phydev->link) {
+		if (info->half_duplex && phydev->duplex != priv->duplex) {
+			new_state = true;
+			priv->duplex = phydev->duplex;
+			ravb_set_duplex_gbeth(ndev);
+		}
+
 		if (phydev->speed != priv->speed) {
 			new_state = true;
 			priv->speed = phydev->speed;
@@ -1121,6 +1134,8 @@ static void ravb_adjust_link(struct net_device *ndev)
 		new_state = true;
 		priv->link = 0;
 		priv->speed = 0;
+		if (info->half_duplex)
+			priv->duplex = -1;
 	}
 
 	/* Enable TX and RX right over here, if E-MAC change is ignored */
@@ -1143,6 +1158,7 @@ static int ravb_phy_init(struct net_device *ndev)
 {
 	struct device_node *np = ndev->dev.parent->of_node;
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
 	struct phy_device *phydev;
 	struct device_node *pn;
 	phy_interface_t iface;
@@ -1150,6 +1166,7 @@ static int ravb_phy_init(struct net_device *ndev)
 
 	priv->link = 0;
 	priv->speed = 0;
+	priv->duplex = -1;
 
 	/* Try connecting to PHY */
 	pn = of_parse_phandle(np, "phy-handle", 0);
@@ -1188,15 +1205,17 @@ static int ravb_phy_init(struct net_device *ndev)
 		netdev_info(ndev, "limited PHY to 100Mbit/s\n");
 	}
 
-	/* 10BASE, Pause and Asym Pause is not supported */
-	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
-	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT);
-	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Pause_BIT);
-	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Asym_Pause_BIT);
+	if (!info->half_duplex) {
+		/* 10BASE, Pause and Asym Pause is not supported */
+		phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
+		phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT);
+		phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Pause_BIT);
+		phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Asym_Pause_BIT);
 
-	/* Half Duplex is not supported */
-	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
-	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_100baseT_Half_BIT);
+		/* Half Duplex is not supported */
+		phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
+		phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_100baseT_Half_BIT);
+	}
 
 	phy_attached_info(phydev);
 
@@ -2175,6 +2194,7 @@ static const struct ravb_hw_info gbeth_hw_info = {
 	.tsrq = TCCR_TSRQ0,
 	.aligned_tx = 1,
 	.tx_counters = 1,
+	.half_duplex = 1,
 };
 
 static const struct of_device_id ravb_match_table[] = {
-- 
2.17.1

[PATCH 10/10] ravb: Initialize GbEthernet E-MAC

[Biju Das]

Initialize GbEthernet E-MAC found on RZ/G2L SoC.
This patch also renames ravb_set_rate to ravb_set_rate_rcar and
ravb_rcar_emac_init to ravb_emac_init_rcar to be consistent with
the naming convention used in sh_eth driver.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v1:
 * Moved CSR0 intialization to later patch.
 * started using ravb_modify for initializing link registers.
---
 drivers/net/ethernet/renesas/ravb.h      | 20 +++++++--
 drivers/net/ethernet/renesas/ravb_main.c | 55 ++++++++++++++++++++----
 2 files changed, 62 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index d83d3b4f3f5f..5dc1324786e0 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -188,6 +188,7 @@ enum ravb_reg {
 	PIR	= 0x0520,
 	PSR	= 0x0528,
 	PIPR	= 0x052c,
+	CXR31	= 0x0530,	/* RZ/G2L only */
 	MPR	= 0x0558,
 	PFTCR	= 0x055c,
 	PFRCR	= 0x0560,
@@ -811,10 +812,11 @@ enum ECMR_BIT {
 	ECMR_TXF	= 0x00010000,	/* Documented for R-Car Gen3 only */
 	ECMR_RXF	= 0x00020000,
 	ECMR_PFR	= 0x00040000,
-	ECMR_ZPF	= 0x00080000,	/* Documented for R-Car Gen3 only */
+	ECMR_ZPF	= 0x00080000,	/* Documented for R-Car Gen3 and RZ/G2L */
 	ECMR_RZPF	= 0x00100000,
 	ECMR_DPAD	= 0x00200000,
 	ECMR_RCSC	= 0x00800000,
+	ECMR_RCPT	= 0x02000000,	/* Documented for RZ/G2L only */
 	ECMR_TRCCM	= 0x04000000,
 };
 
@@ -824,6 +826,7 @@ enum ECSR_BIT {
 	ECSR_MPD	= 0x00000002,
 	ECSR_LCHNG	= 0x00000004,
 	ECSR_PHYI	= 0x00000008,
+	ECSR_PFRI	= 0x00000010,
 };
 
 /* ECSIPR */
@@ -858,9 +861,13 @@ enum MPR_BIT {
 
 /* GECMR */
 enum GECMR_BIT {
-	GECMR_SPEED	= 0x00000001,
-	GECMR_SPEED_100	= 0x00000000,
-	GECMR_SPEED_1000 = 0x00000001,
+	GECMR_SPEED		= 0x00000001,
+	GECMR_SPEED_100		= 0x00000000,
+	GECMR_SPEED_1000	= 0x00000001,
+	GBETH_GECMR_SPEED	= 0x00000030,
+	GBETH_GECMR_SPEED_10	= 0x00000000,
+	GBETH_GECMR_SPEED_100	= 0x00000010,
+	GBETH_GECMR_SPEED_1000	= 0x00000020,
 };
 
 /* The Ethernet AVB descriptor definitions. */
@@ -950,6 +957,11 @@ enum RAVB_QUEUE {
 	RAVB_NC,	/* Network Control Queue */
 };
 
+enum CXR31_BIT {
+	CXR31_SEL_LINK0	= 0x00000001,
+	CXR31_SEL_LINK1	= 0x00000008,
+};
+
 #define DBAT_ENTRY_NUM	22
 #define RX_QUEUE_OFFSET	4
 #define NUM_RX_QUEUE	2
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 3e694738e683..9a4888543384 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -85,10 +85,22 @@ static int ravb_config(struct net_device *ndev)
 
 static void ravb_set_rate_gbeth(struct net_device *ndev)
 {
-	/* Place holder */
+	struct ravb_private *priv = netdev_priv(ndev);
+
+	switch (priv->speed) {
+	case 10:                /* 10BASE */
+		ravb_write(ndev, GBETH_GECMR_SPEED_10, GECMR);
+		break;
+	case 100:               /* 100BASE */
+		ravb_write(ndev, GBETH_GECMR_SPEED_100, GECMR);
+		break;
+	case 1000:              /* 1000BASE */
+		ravb_write(ndev, GBETH_GECMR_SPEED_1000, GECMR);
+		break;
+	}
 }
 
-static void ravb_set_rate(struct net_device *ndev)
+static void ravb_set_rate_rcar(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
 
@@ -449,10 +461,35 @@ static int ravb_ring_init(struct net_device *ndev, int q)
 
 static void ravb_emac_init_gbeth(struct net_device *ndev)
 {
-	/* Place holder */
+	struct ravb_private *priv = netdev_priv(ndev);
+
+	/* Receive frame limit set register */
+	ravb_write(ndev, GBETH_RX_BUFF_MAX + ETH_FCS_LEN, RFLR);
+
+	/* PAUSE prohibition */
+	ravb_write(ndev, ECMR_ZPF | ((priv->duplex > 0) ? ECMR_DM : 0) |
+			 ECMR_TE | ECMR_RE | ECMR_RCPT |
+			 ECMR_TXF | ECMR_RXF | ECMR_PRM, ECMR);
+
+	ravb_set_rate_gbeth(ndev);
+
+	/* Set MAC address */
+	ravb_write(ndev,
+		   (ndev->dev_addr[0] << 24) | (ndev->dev_addr[1] << 16) |
+		   (ndev->dev_addr[2] << 8)  | (ndev->dev_addr[3]), MAHR);
+	ravb_write(ndev, (ndev->dev_addr[4] << 8)  | (ndev->dev_addr[5]), MALR);
+
+	/* E-MAC status register clear */
+	ravb_write(ndev, ECSR_ICD | ECSR_LCHNG | ECSR_PFRI, ECSR);
+
+	/* E-MAC interrupt enable register */
+	ravb_write(ndev, ECSIPR_ICDIP, ECSIPR);
+
+	ravb_modify(ndev, CXR31, CXR31_SEL_LINK1, 0);
+	ravb_modify(ndev, CXR31, CXR31_SEL_LINK0, CXR31_SEL_LINK0);
 }
 
-static void ravb_rcar_emac_init(struct net_device *ndev)
+static void ravb_emac_init_rcar(struct net_device *ndev)
 {
 	/* Receive frame limit set register */
 	ravb_write(ndev, ndev->mtu + ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN, RFLR);
@@ -462,7 +499,7 @@ static void ravb_rcar_emac_init(struct net_device *ndev)
 		   (ndev->features & NETIF_F_RXCSUM ? ECMR_RCSC : 0) |
 		   ECMR_TE | ECMR_RE, ECMR);
 
-	ravb_set_rate(ndev);
+	ravb_set_rate_rcar(ndev);
 
 	/* Set MAC address */
 	ravb_write(ndev,
@@ -2140,10 +2177,10 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.rx_ring_format = ravb_rx_ring_format,
 	.alloc_rx_desc = ravb_alloc_rx_desc,
 	.receive = ravb_rcar_rx,
-	.set_rate = ravb_set_rate,
+	.set_rate = ravb_set_rate_rcar,
 	.set_feature = ravb_set_features_rcar,
 	.dmac_init = ravb_dmac_init_rcar,
-	.emac_init = ravb_rcar_emac_init,
+	.emac_init = ravb_emac_init_rcar,
 	.gstrings_stats = ravb_gstrings_stats,
 	.gstrings_size = sizeof(ravb_gstrings_stats),
 	.net_hw_features = NETIF_F_RXCSUM,
@@ -2164,10 +2201,10 @@ static const struct ravb_hw_info ravb_gen2_hw_info = {
 	.rx_ring_format = ravb_rx_ring_format,
 	.alloc_rx_desc = ravb_alloc_rx_desc,
 	.receive = ravb_rcar_rx,
-	.set_rate = ravb_set_rate,
+	.set_rate = ravb_set_rate_rcar,
 	.set_feature = ravb_set_features_rcar,
 	.dmac_init = ravb_dmac_init_rcar,
-	.emac_init = ravb_rcar_emac_init,
+	.emac_init = ravb_emac_init_rcar,
 	.gstrings_stats = ravb_gstrings_stats,
 	.gstrings_size = sizeof(ravb_gstrings_stats),
 	.net_hw_features = NETIF_F_RXCSUM,
-- 
2.17.1

Re: [PATCH 02/10] ravb: Rename "no_ptp_cfg_active" and "ptp_cfg_active" variables

[Sergey Shtylyov]

On 10/1/21 6:06 PM, Biju Das wrote:

> Rename the variable "no_ptp_cfg_active" with "gptp" and

   This shouldn't be a rename but the extension of the meaning instead...

> "ptp_cfg_active" with "ccc_gac" to match the HW features.
> 
> There is no functional change.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Suggested-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> RFc->v1:
>  * Renamed the variable "no_ptp_cfg_active" with "gptp" and
>    "ptp_cfg_active" with "ccc_gac
> ---
>  drivers/net/ethernet/renesas/ravb.h      |  4 ++--
>  drivers/net/ethernet/renesas/ravb_main.c | 26 ++++++++++++------------
>  2 files changed, 15 insertions(+), 15 deletions(-)

[...]
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 8f2358caef34..dc7654abfe55 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -1274,7 +1274,7 @@ static int ravb_set_ringparam(struct net_device *ndev,
>  	if (netif_running(ndev)) {
>  		netif_device_detach(ndev);
>  		/* Stop PTP Clock driver */
> -		if (info->no_ptp_cfg_active)
> +		if (info->gptp)

   Where have you lost !info->ccc_gac?

>  			ravb_ptp_stop(ndev);
>  		/* Wait for DMA stopping */
>  		error = ravb_stop_dma(ndev);
> @@ -1306,7 +1306,7 @@ static int ravb_set_ringparam(struct net_device *ndev,
>  		ravb_emac_init(ndev);
>  
>  		/* Initialise PTP Clock driver */
> -		if (info->no_ptp_cfg_active)
> +		if (info->gptp)
>  			ravb_ptp_init(ndev, priv->pdev);

    The same question here...

>  		netif_device_attach(ndev);
> @@ -1446,7 +1446,7 @@ static int ravb_open(struct net_device *ndev)
>  	ravb_emac_init(ndev);
>  
>  	/* Initialise PTP Clock driver */
> -	if (info->no_ptp_cfg_active)
> +	if (info->gptp)

   ... and here.

>  		ravb_ptp_init(ndev, priv->pdev);
>  
>  	netif_tx_start_all_queues(ndev);
> @@ -1460,7 +1460,7 @@ static int ravb_open(struct net_device *ndev)
>  
>  out_ptp_stop:
>  	/* Stop PTP Clock driver */
> -	if (info->no_ptp_cfg_active)
> +	if (info->gptp)
>  		ravb_ptp_stop(ndev);

    ... and here.

>  out_free_irq_nc_tx:
>  	if (!info->multi_irqs)
> @@ -1508,7 +1508,7 @@ static void ravb_tx_timeout_work(struct work_struct *work)
>  	netif_tx_stop_all_queues(ndev);
>  
>  	/* Stop PTP Clock driver */
> -	if (info->no_ptp_cfg_active)
> +	if (info->gptp)

    ... and here.

>  		ravb_ptp_stop(ndev);
>  
>  	/* Wait for DMA stopping */
> @@ -1543,7 +1543,7 @@ static void ravb_tx_timeout_work(struct work_struct *work)
>  
>  out:
>  	/* Initialise PTP Clock driver */
> -	if (info->no_ptp_cfg_active)
> +	if (info->gptp)
>  		ravb_ptp_init(ndev, priv->pdev);

    ... and here.
 
>  	netif_tx_start_all_queues(ndev);
> @@ -1752,7 +1752,7 @@ static int ravb_close(struct net_device *ndev)
>  	ravb_write(ndev, 0, TIC);
>  
>  	/* Stop PTP Clock driver */
> -	if (info->no_ptp_cfg_active)
> +	if (info->gptp)

    ... and here.

>  		ravb_ptp_stop(ndev);
>  
>  	/* Set the config mode to stop the AVB-DMAC's processes */
> @@ -2018,7 +2018,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
>  	.internal_delay = 1,
>  	.tx_counters = 1,
>  	.multi_irqs = 1,
> -	.ptp_cfg_active = 1,

   Where is 'gptp'?

> +	.ccc_gac = 1,
>  };
>  
>  static const struct ravb_hw_info ravb_gen2_hw_info = {
[...]
> @@ -2080,7 +2080,7 @@ static void ravb_set_config_mode(struct net_device *ndev)
>  	struct ravb_private *priv = netdev_priv(ndev);
>  	const struct ravb_hw_info *info = priv->info;
>  
> -	if (info->no_ptp_cfg_active) {
> +	if (info->gptp) {

   Where have you lost !info->ccc_gac?

>  		ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_CONFIG);
>  		/* Set CSEL value */
>  		ravb_modify(ndev, CCC, CCC_CSEL, CCC_CSEL_HPB);
[...]

MBR, Sergey

RE: [PATCH 02/10] ravb: Rename "no_ptp_cfg_active" and "ptp_cfg_active" variables

[Biju Das]

Hi Sergei,

> Subject: Re: [PATCH 02/10] ravb: Rename "no_ptp_cfg_active" and
> "ptp_cfg_active" variables
> 
> On 10/1/21 6:06 PM, Biju Das wrote:
> 
> > Rename the variable "no_ptp_cfg_active" with "gptp" and
> 
>    This shouldn't be a rename but the extension of the meaning instead...

This is the original ptp support for both R-Car Gen3 and R-Car Gen2 without config in active mode. Later we added feature support active in config mode for R-Car Gen3 by patch[1].
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/renesas/ravb_main.c?h=v5.15-rc3&id=f5d7837f96e53a8c9b6c49e1bc95cf0ae88b99e8

> 
> > "ptp_cfg_active" with "ccc_gac" to match the HW features.
> >
> > There is no functional change.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Suggested-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > RFc->v1:
> >  * Renamed the variable "no_ptp_cfg_active" with "gptp" and
> >    "ptp_cfg_active" with "ccc_gac
> > ---
> >  drivers/net/ethernet/renesas/ravb.h      |  4 ++--
> >  drivers/net/ethernet/renesas/ravb_main.c | 26
> > ++++++++++++------------
> >  2 files changed, 15 insertions(+), 15 deletions(-)
> 
> [...]
> > diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> > b/drivers/net/ethernet/renesas/ravb_main.c
> > index 8f2358caef34..dc7654abfe55 100644
> > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> > @@ -1274,7 +1274,7 @@ static int ravb_set_ringparam(struct net_device
> *ndev,
> >  	if (netif_running(ndev)) {
> >  		netif_device_detach(ndev);
> >  		/* Stop PTP Clock driver */
> > -		if (info->no_ptp_cfg_active)
> > +		if (info->gptp)
> 
>    Where have you lost !info->ccc_gac?

  As per patch[1], the check is for R-Car Gen2. Why do you need additional check
as per the current driver? 

I see below you are proposing to enable both "gptp" and "ccc_gac" for R-Car Gen3, According to me it is a feature improvement for R-Car Gen3 in which, you can have

1) gPTP support active in config mode
2) gPTP support not active in config mode

But the existing driver code just support "gPTP support active in config mode" for R-Car Gen3.

Do you want me to do feature improvement as well, as part of Gbethernet support?

Please let me know your thoughts.

The same comments applies to all the comments you have mentioned below.

Regards,
Biju



> 
> >  			ravb_ptp_stop(ndev);
> >  		/* Wait for DMA stopping */
> >  		error = ravb_stop_dma(ndev);
> > @@ -1306,7 +1306,7 @@ static int ravb_set_ringparam(struct net_device
> *ndev,
> >  		ravb_emac_init(ndev);
> >
> >  		/* Initialise PTP Clock driver */
> > -		if (info->no_ptp_cfg_active)
> > +		if (info->gptp)
> >  			ravb_ptp_init(ndev, priv->pdev);
> 
>     The same question here...
> 
> >  		netif_device_attach(ndev);
> > @@ -1446,7 +1446,7 @@ static int ravb_open(struct net_device *ndev)
> >  	ravb_emac_init(ndev);
> >
> >  	/* Initialise PTP Clock driver */
> > -	if (info->no_ptp_cfg_active)
> > +	if (info->gptp)
> 
>    ... and here.
> 
> >  		ravb_ptp_init(ndev, priv->pdev);
> >
> >  	netif_tx_start_all_queues(ndev);
> > @@ -1460,7 +1460,7 @@ static int ravb_open(struct net_device *ndev)
> >
> >  out_ptp_stop:
> >  	/* Stop PTP Clock driver */
> > -	if (info->no_ptp_cfg_active)
> > +	if (info->gptp)
> >  		ravb_ptp_stop(ndev);
> 
>     ... and here.
> 
> >  out_free_irq_nc_tx:
> >  	if (!info->multi_irqs)
> > @@ -1508,7 +1508,7 @@ static void ravb_tx_timeout_work(struct
> work_struct *work)
> >  	netif_tx_stop_all_queues(ndev);
> >
> >  	/* Stop PTP Clock driver */
> > -	if (info->no_ptp_cfg_active)
> > +	if (info->gptp)
> 
>     ... and here.
> 
> >  		ravb_ptp_stop(ndev);
> >
> >  	/* Wait for DMA stopping */
> > @@ -1543,7 +1543,7 @@ static void ravb_tx_timeout_work(struct
> > work_struct *work)
> >
> >  out:
> >  	/* Initialise PTP Clock driver */
> > -	if (info->no_ptp_cfg_active)
> > +	if (info->gptp)
> >  		ravb_ptp_init(ndev, priv->pdev);
> 
>     ... and here.
> 
> >  	netif_tx_start_all_queues(ndev);
> > @@ -1752,7 +1752,7 @@ static int ravb_close(struct net_device *ndev)
> >  	ravb_write(ndev, 0, TIC);
> >
> >  	/* Stop PTP Clock driver */
> > -	if (info->no_ptp_cfg_active)
> > +	if (info->gptp)
> 
>     ... and here.
> 
> >  		ravb_ptp_stop(ndev);
> >
> >  	/* Set the config mode to stop the AVB-DMAC's processes */ @@
> > -2018,7 +2018,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info =
> {
> >  	.internal_delay = 1,
> >  	.tx_counters = 1,
> >  	.multi_irqs = 1,
> > -	.ptp_cfg_active = 1,
> 
>    Where is 'gptp'?
> 
> > +	.ccc_gac = 1,
> >  };
> >
> >  static const struct ravb_hw_info ravb_gen2_hw_info = {
> [...]
> > @@ -2080,7 +2080,7 @@ static void ravb_set_config_mode(struct net_device
> *ndev)
> >  	struct ravb_private *priv = netdev_priv(ndev);
> >  	const struct ravb_hw_info *info = priv->info;
> >
> > -	if (info->no_ptp_cfg_active) {
> > +	if (info->gptp) {
> 
>    Where have you lost !info->ccc_gac?
> 
> >  		ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_CONFIG);
> >  		/* Set CSEL value */
> >  		ravb_modify(ndev, CCC, CCC_CSEL, CCC_CSEL_HPB);
> [...]
> 
> MBR, Sergey

Re: [PATCH 00/10] Add Gigabit Ethernet driver support

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Fri,  1 Oct 2021 16:06:26 +0100 you wrote:
> The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC are
> similar to the R-Car Ethernet AVB IP.
> 
> The Gigabit Ethernet IP consists of Ethernet controller (E-MAC), Internal
> TCP/IP Offload Engine (TOE)  and Dedicated Direct memory access controller
> (DMAC).
> 
> [...]

Here is the summary with links:
  - [01/10] ravb: Rename "ravb_set_features_rx_csum" function to "ravb_set_features_rcar"
    https://git.kernel.org/netdev/net-next/c/d9bc9ec45e01
  - [02/10] ravb: Rename "no_ptp_cfg_active" and "ptp_cfg_active" variables
    https://git.kernel.org/netdev/net-next/c/2b061b545cd0
  - [03/10] ravb: Add nc_queue to struct ravb_hw_info
    https://git.kernel.org/netdev/net-next/c/a92f4f0662bf
  - [04/10] ravb: Add support for RZ/G2L SoC
    https://git.kernel.org/netdev/net-next/c/feab85c7ccea
  - [05/10] ravb: Initialize GbEthernet DMAC
    https://git.kernel.org/netdev/net-next/c/660e3d95e21a
  - [06/10] ravb: Exclude gPTP feature support for RZ/G2L
    https://git.kernel.org/netdev/net-next/c/7e09a052dc4e
  - [07/10] ravb: Add tsrq to struct ravb_hw_info
    https://git.kernel.org/netdev/net-next/c/0b395f289451
  - [08/10] ravb: Add magic_pkt to struct ravb_hw_info
    https://git.kernel.org/netdev/net-next/c/ebd5df063ce4
  - [09/10] ravb: Add half_duplex to struct ravb_hw_info
    https://git.kernel.org/netdev/net-next/c/68aa0763c045
  - [10/10] ravb: Initialize GbEthernet E-MAC
    https://git.kernel.org/netdev/net-next/c/16a235199235

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH 02/10] ravb: Rename "no_ptp_cfg_active" and "ptp_cfg_active" variables

[Sergey Shtylyov]

Hello!

   Damn, DaveM continues ignoring my review efforts... :-( will finish reviewing the series anyway.

On 10/2/21 10:53 AM, Biju Das wrote:

>> Subject: Re: [PATCH 02/10] ravb: Rename "no_ptp_cfg_active" and
>> "ptp_cfg_active" variables
>>
>> On 10/1/21 6:06 PM, Biju Das wrote:
>>
>>> Rename the variable "no_ptp_cfg_active" with "gptp" and
>>
>>    This shouldn't be a rename but the extension of the meaning instead...
> 
> This is the original ptp support for both R-Car Gen3 and R-Car Gen2 without config in active mode. Later we added feature support active in config mode for R-Car Gen3 by patch[1].
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/renesas/ravb_main.c?h=v5.15-rc3&id=f5d7837f96e53a8c9b6c49e1bc95cf0ae88b99e8

   And? Do you think I don't remember the driver development history? :-)

>>> "ptp_cfg_active" with "ccc_gac" to match the HW features.
>>>
>>> There is no functional change.
>>>
>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>> Suggested-by: Sergey Shtylyov <s.shtylyov@omp.ru>
>>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>> ---
>>> RFc->v1:
>>>  * Renamed the variable "no_ptp_cfg_active" with "gptp" and
>>>    "ptp_cfg_active" with "ccc_gac
>>> ---
>>>  drivers/net/ethernet/renesas/ravb.h      |  4 ++--
>>>  drivers/net/ethernet/renesas/ravb_main.c | 26
>>> ++++++++++++------------
>>>  2 files changed, 15 insertions(+), 15 deletions(-)
>>
>> [...]
>>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c
>>> b/drivers/net/ethernet/renesas/ravb_main.c
>>> index 8f2358caef34..dc7654abfe55 100644
>>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>>> @@ -1274,7 +1274,7 @@ static int ravb_set_ringparam(struct net_device
>> *ndev,
>>>  	if (netif_running(ndev)) {
>>>  		netif_device_detach(ndev);
>>>  		/* Stop PTP Clock driver */
>>> -		if (info->no_ptp_cfg_active)
>>> +		if (info->gptp)
>>
>>    Where have you lost !info->ccc_gac?
> 
>   As per patch[1], the check is for R-Car Gen2. Why do you need additional check
> as per the current driver?

   Because the driver now supports not only gen2, but also gen3, and RZ/G2L, finally.

> I see below you are proposing to enable both "gptp" and "ccc_gac" for R-Car Gen3,

   Yes, this is how the hardware evolved. gPTP hardware can (optionally) be active outside
the config mode, otherwise there's no difference b/w gen2 and gen3.

> According to me it is a feature improvement for R-Car Gen3 in which, you can have
> 
> 1) gPTP support active in config mode
> 2) gPTP support not active in config mode

   Right.

> But the existing driver code just support "gPTP support active in config mode" for R-Car Gen3.

   And?

> Do you want me to do feature improvement as well, as part of Gbethernet support?

   I thought we agreed on this patch in the previous iteration, To be more clear, by asking
to remove the "double negation", I meant using:

	if (info->gptp && !info->ccc_gac)

versus your:

	if (!info->no_gptp && !info->ccc_gac)

> Please let me know your thoughts.
> 
> The same comments applies to all the comments you have mentioned below.
> 
> Regards,
> Biju

[...]

MBR, Sergey

Re: [PATCH 03/10] ravb: Add nc_queue to struct ravb_hw_info

[Sergey Shtylyov]

On 10/1/21 6:06 PM, Biju Das wrote:

> R-Car supports network control queue whereas RZ/G2L does not support
> it. Add nc_queue to struct ravb_hw_info, so that NC queue is handled
> only by R-Car.
> 
> This patch also renames ravb_rcar_dmac_init to ravb_dmac_init_rcar
> to be consistent with the naming convention used in sh_eth driver.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>

   One little nit below:

> ---
> RFC->v1:
>  * Handled NC queue only for R-Car.
> ---
>  drivers/net/ethernet/renesas/ravb.h      |   3 +-
>  drivers/net/ethernet/renesas/ravb_main.c | 140 +++++++++++++++--------
>  2 files changed, 94 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
> index a33fbcb4aac3..c91e93e5590f 100644
> --- a/drivers/net/ethernet/renesas/ravb.h
> +++ b/drivers/net/ethernet/renesas/ravb.h
> @@ -986,7 +986,7 @@ struct ravb_hw_info {
>  	bool (*receive)(struct net_device *ndev, int *quota, int q);
>  	void (*set_rate)(struct net_device *ndev);
>  	int (*set_feature)(struct net_device *ndev, netdev_features_t features);
> -	void (*dmac_init)(struct net_device *ndev);
> +	int (*dmac_init)(struct net_device *ndev);
>  	void (*emac_init)(struct net_device *ndev);
>  	const char (*gstrings_stats)[ETH_GSTRING_LEN];
>  	size_t gstrings_size;
> @@ -1002,6 +1002,7 @@ struct ravb_hw_info {
>  	unsigned multi_irqs:1;		/* AVB-DMAC and E-MAC has multiple irqs */
>  	unsigned gptp:1;		/* AVB-DMAC has gPTP support */
>  	unsigned ccc_gac:1;		/* AVB-DMAC has gPTP support active in config mode */
> +	unsigned nc_queue:1;		/* AVB-DMAC has NC queue */

   Rather "queues" as there are RX and TX NC queues, no?

[...]
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index dc7654abfe55..8bf13586e90a 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
[...]
> @@ -1698,28 +1717,38 @@ static struct net_device_stats *ravb_get_stats(struct net_device *ndev)
>  
>  	nstats = &ndev->stats;
>  	stats0 = &priv->stats[RAVB_BE];
> -	stats1 = &priv->stats[RAVB_NC];
>  
>  	if (info->tx_counters) {
>  		nstats->tx_dropped += ravb_read(ndev, TROCR);
>  		ravb_write(ndev, 0, TROCR);	/* (write clear) */
>  	}
>  
> -	nstats->rx_packets = stats0->rx_packets + stats1->rx_packets;
> -	nstats->tx_packets = stats0->tx_packets + stats1->tx_packets;
> -	nstats->rx_bytes = stats0->rx_bytes + stats1->rx_bytes;
> -	nstats->tx_bytes = stats0->tx_bytes + stats1->tx_bytes;
> -	nstats->multicast = stats0->multicast + stats1->multicast;
> -	nstats->rx_errors = stats0->rx_errors + stats1->rx_errors;
> -	nstats->rx_crc_errors = stats0->rx_crc_errors + stats1->rx_crc_errors;
> -	nstats->rx_frame_errors =
> -		stats0->rx_frame_errors + stats1->rx_frame_errors;
> -	nstats->rx_length_errors =
> -		stats0->rx_length_errors + stats1->rx_length_errors;
> -	nstats->rx_missed_errors =
> -		stats0->rx_missed_errors + stats1->rx_missed_errors;
> -	nstats->rx_over_errors =
> -		stats0->rx_over_errors + stats1->rx_over_errors;
> +	nstats->rx_packets = stats0->rx_packets;
> +	nstats->tx_packets = stats0->tx_packets;
> +	nstats->rx_bytes = stats0->rx_bytes;
> +	nstats->tx_bytes = stats0->tx_bytes;
> +	nstats->multicast = stats0->multicast;
> +	nstats->rx_errors = stats0->rx_errors;
> +	nstats->rx_crc_errors = stats0->rx_crc_errors;
> +	nstats->rx_frame_errors = stats0->rx_frame_errors;
> +	nstats->rx_length_errors = stats0->rx_length_errors;
> +	nstats->rx_missed_errors = stats0->rx_missed_errors;
> +	nstats->rx_over_errors = stats0->rx_over_errors;
> +	if (info->nc_queue) {
> +		stats1 = &priv->stats[RAVB_NC];
> +
> +		nstats->rx_packets += stats1->rx_packets;
> +		nstats->tx_packets += stats1->tx_packets;
> +		nstats->rx_bytes += stats1->rx_bytes;
> +		nstats->tx_bytes += stats1->tx_bytes;
> +		nstats->multicast += stats1->multicast;
> +		nstats->rx_errors += stats1->rx_errors;
> +		nstats->rx_crc_errors += stats1->rx_crc_errors;
> +		nstats->rx_frame_errors += stats1->rx_frame_errors;
> +		nstats->rx_length_errors += stats1->rx_length_errors;
> +		nstats->rx_missed_errors += stats1->rx_missed_errors;
> +		nstats->rx_over_errors += stats1->rx_over_errors;
> +	}

   Good! :-)

[...]

MBR, Sergey

Re: [PATCH 04/10] ravb: Add support for RZ/G2L SoC

[Sergey Shtylyov]

On 10/1/21 6:06 PM, Biju Das wrote:

> RZ/G2L SoC has Gigabit Ethernet IP consisting of Ethernet controller
> (E-MAC), Internal TCP/IP Offload Engine (TOE) and Dedicated Direct
> memory access controller (DMAC).
> 
> This patch adds compatible string for RZ/G2L and fills up the
> ravb_hw_info struct. Function stubs are added which will be used by
> gbeth_hw_info and will be filled incrementally.

   I've always been against this patch -- we get a support for the GbEther whihc doesn't work
after this patch. I believe we should have the GbEther support in the last patch. of the overall
series.

> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> RFC->v1:
>  * Added compatible string for RZ/G2L.
>  * Added feature bits max_rx_len, aligned_tx and tx_counters for RZ/G2L.
> ---
>  drivers/net/ethernet/renesas/ravb.h      |  2 +
>  drivers/net/ethernet/renesas/ravb_main.c | 62 ++++++++++++++++++++++++
>  2 files changed, 64 insertions(+)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
> index c91e93e5590f..f6398fdcead2 100644
> --- a/drivers/net/ethernet/renesas/ravb.h
> +++ b/drivers/net/ethernet/renesas/ravb.h
[...]
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 8bf13586e90a..dc817b4d95a1 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
[...]
> @@ -2073,12 +2120,27 @@ static const struct ravb_hw_info ravb_gen2_hw_info = {
>  	.nc_queue = 1,
>  };
>  
> +static const struct ravb_hw_info gbeth_hw_info = {
> +	.rx_ring_free = ravb_rx_ring_free_gbeth,
> +	.rx_ring_format = ravb_rx_ring_format_gbeth,
> +	.alloc_rx_desc = ravb_alloc_rx_desc_gbeth,
> +	.receive = ravb_rx_gbeth,
> +	.set_rate = ravb_set_rate_gbeth,
> +	.set_feature = ravb_set_features_gbeth,
> +	.dmac_init = ravb_dmac_init_gbeth,
> +	.emac_init = ravb_emac_init_gbeth,
> +	.max_rx_len = GBETH_RX_BUFF_MAX + RAVB_ALIGN - 1,

   ALIGN(GBETH_RX_BUFF_MAX, RAVB_ALIGN)?

> +	.aligned_tx = 1,
> +	.tx_counters = 1,
> +};
> +

[...]

MBR. Sergey

RE: [PATCH 04/10] ravb: Add support for RZ/G2L SoC

[Biju Das]

> Subject: Re: [PATCH 04/10] ravb: Add support for RZ/G2L SoC
> 
> On 10/1/21 6:06 PM, Biju Das wrote:
> 
> > RZ/G2L SoC has Gigabit Ethernet IP consisting of Ethernet controller
> > (E-MAC), Internal TCP/IP Offload Engine (TOE) and Dedicated Direct
> > memory access controller (DMAC).
> >
> > This patch adds compatible string for RZ/G2L and fills up the
> > ravb_hw_info struct. Function stubs are added which will be used by
> > gbeth_hw_info and will be filled incrementally.
> 
>    I've always been against this patch -- we get a support for the GbEther
> whihc doesn't work after this patch. I believe we should have the GbEther
> support in the last patch. of the overall series.


This is the common practice. We use bricks to build a wall. The function stubs are just
Bricks. 

After filling stubs, we will add SoC dt and board DT, after that one will get GBsupport on
RZ/G2L platform.

Regards,
Biju

> 
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > RFC->v1:
> >  * Added compatible string for RZ/G2L.
> >  * Added feature bits max_rx_len, aligned_tx and tx_counters for RZ/G2L.
> > ---
> >  drivers/net/ethernet/renesas/ravb.h      |  2 +
> >  drivers/net/ethernet/renesas/ravb_main.c | 62
> > ++++++++++++++++++++++++
> >  2 files changed, 64 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/renesas/ravb.h
> > b/drivers/net/ethernet/renesas/ravb.h
> > index c91e93e5590f..f6398fdcead2 100644
> > --- a/drivers/net/ethernet/renesas/ravb.h
> > +++ b/drivers/net/ethernet/renesas/ravb.h
> [...]
> > diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> > b/drivers/net/ethernet/renesas/ravb_main.c
> > index 8bf13586e90a..dc817b4d95a1 100644
> > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> [...]
> > @@ -2073,12 +2120,27 @@ static const struct ravb_hw_info
> ravb_gen2_hw_info = {
> >  	.nc_queue = 1,
> >  };
> >
> > +static const struct ravb_hw_info gbeth_hw_info = {
> > +	.rx_ring_free = ravb_rx_ring_free_gbeth,
> > +	.rx_ring_format = ravb_rx_ring_format_gbeth,
> > +	.alloc_rx_desc = ravb_alloc_rx_desc_gbeth,
> > +	.receive = ravb_rx_gbeth,
> > +	.set_rate = ravb_set_rate_gbeth,
> > +	.set_feature = ravb_set_features_gbeth,
> > +	.dmac_init = ravb_dmac_init_gbeth,
> > +	.emac_init = ravb_emac_init_gbeth,
> > +	.max_rx_len = GBETH_RX_BUFF_MAX + RAVB_ALIGN - 1,
> 
>    ALIGN(GBETH_RX_BUFF_MAX, RAVB_ALIGN)?
> 
> > +	.aligned_tx = 1,
> > +	.tx_counters = 1,
> > +};
> > +
> 
> [...]
> 
> MBR. Sergey

RE: [PATCH 03/10] ravb: Add nc_queue to struct ravb_hw_info

[Biju Das]

Hi Sergei,

> Subject: Re: [PATCH 03/10] ravb: Add nc_queue to struct ravb_hw_info
> 
> On 10/1/21 6:06 PM, Biju Das wrote:
> 
> > R-Car supports network control queue whereas RZ/G2L does not support
> > it. Add nc_queue to struct ravb_hw_info, so that NC queue is handled
> > only by R-Car.
> >
> > This patch also renames ravb_rcar_dmac_init to ravb_dmac_init_rcar to
> > be consistent with the naming convention used in sh_eth driver.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> 
>    One little nit below:
> 
> > ---
> > RFC->v1:
> >  * Handled NC queue only for R-Car.
> > ---
> >  drivers/net/ethernet/renesas/ravb.h      |   3 +-
> >  drivers/net/ethernet/renesas/ravb_main.c | 140
> > +++++++++++++++--------
> >  2 files changed, 94 insertions(+), 49 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/renesas/ravb.h
> > b/drivers/net/ethernet/renesas/ravb.h
> > index a33fbcb4aac3..c91e93e5590f 100644
> > --- a/drivers/net/ethernet/renesas/ravb.h
> > +++ b/drivers/net/ethernet/renesas/ravb.h
> > @@ -986,7 +986,7 @@ struct ravb_hw_info {
> >  	bool (*receive)(struct net_device *ndev, int *quota, int q);
> >  	void (*set_rate)(struct net_device *ndev);
> >  	int (*set_feature)(struct net_device *ndev, netdev_features_t
> features);
> > -	void (*dmac_init)(struct net_device *ndev);
> > +	int (*dmac_init)(struct net_device *ndev);
> >  	void (*emac_init)(struct net_device *ndev);
> >  	const char (*gstrings_stats)[ETH_GSTRING_LEN];
> >  	size_t gstrings_size;
> > @@ -1002,6 +1002,7 @@ struct ravb_hw_info {
> >  	unsigned multi_irqs:1;		/* AVB-DMAC and E-MAC has multiple
> irqs */
> >  	unsigned gptp:1;		/* AVB-DMAC has gPTP support */
> >  	unsigned ccc_gac:1;		/* AVB-DMAC has gPTP support active in
> config mode */
> > +	unsigned nc_queue:1;		/* AVB-DMAC has NC queue */
> 
>    Rather "queues" as there are RX and TX NC queues, no?

It has NC queue on both RX and TX.

If needed, I can send a follow up patch as RFC with the following changes.

unsigned nc_queue:1;		/* AVB-DMAC has NC queue on both RX and TX  */

or 

unsigned nc_queues:1;		/* AVB-DMAC has RX and TX NC queues */

please let me know.

Regards,
Biju

> 
> [...]
> > diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> > b/drivers/net/ethernet/renesas/ravb_main.c
> > index dc7654abfe55..8bf13586e90a 100644
> > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> [...]
> > @@ -1698,28 +1717,38 @@ static struct net_device_stats
> > *ravb_get_stats(struct net_device *ndev)
> >
> >  	nstats = &ndev->stats;
> >  	stats0 = &priv->stats[RAVB_BE];
> > -	stats1 = &priv->stats[RAVB_NC];
> >
> >  	if (info->tx_counters) {
> >  		nstats->tx_dropped += ravb_read(ndev, TROCR);
> >  		ravb_write(ndev, 0, TROCR);	/* (write clear) */
> >  	}
> >
> > -	nstats->rx_packets = stats0->rx_packets + stats1->rx_packets;
> > -	nstats->tx_packets = stats0->tx_packets + stats1->tx_packets;
> > -	nstats->rx_bytes = stats0->rx_bytes + stats1->rx_bytes;
> > -	nstats->tx_bytes = stats0->tx_bytes + stats1->tx_bytes;
> > -	nstats->multicast = stats0->multicast + stats1->multicast;
> > -	nstats->rx_errors = stats0->rx_errors + stats1->rx_errors;
> > -	nstats->rx_crc_errors = stats0->rx_crc_errors + stats1-
> >rx_crc_errors;
> > -	nstats->rx_frame_errors =
> > -		stats0->rx_frame_errors + stats1->rx_frame_errors;
> > -	nstats->rx_length_errors =
> > -		stats0->rx_length_errors + stats1->rx_length_errors;
> > -	nstats->rx_missed_errors =
> > -		stats0->rx_missed_errors + stats1->rx_missed_errors;
> > -	nstats->rx_over_errors =
> > -		stats0->rx_over_errors + stats1->rx_over_errors;
> > +	nstats->rx_packets = stats0->rx_packets;
> > +	nstats->tx_packets = stats0->tx_packets;
> > +	nstats->rx_bytes = stats0->rx_bytes;
> > +	nstats->tx_bytes = stats0->tx_bytes;
> > +	nstats->multicast = stats0->multicast;
> > +	nstats->rx_errors = stats0->rx_errors;
> > +	nstats->rx_crc_errors = stats0->rx_crc_errors;
> > +	nstats->rx_frame_errors = stats0->rx_frame_errors;
> > +	nstats->rx_length_errors = stats0->rx_length_errors;
> > +	nstats->rx_missed_errors = stats0->rx_missed_errors;
> > +	nstats->rx_over_errors = stats0->rx_over_errors;
> > +	if (info->nc_queue) {
> > +		stats1 = &priv->stats[RAVB_NC];
> > +
> > +		nstats->rx_packets += stats1->rx_packets;
> > +		nstats->tx_packets += stats1->tx_packets;
> > +		nstats->rx_bytes += stats1->rx_bytes;
> > +		nstats->tx_bytes += stats1->tx_bytes;
> > +		nstats->multicast += stats1->multicast;
> > +		nstats->rx_errors += stats1->rx_errors;
> > +		nstats->rx_crc_errors += stats1->rx_crc_errors;
> > +		nstats->rx_frame_errors += stats1->rx_frame_errors;
> > +		nstats->rx_length_errors += stats1->rx_length_errors;
> > +		nstats->rx_missed_errors += stats1->rx_missed_errors;
> > +		nstats->rx_over_errors += stats1->rx_over_errors;
> > +	}
> 
>    Good! :-)
> 
> [...]
> 
> MBR, Sergey

RE: [PATCH 02/10] ravb: Rename "no_ptp_cfg_active" and "ptp_cfg_active" variables

[Biju Das]

Hi Sergei,

> Subject: Re: [PATCH 02/10] ravb: Rename "no_ptp_cfg_active" and
> "ptp_cfg_active" variables
> 
> Hello!
> 
>    Damn, DaveM continues ignoring my review efforts... :-( will finish
> reviewing the series anyway.
> 
> On 10/2/21 10:53 AM, Biju Das wrote:
> 
> >> Subject: Re: [PATCH 02/10] ravb: Rename "no_ptp_cfg_active" and
> >> "ptp_cfg_active" variables
> >>
> >> On 10/1/21 6:06 PM, Biju Das wrote:
> >>
> >>> Rename the variable "no_ptp_cfg_active" with "gptp" and
> >>
> >>    This shouldn't be a rename but the extension of the meaning
> instead...
> >
> > This is the original ptp support for both R-Car Gen3 and R-Car Gen2
> without config in active mode. Later we added feature support active in
> config mode for R-Car Gen3 by patch[1].
> > [1]
> > https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.
> > kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2
> > Fcommit%2Fdrivers%2Fnet%2Fethernet%2Frenesas%2Fravb_main.c%3Fh%3Dv5.15
> > -rc3%26id%3Df5d7837f96e53a8c9b6c49e1bc95cf0ae88b99e8&data=04%7C01%
> > 7Cbiju.das.jz%40bp.renesas.com%7Cb4a62982865a4f7cf38408d985d11fef%7C53
> > d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C637687955521294093%7CUnknown%
> > 7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJX
> > VCI6Mn0%3D%7C1000&sdata=6rpdh6hEAUl1yMng2ruFrKflBiDGmq6RylI90Ije3t
> > 4%3D&reserved=0
> 
>    And? Do you think I don't remember the driver development history? :-)
> 
> >>> "ptp_cfg_active" with "ccc_gac" to match the HW features.
> >>>
> >>> There is no functional change.
> >>>
> >>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >>> Suggested-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> >>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>> ---
> >>> RFc->v1:
> >>>  * Renamed the variable "no_ptp_cfg_active" with "gptp" and
> >>>    "ptp_cfg_active" with "ccc_gac
> >>> ---
> >>>  drivers/net/ethernet/renesas/ravb.h      |  4 ++--
> >>>  drivers/net/ethernet/renesas/ravb_main.c | 26
> >>> ++++++++++++------------
> >>>  2 files changed, 15 insertions(+), 15 deletions(-)
> >>
> >> [...]
> >>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> >>> b/drivers/net/ethernet/renesas/ravb_main.c
> >>> index 8f2358caef34..dc7654abfe55 100644
> >>> --- a/drivers/net/ethernet/renesas/ravb_main.c
> >>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> >>> @@ -1274,7 +1274,7 @@ static int ravb_set_ringparam(struct
> >>> net_device
> >> *ndev,
> >>>  	if (netif_running(ndev)) {
> >>>  		netif_device_detach(ndev);
> >>>  		/* Stop PTP Clock driver */
> >>> -		if (info->no_ptp_cfg_active)
> >>> +		if (info->gptp)
> >>
> >>    Where have you lost !info->ccc_gac?
> >
> >   As per patch[1], the check is for R-Car Gen2. Why do you need
> > additional check as per the current driver?
> 
>    Because the driver now supports not only gen2, but also gen3, and
> RZ/G2L, finally.
> 
> > I see below you are proposing to enable both "gptp" and "ccc_gac" for
> > R-Car Gen3,
> 
>    Yes, this is how the hardware evolved. gPTP hardware can (optionally)
> be active outside the config mode, otherwise there's no difference b/w
> gen2 and gen3.
> 
> > According to me it is a feature improvement for R-Car Gen3 in which,
> > you can have
> >
> > 1) gPTP support active in config mode
> > 2) gPTP support not active in config mode
> 
>    Right.
> 
> > But the existing driver code just support "gPTP support active in config
> mode" for R-Car Gen3.
> 
>    And?
> 
> > Do you want me to do feature improvement as well, as part of Gbethernet
> support?
> 
>    I thought we agreed on this patch in the previous iteration, To be more
> clear, by asking to remove the "double negation", I meant using:

I never thought of adding feature improvements as part of Gbethernet support. Any feature improvements after adding GbEthernet support.

If you expressed your ideas like adding gptp, ccc_gac for R-Car Gen3 earlier, then
I should have responded it is feature improvement. So please share your ideas in advance.

Regards,
Biju




> 
> 	if (info->gptp && !info->ccc_gac)
> 
> versus your:
> 
> 	if (!info->no_gptp && !info->ccc_gac)
> 
> > Please let me know your thoughts.
> >
> > The same comments applies to all the comments you have mentioned below.
> >
> > Regards,
> > Biju
> 
> [...]
> 
> MBR, Sergey

Re: [PATCH 04/10] ravb: Add support for RZ/G2L SoC

[Geert Uytterhoeven]

Hi Biju,

On Sun, Oct 3, 2021 at 8:51 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Subject: Re: [PATCH 04/10] ravb: Add support for RZ/G2L SoC
> > On 10/1/21 6:06 PM, Biju Das wrote:
> > > RZ/G2L SoC has Gigabit Ethernet IP consisting of Ethernet controller
> > > (E-MAC), Internal TCP/IP Offload Engine (TOE) and Dedicated Direct
> > > memory access controller (DMAC).
> > >
> > > This patch adds compatible string for RZ/G2L and fills up the
> > > ravb_hw_info struct. Function stubs are added which will be used by
> > > gbeth_hw_info and will be filled incrementally.
> >
> >    I've always been against this patch -- we get a support for the GbEther
> > whihc doesn't work after this patch. I believe we should have the GbEther
> > support in the last patch. of the overall series.
>
> This is the common practice. We use bricks to build a wall. The function stubs are just
> Bricks.
>
> After filling stubs, we will add SoC dt and board DT, after that one will get GBsupport on
> RZ/G2L platform.

Not "after", but "in parallel".  The stubs will be filled in through
the netdev tree (1), while SoC DT and board DT will go through the
renesas-devel and soc trees (2).

So our main worry is: what happens if you have (2) but not (1)?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH 04/10] ravb: Add support for RZ/G2L SoC

[Biju Das]

Hi Geert,

Thanks for the feedback

> Subject: Re: [PATCH 04/10] ravb: Add support for RZ/G2L SoC
> 
> Hi Biju,
> 
> On Sun, Oct 3, 2021 at 8:51 AM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > > Subject: Re: [PATCH 04/10] ravb: Add support for RZ/G2L SoC On
> > > 10/1/21 6:06 PM, Biju Das wrote:
> > > > RZ/G2L SoC has Gigabit Ethernet IP consisting of Ethernet
> > > > controller (E-MAC), Internal TCP/IP Offload Engine (TOE) and
> > > > Dedicated Direct memory access controller (DMAC).
> > > >
> > > > This patch adds compatible string for RZ/G2L and fills up the
> > > > ravb_hw_info struct. Function stubs are added which will be used
> > > > by gbeth_hw_info and will be filled incrementally.
> > >
> > >    I've always been against this patch -- we get a support for the
> > > GbEther whihc doesn't work after this patch. I believe we should
> > > have the GbEther support in the last patch. of the overall series.
> >
> > This is the common practice. We use bricks to build a wall. The
> > function stubs are just Bricks.
> >
> > After filling stubs, we will add SoC dt and board DT, after that one
> > will get GBsupport on RZ/G2L platform.
> 
> Not "after", but "in parallel".  The stubs will be filled in through the
> netdev tree (1), while SoC DT and board DT will go through the renesas-
> devel and soc trees (2).
> 
> So our main worry is: what happens if you have (2) but not (1)?

Please find the test cases

Case a) (1) and then (2) RootFS mounted on NFS
----------------------------------------------------

root@smarc-rzg2l:~# cat /proc/cmdline
ignore_loglevel nfsrootdebug root=/dev/nfs rw nfsroot=192.168.10.1:/tftpboot/RZ-G2L,nfsvers=3 ip=192.168.10.2
root@smarc-rzg2l:~#

Case b) Have (2) but not (1)? RootFS mounted on USB
----------------------------------------------------

root@smarc-rzg2l:~# cat /proc/cmdline
rw rootwait earlycon root=/dev/sda1
root@smarc-rzg2l:~#

Case c) Have (2) but not (1)? RootFS mounted on NFS
---------------------------------------------------

It stops booting as we haven't filled RX stubs.

[    4.457432]  sda: sda1
[    4.465909] sd 0:0:0:0: [sda] Attached SCSI removable disk


If you look at Case B, that is the current case, which boots without any issues. There is no regression
at all with the current changes submitted.

The only issue is mounting with NFS which won't work as we haven't filled stubs to get full functionality.


Regards,
Biju

Re: [PATCH 05/10] ravb: Initialize GbEthernet DMAC

[Sergey Shtylyov]

Hello!

On 10/1/21 6:06 PM, Biju Das wrote:

> Initialize GbEthernet DMAC found on RZ/G2L SoC.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> RFC->v1:
>  * Removed RIC3 initialization from DMAC init, as it is 
>    same as reset value.

   I'm not sure we do a reset everytime...

>  * moved stubs function to earlier patches.
>  * renamed "rgeth" with "gbeth"
> ---
>  drivers/net/ethernet/renesas/ravb.h      |  3 ++-
>  drivers/net/ethernet/renesas/ravb_main.c | 30 +++++++++++++++++++++++-
>  2 files changed, 31 insertions(+), 2 deletions(-)
> 
[...]
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index dc817b4d95a1..5790a9332e7b 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -489,7 +489,35 @@ static void ravb_emac_init(struct net_device *ndev)
>  
>  static int ravb_dmac_init_gbeth(struct net_device *ndev)
>  {
> -	/* Place holder */
> +	int error;
> +
> +	error = ravb_ring_init(ndev, RAVB_BE);
> +	if (error)
> +		return error;
> +
> +	/* Descriptor format */
> +	ravb_ring_format(ndev, RAVB_BE);
> +
> +	/* Set AVB RX */

   AVB? We don't have it, do we?

> +	ravb_write(ndev, 0x60000000, RCR);

   Not even RCR.EFFS? And what do bits 29..30 mean?

[...]
> +	/* Set FIFO size */
> +	ravb_write(ndev, 0x00222200, TGC);

   Do TBD<n> (other than TBD0) fields even exist?

[...]

MBR, Sergey

RE: [PATCH 05/10] ravb: Initialize GbEthernet DMAC

[Biju Das]

Hi Sergey,
Thanks for the comments

> Subject: Re: [PATCH 05/10] ravb: Initialize GbEthernet DMAC
> 
> Hello!
> 
> On 10/1/21 6:06 PM, Biju Das wrote:
> 
> > Initialize GbEthernet DMAC found on RZ/G2L SoC.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > RFC->v1:
> >  * Removed RIC3 initialization from DMAC init, as it is
> >    same as reset value.
> 
>    I'm not sure we do a reset everytime...
> 
> >  * moved stubs function to earlier patches.
> >  * renamed "rgeth" with "gbeth"
> > ---
> >  drivers/net/ethernet/renesas/ravb.h      |  3 ++-
> >  drivers/net/ethernet/renesas/ravb_main.c | 30
> > +++++++++++++++++++++++-
> >  2 files changed, 31 insertions(+), 2 deletions(-)
> >
> [...]
> > diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> > b/drivers/net/ethernet/renesas/ravb_main.c
> > index dc817b4d95a1..5790a9332e7b 100644
> > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> > @@ -489,7 +489,35 @@ static void ravb_emac_init(struct net_device
> > *ndev)
> >
> >  static int ravb_dmac_init_gbeth(struct net_device *ndev)  {
> > -	/* Place holder */
> > +	int error;
> > +
> > +	error = ravb_ring_init(ndev, RAVB_BE);
> > +	if (error)
> > +		return error;
> > +
> > +	/* Descriptor format */
> > +	ravb_ring_format(ndev, RAVB_BE);
> > +
> > +	/* Set AVB RX */
> 
>    AVB? We don't have it, do we?

Good catch. I Will update the comment in next RFC patch.

> 
> > +	ravb_write(ndev, 0x60000000, RCR);
> 
>    Not even RCR.EFFS? And what do bits 29..30 mean?

RZ/G2L Bit 31 is reserved.
Bit 16:30 Reception fifo critical level.
Bit 15:1 reserved
Bit 0 : EFFS

I am not sure, where do you get 29..30? can you please clarify.


> 
> [...]
> > +	/* Set FIFO size */
> > +	ravb_write(ndev, 0x00222200, TGC);
> 
>    Do TBD<n> (other than TBD0) fields even exist?

Only TBD (Bit 8..9) is available to write, rest all are reserved with remaining values
as in "0x00222200"

Regds,
Biju


> 
> [...]
> 
> MBR, Sergey

RE: [PATCH 04/10] ravb: Add support for RZ/G2L SoC

[Biju Das]

Hi Sergei,

> Subject: RE: [PATCH 04/10] ravb: Add support for RZ/G2L SoC
> 
> > Subject: Re: [PATCH 04/10] ravb: Add support for RZ/G2L SoC
> >
> > On 10/1/21 6:06 PM, Biju Das wrote:
> >
> > > RZ/G2L SoC has Gigabit Ethernet IP consisting of Ethernet controller
> > > (E-MAC), Internal TCP/IP Offload Engine (TOE) and Dedicated Direct
> > > memory access controller (DMAC).
> > >
> > > This patch adds compatible string for RZ/G2L and fills up the
> > > ravb_hw_info struct. Function stubs are added which will be used by
> > > gbeth_hw_info and will be filled incrementally.
> >
> >    I've always been against this patch -- we get a support for the
> > GbEther whihc doesn't work after this patch. I believe we should have
> > the GbEther support in the last patch. of the overall series.
> 
> 
> This is the common practice. We use bricks to build a wall. The function
> stubs are just Bricks.
> 
> After filling stubs, we will add SoC dt and board DT, after that one will
> get GBsupport on RZ/G2L platform.
> 
> Regards,
> Biju
> 
> >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > ---
> > > RFC->v1:
> > >  * Added compatible string for RZ/G2L.
> > >  * Added feature bits max_rx_len, aligned_tx and tx_counters for
> RZ/G2L.
> > > ---
> > >  drivers/net/ethernet/renesas/ravb.h      |  2 +
> > >  drivers/net/ethernet/renesas/ravb_main.c | 62
> > > ++++++++++++++++++++++++
> > >  2 files changed, 64 insertions(+)
> > >
> > > diff --git a/drivers/net/ethernet/renesas/ravb.h
> > > b/drivers/net/ethernet/renesas/ravb.h
> > > index c91e93e5590f..f6398fdcead2 100644
> > > --- a/drivers/net/ethernet/renesas/ravb.h
> > > +++ b/drivers/net/ethernet/renesas/ravb.h
> > [...]
> > > diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> > > b/drivers/net/ethernet/renesas/ravb_main.c
> > > index 8bf13586e90a..dc817b4d95a1 100644
> > > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> > [...]
> > > @@ -2073,12 +2120,27 @@ static const struct ravb_hw_info
> > ravb_gen2_hw_info = {
> > >  	.nc_queue = 1,
> > >  };
> > >
> > > +static const struct ravb_hw_info gbeth_hw_info = {
> > > +	.rx_ring_free = ravb_rx_ring_free_gbeth,
> > > +	.rx_ring_format = ravb_rx_ring_format_gbeth,
> > > +	.alloc_rx_desc = ravb_alloc_rx_desc_gbeth,
> > > +	.receive = ravb_rx_gbeth,
> > > +	.set_rate = ravb_set_rate_gbeth,
> > > +	.set_feature = ravb_set_features_gbeth,
> > > +	.dmac_init = ravb_dmac_init_gbeth,
> > > +	.emac_init = ravb_emac_init_gbeth,
> > > +	.max_rx_len = GBETH_RX_BUFF_MAX + RAVB_ALIGN - 1,
> >
> >    ALIGN(GBETH_RX_BUFF_MAX, RAVB_ALIGN)?

Will send this change as RFC.

Regards,
Biju

> >
> > > +	.aligned_tx = 1,
> > > +	.tx_counters = 1,
> > > +};
> > > +
> >
> > [...]
> >
> > MBR. Sergey

Re: [PATCH 05/10] ravb: Initialize GbEthernet DMAC

[Sergei Shtylyov]

On 10/4/21 4:12 PM, Biju Das wrote:

>> Subject: Re: [PATCH 05/10] ravb: Initialize GbEthernet DMAC
>>
>> Hello!
>>
>> On 10/1/21 6:06 PM, Biju Das wrote:
>>
>>> Initialize GbEthernet DMAC found on RZ/G2L SoC.
>>>
>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>> ---
>>> RFC->v1:
>>>  * Removed RIC3 initialization from DMAC init, as it is
>>>    same as reset value.
>>
>>    I'm not sure we do a reset everytime...
>>
>>>  * moved stubs function to earlier patches.
>>>  * renamed "rgeth" with "gbeth"
>>> ---
>>>  drivers/net/ethernet/renesas/ravb.h      |  3 ++-
>>>  drivers/net/ethernet/renesas/ravb_main.c | 30
>>> +++++++++++++++++++++++-
>>>  2 files changed, 31 insertions(+), 2 deletions(-)
>>>
>> [...]
>>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c
>>> b/drivers/net/ethernet/renesas/ravb_main.c
>>> index dc817b4d95a1..5790a9332e7b 100644
>>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>>> @@ -489,7 +489,35 @@ static void ravb_emac_init(struct net_device
>>> *ndev)
>>>
>>>  static int ravb_dmac_init_gbeth(struct net_device *ndev)  {
>>> -	/* Place holder */
>>> +	int error;
>>> +
>>> +	error = ravb_ring_init(ndev, RAVB_BE);
>>> +	if (error)
>>> +		return error;
>>> +
>>> +	/* Descriptor format */
>>> +	ravb_ring_format(ndev, RAVB_BE);
>>> +
>>> +	/* Set AVB RX */
>>
>>    AVB? We don't have it, do we?
> 
> Good catch. I Will update the comment in next RFC patch.

   That's trifles, not worth a patch on its own...

>>
>>> +	ravb_write(ndev, 0x60000000, RCR);
>>
>>    Not even RCR.EFFS? And what do bits 29..30 mean?
> 
> RZ/G2L Bit 31 is reserved.
> Bit 16:30 Reception fifo critical level.
> Bit 15:1 reserved
> Bit 0 : EFFS
> 
> I am not sure, where do you get 29..30? can you please clarify.

   0x60000000 has bits 29..30 set and gen3 manual has these bits reserved. 

>> [...]
>>> +	/* Set FIFO size */
>>> +	ravb_write(ndev, 0x00222200, TGC);
>>
>>    Do TBD<n> (other than TBD0) fields even exist?
> 
> Only TBD (Bit 8..9) is available to write,

   Thought so! :-)

> rest all are reserved with remaining values
> as in "0x00222200"

  Oh, so the defaluts are the sme on RZ/G2L, despite only 1 TX queue?

> Regds,
> Biju

MBR, Sergey

Re: [PATCH 07/10] ravb: Add tsrq to struct ravb_hw_info

[Sergey Shtylyov]

On 10/1/21 6:06 PM, Biju Das wrote:

> R-Car AVB-DMAC has 4 Transmit start request queues, whereas
> RZ/G2L has only 1 Transmit start request queue.

   The TCCR bits are called transmit start request (queue 0/1), not transmit start request queue 0/1.
I think you've read too much value into them for what is just TX queue 0/1.

> Add a tsrq variable to struct ravb_hw_info to handle this
> difference.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> RFC->v1:
>  * Added tsrq variable instead of multi_tsrq feature bit.
> ---
>  drivers/net/ethernet/renesas/ravb.h      | 1 +
>  drivers/net/ethernet/renesas/ravb_main.c | 9 +++++++--
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
> index 9cd3a15743b4..c586070193ef 100644
> --- a/drivers/net/ethernet/renesas/ravb.h
> +++ b/drivers/net/ethernet/renesas/ravb.h
> @@ -997,6 +997,7 @@ struct ravb_hw_info {
>  	netdev_features_t net_features;
>  	int stats_len;
>  	size_t max_rx_len;
> +	u32 tsrq;

   I'd call it 'tccr_value' instead.

>  	unsigned aligned_tx: 1;
>  
>  	/* hardware features */
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index ac141a491ca2..4784832bd93c 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -790,11 +790,13 @@ static void ravb_rcv_snd_enable(struct net_device *ndev)
>  /* function for waiting dma process finished */
>  static int ravb_stop_dma(struct net_device *ndev)
>  {
> +	struct ravb_private *priv = netdev_priv(ndev);
> +	const struct ravb_hw_info *info = priv->info;
>  	int error;
>  
>  	/* Wait for stopping the hardware TX process */
> -	error = ravb_wait(ndev, TCCR,
> -			  TCCR_TSRQ0 | TCCR_TSRQ1 | TCCR_TSRQ2 | TCCR_TSRQ3, 0);
> +	error = ravb_wait(ndev, TCCR, info->tsrq, 0);
> +
>  	if (error)
>  		return error;
>  
> @@ -2128,6 +2130,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
>  	.net_features = NETIF_F_RXCSUM,
>  	.stats_len = ARRAY_SIZE(ravb_gstrings_stats),
>  	.max_rx_len = RX_BUF_SZ + RAVB_ALIGN - 1,
> +	.tsrq = TCCR_TSRQ0 | TCCR_TSRQ1 | TCCR_TSRQ2 | TCCR_TSRQ3,
>  	.internal_delay = 1,
>  	.tx_counters = 1,
>  	.multi_irqs = 1,
> @@ -2150,6 +2153,7 @@ static const struct ravb_hw_info ravb_gen2_hw_info = {
>  	.net_features = NETIF_F_RXCSUM,
>  	.stats_len = ARRAY_SIZE(ravb_gstrings_stats),
>  	.max_rx_len = RX_BUF_SZ + RAVB_ALIGN - 1,
> +	.tsrq = TCCR_TSRQ0 | TCCR_TSRQ1 | TCCR_TSRQ2 | TCCR_TSRQ3,
>  	.aligned_tx = 1,
>  	.gptp = 1,
>  	.nc_queue = 1,
> @@ -2165,6 +2169,7 @@ static const struct ravb_hw_info gbeth_hw_info = {
>  	.dmac_init = ravb_dmac_init_gbeth,
>  	.emac_init = ravb_emac_init_gbeth,
>  	.max_rx_len = GBETH_RX_BUFF_MAX + RAVB_ALIGN - 1,
> +	.tsrq = TCCR_TSRQ0,
>  	.aligned_tx = 1,
>  	.tx_counters = 1,
>  };
> 

[...]

MBR, Sergey

Re: [PATCH 07/10] ravb: Add tsrq to struct ravb_hw_info

[Sergei Shtylyov]

On 10/4/21 9:00 PM, Sergey Shtylyov wrote:

[...]
>    The TCCR bits are called transmit start request (queue 0/1), not transmit start request queue 0/1.
> I think you've read too much value into them for what is just TX queue 0/1.
> 
>> Add a tsrq variable to struct ravb_hw_info to handle this
>> difference.
>>
>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>> ---
>> RFC->v1:
>>  * Added tsrq variable instead of multi_tsrq feature bit.
>> ---
>>  drivers/net/ethernet/renesas/ravb.h      | 1 +
>>  drivers/net/ethernet/renesas/ravb_main.c | 9 +++++++--
>>  2 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
>> index 9cd3a15743b4..c586070193ef 100644
>> --- a/drivers/net/ethernet/renesas/ravb.h
>> +++ b/drivers/net/ethernet/renesas/ravb.h
>> @@ -997,6 +997,7 @@ struct ravb_hw_info {
>>  	netdev_features_t net_features;
>>  	int stats_len;
>>  	size_t max_rx_len;
>> +	u32 tsrq;
> 
>    I'd call it 'tccr_value' instead.

    Or even better, 'tccr_mask'...

[...]

MBR, Sergey

RE: [PATCH 05/10] ravb: Initialize GbEthernet DMAC

[Biju Das]

> -----Original Message-----
> From: Sergei Shtylyov <sergei.shtylyov@gmail.com>
> Sent: 04 October 2021 16:51
> To: Biju Das <biju.das.jz@bp.renesas.com>; Sergey Shtylyov
> <s.shtylyov@omp.ru>; David S. Miller <davem@davemloft.net>; Jakub Kicinski
> <kuba@kernel.org>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>; Sergey Shtylyov
> <s.shtylyov@omprussia.ru>; Adam Ford <aford173@gmail.com>; Andrew Lunn
> <andrew@lunn.ch>; Yuusuke Ashizuka <ashiduka@fujitsu.com>; Yoshihiro
> Shimoda <yoshihiro.shimoda.uh@renesas.com>; netdev@vger.kernel.org; linux-
> renesas-soc@vger.kernel.org; Chris Paterson <Chris.Paterson2@renesas.com>;
> Biju Das <biju.das@bp.renesas.com>; Prabhakar Mahadev Lad
> <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Subject: Re: [PATCH 05/10] ravb: Initialize GbEthernet DMAC
> 
> On 10/4/21 4:12 PM, Biju Das wrote:
> 
> >> Subject: Re: [PATCH 05/10] ravb: Initialize GbEthernet DMAC
> >>
> >> Hello!
> >>
> >> On 10/1/21 6:06 PM, Biju Das wrote:
> >>
> >>> Initialize GbEthernet DMAC found on RZ/G2L SoC.
> >>>
> >>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>> ---
> >>> RFC->v1:
> >>>  * Removed RIC3 initialization from DMAC init, as it is
> >>>    same as reset value.
> >>
> >>    I'm not sure we do a reset everytime...
> >>
> >>>  * moved stubs function to earlier patches.
> >>>  * renamed "rgeth" with "gbeth"
> >>> ---
> >>>  drivers/net/ethernet/renesas/ravb.h      |  3 ++-
> >>>  drivers/net/ethernet/renesas/ravb_main.c | 30
> >>> +++++++++++++++++++++++-
> >>>  2 files changed, 31 insertions(+), 2 deletions(-)
> >>>
> >> [...]
> >>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> >>> b/drivers/net/ethernet/renesas/ravb_main.c
> >>> index dc817b4d95a1..5790a9332e7b 100644
> >>> --- a/drivers/net/ethernet/renesas/ravb_main.c
> >>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> >>> @@ -489,7 +489,35 @@ static void ravb_emac_init(struct net_device
> >>> *ndev)
> >>>
> >>>  static int ravb_dmac_init_gbeth(struct net_device *ndev)  {
> >>> -	/* Place holder */
> >>> +	int error;
> >>> +
> >>> +	error = ravb_ring_init(ndev, RAVB_BE);
> >>> +	if (error)
> >>> +		return error;
> >>> +
> >>> +	/* Descriptor format */
> >>> +	ravb_ring_format(ndev, RAVB_BE);
> >>> +
> >>> +	/* Set AVB RX */
> >>
> >>    AVB? We don't have it, do we?
> >
> > Good catch. I Will update the comment in next RFC patch.
> 
>    That's trifles, not worth a patch on its own...
> 
> >>
> >>> +	ravb_write(ndev, 0x60000000, RCR);
> >>
> >>    Not even RCR.EFFS? And what do bits 29..30 mean?
> >
> > RZ/G2L Bit 31 is reserved.
> > Bit 16:30 Reception fifo critical level.
> > Bit 15:1 reserved
> > Bit 0 : EFFS
> >
> > I am not sure, where do you get 29..30? can you please clarify.
> 
>    0x60000000 has bits 29..30 set and gen3 manual has these bits reserved.
OK.

> 
> >> [...]
> >>> +	/* Set FIFO size */
> >>> +	ravb_write(ndev, 0x00222200, TGC);
> >>
> >>    Do TBD<n> (other than TBD0) fields even exist?
> >
> > Only TBD (Bit 8..9) is available to write,
> 
>    Thought so! :-)
> 
> > rest all are reserved with remaining values as in "0x00222200"
> 
>   Oh, so the defaluts are the sme on RZ/G2L, despite only 1 TX queue?

Yep.

> 
> > Regds,
> > Biju
> 
> MBR, Sergey

RE: [PATCH 07/10] ravb: Add tsrq to struct ravb_hw_info

[Biju Das]

> -----Original Message-----
> From: Sergei Shtylyov <sergei.shtylyov@gmail.com>
> Sent: 04 October 2021 19:37
> To: Sergey Shtylyov <s.shtylyov@omp.ru>; Biju Das
> <biju.das.jz@bp.renesas.com>; David S. Miller <davem@davemloft.net>; Jakub
> Kicinski <kuba@kernel.org>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>; Sergey Shtylyov
> <s.shtylyov@omprussia.ru>; Adam Ford <aford173@gmail.com>; Andrew Lunn
> <andrew@lunn.ch>; Yuusuke Ashizuka <ashiduka@fujitsu.com>; Yoshihiro
> Shimoda <yoshihiro.shimoda.uh@renesas.com>; netdev@vger.kernel.org; linux-
> renesas-soc@vger.kernel.org; Chris Paterson <Chris.Paterson2@renesas.com>;
> Biju Das <biju.das@bp.renesas.com>; Prabhakar Mahadev Lad
> <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Subject: Re: [PATCH 07/10] ravb: Add tsrq to struct ravb_hw_info
> 
> On 10/4/21 9:00 PM, Sergey Shtylyov wrote:
> 
> [...]
> >    The TCCR bits are called transmit start request (queue 0/1), not
> transmit start request queue 0/1.
> > I think you've read too much value into them for what is just TX queue
> 0/1.
> >
> >> Add a tsrq variable to struct ravb_hw_info to handle this difference.
> >>
> >> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >> ---
> >> RFC->v1:
> >>  * Added tsrq variable instead of multi_tsrq feature bit.
> >> ---
> >>  drivers/net/ethernet/renesas/ravb.h      | 1 +
> >>  drivers/net/ethernet/renesas/ravb_main.c | 9 +++++++--
> >>  2 files changed, 8 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/renesas/ravb.h
> >> b/drivers/net/ethernet/renesas/ravb.h
> >> index 9cd3a15743b4..c586070193ef 100644
> >> --- a/drivers/net/ethernet/renesas/ravb.h
> >> +++ b/drivers/net/ethernet/renesas/ravb.h
> >> @@ -997,6 +997,7 @@ struct ravb_hw_info {
> >>  	netdev_features_t net_features;
> >>  	int stats_len;
> >>  	size_t max_rx_len;
> >> +	u32 tsrq;
> >
> >    I'd call it 'tccr_value' instead.
> 
>     Or even better, 'tccr_mask'...

We are not masking anything here right. tccr_value will be ok, as it implies real tccr register value.

Regards,
Biju

> 
> [...]
> 
> MBR, Sergey

Re: [PATCH 07/10] ravb: Add tsrq to struct ravb_hw_info

[Sergey Shtylyov]

On 10/4/21 9:47 PM, Biju Das wrote:

[...]
>>>    The TCCR bits are called transmit start request (queue 0/1), not
>> transmit start request queue 0/1.
>>> I think you've read too much value into them for what is just TX queue
>> 0/1.
>>>
>>>> Add a tsrq variable to struct ravb_hw_info to handle this difference.
>>>>
>>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>>> ---
>>>> RFC->v1:
>>>>  * Added tsrq variable instead of multi_tsrq feature bit.
>>>> ---
>>>>  drivers/net/ethernet/renesas/ravb.h      | 1 +
>>>>  drivers/net/ethernet/renesas/ravb_main.c | 9 +++++++--
>>>>  2 files changed, 8 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/renesas/ravb.h
>>>> b/drivers/net/ethernet/renesas/ravb.h
>>>> index 9cd3a15743b4..c586070193ef 100644
>>>> --- a/drivers/net/ethernet/renesas/ravb.h
>>>> +++ b/drivers/net/ethernet/renesas/ravb.h
>>>> @@ -997,6 +997,7 @@ struct ravb_hw_info {
>>>>  	netdev_features_t net_features;
>>>>  	int stats_len;
>>>>  	size_t max_rx_len;
>>>> +	u32 tsrq;
>>>
>>>    I'd call it 'tccr_value' instead.
>>
>>     Or even better, 'tccr_mask'...
> 
> We are not masking anything here right.

    We do -- we pass the TCCR mask to ravb_wait().

[...]

> Regards,
> Biju

MBR, Sergey

Re: [PATCH 10/10] ravb: Initialize GbEthernet E-MAC

[Sergey Shtylyov]

On 10/1/21 6:06 PM, Biju Das wrote:

> Initialize GbEthernet E-MAC found on RZ/G2L SoC.
> This patch also renames ravb_set_rate to ravb_set_rate_rcar and
> ravb_rcar_emac_init to ravb_emac_init_rcar to be consistent with
> the naming convention used in sh_eth driver.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> RFC->v1:
>  * Moved CSR0 intialization to later patch.
>  * started using ravb_modify for initializing link registers.
> ---
>  drivers/net/ethernet/renesas/ravb.h      | 20 +++++++--
>  drivers/net/ethernet/renesas/ravb_main.c | 55 ++++++++++++++++++++----
>  2 files changed, 62 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
> index d83d3b4f3f5f..5dc1324786e0 100644
> --- a/drivers/net/ethernet/renesas/ravb.h
> +++ b/drivers/net/ethernet/renesas/ravb.h
[...]
> @@ -824,6 +826,7 @@ enum ECSR_BIT {
>  	ECSR_MPD	= 0x00000002,
>  	ECSR_LCHNG	= 0x00000004,
>  	ECSR_PHYI	= 0x00000008,
> +	ECSR_PFRI	= 0x00000010,

   Documented on gen3 and RZ/G2L only?

[...]
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 3e694738e683..9a4888543384 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
[...]
> @@ -449,10 +461,35 @@ static int ravb_ring_init(struct net_device *ndev, int q)
>  
>  static void ravb_emac_init_gbeth(struct net_device *ndev)
>  {
> -	/* Place holder */
> +	struct ravb_private *priv = netdev_priv(ndev);
> +
> +	/* Receive frame limit set register */
> +	ravb_write(ndev, GBETH_RX_BUFF_MAX + ETH_FCS_LEN, RFLR);
> +
> +	/* PAUSE prohibition */

    Should be: 

	/* EMAC Mode: PAUSE prohibition; Duplex; TX; RX */

> +	ravb_write(ndev, ECMR_ZPF | ((priv->duplex > 0) ? ECMR_DM : 0) |
> +			 ECMR_TE | ECMR_RE | ECMR_RCPT |
> +			 ECMR_TXF | ECMR_RXF | ECMR_PRM, ECMR);
> +
> +	ravb_set_rate_gbeth(ndev);
> +
> +	/* Set MAC address */
> +	ravb_write(ndev,
> +		   (ndev->dev_addr[0] << 24) | (ndev->dev_addr[1] << 16) |
> +		   (ndev->dev_addr[2] << 8)  | (ndev->dev_addr[3]), MAHR);
> +	ravb_write(ndev, (ndev->dev_addr[4] << 8)  | (ndev->dev_addr[5]), MALR);
> +
> +	/* E-MAC status register clear */
> +	ravb_write(ndev, ECSR_ICD | ECSR_LCHNG | ECSR_PFRI, ECSR);
> +
> +	/* E-MAC interrupt enable register */
> +	ravb_write(ndev, ECSIPR_ICDIP, ECSIPR);

   Too much repetitive code, I think...

> +
> +	ravb_modify(ndev, CXR31, CXR31_SEL_LINK1, 0);
> +	ravb_modify(ndev, CXR31, CXR31_SEL_LINK0, CXR31_SEL_LINK0);

   Can't be done in a single RMW?

[...]

MBR, Sergey

RE: [PATCH 10/10] ravb: Initialize GbEthernet E-MAC

[Biju Das]

> -----Original Message-----
> From: Sergey Shtylyov <s.shtylyov@omp.ru>
> Sent: 04 October 2021 19:56
> To: Biju Das <biju.das.jz@bp.renesas.com>; David S. Miller
> <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>
> Cc: Sergei Shtylyov <sergei.shtylyov@gmail.com>; Geert Uytterhoeven
> <geert+renesas@glider.be>; Sergey Shtylyov <s.shtylyov@omprussia.ru>; Adam
> Ford <aford173@gmail.com>; Andrew Lunn <andrew@lunn.ch>; Yuusuke Ashizuka
> <ashiduka@fujitsu.com>; Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com>; netdev@vger.kernel.org; linux-renesas-
> soc@vger.kernel.org; Chris Paterson <Chris.Paterson2@renesas.com>; Biju
> Das <biju.das@bp.renesas.com>; Prabhakar Mahadev Lad <prabhakar.mahadev-
> lad.rj@bp.renesas.com>
> Subject: Re: [PATCH 10/10] ravb: Initialize GbEthernet E-MAC
> 
> On 10/1/21 6:06 PM, Biju Das wrote:
> 
> > Initialize GbEthernet E-MAC found on RZ/G2L SoC.
> > This patch also renames ravb_set_rate to ravb_set_rate_rcar and
> > ravb_rcar_emac_init to ravb_emac_init_rcar to be consistent with the
> > naming convention used in sh_eth driver.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > RFC->v1:
> >  * Moved CSR0 intialization to later patch.
> >  * started using ravb_modify for initializing link registers.
> > ---
> >  drivers/net/ethernet/renesas/ravb.h      | 20 +++++++--
> >  drivers/net/ethernet/renesas/ravb_main.c | 55
> > ++++++++++++++++++++----
> >  2 files changed, 62 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/renesas/ravb.h
> > b/drivers/net/ethernet/renesas/ravb.h
> > index d83d3b4f3f5f..5dc1324786e0 100644
> > --- a/drivers/net/ethernet/renesas/ravb.h
> > +++ b/drivers/net/ethernet/renesas/ravb.h
> [...]
> > @@ -824,6 +826,7 @@ enum ECSR_BIT {
> >  	ECSR_MPD	= 0x00000002,
> >  	ECSR_LCHNG	= 0x00000004,
> >  	ECSR_PHYI	= 0x00000008,
> > +	ECSR_PFRI	= 0x00000010,
> 
>    Documented on gen3 and RZ/G2L only?
> 
> [...]
> > diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> > b/drivers/net/ethernet/renesas/ravb_main.c
> > index 3e694738e683..9a4888543384 100644
> > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> [...]
> > @@ -449,10 +461,35 @@ static int ravb_ring_init(struct net_device
> > *ndev, int q)
> >
> >  static void ravb_emac_init_gbeth(struct net_device *ndev)  {
> > -	/* Place holder */
> > +	struct ravb_private *priv = netdev_priv(ndev);
> > +
> > +	/* Receive frame limit set register */
> > +	ravb_write(ndev, GBETH_RX_BUFF_MAX + ETH_FCS_LEN, RFLR);
> > +
> > +	/* PAUSE prohibition */
> 
>     Should be:
> 
> 	/* EMAC Mode: PAUSE prohibition; Duplex; TX; RX */
> 
> > +	ravb_write(ndev, ECMR_ZPF | ((priv->duplex > 0) ? ECMR_DM : 0) |
> > +			 ECMR_TE | ECMR_RE | ECMR_RCPT |
> > +			 ECMR_TXF | ECMR_RXF | ECMR_PRM, ECMR);
> > +
> > +	ravb_set_rate_gbeth(ndev);
> > +
> > +	/* Set MAC address */
> > +	ravb_write(ndev,
> > +		   (ndev->dev_addr[0] << 24) | (ndev->dev_addr[1] << 16) |
> > +		   (ndev->dev_addr[2] << 8)  | (ndev->dev_addr[3]), MAHR);
> > +	ravb_write(ndev, (ndev->dev_addr[4] << 8)  | (ndev->dev_addr[5]),
> > +MALR);
> > +
> > +	/* E-MAC status register clear */
> > +	ravb_write(ndev, ECSR_ICD | ECSR_LCHNG | ECSR_PFRI, ECSR);
> > +
> > +	/* E-MAC interrupt enable register */
> > +	ravb_write(ndev, ECSIPR_ICDIP, ECSIPR);
> 
>    Too much repetitive code, I think...

Can you please clarify what are the codes repetitive here?

> 
> > +
> > +	ravb_modify(ndev, CXR31, CXR31_SEL_LINK1, 0);
> > +	ravb_modify(ndev, CXR31, CXR31_SEL_LINK0, CXR31_SEL_LINK0);
> 
>    Can't be done in a single RMW?

Will do.

> 
> [...]
> 
> MBR, Sergey

Re: [PATCH 10/10] ravb: Initialize GbEthernet E-MAC

[Sergey Shtylyov]

On 10/4/21 10:23 PM, Biju Das wrote:

[...]
>>> Initialize GbEthernet E-MAC found on RZ/G2L SoC.
>>> This patch also renames ravb_set_rate to ravb_set_rate_rcar and
>>> ravb_rcar_emac_init to ravb_emac_init_rcar to be consistent with the
>>> naming convention used in sh_eth driver.
>>>
>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>> ---
>>> RFC->v1:
>>>  * Moved CSR0 intialization to later patch.
>>>  * started using ravb_modify for initializing link registers.
>>> ---
>>>  drivers/net/ethernet/renesas/ravb.h      | 20 +++++++--
>>>  drivers/net/ethernet/renesas/ravb_main.c | 55
>>> ++++++++++++++++++++----
>>>  2 files changed, 62 insertions(+), 13 deletions(-)
>>>
[...]
>>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c
>>> b/drivers/net/ethernet/renesas/ravb_main.c
>>> index 3e694738e683..9a4888543384 100644
>>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>> [...]
>>> @@ -449,10 +461,35 @@ static int ravb_ring_init(struct net_device
>>> *ndev, int q)
>>>
>>>  static void ravb_emac_init_gbeth(struct net_device *ndev)  {
>>> -	/* Place holder */
>>> +	struct ravb_private *priv = netdev_priv(ndev);
>>> +
>>> +	/* Receive frame limit set register */
>>> +	ravb_write(ndev, GBETH_RX_BUFF_MAX + ETH_FCS_LEN, RFLR);
>>> +
>>> +	/* PAUSE prohibition */
>>
>>     Should be:
>>
>> 	/* EMAC Mode: PAUSE prohibition; Duplex; TX; RX */
>>
>>> +	ravb_write(ndev, ECMR_ZPF | ((priv->duplex > 0) ? ECMR_DM : 0) |
>>> +			 ECMR_TE | ECMR_RE | ECMR_RCPT |
>>> +			 ECMR_TXF | ECMR_RXF | ECMR_PRM, ECMR);
>>> +
>>> +	ravb_set_rate_gbeth(ndev);
>>> +
>>> +	/* Set MAC address */
>>> +	ravb_write(ndev,
>>> +		   (ndev->dev_addr[0] << 24) | (ndev->dev_addr[1] << 16) |
>>> +		   (ndev->dev_addr[2] << 8)  | (ndev->dev_addr[3]), MAHR);
>>> +	ravb_write(ndev, (ndev->dev_addr[4] << 8)  | (ndev->dev_addr[5]),
>>> +MALR);
>>> +
>>> +	/* E-MAC status register clear */
>>> +	ravb_write(ndev, ECSR_ICD | ECSR_LCHNG | ECSR_PFRI, ECSR);
>>> +
>>> +	/* E-MAC interrupt enable register */
>>> +	ravb_write(ndev, ECSIPR_ICDIP, ECSIPR);
>>
>>    Too much repetitive code, I think...
> 
> Can you please clarify what are the codes repetitive here?

   MAHR/MALR reading, mainly...
   The following code turned out to be different from gen2/3 indeed...

[...]

MBR, Sergey

RE: [PATCH 07/10] ravb: Add tsrq to struct ravb_hw_info

[Biju Das]

> -----Original Message-----
> From: Sergey Shtylyov <s.shtylyov@omp.ru>
> Sent: 04 October 2021 19:54
> To: Biju Das <biju.das.jz@bp.renesas.com>; Sergei Shtylyov
> <sergei.shtylyov@gmail.com>; David S. Miller <davem@davemloft.net>; Jakub
> Kicinski <kuba@kernel.org>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>; Sergey Shtylyov
> <s.shtylyov@omprussia.ru>; Adam Ford <aford173@gmail.com>; Andrew Lunn
> <andrew@lunn.ch>; Yuusuke Ashizuka <ashiduka@fujitsu.com>; Yoshihiro
> Shimoda <yoshihiro.shimoda.uh@renesas.com>; netdev@vger.kernel.org; linux-
> renesas-soc@vger.kernel.org; Chris Paterson <Chris.Paterson2@renesas.com>;
> Biju Das <biju.das@bp.renesas.com>; Prabhakar Mahadev Lad
> <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Subject: Re: [PATCH 07/10] ravb: Add tsrq to struct ravb_hw_info
> 
> On 10/4/21 9:47 PM, Biju Das wrote:
> 
> [...]
> >>>    The TCCR bits are called transmit start request (queue 0/1), not
> >> transmit start request queue 0/1.
> >>> I think you've read too much value into them for what is just TX queue
> >> 0/1.
> >>>
> >>>> Add a tsrq variable to struct ravb_hw_info to handle this difference.
> >>>>
> >>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >>>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>>> ---
> >>>> RFC->v1:
> >>>>  * Added tsrq variable instead of multi_tsrq feature bit.
> >>>> ---
> >>>>  drivers/net/ethernet/renesas/ravb.h      | 1 +
> >>>>  drivers/net/ethernet/renesas/ravb_main.c | 9 +++++++--
> >>>>  2 files changed, 8 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/drivers/net/ethernet/renesas/ravb.h
> >>>> b/drivers/net/ethernet/renesas/ravb.h
> >>>> index 9cd3a15743b4..c586070193ef 100644
> >>>> --- a/drivers/net/ethernet/renesas/ravb.h
> >>>> +++ b/drivers/net/ethernet/renesas/ravb.h
> >>>> @@ -997,6 +997,7 @@ struct ravb_hw_info {
> >>>>  	netdev_features_t net_features;
> >>>>  	int stats_len;
> >>>>  	size_t max_rx_len;
> >>>> +	u32 tsrq;
> >>>
> >>>    I'd call it 'tccr_value' instead.
> >>
> >>     Or even better, 'tccr_mask'...
> >
> > We are not masking anything here right.
> 
>     We do -- we pass the TCCR mask to ravb_wait().

Agreed. will use "tccr_mask" in next RFC version.

> 
> [...]
> 
> > Regards,
> > Biju
> 
> MBR, Sergey

RE: [PATCH 10/10] ravb: Initialize GbEthernet E-MAC

[Biju Das]

> -----Original Message-----
> From: Sergey Shtylyov <s.shtylyov@omp.ru>
> Sent: 04 October 2021 20:28
> To: Biju Das <biju.das.jz@bp.renesas.com>; David S. Miller
> <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>
> Cc: Sergei Shtylyov <sergei.shtylyov@gmail.com>; Geert Uytterhoeven
> <geert+renesas@glider.be>; Sergey Shtylyov <s.shtylyov@omprussia.ru>; Adam
> Ford <aford173@gmail.com>; Andrew Lunn <andrew@lunn.ch>; Yuusuke Ashizuka
> <ashiduka@fujitsu.com>; Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com>; netdev@vger.kernel.org; linux-renesas-
> soc@vger.kernel.org; Chris Paterson <Chris.Paterson2@renesas.com>; Biju
> Das <biju.das@bp.renesas.com>; Prabhakar Mahadev Lad <prabhakar.mahadev-
> lad.rj@bp.renesas.com>
> Subject: Re: [PATCH 10/10] ravb: Initialize GbEthernet E-MAC
> 
> On 10/4/21 10:23 PM, Biju Das wrote:
> 
> [...]
> >>> Initialize GbEthernet E-MAC found on RZ/G2L SoC.
> >>> This patch also renames ravb_set_rate to ravb_set_rate_rcar and
> >>> ravb_rcar_emac_init to ravb_emac_init_rcar to be consistent with the
> >>> naming convention used in sh_eth driver.
> >>>
> >>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>> ---
> >>> RFC->v1:
> >>>  * Moved CSR0 intialization to later patch.
> >>>  * started using ravb_modify for initializing link registers.
> >>> ---
> >>>  drivers/net/ethernet/renesas/ravb.h      | 20 +++++++--
> >>>  drivers/net/ethernet/renesas/ravb_main.c | 55
> >>> ++++++++++++++++++++----
> >>>  2 files changed, 62 insertions(+), 13 deletions(-)
> >>>
> [...]
> >>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> >>> b/drivers/net/ethernet/renesas/ravb_main.c
> >>> index 3e694738e683..9a4888543384 100644
> >>> --- a/drivers/net/ethernet/renesas/ravb_main.c
> >>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> >> [...]
> >>> @@ -449,10 +461,35 @@ static int ravb_ring_init(struct net_device
> >>> *ndev, int q)
> >>>
> >>>  static void ravb_emac_init_gbeth(struct net_device *ndev)  {
> >>> -	/* Place holder */
> >>> +	struct ravb_private *priv = netdev_priv(ndev);
> >>> +
> >>> +	/* Receive frame limit set register */
> >>> +	ravb_write(ndev, GBETH_RX_BUFF_MAX + ETH_FCS_LEN, RFLR);
> >>> +
> >>> +	/* PAUSE prohibition */
> >>
> >>     Should be:
> >>
> >> 	/* EMAC Mode: PAUSE prohibition; Duplex; TX; RX */

Why do we need ";" in between? Just checking or ":" is sufficient throughout?

> >>
> >>> +	ravb_write(ndev, ECMR_ZPF | ((priv->duplex > 0) ? ECMR_DM : 0) |
> >>> +			 ECMR_TE | ECMR_RE | ECMR_RCPT |
> >>> +			 ECMR_TXF | ECMR_RXF | ECMR_PRM, ECMR);
> >>> +
> >>> +	ravb_set_rate_gbeth(ndev);
> >>> +
> >>> +	/* Set MAC address */
> >>> +	ravb_write(ndev,
> >>> +		   (ndev->dev_addr[0] << 24) | (ndev->dev_addr[1] << 16) |
> >>> +		   (ndev->dev_addr[2] << 8)  | (ndev->dev_addr[3]), MAHR);
> >>> +	ravb_write(ndev, (ndev->dev_addr[4] << 8)  | (ndev->dev_addr[5]),
> >>> +MALR);
> >>> +
> >>> +	/* E-MAC status register clear */
> >>> +	ravb_write(ndev, ECSR_ICD | ECSR_LCHNG | ECSR_PFRI, ECSR);
> >>> +
> >>> +	/* E-MAC interrupt enable register */
> >>> +	ravb_write(ndev, ECSIPR_ICDIP, ECSIPR);
> >>
> >>    Too much repetitive code, I think...
> >
> > Can you please clarify what are the codes repetitive here?
> 
>    MAHR/MALR reading, mainly...
>    The following code turned out to be different from gen2/3 indeed...

It is OK. Only for just saving 2 writes we don't need to introduce a new function.

Regards,
Biju



> 
> [...]
> 
> MBR, Sergey

Re: [PATCH 03/10] ravb: Add nc_queue to struct ravb_hw_info

[Sergei Shtylyov]

On 10/3/21 9:58 AM, Biju Das wrote:

>>> R-Car supports network control queue whereas RZ/G2L does not support
>>> it. Add nc_queue to struct ravb_hw_info, so that NC queue is handled
>>> only by R-Car.
>>>
>>> This patch also renames ravb_rcar_dmac_init to ravb_dmac_init_rcar to
>>> be consistent with the naming convention used in sh_eth driver.
>>>
>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>
>> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
>>
>>    One little nit below:
>>
>>> ---
>>> RFC->v1:
>>>  * Handled NC queue only for R-Car.
>>> ---
>>>  drivers/net/ethernet/renesas/ravb.h      |   3 +-
>>>  drivers/net/ethernet/renesas/ravb_main.c | 140
>>> +++++++++++++++--------
>>>  2 files changed, 94 insertions(+), 49 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/renesas/ravb.h
>>> b/drivers/net/ethernet/renesas/ravb.h
>>> index a33fbcb4aac3..c91e93e5590f 100644
>>> --- a/drivers/net/ethernet/renesas/ravb.h
>>> +++ b/drivers/net/ethernet/renesas/ravb.h
>>> @@ -986,7 +986,7 @@ struct ravb_hw_info {
>>>  	bool (*receive)(struct net_device *ndev, int *quota, int q);
>>>  	void (*set_rate)(struct net_device *ndev);
>>>  	int (*set_feature)(struct net_device *ndev, netdev_features_t
>> features);
>>> -	void (*dmac_init)(struct net_device *ndev);
>>> +	int (*dmac_init)(struct net_device *ndev);
>>>  	void (*emac_init)(struct net_device *ndev);
>>>  	const char (*gstrings_stats)[ETH_GSTRING_LEN];
>>>  	size_t gstrings_size;
>>> @@ -1002,6 +1002,7 @@ struct ravb_hw_info {
>>>  	unsigned multi_irqs:1;		/* AVB-DMAC and E-MAC has multiple
>> irqs */
>>>  	unsigned gptp:1;		/* AVB-DMAC has gPTP support */
>>>  	unsigned ccc_gac:1;		/* AVB-DMAC has gPTP support active in
>> config mode */
>>> +	unsigned nc_queue:1;		/* AVB-DMAC has NC queue */
>>
>>    Rather "queues" as there are RX and TX NC queues, no?
> 
> It has NC queue on both RX and TX.
> 
> If needed, I can send a follow up patch as RFC with the following changes.
> 
> unsigned nc_queue:1;		/* AVB-DMAC has NC queue on both RX and TX  */
> 
> or 
> 
> unsigned nc_queues:1;		/* AVB-DMAC has RX and TX NC queues */
> 
> please let me know.

   Yes, please do it.

> Regards,
> Biju

MNR, Sergey

RE: [PATCH 03/10] ravb: Add nc_queue to struct ravb_hw_info

[Biju Das]

Hi Sergey,

> -----Original Message-----
> From: Sergei Shtylyov <sergei.shtylyov@gmail.com>
> Sent: 06 October 2021 20:46
> To: Biju Das <biju.das.jz@bp.renesas.com>; Sergey Shtylyov
> <s.shtylyov@omp.ru>; David S. Miller <davem@davemloft.net>; Jakub Kicinski
> <kuba@kernel.org>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>; Sergey Shtylyov
> <s.shtylyov@omprussia.ru>; Adam Ford <aford173@gmail.com>; Andrew Lunn
> <andrew@lunn.ch>; Yuusuke Ashizuka <ashiduka@fujitsu.com>; Yoshihiro
> Shimoda <yoshihiro.shimoda.uh@renesas.com>; netdev@vger.kernel.org; linux-
> renesas-soc@vger.kernel.org; Chris Paterson <Chris.Paterson2@renesas.com>;
> Biju Das <biju.das@bp.renesas.com>; Prabhakar Mahadev Lad
> <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Subject: Re: [PATCH 03/10] ravb: Add nc_queue to struct ravb_hw_info
> 
> On 10/3/21 9:58 AM, Biju Das wrote:
> 
> >>> R-Car supports network control queue whereas RZ/G2L does not support
> >>> it. Add nc_queue to struct ravb_hw_info, so that NC queue is handled
> >>> only by R-Car.
> >>>
> >>> This patch also renames ravb_rcar_dmac_init to ravb_dmac_init_rcar
> >>> to be consistent with the naming convention used in sh_eth driver.
> >>>
> >>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>
> >> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> >>
> >>    One little nit below:
> >>
> >>> ---
> >>> RFC->v1:
> >>>  * Handled NC queue only for R-Car.
> >>> ---
> >>>  drivers/net/ethernet/renesas/ravb.h      |   3 +-
> >>>  drivers/net/ethernet/renesas/ravb_main.c | 140
> >>> +++++++++++++++--------
> >>>  2 files changed, 94 insertions(+), 49 deletions(-)
> >>>
> >>> diff --git a/drivers/net/ethernet/renesas/ravb.h
> >>> b/drivers/net/ethernet/renesas/ravb.h
> >>> index a33fbcb4aac3..c91e93e5590f 100644
> >>> --- a/drivers/net/ethernet/renesas/ravb.h
> >>> +++ b/drivers/net/ethernet/renesas/ravb.h
> >>> @@ -986,7 +986,7 @@ struct ravb_hw_info {
> >>>  	bool (*receive)(struct net_device *ndev, int *quota, int q);
> >>>  	void (*set_rate)(struct net_device *ndev);
> >>>  	int (*set_feature)(struct net_device *ndev, netdev_features_t
> >> features);
> >>> -	void (*dmac_init)(struct net_device *ndev);
> >>> +	int (*dmac_init)(struct net_device *ndev);
> >>>  	void (*emac_init)(struct net_device *ndev);
> >>>  	const char (*gstrings_stats)[ETH_GSTRING_LEN];
> >>>  	size_t gstrings_size;
> >>> @@ -1002,6 +1002,7 @@ struct ravb_hw_info {
> >>>  	unsigned multi_irqs:1;		/* AVB-DMAC and E-MAC has multiple
> >> irqs */
> >>>  	unsigned gptp:1;		/* AVB-DMAC has gPTP support */
> >>>  	unsigned ccc_gac:1;		/* AVB-DMAC has gPTP support active in
> >> config mode */
> >>> +	unsigned nc_queue:1;		/* AVB-DMAC has NC queue */
> >>
> >>    Rather "queues" as there are RX and TX NC queues, no?
> >
> > It has NC queue on both RX and TX.
> >
> > If needed, I can send a follow up patch as RFC with the following
> changes.
> >
> > unsigned nc_queue:1;		/* AVB-DMAC has NC queue on both RX and TX
> */
> >
> > or
> >
> > unsigned nc_queues:1;		/* AVB-DMAC has RX and TX NC queues */
> >
> > please let me know.
> 
>    Yes, please do it.

OK I will go with below, as RX has single NC queue and TX has single NC queue.

unsigned nc_queue:1;		/* AVB-DMAC has NC queue on both RX and TX */

Cheers,
Biju

> 
> > Regards,
> > Biju
> 
> MNR, Sergey