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

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:

quoted

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 <redacted>

   One little nit below:

quoted

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

quoted

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

quoted

 	unsigned gptp:1;		/* AVB-DMAC has gPTP support */
 	unsigned ccc_gac:1;		/* AVB-DMAC has gPTP support active in

config mode */

quoted

+	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

[...]

quoted

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

[...]

quoted

@@ -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