linux-renesas-soc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/8] Add Gigabit Ethernet driver support
@ 2021-08-02 10:26 Biju Das
  2021-08-02 10:26 ` [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to driver data Biju Das
                   ` (7 more replies)
  0 siblings, 8 replies; 59+ messages in thread
From: Biju Das @ 2021-08-02 10:26 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: Biju Das, Sergei Shtylyov, Geert Uytterhoeven, Sergey Shtylyov,
	Adam Ford, Andrew Lunn, Yuusuke Ashizuka, Yoshihiro Shimoda,
	Yang Yingliang, netdev, linux-renesas-soc, Chris Paterson,
	Biju Das, Prabhakar Mahadev Lad

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.

Currently a runtime decision based on the chip type is used to distinguish
the HW differences between the SoC families.

This patch series is in preparation for supporting the RZ/G2L SoC by
replacing driver data chip type with struct ravb_hw_info by moving chip
type to it and also adding gstrings_stats, gstrings_size, net_hw_features,
net_features, num_gstat_queue, num_tx_desc, stats_len, skb_sz variables to
it. This patch also adds the feature bit for {RX, TX} clock internal
delays and TX Drop counters HW features found on R-Car Gen3 to struct
ravb_hw_info.

This patch series is based on net-next.

v1->v2:
 * Replaced driver data chip type with struct ravb_hw_info
 * Added gstrings_stats, gstrings_size, net_hw_features, net_features,
   num_gstat_queue, num_tx_desc, stats_len, skb_sz to struct ravb_hw_info
 * Added internal_delay and tx_drop_cntrs hw feature bit to struct ravb_hw_info

RFC->V1
  * Incorporated feedback from Andrew, Sergei, Geert and Prabhakar
  * https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=515525

Biju Das (8):
  ravb: Add struct ravb_hw_info to driver data
  ravb: Add skb_sz to struct ravb_hw_info
  ravb: Add num_gstat_queue to struct ravb_hw_info
  ravb: Add stats_len to struct ravb_hw_info
  ravb: Add gstrings_stats and gstrings_size to struct ravb_hw_info
  ravb: Add net_features and net_hw_features to struct ravb_hw_info
  ravb: Add internal delay hw feature to struct ravb_hw_info
  ravb: Add tx_drop_cntrs to struct ravb_hw_info

 drivers/net/ethernet/renesas/ravb.h      | 18 +++++
 drivers/net/ethernet/renesas/ravb_main.c | 91 ++++++++++++++++--------
 2 files changed, 80 insertions(+), 29 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 59+ messages in thread

* [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to driver data
  2021-08-02 10:26 [PATCH net-next v2 0/8] Add Gigabit Ethernet driver support Biju Das
@ 2021-08-02 10:26 ` Biju Das
  2021-08-02 15:02   ` Andrew Lunn
                     ` (2 more replies)
  2021-08-02 10:26 ` [PATCH net-next v2 2/8] ravb: Add skb_sz to struct ravb_hw_info Biju Das
                   ` (6 subsequent siblings)
  7 siblings, 3 replies; 59+ messages in thread
From: Biju Das @ 2021-08-02 10:26 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: Biju Das, Sergei Shtylyov, Geert Uytterhoeven, Sergey Shtylyov,
	Adam Ford, Andrew Lunn, Yuusuke Ashizuka, Yoshihiro Shimoda,
	netdev, linux-renesas-soc, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad

The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC are
similar to the R-Car Ethernet AVB IP. With a few changes in the driver we
can support both IPs.

Currently a runtime decision based on the chip type is used to distinguish
the HW differences between the SoC families.

The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2 and
RZ/G2L it is 2. For cases like this it is better to select the number of
TX descriptors by using a structure with a value, rather than a runtime
decision based on the chip type.

This patch adds the num_tx_desc variable to struct ravb_hw_info and also
replaces the driver data chip type with struct ravb_hw_info by moving chip
type to it.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
 * Incorporated Andrew and Sergei's review comments for making it smaller patch
   and provided detailed description.
---
 drivers/net/ethernet/renesas/ravb.h      |  7 +++++
 drivers/net/ethernet/renesas/ravb_main.c | 38 +++++++++++++++---------
 2 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 80e62ca2e3d3..cfb972c05b34 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -988,6 +988,11 @@ enum ravb_chip_id {
 	RCAR_GEN3,
 };
 
+struct ravb_hw_info {
+	enum ravb_chip_id chip_id;
+	int num_tx_desc;
+};
+
 struct ravb_private {
 	struct net_device *ndev;
 	struct platform_device *pdev;
@@ -1040,6 +1045,8 @@ struct ravb_private {
 	unsigned txcidm:1;		/* TX Clock Internal Delay Mode */
 	unsigned rgmii_override:1;	/* Deprecated rgmii-*id behavior */
 	int num_tx_desc;		/* TX descriptors per packet */
+
+	const struct ravb_hw_info *info;
 };
 
 static inline u32 ravb_read(struct net_device *ndev, enum ravb_reg reg)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index f4dfe9f71d06..ffbd224d8780 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1924,12 +1924,22 @@ static int ravb_mdio_release(struct ravb_private *priv)
 	return 0;
 }
 
+static const struct ravb_hw_info ravb_gen3_hw_info = {
+	.chip_id = RCAR_GEN3,
+	.num_tx_desc = NUM_TX_DESC_GEN3,
+};
+
+static const struct ravb_hw_info ravb_gen2_hw_info = {
+	.chip_id = RCAR_GEN2,
+	.num_tx_desc = NUM_TX_DESC_GEN2,
+};
+
 static const struct of_device_id ravb_match_table[] = {
-	{ .compatible = "renesas,etheravb-r8a7790", .data = (void *)RCAR_GEN2 },
-	{ .compatible = "renesas,etheravb-r8a7794", .data = (void *)RCAR_GEN2 },
-	{ .compatible = "renesas,etheravb-rcar-gen2", .data = (void *)RCAR_GEN2 },
-	{ .compatible = "renesas,etheravb-r8a7795", .data = (void *)RCAR_GEN3 },
-	{ .compatible = "renesas,etheravb-rcar-gen3", .data = (void *)RCAR_GEN3 },
+	{ .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 },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ravb_match_table);
@@ -2034,8 +2044,8 @@ static void ravb_set_delay_mode(struct net_device *ndev)
 static int ravb_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
+	const struct ravb_hw_info *info;
 	struct ravb_private *priv;
-	enum ravb_chip_id chip_id;
 	struct net_device *ndev;
 	int error, irq, q;
 	struct resource *res;
@@ -2058,9 +2068,9 @@ static int ravb_probe(struct platform_device *pdev)
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_get_sync(&pdev->dev);
 
-	chip_id = (enum ravb_chip_id)of_device_get_match_data(&pdev->dev);
+	info = of_device_get_match_data(&pdev->dev);
 
-	if (chip_id == RCAR_GEN3)
+	if (info->chip_id == RCAR_GEN3)
 		irq = platform_get_irq_byname(pdev, "ch22");
 	else
 		irq = platform_get_irq(pdev, 0);
@@ -2073,6 +2083,7 @@ static int ravb_probe(struct platform_device *pdev)
 	SET_NETDEV_DEV(ndev, &pdev->dev);
 
 	priv = netdev_priv(ndev);
+	priv->info = info;
 	priv->ndev = ndev;
 	priv->pdev = pdev;
 	priv->num_tx_ring[RAVB_BE] = BE_TX_RING_SIZE;
@@ -2099,7 +2110,7 @@ static int ravb_probe(struct platform_device *pdev)
 	priv->avb_link_active_low =
 		of_property_read_bool(np, "renesas,ether-link-active-low");
 
-	if (chip_id == RCAR_GEN3) {
+	if (info->chip_id == RCAR_GEN3) {
 		irq = platform_get_irq_byname(pdev, "ch24");
 		if (irq < 0) {
 			error = irq;
@@ -2124,7 +2135,7 @@ static int ravb_probe(struct platform_device *pdev)
 		}
 	}
 
-	priv->chip_id = chip_id;
+	priv->chip_id = info->chip_id;
 
 	priv->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(priv->clk)) {
@@ -2142,8 +2153,7 @@ static int ravb_probe(struct platform_device *pdev)
 	ndev->max_mtu = 2048 - (ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN);
 	ndev->min_mtu = ETH_MIN_MTU;
 
-	priv->num_tx_desc = chip_id == RCAR_GEN2 ?
-		NUM_TX_DESC_GEN2 : NUM_TX_DESC_GEN3;
+	priv->num_tx_desc = info->num_tx_desc;
 
 	/* Set function */
 	ndev->netdev_ops = &ravb_netdev_ops;
@@ -2184,7 +2194,7 @@ static int ravb_probe(struct platform_device *pdev)
 	INIT_LIST_HEAD(&priv->ts_skb_list);
 
 	/* Initialise PTP Clock driver */
-	if (chip_id != RCAR_GEN2)
+	if (info->chip_id != RCAR_GEN2)
 		ravb_ptp_init(ndev, pdev);
 
 	/* Debug message level */
@@ -2232,7 +2242,7 @@ static int ravb_probe(struct platform_device *pdev)
 			  priv->desc_bat_dma);
 
 	/* Stop PTP Clock driver */
-	if (chip_id != RCAR_GEN2)
+	if (info->chip_id != RCAR_GEN2)
 		ravb_ptp_stop(ndev);
 out_disable_refclk:
 	clk_disable_unprepare(priv->refclk);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH net-next v2 2/8] ravb: Add skb_sz to struct ravb_hw_info
  2021-08-02 10:26 [PATCH net-next v2 0/8] Add Gigabit Ethernet driver support Biju Das
  2021-08-02 10:26 ` [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to driver data Biju Das
@ 2021-08-02 10:26 ` Biju Das
  2021-08-02 15:08   ` Andrew Lunn
  2021-08-02 20:54   ` Sergei Shtylyov
  2021-08-02 10:26 ` [PATCH net-next v2 3/8] ravb: Add num_gstat_queue " Biju Das
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 59+ messages in thread
From: Biju Das @ 2021-08-02 10:26 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: Biju Das, Sergei Shtylyov, Geert Uytterhoeven, Sergey Shtylyov,
	Adam Ford, Andrew Lunn, Yuusuke Ashizuka, Yoshihiro Shimoda,
	netdev, linux-renesas-soc, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad

The maximum descriptor size that can be specified on the reception side for
R-Car is 2048 bytes, whereas for RZ/G2L it is 8096.

Add the skb_size variable to struct ravb_hw_info for allocating different
skb buffer sizes for R-Car and RZ/G2L using the netdev_alloc_skb function.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
 * Incorporated Andrew and Sergei's review comments for making it smaller patch
   and provided detailed description.
---
 drivers/net/ethernet/renesas/ravb.h      |  1 +
 drivers/net/ethernet/renesas/ravb_main.c | 10 ++++++----
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index cfb972c05b34..16d1711a0731 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -991,6 +991,7 @@ enum ravb_chip_id {
 struct ravb_hw_info {
 	enum ravb_chip_id chip_id;
 	int num_tx_desc;
+	size_t skb_sz;
 };
 
 struct ravb_private {
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index ffbd224d8780..08146c1975e5 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -339,6 +339,7 @@ static void ravb_ring_format(struct net_device *ndev, int q)
 static int ravb_ring_init(struct net_device *ndev, int q)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
 	int num_tx_desc = priv->num_tx_desc;
 	struct sk_buff *skb;
 	int ring_size;
@@ -353,7 +354,7 @@ static int ravb_ring_init(struct net_device *ndev, int q)
 		goto error;
 
 	for (i = 0; i < priv->num_rx_ring[q]; i++) {
-		skb = netdev_alloc_skb(ndev, RX_BUF_SZ + RAVB_ALIGN - 1);
+		skb = netdev_alloc_skb(ndev, info->skb_sz);
 		if (!skb)
 			goto error;
 		ravb_set_buffer_align(skb);
@@ -535,6 +536,7 @@ static void ravb_rx_csum(struct sk_buff *skb)
 static bool ravb_rx(struct net_device *ndev, int *quota, int q)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
 	int entry = priv->cur_rx[q] % priv->num_rx_ring[q];
 	int boguscnt = (priv->dirty_rx[q] + priv->num_rx_ring[q]) -
 			priv->cur_rx[q];
@@ -619,9 +621,7 @@ static bool ravb_rx(struct net_device *ndev, int *quota, int q)
 		desc->ds_cc = cpu_to_le16(RX_BUF_SZ);
 
 		if (!priv->rx_skb[q][entry]) {
-			skb = netdev_alloc_skb(ndev,
-					       RX_BUF_SZ +
-					       RAVB_ALIGN - 1);
+			skb = netdev_alloc_skb(ndev, info->skb_sz);
 			if (!skb)
 				break;	/* Better luck next round. */
 			ravb_set_buffer_align(skb);
@@ -1927,11 +1927,13 @@ static int ravb_mdio_release(struct ravb_private *priv)
 static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.chip_id = RCAR_GEN3,
 	.num_tx_desc = NUM_TX_DESC_GEN3,
+	.skb_sz = RX_BUF_SZ + RAVB_ALIGN - 1,
 };
 
 static const struct ravb_hw_info ravb_gen2_hw_info = {
 	.chip_id = RCAR_GEN2,
 	.num_tx_desc = NUM_TX_DESC_GEN2,
+	.skb_sz = RX_BUF_SZ + RAVB_ALIGN - 1,
 };
 
 static const struct of_device_id ravb_match_table[] = {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH net-next v2 3/8] ravb: Add num_gstat_queue to struct ravb_hw_info
  2021-08-02 10:26 [PATCH net-next v2 0/8] Add Gigabit Ethernet driver support Biju Das
  2021-08-02 10:26 ` [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to driver data Biju Das
  2021-08-02 10:26 ` [PATCH net-next v2 2/8] ravb: Add skb_sz to struct ravb_hw_info Biju Das
@ 2021-08-02 10:26 ` Biju Das
  2021-08-02 15:09   ` Andrew Lunn
  2021-08-03 18:21   ` Sergei Shtylyov
  2021-08-02 10:26 ` [PATCH net-next v2 4/8] ravb: Add stats_len " Biju Das
                   ` (4 subsequent siblings)
  7 siblings, 2 replies; 59+ messages in thread
From: Biju Das @ 2021-08-02 10:26 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: Biju Das, Sergei Shtylyov, Geert Uytterhoeven, Sergey Shtylyov,
	Adam Ford, Andrew Lunn, Yuusuke Ashizuka, Yoshihiro Shimoda,
	netdev, linux-renesas-soc, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad

The number of queues used in retrieving device stats for R-Car is 2,
whereas for RZ/G2L it is 1.

Add the num_gstat_queue variable to struct ravb_hw_info, to add subsequent
SoCs without any code changes to the ravb_get_ethtool_stats function.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
 * Incorporated Andrew and Sergei's review comments for making it smaller patch
   and provided detailed description.
---
 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 16d1711a0731..38552e0319d3 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -990,6 +990,7 @@ enum ravb_chip_id {
 
 struct ravb_hw_info {
 	enum ravb_chip_id chip_id;
+	int num_gstat_queue;
 	int num_tx_desc;
 	size_t skb_sz;
 };
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 08146c1975e5..30132693edd7 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1149,11 +1149,12 @@ 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 i = 0;
 	int q;
 
 	/* Device-specific stats */
-	for (q = RAVB_BE; q < NUM_RX_QUEUE; q++) {
+	for (q = RAVB_BE; q < info->num_gstat_queue; q++) {
 		struct net_device_stats *stats = &priv->stats[q];
 
 		data[i++] = priv->cur_rx[q];
@@ -1926,12 +1927,14 @@ static int ravb_mdio_release(struct ravb_private *priv)
 
 static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.chip_id = RCAR_GEN3,
+	.num_gstat_queue = NUM_RX_QUEUE,
 	.num_tx_desc = NUM_TX_DESC_GEN3,
 	.skb_sz = RX_BUF_SZ + RAVB_ALIGN - 1,
 };
 
 static const struct ravb_hw_info ravb_gen2_hw_info = {
 	.chip_id = RCAR_GEN2,
+	.num_gstat_queue = NUM_RX_QUEUE,
 	.num_tx_desc = NUM_TX_DESC_GEN2,
 	.skb_sz = RX_BUF_SZ + RAVB_ALIGN - 1,
 };
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH net-next v2 4/8] ravb: Add stats_len to struct ravb_hw_info
  2021-08-02 10:26 [PATCH net-next v2 0/8] Add Gigabit Ethernet driver support Biju Das
                   ` (2 preceding siblings ...)
  2021-08-02 10:26 ` [PATCH net-next v2 3/8] ravb: Add num_gstat_queue " Biju Das
@ 2021-08-02 10:26 ` Biju Das
  2021-08-03 18:35   ` Sergei Shtylyov
  2021-08-02 10:26 ` [PATCH net-next v2 5/8] ravb: Add gstrings_stats and gstrings_size " Biju Das
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 59+ messages in thread
From: Biju Das @ 2021-08-02 10:26 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: Biju Das, Sergei Shtylyov, Geert Uytterhoeven, Sergey Shtylyov,
	Adam Ford, Andrew Lunn, Yuusuke Ashizuka, Yoshihiro Shimoda,
	netdev, linux-renesas-soc, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad

R-Car provides 30 device stats, whereas RZ/G2L provides only 15. In
addition, RZ/G2L has stats "rx_queue_0_csum_offload_errors" instead of
"rx_queue_0_missed_errors".

Replace RAVB_STATS_LEN macro with a structure variable stats_len to
struct ravb_hw_info, to support subsequent SoCs without any code changes
to the ravb_get_sset_count function.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
 * Incorporated Andrew and Sergei's review comments for making it smaller patch
   and provided detailed description.
---
 drivers/net/ethernet/renesas/ravb.h      | 1 +
 drivers/net/ethernet/renesas/ravb_main.c | 9 ++++++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 38552e0319d3..a42c34eaebc2 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -992,6 +992,7 @@ struct ravb_hw_info {
 	enum ravb_chip_id chip_id;
 	int num_gstat_queue;
 	int num_tx_desc;
+	int stats_len;
 	size_t skb_sz;
 };
 
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 30132693edd7..baeb868b07ed 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1133,13 +1133,14 @@ static const char ravb_gstrings_stats[][ETH_GSTRING_LEN] = {
 	"rx_queue_1_over_errors",
 };
 
-#define RAVB_STATS_LEN	ARRAY_SIZE(ravb_gstrings_stats)
-
 static int ravb_get_sset_count(struct net_device *netdev, int sset)
 {
+	struct ravb_private *priv = netdev_priv(netdev);
+	const struct ravb_hw_info *info = priv->info;
+
 	switch (sset) {
 	case ETH_SS_STATS:
-		return RAVB_STATS_LEN;
+		return info->stats_len;
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -1929,6 +1930,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.chip_id = RCAR_GEN3,
 	.num_gstat_queue = NUM_RX_QUEUE,
 	.num_tx_desc = NUM_TX_DESC_GEN3,
+	.stats_len = ARRAY_SIZE(ravb_gstrings_stats),
 	.skb_sz = RX_BUF_SZ + RAVB_ALIGN - 1,
 };
 
@@ -1936,6 +1938,7 @@ static const struct ravb_hw_info ravb_gen2_hw_info = {
 	.chip_id = RCAR_GEN2,
 	.num_gstat_queue = NUM_RX_QUEUE,
 	.num_tx_desc = NUM_TX_DESC_GEN2,
+	.stats_len = ARRAY_SIZE(ravb_gstrings_stats),
 	.skb_sz = RX_BUF_SZ + RAVB_ALIGN - 1,
 };
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH net-next v2 5/8] ravb: Add gstrings_stats and gstrings_size to struct ravb_hw_info
  2021-08-02 10:26 [PATCH net-next v2 0/8] Add Gigabit Ethernet driver support Biju Das
                   ` (3 preceding siblings ...)
  2021-08-02 10:26 ` [PATCH net-next v2 4/8] ravb: Add stats_len " Biju Das
@ 2021-08-02 10:26 ` Biju Das
  2021-08-02 15:11   ` Andrew Lunn
  2021-08-04 20:36   ` Sergei Shtylyov
  2021-08-02 10:26 ` [PATCH net-next v2 6/8] ravb: Add net_features and net_hw_features " Biju Das
                   ` (2 subsequent siblings)
  7 siblings, 2 replies; 59+ messages in thread
From: Biju Das @ 2021-08-02 10:26 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: Biju Das, Sergei Shtylyov, Geert Uytterhoeven, Sergey Shtylyov,
	Adam Ford, Andrew Lunn, Yuusuke Ashizuka, Yoshihiro Shimoda,
	netdev, linux-renesas-soc, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad

The device stats strings for R-Car and RZ/G2L are different.

R-Car provides 30 device stats, whereas RZ/G2L provides only 15. In
addition, RZ/G2L has stats "rx_queue_0_csum_offload_errors" instead of
"rx_queue_0_missed_errors".

Add structure variables gstrings_stats and gstrings_size to struct
ravb_hw_info, so that subsequent SoCs can be added without any code
changes in the ravb_get_strings function.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
 * Incorporated Andrew and Sergei's review comments for making it smaller patch
   and provided detailed description.
---
 drivers/net/ethernet/renesas/ravb.h      | 2 ++
 drivers/net/ethernet/renesas/ravb_main.c | 9 ++++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index a42c34eaebc2..b765b2b7d9e9 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -989,6 +989,8 @@ enum ravb_chip_id {
 };
 
 struct ravb_hw_info {
+	const char (*gstrings_stats)[ETH_GSTRING_LEN];
+	size_t gstrings_size;
 	enum ravb_chip_id chip_id;
 	int num_gstat_queue;
 	int num_tx_desc;
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index baeb868b07ed..7a69668cb512 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1178,9 +1178,12 @@ static void ravb_get_ethtool_stats(struct net_device *ndev,
 
 static void ravb_get_strings(struct net_device *ndev, u32 stringset, u8 *data)
 {
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
+
 	switch (stringset) {
 	case ETH_SS_STATS:
-		memcpy(data, ravb_gstrings_stats, sizeof(ravb_gstrings_stats));
+		memcpy(data, info->gstrings_stats, info->gstrings_size);
 		break;
 	}
 }
@@ -1927,6 +1930,8 @@ static int ravb_mdio_release(struct ravb_private *priv)
 }
 
 static const struct ravb_hw_info ravb_gen3_hw_info = {
+	.gstrings_stats = ravb_gstrings_stats,
+	.gstrings_size = sizeof(ravb_gstrings_stats),
 	.chip_id = RCAR_GEN3,
 	.num_gstat_queue = NUM_RX_QUEUE,
 	.num_tx_desc = NUM_TX_DESC_GEN3,
@@ -1935,6 +1940,8 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
 };
 
 static const struct ravb_hw_info ravb_gen2_hw_info = {
+	.gstrings_stats = ravb_gstrings_stats,
+	.gstrings_size = sizeof(ravb_gstrings_stats),
 	.chip_id = RCAR_GEN2,
 	.num_gstat_queue = NUM_RX_QUEUE,
 	.num_tx_desc = NUM_TX_DESC_GEN2,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH net-next v2 6/8] ravb: Add net_features and net_hw_features to struct ravb_hw_info
  2021-08-02 10:26 [PATCH net-next v2 0/8] Add Gigabit Ethernet driver support Biju Das
                   ` (4 preceding siblings ...)
  2021-08-02 10:26 ` [PATCH net-next v2 5/8] ravb: Add gstrings_stats and gstrings_size " Biju Das
@ 2021-08-02 10:26 ` Biju Das
  2021-08-02 15:12   ` Andrew Lunn
                     ` (2 more replies)
  2021-08-02 10:26 ` [PATCH net-next v2 7/8] ravb: Add internal delay hw feature " Biju Das
  2021-08-02 10:26 ` [PATCH net-next v2 8/8] ravb: Add tx_drop_cntrs " Biju Das
  7 siblings, 3 replies; 59+ messages in thread
From: Biju Das @ 2021-08-02 10:26 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: Biju Das, Sergei Shtylyov, Geert Uytterhoeven, Sergey Shtylyov,
	Adam Ford, Andrew Lunn, Yuusuke Ashizuka, Yoshihiro Shimoda,
	netdev, linux-renesas-soc, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad

On R-Car the checksum calculation on RX frames is done by the E-MAC
module, whereas on RZ/G2L it is done by the TOE.

TOE calculates the checksum of received frames from E-MAC and outputs it to
DMAC. TOE also calculates the checksum of transmission frames from DMAC and
outputs it E-MAC.

Add net_features and net_hw_features to struct ravb_hw_info, to support
subsequent SoCs without any code changes in the ravb_probe function.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
 * Incorporated Andrew and Sergei's review comments for making it smaller patch
   and provided detailed description.
---
 drivers/net/ethernet/renesas/ravb.h      |  2 ++
 drivers/net/ethernet/renesas/ravb_main.c | 12 ++++++++----
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index b765b2b7d9e9..3df813b2e253 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -991,6 +991,8 @@ enum ravb_chip_id {
 struct ravb_hw_info {
 	const char (*gstrings_stats)[ETH_GSTRING_LEN];
 	size_t gstrings_size;
+	netdev_features_t net_hw_features;
+	netdev_features_t net_features;
 	enum ravb_chip_id chip_id;
 	int num_gstat_queue;
 	int num_tx_desc;
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 7a69668cb512..2ac962b5b8fb 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1932,6 +1932,8 @@ static int ravb_mdio_release(struct ravb_private *priv)
 static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.gstrings_stats = ravb_gstrings_stats,
 	.gstrings_size = sizeof(ravb_gstrings_stats),
+	.net_hw_features = NETIF_F_RXCSUM,
+	.net_features = NETIF_F_RXCSUM,
 	.chip_id = RCAR_GEN3,
 	.num_gstat_queue = NUM_RX_QUEUE,
 	.num_tx_desc = NUM_TX_DESC_GEN3,
@@ -1942,6 +1944,8 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
 static const struct ravb_hw_info ravb_gen2_hw_info = {
 	.gstrings_stats = ravb_gstrings_stats,
 	.gstrings_size = sizeof(ravb_gstrings_stats),
+	.net_hw_features = NETIF_F_RXCSUM,
+	.net_features = NETIF_F_RXCSUM,
 	.chip_id = RCAR_GEN2,
 	.num_gstat_queue = NUM_RX_QUEUE,
 	.num_tx_desc = NUM_TX_DESC_GEN2,
@@ -2077,14 +2081,14 @@ static int ravb_probe(struct platform_device *pdev)
 	if (!ndev)
 		return -ENOMEM;
 
-	ndev->features = NETIF_F_RXCSUM;
-	ndev->hw_features = NETIF_F_RXCSUM;
+	info = of_device_get_match_data(&pdev->dev);
+
+	ndev->features = info->net_features;
+	ndev->hw_features = info->net_hw_features;
 
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_get_sync(&pdev->dev);
 
-	info = of_device_get_match_data(&pdev->dev);
-
 	if (info->chip_id == RCAR_GEN3)
 		irq = platform_get_irq_byname(pdev, "ch22");
 	else
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH net-next v2 7/8] ravb: Add internal delay hw feature to struct ravb_hw_info
  2021-08-02 10:26 [PATCH net-next v2 0/8] Add Gigabit Ethernet driver support Biju Das
                   ` (5 preceding siblings ...)
  2021-08-02 10:26 ` [PATCH net-next v2 6/8] ravb: Add net_features and net_hw_features " Biju Das
@ 2021-08-02 10:26 ` Biju Das
  2021-08-02 15:13   ` Andrew Lunn
                     ` (2 more replies)
  2021-08-02 10:26 ` [PATCH net-next v2 8/8] ravb: Add tx_drop_cntrs " Biju Das
  7 siblings, 3 replies; 59+ messages in thread
From: Biju Das @ 2021-08-02 10:26 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: Biju Das, Sergei Shtylyov, Geert Uytterhoeven, Sergey Shtylyov,
	Adam Ford, Andrew Lunn, Yuusuke Ashizuka, Yoshihiro Shimoda,
	netdev, linux-renesas-soc, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad

R-Car Gen3 supports TX and RX clock internal delay modes, whereas R-Car
Gen2 and RZ/G2L do not support it.
Add an internal_delay hw feature bit to struct ravb_hw_info to enable this
only for R-Car Gen3.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
 * Incorporated Andrew and Sergei's review comments for making it smaller patch
   and provided detailed description.
---
 drivers/net/ethernet/renesas/ravb.h      | 3 +++
 drivers/net/ethernet/renesas/ravb_main.c | 6 ++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 3df813b2e253..0d640dbe1eed 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -998,6 +998,9 @@ struct ravb_hw_info {
 	int num_tx_desc;
 	int stats_len;
 	size_t skb_sz;
+
+	/* hardware features */
+	unsigned internal_delay:1;	/* RAVB has internal delays */
 };
 
 struct ravb_private {
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 2ac962b5b8fb..02acae4d51c1 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1939,6 +1939,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.num_tx_desc = NUM_TX_DESC_GEN3,
 	.stats_len = ARRAY_SIZE(ravb_gstrings_stats),
 	.skb_sz = RX_BUF_SZ + RAVB_ALIGN - 1,
+	.internal_delay = 1,
 };
 
 static const struct ravb_hw_info ravb_gen2_hw_info = {
@@ -2189,7 +2190,7 @@ static int ravb_probe(struct platform_device *pdev)
 	/* Request GTI loading */
 	ravb_modify(ndev, GCCR, GCCR_LTI, GCCR_LTI);
 
-	if (priv->chip_id != RCAR_GEN2) {
+	if (info->internal_delay) {
 		ravb_parse_delay_mode(np, ndev);
 		ravb_set_delay_mode(ndev);
 	}
@@ -2362,6 +2363,7 @@ static int __maybe_unused ravb_resume(struct device *dev)
 {
 	struct net_device *ndev = dev_get_drvdata(dev);
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
 	int ret = 0;
 
 	/* If WoL is enabled set reset mode to rearm the WoL logic */
@@ -2384,7 +2386,7 @@ static int __maybe_unused ravb_resume(struct device *dev)
 	/* Request GTI loading */
 	ravb_modify(ndev, GCCR, GCCR_LTI, GCCR_LTI);
 
-	if (priv->chip_id != RCAR_GEN2)
+	if (info->internal_delay)
 		ravb_set_delay_mode(ndev);
 
 	/* Restore descriptor base address table */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH net-next v2 8/8] ravb: Add tx_drop_cntrs to struct ravb_hw_info
  2021-08-02 10:26 [PATCH net-next v2 0/8] Add Gigabit Ethernet driver support Biju Das
                   ` (6 preceding siblings ...)
  2021-08-02 10:26 ` [PATCH net-next v2 7/8] ravb: Add internal delay hw feature " Biju Das
@ 2021-08-02 10:26 ` Biju Das
  2021-08-02 15:14   ` Andrew Lunn
  2021-08-04 20:50   ` Sergei Shtylyov
  7 siblings, 2 replies; 59+ messages in thread
From: Biju Das @ 2021-08-02 10:26 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: Biju Das, Sergei Shtylyov, Geert Uytterhoeven, Sergey Shtylyov,
	Adam Ford, Andrew Lunn, Yuusuke Ashizuka, Yoshihiro Shimoda,
	netdev, linux-renesas-soc, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad

The register for retrieving TX drop counters is present only on R-Car Gen3
and RZ/G2L; it is not present on R-Car Gen2.

Add the tx_drop_cntrs hw feature bit to struct ravb_hw_info, to enable this
feature specifically for R-Car Gen3 now and later extend it to RZ/G2L.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
 * Incorporated Andrew and Sergei's review comments for making it smaller patch
   and provided detailed description.
---
 drivers/net/ethernet/renesas/ravb.h      | 1 +
 drivers/net/ethernet/renesas/ravb_main.c | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 0d640dbe1eed..35fbb9f60ba8 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -1001,6 +1001,7 @@ struct ravb_hw_info {
 
 	/* hardware features */
 	unsigned internal_delay:1;	/* RAVB has internal delays */
+	unsigned tx_drop_cntrs:1;	/* RAVB has TX error counters */
 };
 
 struct ravb_private {
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 02acae4d51c1..6af3f978c84c 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1633,13 +1633,14 @@ static u16 ravb_select_queue(struct net_device *ndev, struct sk_buff *skb,
 static struct net_device_stats *ravb_get_stats(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
 	struct net_device_stats *nstats, *stats0, *stats1;
 
 	nstats = &ndev->stats;
 	stats0 = &priv->stats[RAVB_BE];
 	stats1 = &priv->stats[RAVB_NC];
 
-	if (priv->chip_id == RCAR_GEN3) {
+	if (info->tx_drop_cntrs) {
 		nstats->tx_dropped += ravb_read(ndev, TROCR);
 		ravb_write(ndev, 0, TROCR);	/* (write clear) */
 	}
@@ -1940,6 +1941,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.stats_len = ARRAY_SIZE(ravb_gstrings_stats),
 	.skb_sz = RX_BUF_SZ + RAVB_ALIGN - 1,
 	.internal_delay = 1,
+	.tx_drop_cntrs = 1,
 };
 
 static const struct ravb_hw_info ravb_gen2_hw_info = {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to driver data
  2021-08-02 10:26 ` [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to driver data Biju Das
@ 2021-08-02 15:02   ` Andrew Lunn
  2021-08-02 20:42   ` Sergei Shtylyov
  2021-08-09 12:06   ` Geert Uytterhoeven
  2 siblings, 0 replies; 59+ messages in thread
From: Andrew Lunn @ 2021-08-02 15:02 UTC (permalink / raw)
  To: Biju Das
  Cc: David S. Miller, Jakub Kicinski, Sergei Shtylyov,
	Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Yuusuke Ashizuka,
	Yoshihiro Shimoda, netdev, linux-renesas-soc, Chris Paterson,
	Biju Das, Prabhakar Mahadev Lad

On Mon, Aug 02, 2021 at 11:26:47AM +0100, Biju Das wrote:
> The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC are
> similar to the R-Car Ethernet AVB IP. With a few changes in the driver we
> can support both IPs.
> 
> Currently a runtime decision based on the chip type is used to distinguish
> the HW differences between the SoC families.
> 
> The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2 and
> RZ/G2L it is 2. For cases like this it is better to select the number of
> TX descriptors by using a structure with a value, rather than a runtime
> decision based on the chip type.
> 
> This patch adds the num_tx_desc variable to struct ravb_hw_info and also
> replaces the driver data chip type with struct ravb_hw_info by moving chip
> type to it.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Hi Biju

This is better. A lot clearer what is going on. I personally would of
done the num_tx_desc change as a separate patch, but this is O.K.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 2/8] ravb: Add skb_sz to struct ravb_hw_info
  2021-08-02 10:26 ` [PATCH net-next v2 2/8] ravb: Add skb_sz to struct ravb_hw_info Biju Das
@ 2021-08-02 15:08   ` Andrew Lunn
  2021-08-02 20:54   ` Sergei Shtylyov
  1 sibling, 0 replies; 59+ messages in thread
From: Andrew Lunn @ 2021-08-02 15:08 UTC (permalink / raw)
  To: Biju Das
  Cc: David S. Miller, Jakub Kicinski, Sergei Shtylyov,
	Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Yuusuke Ashizuka,
	Yoshihiro Shimoda, netdev, linux-renesas-soc, Chris Paterson,
	Biju Das, Prabhakar Mahadev Lad

On Mon, Aug 02, 2021 at 11:26:48AM +0100, Biju Das wrote:
> The maximum descriptor size that can be specified on the reception side for
> R-Car is 2048 bytes, whereas for RZ/G2L it is 8096.
> 
> Add the skb_size variable to struct ravb_hw_info for allocating different
> skb buffer sizes for R-Car and RZ/G2L using the netdev_alloc_skb function.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 3/8] ravb: Add num_gstat_queue to struct ravb_hw_info
  2021-08-02 10:26 ` [PATCH net-next v2 3/8] ravb: Add num_gstat_queue " Biju Das
@ 2021-08-02 15:09   ` Andrew Lunn
  2021-08-03 18:21   ` Sergei Shtylyov
  1 sibling, 0 replies; 59+ messages in thread
From: Andrew Lunn @ 2021-08-02 15:09 UTC (permalink / raw)
  To: Biju Das
  Cc: David S. Miller, Jakub Kicinski, Sergei Shtylyov,
	Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Yuusuke Ashizuka,
	Yoshihiro Shimoda, netdev, linux-renesas-soc, Chris Paterson,
	Biju Das, Prabhakar Mahadev Lad

On Mon, Aug 02, 2021 at 11:26:49AM +0100, Biju Das wrote:
> The number of queues used in retrieving device stats for R-Car is 2,
> whereas for RZ/G2L it is 1.
> 
> Add the num_gstat_queue variable to struct ravb_hw_info, to add subsequent
> SoCs without any code changes to the ravb_get_ethtool_stats function.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 5/8] ravb: Add gstrings_stats and gstrings_size to struct ravb_hw_info
  2021-08-02 10:26 ` [PATCH net-next v2 5/8] ravb: Add gstrings_stats and gstrings_size " Biju Das
@ 2021-08-02 15:11   ` Andrew Lunn
  2021-08-04 20:36   ` Sergei Shtylyov
  1 sibling, 0 replies; 59+ messages in thread
From: Andrew Lunn @ 2021-08-02 15:11 UTC (permalink / raw)
  To: Biju Das
  Cc: David S. Miller, Jakub Kicinski, Sergei Shtylyov,
	Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Yuusuke Ashizuka,
	Yoshihiro Shimoda, netdev, linux-renesas-soc, Chris Paterson,
	Biju Das, Prabhakar Mahadev Lad

On Mon, Aug 02, 2021 at 11:26:51AM +0100, Biju Das wrote:
> The device stats strings for R-Car and RZ/G2L are different.
> 
> R-Car provides 30 device stats, whereas RZ/G2L provides only 15. In
> addition, RZ/G2L has stats "rx_queue_0_csum_offload_errors" instead of
> "rx_queue_0_missed_errors".
> 
> Add structure variables gstrings_stats and gstrings_size to struct
> ravb_hw_info, so that subsequent SoCs can be added without any code
> changes in the ravb_get_strings function.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 6/8] ravb: Add net_features and net_hw_features to struct ravb_hw_info
  2021-08-02 10:26 ` [PATCH net-next v2 6/8] ravb: Add net_features and net_hw_features " Biju Das
@ 2021-08-02 15:12   ` Andrew Lunn
  2021-08-05 19:07   ` Sergei Shtylyov
  2021-08-06 20:31   ` Sergei Shtylyov
  2 siblings, 0 replies; 59+ messages in thread
From: Andrew Lunn @ 2021-08-02 15:12 UTC (permalink / raw)
  To: Biju Das
  Cc: David S. Miller, Jakub Kicinski, Sergei Shtylyov,
	Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Yuusuke Ashizuka,
	Yoshihiro Shimoda, netdev, linux-renesas-soc, Chris Paterson,
	Biju Das, Prabhakar Mahadev Lad

On Mon, Aug 02, 2021 at 11:26:52AM +0100, Biju Das wrote:
> On R-Car the checksum calculation on RX frames is done by the E-MAC
> module, whereas on RZ/G2L it is done by the TOE.
> 
> TOE calculates the checksum of received frames from E-MAC and outputs it to
> DMAC. TOE also calculates the checksum of transmission frames from DMAC and
> outputs it E-MAC.
> 
> Add net_features and net_hw_features to struct ravb_hw_info, to support
> subsequent SoCs without any code changes in the ravb_probe function.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 7/8] ravb: Add internal delay hw feature to struct ravb_hw_info
  2021-08-02 10:26 ` [PATCH net-next v2 7/8] ravb: Add internal delay hw feature " Biju Das
@ 2021-08-02 15:13   ` Andrew Lunn
  2021-08-03 21:06   ` Sergei Shtylyov
  2021-08-03 21:12   ` Sergei Shtylyov
  2 siblings, 0 replies; 59+ messages in thread
From: Andrew Lunn @ 2021-08-02 15:13 UTC (permalink / raw)
  To: Biju Das
  Cc: David S. Miller, Jakub Kicinski, Sergei Shtylyov,
	Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Yuusuke Ashizuka,
	Yoshihiro Shimoda, netdev, linux-renesas-soc, Chris Paterson,
	Biju Das, Prabhakar Mahadev Lad

On Mon, Aug 02, 2021 at 11:26:53AM +0100, Biju Das wrote:
> R-Car Gen3 supports TX and RX clock internal delay modes, whereas R-Car
> Gen2 and RZ/G2L do not support it.
> Add an internal_delay hw feature bit to struct ravb_hw_info to enable this
> only for R-Car Gen3.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 8/8] ravb: Add tx_drop_cntrs to struct ravb_hw_info
  2021-08-02 10:26 ` [PATCH net-next v2 8/8] ravb: Add tx_drop_cntrs " Biju Das
@ 2021-08-02 15:14   ` Andrew Lunn
  2021-08-04 20:50   ` Sergei Shtylyov
  1 sibling, 0 replies; 59+ messages in thread
From: Andrew Lunn @ 2021-08-02 15:14 UTC (permalink / raw)
  To: Biju Das
  Cc: David S. Miller, Jakub Kicinski, Sergei Shtylyov,
	Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Yuusuke Ashizuka,
	Yoshihiro Shimoda, netdev, linux-renesas-soc, Chris Paterson,
	Biju Das, Prabhakar Mahadev Lad

On Mon, Aug 02, 2021 at 11:26:54AM +0100, Biju Das wrote:
> The register for retrieving TX drop counters is present only on R-Car Gen3
> and RZ/G2L; it is not present on R-Car Gen2.
> 
> Add the tx_drop_cntrs hw feature bit to struct ravb_hw_info, to enable this
> feature specifically for R-Car Gen3 now and later extend it to RZ/G2L.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to driver data
  2021-08-02 10:26 ` [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to driver data Biju Das
  2021-08-02 15:02   ` Andrew Lunn
@ 2021-08-02 20:42   ` Sergei Shtylyov
  2021-08-03  5:57     ` Biju Das
  2021-08-09 12:06   ` Geert Uytterhoeven
  2 siblings, 1 reply; 59+ messages in thread
From: Sergei Shtylyov @ 2021-08-02 20:42 UTC (permalink / raw)
  To: Biju Das, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

On 8/2/21 1:26 PM, Biju Das wrote:

> The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC are
> similar to the R-Car Ethernet AVB IP. With a few changes in the driver we
> can support both IPs.
> 
> Currently a runtime decision based on the chip type is used to distinguish
> the HW differences between the SoC families.
> 
> The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2 and
> RZ/G2L it is 2. For cases like this it is better to select the number of
> TX descriptors by using a structure with a value, rather than a runtime
> decision based on the chip type.
> 
> This patch adds the num_tx_desc variable to struct ravb_hw_info and also
> replaces the driver data chip type with struct ravb_hw_info by moving chip
> type to it.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v2:
>  * Incorporated Andrew and Sergei's review comments for making it smaller patch
>    and provided detailed description.
> ---
>  drivers/net/ethernet/renesas/ravb.h      |  7 +++++
>  drivers/net/ethernet/renesas/ravb_main.c | 38 +++++++++++++++---------
>  2 files changed, 31 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
> index 80e62ca2e3d3..cfb972c05b34 100644
> --- a/drivers/net/ethernet/renesas/ravb.h
> +++ b/drivers/net/ethernet/renesas/ravb.h
> @@ -988,6 +988,11 @@ enum ravb_chip_id {
>  	RCAR_GEN3,
>  };
>  
> +struct ravb_hw_info {
> +	enum ravb_chip_id chip_id;
> +	int num_tx_desc;

   I think this is rather the driver's choice, than the h/w feature... Perhaps a rename
would help with that? :-)

[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 2/8] ravb: Add skb_sz to struct ravb_hw_info
  2021-08-02 10:26 ` [PATCH net-next v2 2/8] ravb: Add skb_sz to struct ravb_hw_info Biju Das
  2021-08-02 15:08   ` Andrew Lunn
@ 2021-08-02 20:54   ` Sergei Shtylyov
  2021-08-03  7:13     ` Biju Das
  1 sibling, 1 reply; 59+ messages in thread
From: Sergei Shtylyov @ 2021-08-02 20:54 UTC (permalink / raw)
  To: Biju Das, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

On 8/2/21 1:26 PM, Biju Das wrote:

> The maximum descriptor size that can be specified on the reception side for
> R-Car is 2048 bytes, whereas for RZ/G2L it is 8096.
> 
> Add the skb_size variable to struct ravb_hw_info for allocating different
> skb buffer sizes for R-Car and RZ/G2L using the netdev_alloc_skb function.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v2:
>  * Incorporated Andrew and Sergei's review comments for making it smaller patch
>    and provided detailed description.
> ---
>  drivers/net/ethernet/renesas/ravb.h      |  1 +
>  drivers/net/ethernet/renesas/ravb_main.c | 10 ++++++----
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
> index cfb972c05b34..16d1711a0731 100644
> --- a/drivers/net/ethernet/renesas/ravb.h
> +++ b/drivers/net/ethernet/renesas/ravb.h
> @@ -991,6 +991,7 @@ enum ravb_chip_id {
>  struct ravb_hw_info {
>  	enum ravb_chip_id chip_id;
>  	int num_tx_desc;
> +	size_t skb_sz;

   Bad naming -- refers to software ISO hatdware, I suggest max_rx_len or s/th of that sort.

[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 59+ messages in thread

* RE: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to driver data
  2021-08-02 20:42   ` Sergei Shtylyov
@ 2021-08-03  5:57     ` Biju Das
  2021-08-03  6:36       ` Biju Das
  2021-08-04 19:27       ` Sergei Shtylyov
  0 siblings, 2 replies; 59+ messages in thread
From: Biju Das @ 2021-08-03  5:57 UTC (permalink / raw)
  To: Sergei Shtylyov, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hi Sergei,

Thanks for the feedback.

> Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to
> driver data
> 
> On 8/2/21 1:26 PM, Biju Das wrote:
> 
> > The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC
> > are similar to the R-Car Ethernet AVB IP. With a few changes in the
> > driver we can support both IPs.
> >
> > Currently a runtime decision based on the chip type is used to
> > distinguish the HW differences between the SoC families.
> >
> > The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2
> > and RZ/G2L it is 2. For cases like this it is better to select the
> > number of TX descriptors by using a structure with a value, rather
> > than a runtime decision based on the chip type.
> >
> > This patch adds the num_tx_desc variable to struct ravb_hw_info and
> > also replaces the driver data chip type with struct ravb_hw_info by
> > moving chip type to it.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > v2:
> >  * Incorporated Andrew and Sergei's review comments for making it
> smaller patch
> >    and provided detailed description.
> > ---
> >  drivers/net/ethernet/renesas/ravb.h      |  7 +++++
> >  drivers/net/ethernet/renesas/ravb_main.c | 38
> > +++++++++++++++---------
> >  2 files changed, 31 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/renesas/ravb.h
> > b/drivers/net/ethernet/renesas/ravb.h
> > index 80e62ca2e3d3..cfb972c05b34 100644
> > --- a/drivers/net/ethernet/renesas/ravb.h
> > +++ b/drivers/net/ethernet/renesas/ravb.h
> > @@ -988,6 +988,11 @@ enum ravb_chip_id {
> >  	RCAR_GEN3,
> >  };
> >
> > +struct ravb_hw_info {
> > +	enum ravb_chip_id chip_id;
> > +	int num_tx_desc;
> 
>    I think this is rather the driver's choice, than the h/w feature...
> Perhaps a rename would help with that? :-)

It is consistent with current naming convention used by the driver. NUM_TX_DESC macro is replaced by num_tx_desc and  the below run time decision based on chip type for H/W configuration for Gen2/Gen3 is replaced by info->num_tx_desc.

priv->num_tx_desc = chip_id == RCAR_GEN2 ? NUM_TX_DESC_GEN2 : NUM_TX_DESC_GEN3;

Please let me know, if I am missing anything,

Previously there is a suggestion to change the generic struct ravb_driver_data(which holds driver differences and HW features) with struct ravb_hw_info. 

Regards,
Biju

> 
> [...]
> 
> MBR, Sergei

^ permalink raw reply	[flat|nested] 59+ messages in thread

* RE: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to driver data
  2021-08-03  5:57     ` Biju Das
@ 2021-08-03  6:36       ` Biju Das
  2021-08-04 19:27       ` Sergei Shtylyov
  1 sibling, 0 replies; 59+ messages in thread
From: Biju Das @ 2021-08-03  6:36 UTC (permalink / raw)
  To: Biju Das, Sergei Shtylyov, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hi Sergei,

> Subject: RE: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to
> driver data
> 
> Hi Sergei,
> 
> Thanks for the feedback.
> 
> > Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to
> > driver data
> >
> > On 8/2/21 1:26 PM, Biju Das wrote:
> >
> > > The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC
> > > are similar to the R-Car Ethernet AVB IP. With a few changes in the
> > > driver we can support both IPs.
> > >
> > > Currently a runtime decision based on the chip type is used to
> > > distinguish the HW differences between the SoC families.
> > >
> > > The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car
> > > Gen2 and RZ/G2L it is 2. For cases like this it is better to select
> > > the number of TX descriptors by using a structure with a value,
> > > rather than a runtime decision based on the chip type.
> > >
> > > This patch adds the num_tx_desc variable to struct ravb_hw_info and
> > > also replaces the driver data chip type with struct ravb_hw_info by
> > > moving chip type to it.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > ---
> > > v2:
> > >  * Incorporated Andrew and Sergei's review comments for making it
> > smaller patch
> > >    and provided detailed description.
> > > ---
> > >  drivers/net/ethernet/renesas/ravb.h      |  7 +++++
> > >  drivers/net/ethernet/renesas/ravb_main.c | 38
> > > +++++++++++++++---------
> > >  2 files changed, 31 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/renesas/ravb.h
> > > b/drivers/net/ethernet/renesas/ravb.h
> > > index 80e62ca2e3d3..cfb972c05b34 100644
> > > --- a/drivers/net/ethernet/renesas/ravb.h
> > > +++ b/drivers/net/ethernet/renesas/ravb.h
> > > @@ -988,6 +988,11 @@ enum ravb_chip_id {
> > >  	RCAR_GEN3,
> > >  };
> > >
> > > +struct ravb_hw_info {
> > > +	enum ravb_chip_id chip_id;
> > > +	int num_tx_desc;
> >
> >    I think this is rather the driver's choice, than the h/w feature...
> > Perhaps a rename would help with that? :-)
> 
> It is consistent with current naming convention used by the driver.
> NUM_TX_DESC macro is replaced by num_tx_desc.
      
So the name should be ok. 

Indeed we are agreed to add function pointers to struct ravb_hw_info to avoid another level of indirection.

If the concern is related to duplication of data(ie,priv->num_tx_desc vs info->num_tx_desc)
I have a plan to remove priv->num_tx_desc with info->num_tx_desc  later.

Regards,
Biju





^ permalink raw reply	[flat|nested] 59+ messages in thread

* RE: [PATCH net-next v2 2/8] ravb: Add skb_sz to struct ravb_hw_info
  2021-08-02 20:54   ` Sergei Shtylyov
@ 2021-08-03  7:13     ` Biju Das
  0 siblings, 0 replies; 59+ messages in thread
From: Biju Das @ 2021-08-03  7:13 UTC (permalink / raw)
  To: Sergei Shtylyov, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad


Hi Sergei,

Thanks for the feedback.

> Subject: Re: [PATCH net-next v2 2/8] ravb: Add skb_sz to struct
> ravb_hw_info
> 
> On 8/2/21 1:26 PM, Biju Das wrote:
> 
> > The maximum descriptor size that can be specified on the reception
> > side for R-Car is 2048 bytes, whereas for RZ/G2L it is 8096.
> >
> > Add the skb_size variable to struct ravb_hw_info for allocating
> > different skb buffer sizes for R-Car and RZ/G2L using the
> netdev_alloc_skb function.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > v2:
> >  * Incorporated Andrew and Sergei's review comments for making it
> smaller patch
> >    and provided detailed description.
> > ---
> >  drivers/net/ethernet/renesas/ravb.h      |  1 +
> >  drivers/net/ethernet/renesas/ravb_main.c | 10 ++++++----
> >  2 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/renesas/ravb.h
> > b/drivers/net/ethernet/renesas/ravb.h
> > index cfb972c05b34..16d1711a0731 100644
> > --- a/drivers/net/ethernet/renesas/ravb.h
> > +++ b/drivers/net/ethernet/renesas/ravb.h
> > @@ -991,6 +991,7 @@ enum ravb_chip_id {  struct ravb_hw_info {
> >  	enum ravb_chip_id chip_id;
> >  	int num_tx_desc;
> > +	size_t skb_sz;
> 
>    Bad naming -- refers to software ISO hatdware, I suggest max_rx_len or
> s/th of that sort.

From the api description
*	netdev_alloc_skb - allocate an skbuff for rx on a specific device
*	@length: length to allocate

Since it allocates skbuff, I thought skb_sz (size of skb buffer) is a good name.

Is there any restriction in Linux, not to use skb_sz because of 
"software ISO hardware" as you mentioned?
I may have chosen bad name because of this restriction. 

Please correct me, if that is the case.

Regards,
Biju

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 3/8] ravb: Add num_gstat_queue to struct ravb_hw_info
  2021-08-02 10:26 ` [PATCH net-next v2 3/8] ravb: Add num_gstat_queue " Biju Das
  2021-08-02 15:09   ` Andrew Lunn
@ 2021-08-03 18:21   ` Sergei Shtylyov
  2021-08-03 19:13     ` Biju Das
  1 sibling, 1 reply; 59+ messages in thread
From: Sergei Shtylyov @ 2021-08-03 18:21 UTC (permalink / raw)
  To: Biju Das, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hello!

On 8/2/21 1:26 PM, Biju Das wrote:

> The number of queues used in retrieving device stats for R-Car is 2,
> whereas for RZ/G2L it is 1.

   Mhm, how many RX queues are on your platform, 1? Then we don't need so specific name, just num_rx_queue.

> Add the num_gstat_queue variable to struct ravb_hw_info, to add subsequent
> SoCs without any code changes to the ravb_get_ethtool_stats function.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 4/8] ravb: Add stats_len to struct ravb_hw_info
  2021-08-02 10:26 ` [PATCH net-next v2 4/8] ravb: Add stats_len " Biju Das
@ 2021-08-03 18:35   ` Sergei Shtylyov
  2021-08-03 18:47     ` Biju Das
  0 siblings, 1 reply; 59+ messages in thread
From: Sergei Shtylyov @ 2021-08-03 18:35 UTC (permalink / raw)
  To: Biju Das, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

On 8/2/21 1:26 PM, Biju Das wrote:

> R-Car provides 30 device stats, whereas RZ/G2L provides only 15. In
> addition, RZ/G2L has stats "rx_queue_0_csum_offload_errors" instead of
> "rx_queue_0_missed_errors".
> 
> Replace RAVB_STATS_LEN macro with a structure variable stats_len to
> struct ravb_hw_info, to support subsequent SoCs without any code changes
> to the ravb_get_sset_count function.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
[...]

   Finally a patch that I can agree with. :-)

Reviewed-by: ergei Shtylyov <sergei.shtylyov@gmail.com>

MBR, Sergei

^ permalink raw reply	[flat|nested] 59+ messages in thread

* RE: [PATCH net-next v2 4/8] ravb: Add stats_len to struct ravb_hw_info
  2021-08-03 18:35   ` Sergei Shtylyov
@ 2021-08-03 18:47     ` Biju Das
  2021-08-03 19:20       ` Sergei Shtylyov
  0 siblings, 1 reply; 59+ messages in thread
From: Biju Das @ 2021-08-03 18:47 UTC (permalink / raw)
  To: Sergei Shtylyov, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad


Hi Sergei,

> Subject: Re: [PATCH net-next v2 4/8] ravb: Add stats_len to struct
> ravb_hw_info
> 
> On 8/2/21 1:26 PM, Biju Das wrote:
> 
> > R-Car provides 30 device stats, whereas RZ/G2L provides only 15. In
> > addition, RZ/G2L has stats "rx_queue_0_csum_offload_errors" instead of
> > "rx_queue_0_missed_errors".
> >
> > Replace RAVB_STATS_LEN macro with a structure variable stats_len to
> > struct ravb_hw_info, to support subsequent SoCs without any code
> > changes to the ravb_get_sset_count function.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> [...]
> 
>    Finally a patch that I can agree with. :-)
> 
> Reviewed-by: ergei Shtylyov <sergei.shtylyov@gmail.com>
              ^Typo here.

Cheers,
Biju

^ permalink raw reply	[flat|nested] 59+ messages in thread

* RE: [PATCH net-next v2 3/8] ravb: Add num_gstat_queue to struct ravb_hw_info
  2021-08-03 18:21   ` Sergei Shtylyov
@ 2021-08-03 19:13     ` Biju Das
  2021-08-03 19:22       ` Sergei Shtylyov
  0 siblings, 1 reply; 59+ messages in thread
From: Biju Das @ 2021-08-03 19:13 UTC (permalink / raw)
  To: Sergei Shtylyov, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hi Sergei,

Thanks for the feedback.

> Subject: Re: [PATCH net-next v2 3/8] ravb: Add num_gstat_queue to struct
> ravb_hw_info
> 
> Hello!
> 
> On 8/2/21 1:26 PM, Biju Das wrote:
> 
> > The number of queues used in retrieving device stats for R-Car is 2,
> > whereas for RZ/G2L it is 1.

> 
>    Mhm, how many RX queues are on your platform, 1? Then we don't need so
> specific name, just num_rx_queue.

There are 2 RX queues, but we provide only device stats information from first queue.

R-Car = 2x15 = 30 device stats
RZ/G2L = 1x15 = 15 device stats.

Cheers,
Biju


> 
> > Add the num_gstat_queue variable to struct ravb_hw_info, to add
> > subsequent SoCs without any code changes to the ravb_get_ethtool_stats
> function.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> [...]
> 
> MBR, Sergei

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 4/8] ravb: Add stats_len to struct ravb_hw_info
  2021-08-03 18:47     ` Biju Das
@ 2021-08-03 19:20       ` Sergei Shtylyov
  0 siblings, 0 replies; 59+ messages in thread
From: Sergei Shtylyov @ 2021-08-03 19:20 UTC (permalink / raw)
  To: Biju Das, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

On 8/3/21 9:47 PM, Biju Das wrote:

[...]
>>> R-Car provides 30 device stats, whereas RZ/G2L provides only 15. In
>>> addition, RZ/G2L has stats "rx_queue_0_csum_offload_errors" instead of
>>> "rx_queue_0_missed_errors".
>>>
>>> Replace RAVB_STATS_LEN macro with a structure variable stats_len to
>>> struct ravb_hw_info, to support subsequent SoCs without any code
>>> changes to the ravb_get_sset_count function.
>>>
>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>> [...]
>>
>>    Finally a patch that I can agree with. :-)
>>
>> Reviewed-by: ergei Shtylyov <sergei.shtylyov@gmail.com>
>               ^Typo here.

   Sorry, here's a good one:

Reviewed-by: Sergei Shtylyov <sergei.shtylyov@gmail.com>

> Cheers,
> Biju

MBR, Sergei

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 3/8] ravb: Add num_gstat_queue to struct ravb_hw_info
  2021-08-03 19:13     ` Biju Das
@ 2021-08-03 19:22       ` Sergei Shtylyov
  2021-08-03 19:47         ` Biju Das
  0 siblings, 1 reply; 59+ messages in thread
From: Sergei Shtylyov @ 2021-08-03 19:22 UTC (permalink / raw)
  To: Biju Das, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

On 8/3/21 10:13 PM, Biju Das wrote:

[...]
>>> The number of queues used in retrieving device stats for R-Car is 2,
>>> whereas for RZ/G2L it is 1.
> 
>>
>>    Mhm, how many RX queues are on your platform, 1? Then we don't need so
>> specific name, just num_rx_queue.
> 
> There are 2 RX queues, but we provide only device stats information from first queue.
> 
> R-Car = 2x15 = 30 device stats
> RZ/G2L = 1x15 = 15 device stats.

    That's pretty strange... how the RX queue #1 is called? How many RX queues are, at all?

> Cheers,
> Biju

MBR, Sergei

^ permalink raw reply	[flat|nested] 59+ messages in thread

* RE: [PATCH net-next v2 3/8] ravb: Add num_gstat_queue to struct ravb_hw_info
  2021-08-03 19:22       ` Sergei Shtylyov
@ 2021-08-03 19:47         ` Biju Das
  2021-08-17 15:08           ` Biju Das
  0 siblings, 1 reply; 59+ messages in thread
From: Biju Das @ 2021-08-03 19:47 UTC (permalink / raw)
  To: Sergei Shtylyov, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hi Sergei,

> Subject: Re: [PATCH net-next v2 3/8] ravb: Add num_gstat_queue to struct
> ravb_hw_info
> 
> On 8/3/21 10:13 PM, Biju Das wrote:
> 
> [...]
> >>> The number of queues used in retrieving device stats for R-Car is 2,
> >>> whereas for RZ/G2L it is 1.
> >
> >>
> >>    Mhm, how many RX queues are on your platform, 1? Then we don't
> >> need so specific name, just num_rx_queue.
> >
> > There are 2 RX queues, but we provide only device stats information from
> first queue.
> >
> > R-Car = 2x15 = 30 device stats
> > RZ/G2L = 1x15 = 15 device stats.
> 
>     That's pretty strange... how the RX queue #1 is called? How many RX
> queues are, at all?

For both R-Car and RZ/G2L,
------------------------- 
#define NUM_RX_QUEUE    2
#define NUM_TX_QUEUE    2

Target device stat output for RZ/G2L:-
------------------------------------
root@smarc-rzg2l:~# ethtool -S eth0
NIC statistics:
     rx_queue_0_current: 21852
     tx_queue_0_current: 18854
     rx_queue_0_dirty: 21852
     tx_queue_0_dirty: 18854
     rx_queue_0_packets: 21852
     tx_queue_0_packets: 9427
     rx_queue_0_bytes: 28224093
     tx_queue_0_bytes: 1659438
     rx_queue_0_mcast_packets: 498
     rx_queue_0_errors: 0
     rx_queue_0_crc_errors: 0
     rx_queue_0_frame_errors: 0
     rx_queue_0_length_errors: 0
     rx_queue_0_csum_offload_errors: 0
     rx_queue_0_over_errors: 0
root@smarc-rzg2l:~#


Target device stat output for R-Car Gen3:-
----------------------------------------
root@hihope-rzg2m:~#  ethtool -S eth0
NIC statistics:
     rx_queue_0_current: 34215
     tx_queue_0_current: 14158
     rx_queue_0_dirty: 34215
     tx_queue_0_dirty: 14158
     rx_queue_0_packets: 34215
     tx_queue_0_packets: 14158
     rx_queue_0_bytes: 38313586
     tx_queue_0_bytes: 3222182
     rx_queue_0_mcast_packets: 499
     rx_queue_0_errors: 0
     rx_queue_0_crc_errors: 0
     rx_queue_0_frame_errors: 0
     rx_queue_0_length_errors: 0
     rx_queue_0_missed_errors: 0
     rx_queue_0_over_errors: 0
     rx_queue_1_current: 0
     tx_queue_1_current: 0
     rx_queue_1_dirty: 0
     tx_queue_1_dirty: 0
     rx_queue_1_packets: 0
     tx_queue_1_packets: 0
     rx_queue_1_bytes: 0
     tx_queue_1_bytes: 0
     rx_queue_1_mcast_packets: 0
     rx_queue_1_errors: 0
     rx_queue_1_crc_errors: 0
     rx_queue_1_frame_errors: 0
     rx_queue_1_length_errors: 0
     rx_queue_1_missed_errors: 0
     rx_queue_1_over_errors: 0

Cheers,
Biju


^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 7/8] ravb: Add internal delay hw feature to struct ravb_hw_info
  2021-08-02 10:26 ` [PATCH net-next v2 7/8] ravb: Add internal delay hw feature " Biju Das
  2021-08-02 15:13   ` Andrew Lunn
@ 2021-08-03 21:06   ` Sergei Shtylyov
  2021-08-04  6:19     ` Biju Das
  2021-08-03 21:12   ` Sergei Shtylyov
  2 siblings, 1 reply; 59+ messages in thread
From: Sergei Shtylyov @ 2021-08-03 21:06 UTC (permalink / raw)
  To: Biju Das, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

On 8/2/21 1:26 PM, Biju Das wrote:

> R-Car Gen3 supports TX and RX clock internal delay modes, whereas R-Car
> Gen2 and RZ/G2L do not support it.
> Add an internal_delay hw feature bit to struct ravb_hw_info to enable this
> only for R-Car Gen3.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
[...]

   OK, this one also seems uncontroversial:

Reviewed-by: Sergei Shtylyov <sergei.shtylyov@gmail.com>

MBR, Sergei

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 7/8] ravb: Add internal delay hw feature to struct ravb_hw_info
  2021-08-02 10:26 ` [PATCH net-next v2 7/8] ravb: Add internal delay hw feature " Biju Das
  2021-08-02 15:13   ` Andrew Lunn
  2021-08-03 21:06   ` Sergei Shtylyov
@ 2021-08-03 21:12   ` Sergei Shtylyov
  2021-08-04  5:13     ` Biju Das
  2 siblings, 1 reply; 59+ messages in thread
From: Sergei Shtylyov @ 2021-08-03 21:12 UTC (permalink / raw)
  To: Biju Das, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

On 8/2/21 1:26 PM, Biju Das wrote:

> R-Car Gen3 supports TX and RX clock internal delay modes, whereas R-Car
> Gen2 and RZ/G2L do not support it.
> Add an internal_delay hw feature bit to struct ravb_hw_info to enable this
> only for R-Car Gen3.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v2:
>  * Incorporated Andrew and Sergei's review comments for making it smaller patch
>    and provided detailed description.
> ---
>  drivers/net/ethernet/renesas/ravb.h      | 3 +++
>  drivers/net/ethernet/renesas/ravb_main.c | 6 ++++--
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
> index 3df813b2e253..0d640dbe1eed 100644
> --- a/drivers/net/ethernet/renesas/ravb.h
> +++ b/drivers/net/ethernet/renesas/ravb.h
> @@ -998,6 +998,9 @@ struct ravb_hw_info {
>  	int num_tx_desc;
>  	int stats_len;
>  	size_t skb_sz;
> +
> +	/* hardware features */
> +	unsigned internal_delay:1;	/* RAVB has internal delays */

   Oops, missed it initially:
   RAVB? That's not a device name, according to the manuals. It seems to be the driver's name.
I'd drop this comment...

[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 59+ messages in thread

* RE: [PATCH net-next v2 7/8] ravb: Add internal delay hw feature to struct ravb_hw_info
  2021-08-03 21:12   ` Sergei Shtylyov
@ 2021-08-04  5:13     ` Biju Das
  2021-08-04  9:51       ` Sergey Shtylyov
  2021-08-04 10:20       ` Sergei Shtylyov
  0 siblings, 2 replies; 59+ messages in thread
From: Biju Das @ 2021-08-04  5:13 UTC (permalink / raw)
  To: Sergei Shtylyov, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hi Sergei,

Thanks for the feedback

> Subject: Re: [PATCH net-next v2 7/8] ravb: Add internal delay hw feature
> to struct ravb_hw_info
> 
> On 8/2/21 1:26 PM, Biju Das wrote:
> 
> > R-Car Gen3 supports TX and RX clock internal delay modes, whereas
> > R-Car
> > Gen2 and RZ/G2L do not support it.
> > Add an internal_delay hw feature bit to struct ravb_hw_info to enable
> > this only for R-Car Gen3.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > v2:
> >  * Incorporated Andrew and Sergei's review comments for making it
> smaller patch
> >    and provided detailed description.
> > ---
> >  drivers/net/ethernet/renesas/ravb.h      | 3 +++
> >  drivers/net/ethernet/renesas/ravb_main.c | 6 ++++--
> >  2 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/renesas/ravb.h
> > b/drivers/net/ethernet/renesas/ravb.h
> > index 3df813b2e253..0d640dbe1eed 100644
> > --- a/drivers/net/ethernet/renesas/ravb.h
> > +++ b/drivers/net/ethernet/renesas/ravb.h
> > @@ -998,6 +998,9 @@ struct ravb_hw_info {
> >  	int num_tx_desc;
> >  	int stats_len;
> >  	size_t skb_sz;
> > +
> > +	/* hardware features */
> > +	unsigned internal_delay:1;	/* RAVB has internal delays */
> 
>    Oops, missed it initially:
>    RAVB? That's not a device name, according to the manuals. It seems to
> be the driver's name.

OK. will change it to AVB-DMAC has internal delays.

Cheers,
Biju


^ permalink raw reply	[flat|nested] 59+ messages in thread

* RE: [PATCH net-next v2 7/8] ravb: Add internal delay hw feature to struct ravb_hw_info
  2021-08-03 21:06   ` Sergei Shtylyov
@ 2021-08-04  6:19     ` Biju Das
  0 siblings, 0 replies; 59+ messages in thread
From: Biju Das @ 2021-08-04  6:19 UTC (permalink / raw)
  To: Sergei Shtylyov, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hi Sergei,

Thanks for the feedback.

> Subject: Re: [PATCH net-next v2 7/8] ravb: Add internal delay hw feature
> to struct ravb_hw_info
> 
> On 8/2/21 1:26 PM, Biju Das wrote:
> 
> > R-Car Gen3 supports TX and RX clock internal delay modes, whereas
> > R-Car
> > Gen2 and RZ/G2L do not support it.
> > Add an internal_delay hw feature bit to struct ravb_hw_info to enable
> > this only for R-Car Gen3.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> [...]
> 
>    OK, this one also seems uncontroversial:

So far the comments I received

1) I have replaced NUM_TX_DESC to num_tx_desc. But you are recommending to rename it,
        is ravb_num_tx_desc good choice?

2) skb_sz to max_rx_len, I am ok for it, if there is no objection from others.

3) patches related to device stats.

I already provided the output of ethtool -S eth0 for both R-Car and RZ/G2L. 

For RZ/G2L there is an "rx_queue_0_csum_offload_errors: 0", instead of
"rx_queue_0_missed_errors: 0", Both uses MSC bit 6 for collecting this info.

To provide correct output to the user using command "ethtool -S eth0",
RZ/G2L need to have a different string LUT.

Q1) Do you agree with this?

Cheers,
Biju

> 
> Reviewed-by: Sergei Shtylyov <sergei.shtylyov@gmail.com>
> 
> MBR, Sergei

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 7/8] ravb: Add internal delay hw feature to struct ravb_hw_info
  2021-08-04  5:13     ` Biju Das
@ 2021-08-04  9:51       ` Sergey Shtylyov
  2021-08-04 10:08         ` Biju Das
  2021-08-04 10:20       ` Sergei Shtylyov
  1 sibling, 1 reply; 59+ messages in thread
From: Sergey Shtylyov @ 2021-08-04  9:51 UTC (permalink / raw)
  To: Biju Das, Sergei Shtylyov, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

On 04.08.2021 8:13, Biju Das wrote:
> Hi Sergei,
> 
> Thanks for the feedback
> 
>> Subject: Re: [PATCH net-next v2 7/8] ravb: Add internal delay hw feature
>> to struct ravb_hw_info
>>
>> On 8/2/21 1:26 PM, Biju Das wrote:
>>
>>> R-Car Gen3 supports TX and RX clock internal delay modes, whereas
>>> R-Car
>>> Gen2 and RZ/G2L do not support it.
>>> Add an internal_delay hw feature bit to struct ravb_hw_info to enable
>>> this only for R-Car Gen3.
>>>
>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>> ---
>>> v2:
>>>   * Incorporated Andrew and Sergei's review comments for making it
>> smaller patch
>>>     and provided detailed description.
>>> ---
>>>   drivers/net/ethernet/renesas/ravb.h      | 3 +++
>>>   drivers/net/ethernet/renesas/ravb_main.c | 6 ++++--
>>>   2 files changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/renesas/ravb.h
>>> b/drivers/net/ethernet/renesas/ravb.h
>>> index 3df813b2e253..0d640dbe1eed 100644
>>> --- a/drivers/net/ethernet/renesas/ravb.h
>>> +++ b/drivers/net/ethernet/renesas/ravb.h
>>> @@ -998,6 +998,9 @@ struct ravb_hw_info {
>>>   	int num_tx_desc;
>>>   	int stats_len;
>>>   	size_t skb_sz;
>>> +
>>> +	/* hardware features */
>>> +	unsigned internal_delay:1;	/* RAVB has internal delays */
>>
>>     Oops, missed it initially:
>>     RAVB? That's not a device name, according to the manuals. It seems to
>> be the driver's name.
> 
> OK. will change it to AVB-DMAC has internal delays.

    Please don't -- E-MAC has them, not AVB-DMAC.

> Cheers,
> Biju

NBR, Sergei

^ permalink raw reply	[flat|nested] 59+ messages in thread

* RE: [PATCH net-next v2 7/8] ravb: Add internal delay hw feature to struct ravb_hw_info
  2021-08-04  9:51       ` Sergey Shtylyov
@ 2021-08-04 10:08         ` Biju Das
  2021-08-04 10:34           ` Sergei Shtylyov
  0 siblings, 1 reply; 59+ messages in thread
From: Biju Das @ 2021-08-04 10:08 UTC (permalink / raw)
  To: Sergey Shtylyov, Sergei Shtylyov, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hi Sergei,

Thanks for feedback

> Subject: Re: [PATCH net-next v2 7/8] ravb: Add internal delay hw feature
> to struct ravb_hw_info
> 
> On 04.08.2021 8:13, Biju Das wrote:
> > Hi Sergei,
> >
> > Thanks for the feedback
> >
> >> Subject: Re: [PATCH net-next v2 7/8] ravb: Add internal delay hw
> >> feature to struct ravb_hw_info
> >>
> >> On 8/2/21 1:26 PM, Biju Das wrote:
> >>
> >>> R-Car Gen3 supports TX and RX clock internal delay modes, whereas
> >>> R-Car
> >>> Gen2 and RZ/G2L do not support it.
> >>> Add an internal_delay hw feature bit to struct ravb_hw_info to
> >>> enable this only for R-Car Gen3.
> >>>
> >>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>> ---
> >>> v2:
> >>>   * Incorporated Andrew and Sergei's review comments for making it
> >> smaller patch
> >>>     and provided detailed description.
> >>> ---
> >>>   drivers/net/ethernet/renesas/ravb.h      | 3 +++
> >>>   drivers/net/ethernet/renesas/ravb_main.c | 6 ++++--
> >>>   2 files changed, 7 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/net/ethernet/renesas/ravb.h
> >>> b/drivers/net/ethernet/renesas/ravb.h
> >>> index 3df813b2e253..0d640dbe1eed 100644
> >>> --- a/drivers/net/ethernet/renesas/ravb.h
> >>> +++ b/drivers/net/ethernet/renesas/ravb.h
> >>> @@ -998,6 +998,9 @@ struct ravb_hw_info {
> >>>   	int num_tx_desc;
> >>>   	int stats_len;
> >>>   	size_t skb_sz;
> >>> +
> >>> +	/* hardware features */
> >>> +	unsigned internal_delay:1;	/* RAVB has internal delays */
> >>
> >>     Oops, missed it initially:
> >>     RAVB? That's not a device name, according to the manuals. It
> >> seems to be the driver's name.
> >
> > OK. will change it to AVB-DMAC has internal delays.
> 
>     Please don't -- E-MAC has them, not AVB-DMAC.

By looking at HW manual for R-Car AVB-DMAC (APSR register, offset:-0x08C) has TDM and RDM registers for Setting internal delay mode which can give TX clock delay up to 2.0ns and RX Clock delay 2.8ns.

Please correct me, if this is not the case.

Regards,
Biju



> 
> > Cheers,
> > Biju
> 
> NBR, Sergei

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 7/8] ravb: Add internal delay hw feature to struct ravb_hw_info
  2021-08-04  5:13     ` Biju Das
  2021-08-04  9:51       ` Sergey Shtylyov
@ 2021-08-04 10:20       ` Sergei Shtylyov
  2021-08-04 10:32         ` Biju Das
  1 sibling, 1 reply; 59+ messages in thread
From: Sergei Shtylyov @ 2021-08-04 10:20 UTC (permalink / raw)
  To: Biju Das, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

On 04.08.2021 8:13, Biju Das wrote:

[...]
>>> R-Car Gen3 supports TX and RX clock internal delay modes, whereas
>>> R-Car
>>> Gen2 and RZ/G2L do not support it.
>>> Add an internal_delay hw feature bit to struct ravb_hw_info to enable
>>> this only for R-Car Gen3.
>>>
>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>> ---
>>> v2:
>>>   * Incorporated Andrew and Sergei's review comments for making it
>> smaller patch
>>>     and provided detailed description.
>>> ---
>>>   drivers/net/ethernet/renesas/ravb.h      | 3 +++
>>>   drivers/net/ethernet/renesas/ravb_main.c | 6 ++++--
>>>   2 files changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/renesas/ravb.h
>>> b/drivers/net/ethernet/renesas/ravb.h
>>> index 3df813b2e253..0d640dbe1eed 100644
>>> --- a/drivers/net/ethernet/renesas/ravb.h
>>> +++ b/drivers/net/ethernet/renesas/ravb.h
>>> @@ -998,6 +998,9 @@ struct ravb_hw_info {
>>>   	int num_tx_desc;
>>>   	int stats_len;
>>>   	size_t skb_sz;
>>> +
>>> +	/* hardware features */
>>> +	unsigned internal_delay:1;	/* RAVB has internal delays */
>>
>>     Oops, missed it initially:
>>     RAVB? That's not a device name, according to the manuals. It seems to
>> be the driver's name.
> 
> OK. will change it to AVB-DMAC has internal delays.

    Please don't -- E-MAC has them, not AVB-DMAC.

> Cheers,
> Biju

MBR, Sergei

^ permalink raw reply	[flat|nested] 59+ messages in thread

* RE: [PATCH net-next v2 7/8] ravb: Add internal delay hw feature to struct ravb_hw_info
  2021-08-04 10:20       ` Sergei Shtylyov
@ 2021-08-04 10:32         ` Biju Das
  0 siblings, 0 replies; 59+ messages in thread
From: Biju Das @ 2021-08-04 10:32 UTC (permalink / raw)
  To: Sergei Shtylyov, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hi Sergei,

> Subject: Re: [PATCH net-next v2 7/8] ravb: Add internal delay hw feature
> to struct ravb_hw_info
> 
> On 04.08.2021 8:13, Biju Das wrote:
> 
> [...]
> >>> R-Car Gen3 supports TX and RX clock internal delay modes, whereas
> >>> R-Car
> >>> Gen2 and RZ/G2L do not support it.
> >>> Add an internal_delay hw feature bit to struct ravb_hw_info to
> >>> enable this only for R-Car Gen3.
> >>>
> >>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>> ---
> >>> v2:
> >>>   * Incorporated Andrew and Sergei's review comments for making it
> >> smaller patch
> >>>     and provided detailed description.
> >>> ---
> >>>   drivers/net/ethernet/renesas/ravb.h      | 3 +++
> >>>   drivers/net/ethernet/renesas/ravb_main.c | 6 ++++--
> >>>   2 files changed, 7 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/net/ethernet/renesas/ravb.h
> >>> b/drivers/net/ethernet/renesas/ravb.h
> >>> index 3df813b2e253..0d640dbe1eed 100644
> >>> --- a/drivers/net/ethernet/renesas/ravb.h
> >>> +++ b/drivers/net/ethernet/renesas/ravb.h
> >>> @@ -998,6 +998,9 @@ struct ravb_hw_info {
> >>>   	int num_tx_desc;
> >>>   	int stats_len;
> >>>   	size_t skb_sz;
> >>> +
> >>> +	/* hardware features */
> >>> +	unsigned internal_delay:1;	/* RAVB has internal delays */
> >>
> >>     Oops, missed it initially:
> >>     RAVB? That's not a device name, according to the manuals. It
> >> seems to be the driver's name.
> >
> > OK. will change it to AVB-DMAC has internal delays.
> 
>     Please don't -- E-MAC has them, not AVB-DMAC.

Since the register for setting internal delay mode is coming from product specific register(0x8c),
I am agreeing with your statement, "E-MAC has internal delays".

Cheers,
Biju

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 7/8] ravb: Add internal delay hw feature to struct ravb_hw_info
  2021-08-04 10:08         ` Biju Das
@ 2021-08-04 10:34           ` Sergei Shtylyov
  0 siblings, 0 replies; 59+ messages in thread
From: Sergei Shtylyov @ 2021-08-04 10:34 UTC (permalink / raw)
  To: Biju Das, Sergey Shtylyov, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

On 04.08.2021 13:08, Biju Das wrote:
> Hi Sergei,
> 
> Thanks for feedback
> 
>> Subject: Re: [PATCH net-next v2 7/8] ravb: Add internal delay hw feature
>> to struct ravb_hw_info
>>
>> On 04.08.2021 8:13, Biju Das wrote:
>>> Hi Sergei,
>>>
>>> Thanks for the feedback
>>>
>>>> Subject: Re: [PATCH net-next v2 7/8] ravb: Add internal delay hw
>>>> feature to struct ravb_hw_info
>>>>
>>>> On 8/2/21 1:26 PM, Biju Das wrote:
>>>>
>>>>> R-Car Gen3 supports TX and RX clock internal delay modes, whereas
>>>>> R-Car
>>>>> Gen2 and RZ/G2L do not support it.
>>>>> Add an internal_delay hw feature bit to struct ravb_hw_info to
>>>>> enable this only for R-Car Gen3.
>>>>>
>>>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>>>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>>>> ---
>>>>> v2:
>>>>>    * Incorporated Andrew and Sergei's review comments for making it
>>>> smaller patch
>>>>>      and provided detailed description.
>>>>> ---
>>>>>    drivers/net/ethernet/renesas/ravb.h      | 3 +++
>>>>>    drivers/net/ethernet/renesas/ravb_main.c | 6 ++++--
>>>>>    2 files changed, 7 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/renesas/ravb.h
>>>>> b/drivers/net/ethernet/renesas/ravb.h
>>>>> index 3df813b2e253..0d640dbe1eed 100644
>>>>> --- a/drivers/net/ethernet/renesas/ravb.h
>>>>> +++ b/drivers/net/ethernet/renesas/ravb.h
>>>>> @@ -998,6 +998,9 @@ struct ravb_hw_info {
>>>>>    	int num_tx_desc;
>>>>>    	int stats_len;
>>>>>    	size_t skb_sz;
>>>>> +
>>>>> +	/* hardware features */
>>>>> +	unsigned internal_delay:1;	/* RAVB has internal delays */
>>>>
>>>>      Oops, missed it initially:
>>>>      RAVB? That's not a device name, according to the manuals. It
>>>> seems to be the driver's name.
>>>
>>> OK. will change it to AVB-DMAC has internal delays.
>>
>>      Please don't -- E-MAC has them, not AVB-DMAC.
> 
> By looking at HW manual for R-Car AVB-DMAC (APSR register, offset:-0x08C) has TDM and RDM registers for Setting internal delay mode which can give TX clock delay up to 2.0ns and RX Clock delay 2.8ns.
> 
> Please correct me, if this is not the case.

    You're correct indeed -- though being counter-intuitive, APSR belongs to 
the AVB-DMAC block. Sorry about that. :-/

>  Regards,
> Biju

NBR, Sergei

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to driver data
  2021-08-03  5:57     ` Biju Das
  2021-08-03  6:36       ` Biju Das
@ 2021-08-04 19:27       ` Sergei Shtylyov
  2021-08-04 20:27         ` Sergei Shtylyov
  1 sibling, 1 reply; 59+ messages in thread
From: Sergei Shtylyov @ 2021-08-04 19:27 UTC (permalink / raw)
  To: Biju Das, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

On 8/3/21 8:57 AM, Biju Das wrote:

>> Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to
>> driver data
>>
>> On 8/2/21 1:26 PM, Biju Das wrote:
>>
>>> The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC
>>> are similar to the R-Car Ethernet AVB IP. With a few changes in the
>>> driver we can support both IPs.
>>>
>>> Currently a runtime decision based on the chip type is used to
>>> distinguish the HW differences between the SoC families.
>>>
>>> The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2
>>> and RZ/G2L it is 2. For cases like this it is better to select the
>>> number of TX descriptors by using a structure with a value, rather
>>> than a runtime decision based on the chip type.
>>>
>>> This patch adds the num_tx_desc variable to struct ravb_hw_info and
>>> also replaces the driver data chip type with struct ravb_hw_info by
>>> moving chip type to it.
>>>
>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>> ---
>>> v2:
>>>  * Incorporated Andrew and Sergei's review comments for making it
>> smaller patch
>>>    and provided detailed description.
>>> ---
>>>  drivers/net/ethernet/renesas/ravb.h      |  7 +++++
>>>  drivers/net/ethernet/renesas/ravb_main.c | 38
>>> +++++++++++++++---------
>>>  2 files changed, 31 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/renesas/ravb.h
>>> b/drivers/net/ethernet/renesas/ravb.h
>>> index 80e62ca2e3d3..cfb972c05b34 100644
>>> --- a/drivers/net/ethernet/renesas/ravb.h
>>> +++ b/drivers/net/ethernet/renesas/ravb.h
>>> @@ -988,6 +988,11 @@ enum ravb_chip_id {
>>>  	RCAR_GEN3,
>>>  };
>>>
>>> +struct ravb_hw_info {
>>> +	enum ravb_chip_id chip_id;
>>> +	int num_tx_desc;

   How about leaving that field in the *struct* ravb_private? And adding the following instead:

	unsigned unaligned_tx: 1;

>>    I think this is rather the driver's choice, than the h/w feature...
>> Perhaps a rename would help with that? :-)
> 
> It is consistent with current naming convention used by the driver. NUM_TX_DESC macro is replaced by num_tx_desc and  the below run time decision based on chip type for H/W configuration for Gen2/Gen3 is replaced by info->num_tx_desc.
> 
> priv->num_tx_desc = chip_id == RCAR_GEN2 ? NUM_TX_DESC_GEN2 : NUM_TX_DESC_GEN3;

   .. and then:

	priv->num_tx_desc =  info->unaligned_tx ? NUM_TX_DESC_GEN2 : NUM_TX_DESC_GEN3;

> Please let me know, if I am missing anything,
> 
> Previously there is a suggestion to change the generic struct ravb_driver_data (which holds driver differences and HW features) with struct ravb_hw_info.

    Well, my plan was to place all the hardware features supported into the *struct* ravb_hw_info and leave all
the driver's software data in the *struct* ravb_private.

> Regards,
> Biju
[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to driver data
  2021-08-04 19:27       ` Sergei Shtylyov
@ 2021-08-04 20:27         ` Sergei Shtylyov
  0 siblings, 0 replies; 59+ messages in thread
From: Sergei Shtylyov @ 2021-08-04 20:27 UTC (permalink / raw)
  To: Biju Das, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

On 8/4/21 10:27 PM, Sergei Shtylyov wrote:

>>> Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to
>>> driver data
>>>
>>> On 8/2/21 1:26 PM, Biju Das wrote:
>>>
>>>> The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC
>>>> are similar to the R-Car Ethernet AVB IP. With a few changes in the
>>>> driver we can support both IPs.
>>>>
>>>> Currently a runtime decision based on the chip type is used to
>>>> distinguish the HW differences between the SoC families.
>>>>
>>>> The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2
>>>> and RZ/G2L it is 2. For cases like this it is better to select the
>>>> number of TX descriptors by using a structure with a value, rather
>>>> than a runtime decision based on the chip type.
>>>>
>>>> This patch adds the num_tx_desc variable to struct ravb_hw_info and
>>>> also replaces the driver data chip type with struct ravb_hw_info by
>>>> moving chip type to it.
>>>>
>>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>>> ---
>>>> v2:
>>>>  * Incorporated Andrew and Sergei's review comments for making it
>>> smaller patch
>>>>    and provided detailed description.
>>>> ---
>>>>  drivers/net/ethernet/renesas/ravb.h      |  7 +++++
>>>>  drivers/net/ethernet/renesas/ravb_main.c | 38
>>>> +++++++++++++++---------
>>>>  2 files changed, 31 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/renesas/ravb.h
>>>> b/drivers/net/ethernet/renesas/ravb.h
>>>> index 80e62ca2e3d3..cfb972c05b34 100644
>>>> --- a/drivers/net/ethernet/renesas/ravb.h
>>>> +++ b/drivers/net/ethernet/renesas/ravb.h
>>>> @@ -988,6 +988,11 @@ enum ravb_chip_id {
>>>>  	RCAR_GEN3,
>>>>  };
>>>>
>>>> +struct ravb_hw_info {
>>>> +	enum ravb_chip_id chip_id;
>>>> +	int num_tx_desc;
> 
>    How about leaving that field in the *struct* ravb_private? And adding the following instead:
> 
> 	unsigned unaligned_tx: 1;

   Or aligned_tx, so that gen2 has it set, and gen3 has it cleared.

> 
>>>    I think this is rather the driver's choice, than the h/w feature...
>>> Perhaps a rename would help with that? :-)
>>
>> It is consistent with current naming convention used by the driver. NUM_TX_DESC macro is replaced by num_tx_desc and  the below run time decision based on chip type for H/W configuration for Gen2/Gen3 is replaced by info->num_tx_desc.
>>
>> priv->num_tx_desc = chip_id == RCAR_GEN2 ? NUM_TX_DESC_GEN2 : NUM_TX_DESC_GEN3;
> 
>    .. and then:
> 
> 	priv->num_tx_desc =  info->unaligned_tx ? NUM_TX_DESC_GEN2 : NUM_TX_DESC_GEN3;

   Sorry, mixed the values, should have been:

	priv->num_tx_desc =  info->unaligned_tx ? NUM_TX_DESC_GEN3 : NUM_TX_DESC_GEN2;

>> Please let me know, if I am missing anything,
>>
>> Previously there is a suggestion to change the generic struct ravb_driver_data (which holds driver differences and HW features) with struct ravb_hw_info.
> 
>     Well, my plan was to place all the hardware features supported into the *struct* ravb_hw_info and leave all
> the driver's software data in the *struct* ravb_private.

   ... just like *struct* sh_eth_cpu_data and sh_eth_private in the sh_eth driver.

>> Regards,
>> Biju

MBR, Sergei

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 5/8] ravb: Add gstrings_stats and gstrings_size to struct ravb_hw_info
  2021-08-02 10:26 ` [PATCH net-next v2 5/8] ravb: Add gstrings_stats and gstrings_size " Biju Das
  2021-08-02 15:11   ` Andrew Lunn
@ 2021-08-04 20:36   ` Sergei Shtylyov
  1 sibling, 0 replies; 59+ messages in thread
From: Sergei Shtylyov @ 2021-08-04 20:36 UTC (permalink / raw)
  To: Biju Das, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

On 8/2/21 1:26 PM, Biju Das wrote:

> The device stats strings for R-Car and RZ/G2L are different.
> 
> R-Car provides 30 device stats, whereas RZ/G2L provides only 15. In
> addition, RZ/G2L has stats "rx_queue_0_csum_offload_errors" instead of
> "rx_queue_0_missed_errors".
> 
> Add structure variables gstrings_stats and gstrings_size to struct
> ravb_hw_info, so that subsequent SoCs can be added without any code
> changes in the ravb_get_strings function.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Reviewed-by: Sergei Shtylyov <sergei.shtylyov@gmail.com>

[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 8/8] ravb: Add tx_drop_cntrs to struct ravb_hw_info
  2021-08-02 10:26 ` [PATCH net-next v2 8/8] ravb: Add tx_drop_cntrs " Biju Das
  2021-08-02 15:14   ` Andrew Lunn
@ 2021-08-04 20:50   ` Sergei Shtylyov
  2021-08-17 15:47     ` Biju Das
  1 sibling, 1 reply; 59+ messages in thread
From: Sergei Shtylyov @ 2021-08-04 20:50 UTC (permalink / raw)
  To: Biju Das, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

On 8/2/21 1:26 PM, Biju Das wrote:

> The register for retrieving TX drop counters is present only on R-Car Gen3
> and RZ/G2L; it is not present on R-Car Gen2.
> 
> Add the tx_drop_cntrs hw feature bit to struct ravb_hw_info, to enable this
> feature specifically for R-Car Gen3 now and later extend it to RZ/G2L.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v2:
>  * Incorporated Andrew and Sergei's review comments for making it smaller patch
>    and provided detailed description.
> ---
>  drivers/net/ethernet/renesas/ravb.h      | 1 +
>  drivers/net/ethernet/renesas/ravb_main.c | 4 +++-
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
> index 0d640dbe1eed..35fbb9f60ba8 100644
> --- a/drivers/net/ethernet/renesas/ravb.h
> +++ b/drivers/net/ethernet/renesas/ravb.h
> @@ -1001,6 +1001,7 @@ struct ravb_hw_info {
>  
>  	/* hardware features */
>  	unsigned internal_delay:1;	/* RAVB has internal delays */
> +	unsigned tx_drop_cntrs:1;	/* RAVB has TX error counters */

   I suggest 'tx_counters' -- this name comes from the sh_eth driver for the same regs
(but negated meaning). And please don't call the hardware RAVB. :-)

[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 6/8] ravb: Add net_features and net_hw_features to struct ravb_hw_info
  2021-08-02 10:26 ` [PATCH net-next v2 6/8] ravb: Add net_features and net_hw_features " Biju Das
  2021-08-02 15:12   ` Andrew Lunn
@ 2021-08-05 19:07   ` Sergei Shtylyov
  2021-08-05 19:18     ` Biju Das
  2021-08-06 20:31   ` Sergei Shtylyov
  2 siblings, 1 reply; 59+ messages in thread
From: Sergei Shtylyov @ 2021-08-05 19:07 UTC (permalink / raw)
  To: Biju Das, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

On 8/2/21 1:26 PM, Biju Das wrote:

> On R-Car the checksum calculation on RX frames is done by the E-MAC
> module, whereas on RZ/G2L it is done by the TOE.
> 
> TOE calculates the checksum of received frames from E-MAC and outputs it to
> DMAC. TOE also calculates the checksum of transmission frames from DMAC and
> outputs it E-MAC.
> 
> Add net_features and net_hw_features to struct ravb_hw_info, to support
> subsequent SoCs without any code changes in the ravb_probe function.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

[...]
> diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
> index b765b2b7d9e9..3df813b2e253 100644
> --- a/drivers/net/ethernet/renesas/ravb.h
> +++ b/drivers/net/ethernet/renesas/ravb.h
> @@ -991,6 +991,8 @@ enum ravb_chip_id {
>  struct ravb_hw_info {
>  	const char (*gstrings_stats)[ETH_GSTRING_LEN];
>  	size_t gstrings_size;
> +	netdev_features_t net_hw_features;
> +	netdev_features_t net_features;

   Do we really need both of these here? It seems like the 'feartures' mirrors the enabled features?

>  	enum ravb_chip_id chip_id;
>  	int num_gstat_queue;
>  	int num_tx_desc;
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 7a69668cb512..2ac962b5b8fb 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
[...]
> @@ -2077,14 +2081,14 @@ static int ravb_probe(struct platform_device *pdev)
>  	if (!ndev)
>  		return -ENOMEM;
>  
> -	ndev->features = NETIF_F_RXCSUM;
> -	ndev->hw_features = NETIF_F_RXCSUM;
> +	info = of_device_get_match_data(&pdev->dev);
> +
> +	ndev->features = info->net_features;
> +	ndev->hw_features = info->net_hw_features;

   What value you plan to set her for GbEth, NETIF_F_HW_CSUM?

[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 59+ messages in thread

* RE: [PATCH net-next v2 6/8] ravb: Add net_features and net_hw_features to struct ravb_hw_info
  2021-08-05 19:07   ` Sergei Shtylyov
@ 2021-08-05 19:18     ` Biju Das
  2021-08-06 20:20       ` Sergei Shtylyov
  0 siblings, 1 reply; 59+ messages in thread
From: Biju Das @ 2021-08-05 19:18 UTC (permalink / raw)
  To: Sergei Shtylyov, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hi Sergei,

Thanks for the feedback.

> Subject: Re: [PATCH net-next v2 6/8] ravb: Add net_features and
> net_hw_features to struct ravb_hw_info
> 
> On 8/2/21 1:26 PM, Biju Das wrote:
> 
> > On R-Car the checksum calculation on RX frames is done by the E-MAC
> > module, whereas on RZ/G2L it is done by the TOE.
> >
> > TOE calculates the checksum of received frames from E-MAC and outputs
> > it to DMAC. TOE also calculates the checksum of transmission frames
> > from DMAC and outputs it E-MAC.
> >
> > Add net_features and net_hw_features to struct ravb_hw_info, to
> > support subsequent SoCs without any code changes in the ravb_probe
> function.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> [...]
> > diff --git a/drivers/net/ethernet/renesas/ravb.h
> > b/drivers/net/ethernet/renesas/ravb.h
> > index b765b2b7d9e9..3df813b2e253 100644
> > --- a/drivers/net/ethernet/renesas/ravb.h
> > +++ b/drivers/net/ethernet/renesas/ravb.h
> > @@ -991,6 +991,8 @@ enum ravb_chip_id {  struct ravb_hw_info {
> >  	const char (*gstrings_stats)[ETH_GSTRING_LEN];
> >  	size_t gstrings_size;
> > +	netdev_features_t net_hw_features;
> > +	netdev_features_t net_features;
> 
>    Do we really need both of these here? 

R-Car has only Rx Checksum on E-Mac, where as Geth supports Rx Check Sum on E-Mac or Rx/Tx CheckSum on TOE.
So there is a hw difference. Please let me know what is the best way to handle this?

>It seems like the 'feartures'
> mirrors the enabled features?

Can you please explain this little bit?

> 
> >  	enum ravb_chip_id chip_id;
> >  	int num_gstat_queue;
> >  	int num_tx_desc;
> > diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> > b/drivers/net/ethernet/renesas/ravb_main.c
> > index 7a69668cb512..2ac962b5b8fb 100644
> > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> [...]
> > @@ -2077,14 +2081,14 @@ static int ravb_probe(struct platform_device
> *pdev)
> >  	if (!ndev)
> >  		return -ENOMEM;
> >
> > -	ndev->features = NETIF_F_RXCSUM;
> > -	ndev->hw_features = NETIF_F_RXCSUM;
> > +	info = of_device_get_match_data(&pdev->dev);
> > +
> > +	ndev->features = info->net_features;
> > +	ndev->hw_features = info->net_hw_features;
> 
>    What value you plan to set her for GbEth, NETIF_F_HW_CSUM?

Yes, That is the plan.
.net_hw_features = (NETIF_F_HW_CSUM | NETIF_F_RXCSUM),

Regards,
Biju

> 
> [...]
> 
> MBR, Sergei

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 6/8] ravb: Add net_features and net_hw_features to struct ravb_hw_info
  2021-08-05 19:18     ` Biju Das
@ 2021-08-06 20:20       ` Sergei Shtylyov
  2021-08-12  7:35         ` Biju Das
  0 siblings, 1 reply; 59+ messages in thread
From: Sergei Shtylyov @ 2021-08-06 20:20 UTC (permalink / raw)
  To: Biju Das, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hello!

On 8/5/21 10:18 PM, Biju Das wrote:

[...]
>>> On R-Car the checksum calculation on RX frames is done by the E-MAC
>>> module, whereas on RZ/G2L it is done by the TOE.
>>>
>>> TOE calculates the checksum of received frames from E-MAC and outputs
>>> it to DMAC. TOE also calculates the checksum of transmission frames
>>> from DMAC and outputs it E-MAC.
>>>
>>> Add net_features and net_hw_features to struct ravb_hw_info, to
>>> support subsequent SoCs without any code changes in the ravb_probe
>> function.
>>>
>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>
>> [...]
>>> diff --git a/drivers/net/ethernet/renesas/ravb.h
>>> b/drivers/net/ethernet/renesas/ravb.h
>>> index b765b2b7d9e9..3df813b2e253 100644
>>> --- a/drivers/net/ethernet/renesas/ravb.h
>>> +++ b/drivers/net/ethernet/renesas/ravb.h
>>> @@ -991,6 +991,8 @@ enum ravb_chip_id {  struct ravb_hw_info {
>>>  	const char (*gstrings_stats)[ETH_GSTRING_LEN];
>>>  	size_t gstrings_size;
>>> +	netdev_features_t net_hw_features;
>>> +	netdev_features_t net_features;
>>
>>    Do we really need both of these here? 
> 
> R-Car has only Rx Checksum on E-Mac, where as Geth supports Rx Check Sum on E-Mac or Rx/Tx CheckSum on TOE.
> So there is a hw difference. Please let me know what is the best way to handle this?

   I meant that we could go with only one field of the net_features... Alternatively, we could use our own
feature bits...

>> It seems like the 'feartures'
>> mirrors the enabled features?
> 
> Can you please explain this little bit?

   Looks like I was wrong. :-)

[...]

> Regards,
> Biju

[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 6/8] ravb: Add net_features and net_hw_features to struct ravb_hw_info
  2021-08-02 10:26 ` [PATCH net-next v2 6/8] ravb: Add net_features and net_hw_features " Biju Das
  2021-08-02 15:12   ` Andrew Lunn
  2021-08-05 19:07   ` Sergei Shtylyov
@ 2021-08-06 20:31   ` Sergei Shtylyov
  2 siblings, 0 replies; 59+ messages in thread
From: Sergei Shtylyov @ 2021-08-06 20:31 UTC (permalink / raw)
  To: Biju Das, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

On 8/2/21 1:26 PM, Biju Das wrote:

> On R-Car the checksum calculation on RX frames is done by the E-MAC
> module, whereas on RZ/G2L it is done by the TOE.
> 
> TOE calculates the checksum of received frames from E-MAC and outputs it to
> DMAC. TOE also calculates the checksum of transmission frames from DMAC and
> outputs it E-MAC.
> 
> Add net_features and net_hw_features to struct ravb_hw_info, to support
> subsequent SoCs without any code changes in the ravb_probe function.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Reviewed-by: Sergei Shtylyov <sergei.shtylyov@gmail.com>


[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to driver data
  2021-08-02 10:26 ` [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to driver data Biju Das
  2021-08-02 15:02   ` Andrew Lunn
  2021-08-02 20:42   ` Sergei Shtylyov
@ 2021-08-09 12:06   ` Geert Uytterhoeven
  2021-08-12  7:26     ` Biju Das
  2 siblings, 1 reply; 59+ messages in thread
From: Geert Uytterhoeven @ 2021-08-09 12:06 UTC (permalink / raw)
  To: Biju Das
  Cc: David S. Miller, Jakub Kicinski, Sergei Shtylyov,
	Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, Linux-Renesas,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hi Biju,

On Mon, Aug 2, 2021 at 12:27 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC are
> similar to the R-Car Ethernet AVB IP. With a few changes in the driver we
> can support both IPs.
>
> Currently a runtime decision based on the chip type is used to distinguish
> the HW differences between the SoC families.
>
> The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2 and
> RZ/G2L it is 2. For cases like this it is better to select the number of
> TX descriptors by using a structure with a value, rather than a runtime
> decision based on the chip type.
>
> This patch adds the num_tx_desc variable to struct ravb_hw_info and also
> replaces the driver data chip type with struct ravb_hw_info by moving chip
> type to it.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/net/ethernet/renesas/ravb.h
> +++ b/drivers/net/ethernet/renesas/ravb.h
> @@ -988,6 +988,11 @@ enum ravb_chip_id {
>         RCAR_GEN3,
>  };
>
> +struct ravb_hw_info {
> +       enum ravb_chip_id chip_id;
> +       int num_tx_desc;

Why not "unsigned int"? ...
This comment applies to a few more subsequent patches.

> +};
> +
>  struct ravb_private {
>         struct net_device *ndev;
>         struct platform_device *pdev;
> @@ -1040,6 +1045,8 @@ struct ravb_private {
>         unsigned txcidm:1;              /* TX Clock Internal Delay Mode */
>         unsigned rgmii_override:1;      /* Deprecated rgmii-*id behavior */
>         int num_tx_desc;                /* TX descriptors per packet */

... oh, here's the original culprit.

> +
> +       const struct ravb_hw_info *info;
>  };
>

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

^ permalink raw reply	[flat|nested] 59+ messages in thread

* RE: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to driver data
  2021-08-09 12:06   ` Geert Uytterhoeven
@ 2021-08-12  7:26     ` Biju Das
  2021-08-12  7:58       ` Geert Uytterhoeven
  0 siblings, 1 reply; 59+ messages in thread
From: Biju Das @ 2021-08-12  7:26 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: David S. Miller, Jakub Kicinski, Sergei Shtylyov,
	Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, Linux-Renesas,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to
> driver data
> 
> Hi Biju,
> 
> On Mon, Aug 2, 2021 at 12:27 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC
> > are similar to the R-Car Ethernet AVB IP. With a few changes in the
> > driver we can support both IPs.
> >
> > Currently a runtime decision based on the chip type is used to
> > distinguish the HW differences between the SoC families.
> >
> > The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2
> > and RZ/G2L it is 2. For cases like this it is better to select the
> > number of TX descriptors by using a structure with a value, rather
> > than a runtime decision based on the chip type.
> >
> > This patch adds the num_tx_desc variable to struct ravb_hw_info and
> > also replaces the driver data chip type with struct ravb_hw_info by
> > moving chip type to it.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/net/ethernet/renesas/ravb.h
> > +++ b/drivers/net/ethernet/renesas/ravb.h
> > @@ -988,6 +988,11 @@ enum ravb_chip_id {
> >         RCAR_GEN3,
> >  };
> >
> > +struct ravb_hw_info {
> > +       enum ravb_chip_id chip_id;
> > +       int num_tx_desc;
> 
> Why not "unsigned int"? ...
> This comment applies to a few more subsequent patches.

To avoid signed and unsigned comparison warnings.

> 
> > +};
> > +
> >  struct ravb_private {
> >         struct net_device *ndev;
> >         struct platform_device *pdev;
> > @@ -1040,6 +1045,8 @@ struct ravb_private {
> >         unsigned txcidm:1;              /* TX Clock Internal Delay Mode
> */
> >         unsigned rgmii_override:1;      /* Deprecated rgmii-*id behavior
> */
> >         int num_tx_desc;                /* TX descriptors per packet */
> 
> ... oh, here's the original culprit.

Exactly, this the reason. 

Do you want me to change this into unsigned int? Please let me know.

Regards,
Biju

> 
> > +
> > +       const struct ravb_hw_info *info;
> >  };
> >
> 
> 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

^ permalink raw reply	[flat|nested] 59+ messages in thread

* RE: [PATCH net-next v2 6/8] ravb: Add net_features and net_hw_features to struct ravb_hw_info
  2021-08-06 20:20       ` Sergei Shtylyov
@ 2021-08-12  7:35         ` Biju Das
  0 siblings, 0 replies; 59+ messages in thread
From: Biju Das @ 2021-08-12  7:35 UTC (permalink / raw)
  To: Sergei Shtylyov, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hi Sergei,

Thanks for the feedback.

> Subject: Re: [PATCH net-next v2 6/8] ravb: Add net_features and
> net_hw_features to struct ravb_hw_info
> 
> Hello!
> 
> On 8/5/21 10:18 PM, Biju Das wrote:
> 
> [...]
> >>> On R-Car the checksum calculation on RX frames is done by the E-MAC
> >>> module, whereas on RZ/G2L it is done by the TOE.
> >>>
> >>> TOE calculates the checksum of received frames from E-MAC and
> >>> outputs it to DMAC. TOE also calculates the checksum of transmission
> >>> frames from DMAC and outputs it E-MAC.
> >>>
> >>> Add net_features and net_hw_features to struct ravb_hw_info, to
> >>> support subsequent SoCs without any code changes in the ravb_probe
> >> function.
> >>>
> >>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>
> >> [...]
> >>> diff --git a/drivers/net/ethernet/renesas/ravb.h
> >>> b/drivers/net/ethernet/renesas/ravb.h
> >>> index b765b2b7d9e9..3df813b2e253 100644
> >>> --- a/drivers/net/ethernet/renesas/ravb.h
> >>> +++ b/drivers/net/ethernet/renesas/ravb.h
> >>> @@ -991,6 +991,8 @@ enum ravb_chip_id {  struct ravb_hw_info {
> >>>  	const char (*gstrings_stats)[ETH_GSTRING_LEN];
> >>>  	size_t gstrings_size;
> >>> +	netdev_features_t net_hw_features;
> >>> +	netdev_features_t net_features;
> >>
> >>    Do we really need both of these here?
> >
> > R-Car has only Rx Checksum on E-Mac, where as Geth supports Rx Check Sum
> on E-Mac or Rx/Tx CheckSum on TOE.
> > So there is a hw difference. Please let me know what is the best way to
> handle this?
> 
>    I meant that we could go with only one field of the net_features...
> Alternatively, we could use our own feature bits...

Basically are you suggesting some thing like unsigned hwcsum_tx_rx: 1;

Then,

if (hwcsum_tx_rx) {
	ndev->hw_features = NETIF_F_HW_CSUM | NETIF_F_RXCSUM;
} else {
	ndev->features = NETIF_F_RXCSUM;
	ndev->hw_features = NETIF_F_RXCSUM;
}

Cheers,
Biju


^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to driver data
  2021-08-12  7:26     ` Biju Das
@ 2021-08-12  7:58       ` Geert Uytterhoeven
  2021-08-12  8:13         ` Biju Das
  0 siblings, 1 reply; 59+ messages in thread
From: Geert Uytterhoeven @ 2021-08-12  7:58 UTC (permalink / raw)
  To: Biju Das
  Cc: David S. Miller, Jakub Kicinski, Sergei Shtylyov,
	Sergey Shtylyov, Adam Ford, Andrew Lunn, Yuusuke Ashizuka,
	Yoshihiro Shimoda, netdev, Linux-Renesas, Chris Paterson,
	Biju Das, Prabhakar Mahadev Lad

Hi Biju,

On Thu, Aug 12, 2021 at 9:26 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > -----Original Message-----
> > On Mon, Aug 2, 2021 at 12:27 PM Biju Das <biju.das.jz@bp.renesas.com>
> > wrote:
> > > The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC
> > > are similar to the R-Car Ethernet AVB IP. With a few changes in the
> > > driver we can support both IPs.
> > >
> > > Currently a runtime decision based on the chip type is used to
> > > distinguish the HW differences between the SoC families.
> > >
> > > The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2
> > > and RZ/G2L it is 2. For cases like this it is better to select the
> > > number of TX descriptors by using a structure with a value, rather
> > > than a runtime decision based on the chip type.
> > >
> > > This patch adds the num_tx_desc variable to struct ravb_hw_info and
> > > also replaces the driver data chip type with struct ravb_hw_info by
> > > moving chip type to it.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Thanks for your patch!
> >
> > > --- a/drivers/net/ethernet/renesas/ravb.h
> > > +++ b/drivers/net/ethernet/renesas/ravb.h
> > > @@ -988,6 +988,11 @@ enum ravb_chip_id {
> > >         RCAR_GEN3,
> > >  };
> > >
> > > +struct ravb_hw_info {
> > > +       enum ravb_chip_id chip_id;
> > > +       int num_tx_desc;
> >
> > Why not "unsigned int"? ...
> > This comment applies to a few more subsequent patches.
>
> To avoid signed and unsigned comparison warnings.
>
> >
> > > +};
> > > +
> > >  struct ravb_private {
> > >         struct net_device *ndev;
> > >         struct platform_device *pdev;
> > > @@ -1040,6 +1045,8 @@ struct ravb_private {
> > >         unsigned txcidm:1;              /* TX Clock Internal Delay Mode
> > */
> > >         unsigned rgmii_override:1;      /* Deprecated rgmii-*id behavior
> > */
> > >         int num_tx_desc;                /* TX descriptors per packet */
> >
> > ... oh, here's the original culprit.
>
> Exactly, this the reason.
>
> Do you want me to change this into unsigned int? Please let me know.

Up to you (or the maintainer? ;-)

For new fields (in the other patches), I would use unsigned for all
unsigned values.  Signed values have more pitfalls related to
undefined behavior.

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

^ permalink raw reply	[flat|nested] 59+ messages in thread

* RE: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to driver data
  2021-08-12  7:58       ` Geert Uytterhoeven
@ 2021-08-12  8:13         ` Biju Das
  2021-08-17 11:24           ` Biju Das
  0 siblings, 1 reply; 59+ messages in thread
From: Biju Das @ 2021-08-12  8:13 UTC (permalink / raw)
  To: Geert Uytterhoeven, Sergey Shtylyov
  Cc: David S. Miller, Jakub Kicinski, Sergei Shtylyov, Adam Ford,
	Andrew Lunn, Yuusuke Ashizuka, Yoshihiro Shimoda, netdev,
	Linux-Renesas, Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to
> driver data
> 
> Hi Biju,
> 
> On Thu, Aug 12, 2021 at 9:26 AM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > > -----Original Message-----
> > > On Mon, Aug 2, 2021 at 12:27 PM Biju Das
> > > <biju.das.jz@bp.renesas.com>
> > > wrote:
> > > > The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L
> > > > SoC are similar to the R-Car Ethernet AVB IP. With a few changes
> > > > in the driver we can support both IPs.
> > > >
> > > > Currently a runtime decision based on the chip type is used to
> > > > distinguish the HW differences between the SoC families.
> > > >
> > > > The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car
> > > > Gen2 and RZ/G2L it is 2. For cases like this it is better to
> > > > select the number of TX descriptors by using a structure with a
> > > > value, rather than a runtime decision based on the chip type.
> > > >
> > > > This patch adds the num_tx_desc variable to struct ravb_hw_info
> > > > and also replaces the driver data chip type with struct
> > > > ravb_hw_info by moving chip type to it.
> > > >
> > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > Reviewed-by: Lad Prabhakar
> > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > Thanks for your patch!
> > >
> > > > --- a/drivers/net/ethernet/renesas/ravb.h
> > > > +++ b/drivers/net/ethernet/renesas/ravb.h
> > > > @@ -988,6 +988,11 @@ enum ravb_chip_id {
> > > >         RCAR_GEN3,
> > > >  };
> > > >
> > > > +struct ravb_hw_info {
> > > > +       enum ravb_chip_id chip_id;
> > > > +       int num_tx_desc;
> > >
> > > Why not "unsigned int"? ...
> > > This comment applies to a few more subsequent patches.
> >
> > To avoid signed and unsigned comparison warnings.
> >
> > >
> > > > +};
> > > > +
> > > >  struct ravb_private {
> > > >         struct net_device *ndev;
> > > >         struct platform_device *pdev; @@ -1040,6 +1045,8 @@ struct
> > > > ravb_private {
> > > >         unsigned txcidm:1;              /* TX Clock Internal Delay
> Mode
> > > */
> > > >         unsigned rgmii_override:1;      /* Deprecated rgmii-*id
> behavior
> > > */
> > > >         int num_tx_desc;                /* TX descriptors per packet
> */
> > >
> > > ... oh, here's the original culprit.
> >
> > Exactly, this the reason.
> >
> > Do you want me to change this into unsigned int? Please let me know.
> 
> Up to you (or the maintainer? ;-)
> 
> For new fields (in the other patches), I would use unsigned for all
> unsigned values.  Signed values have more pitfalls related to undefined
> behavior.

Sergei, What is your thoughts here? Please let me know.

Cheers,
Biju

^ permalink raw reply	[flat|nested] 59+ messages in thread

* RE: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to driver data
  2021-08-12  8:13         ` Biju Das
@ 2021-08-17 11:24           ` Biju Das
  2021-08-17 20:11             ` Sergey Shtylyov
  0 siblings, 1 reply; 59+ messages in thread
From: Biju Das @ 2021-08-17 11:24 UTC (permalink / raw)
  To: Biju Das, Geert Uytterhoeven, Sergey Shtylyov
  Cc: David S. Miller, Jakub Kicinski, Sergei Shtylyov, Adam Ford,
	Andrew Lunn, Yuusuke Ashizuka, Yoshihiro Shimoda, netdev,
	Linux-Renesas, Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hi all,

> Subject: RE: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to
> driver data
> 
> Hi Geert,
> 
> Thanks for the feedback.
> 
> > Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to
> > driver data
> >
> > Hi Biju,
> >
> > On Thu, Aug 12, 2021 at 9:26 AM Biju Das <biju.das.jz@bp.renesas.com>
> > wrote:
> > > > -----Original Message-----
> > > > On Mon, Aug 2, 2021 at 12:27 PM Biju Das
> > > > <biju.das.jz@bp.renesas.com>
> > > > wrote:
> > > > > The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L
> > > > > SoC are similar to the R-Car Ethernet AVB IP. With a few changes
> > > > > in the driver we can support both IPs.
> > > > >
> > > > > Currently a runtime decision based on the chip type is used to
> > > > > distinguish the HW differences between the SoC families.
> > > > >
> > > > > The number of TX descriptors for R-Car Gen3 is 1 whereas on
> > > > > R-Car
> > > > > Gen2 and RZ/G2L it is 2. For cases like this it is better to
> > > > > select the number of TX descriptors by using a structure with a
> > > > > value, rather than a runtime decision based on the chip type.
> > > > >
> > > > > This patch adds the num_tx_desc variable to struct ravb_hw_info
> > > > > and also replaces the driver data chip type with struct
> > > > > ravb_hw_info by moving chip type to it.
> > > > >
> > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > > Reviewed-by: Lad Prabhakar
> > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > >
> > > > Thanks for your patch!
> > > >
> > > > > --- a/drivers/net/ethernet/renesas/ravb.h
> > > > > +++ b/drivers/net/ethernet/renesas/ravb.h
> > > > > @@ -988,6 +988,11 @@ enum ravb_chip_id {
> > > > >         RCAR_GEN3,
> > > > >  };
> > > > >
> > > > > +struct ravb_hw_info {
> > > > > +       enum ravb_chip_id chip_id;
> > > > > +       int num_tx_desc;
> > > >
> > > > Why not "unsigned int"? ...
> > > > This comment applies to a few more subsequent patches.
> > >
> > > To avoid signed and unsigned comparison warnings.
> > >
> > > >
> > > > > +};
> > > > > +
> > > > >  struct ravb_private {
> > > > >         struct net_device *ndev;
> > > > >         struct platform_device *pdev; @@ -1040,6 +1045,8 @@
> > > > > struct ravb_private {
> > > > >         unsigned txcidm:1;              /* TX Clock Internal Delay
> > Mode
> > > > */
> > > > >         unsigned rgmii_override:1;      /* Deprecated rgmii-*id
> > behavior
> > > > */
> > > > >         int num_tx_desc;                /* TX descriptors per
> packet
> > */
> > > >
> > > > ... oh, here's the original culprit.
> > >
> > > Exactly, this the reason.
> > >
> > > Do you want me to change this into unsigned int? Please let me know.
> >
> > Up to you (or the maintainer? ;-)
> >
> > For new fields (in the other patches), I would use unsigned for all
> > unsigned values.  Signed values have more pitfalls related to
> > undefined behavior.
> 
> Sergei, What is your thoughts here? Please let me know.

Here is my plan.

I will split this patch into two as Andrew suggested and 

Then on the second patch will add as info->unaligned_tx as Sergei suggested.

Now the only open point is related to the data type of "int num_tx_desc"
and to align with sh_eth driver I will keep int.

Regards,
Biju

^ permalink raw reply	[flat|nested] 59+ messages in thread

* RE: [PATCH net-next v2 3/8] ravb: Add num_gstat_queue to struct ravb_hw_info
  2021-08-03 19:47         ` Biju Das
@ 2021-08-17 15:08           ` Biju Das
  0 siblings, 0 replies; 59+ messages in thread
From: Biju Das @ 2021-08-17 15:08 UTC (permalink / raw)
  To: Biju Das, Sergei Shtylyov, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hi All,

I have tested without this patch and got expected results. So I am dropping this patch in the next version.

Cheers,
Biju

> Subject: RE: [PATCH net-next v2 3/8] ravb: Add num_gstat_queue to struct
> ravb_hw_info
> 
> Hi Sergei,
> 
> > Subject: Re: [PATCH net-next v2 3/8] ravb: Add num_gstat_queue to
> > struct ravb_hw_info
> >
> > On 8/3/21 10:13 PM, Biju Das wrote:
> >
> > [...]
> > >>> The number of queues used in retrieving device stats for R-Car is
> > >>> 2, whereas for RZ/G2L it is 1.
> > >
> > >>
> > >>    Mhm, how many RX queues are on your platform, 1? Then we don't
> > >> need so specific name, just num_rx_queue.
> > >
> > > There are 2 RX queues, but we provide only device stats information
> > > from
> > first queue.
> > >
> > > R-Car = 2x15 = 30 device stats
> > > RZ/G2L = 1x15 = 15 device stats.
> >
> >     That's pretty strange... how the RX queue #1 is called? How many
> > RX queues are, at all?
> 
> For both R-Car and RZ/G2L,
> -------------------------
> #define NUM_RX_QUEUE    2
> #define NUM_TX_QUEUE    2
> 
> Target device stat output for RZ/G2L:-
> ------------------------------------
> root@smarc-rzg2l:~# ethtool -S eth0
> NIC statistics:
>      rx_queue_0_current: 21852
>      tx_queue_0_current: 18854
>      rx_queue_0_dirty: 21852
>      tx_queue_0_dirty: 18854
>      rx_queue_0_packets: 21852
>      tx_queue_0_packets: 9427
>      rx_queue_0_bytes: 28224093
>      tx_queue_0_bytes: 1659438
>      rx_queue_0_mcast_packets: 498
>      rx_queue_0_errors: 0
>      rx_queue_0_crc_errors: 0
>      rx_queue_0_frame_errors: 0
>      rx_queue_0_length_errors: 0
>      rx_queue_0_csum_offload_errors: 0
>      rx_queue_0_over_errors: 0
> root@smarc-rzg2l:~#
> 
> 
> Target device stat output for R-Car Gen3:-
> ----------------------------------------
> root@hihope-rzg2m:~#  ethtool -S eth0
> NIC statistics:
>      rx_queue_0_current: 34215
>      tx_queue_0_current: 14158
>      rx_queue_0_dirty: 34215
>      tx_queue_0_dirty: 14158
>      rx_queue_0_packets: 34215
>      tx_queue_0_packets: 14158
>      rx_queue_0_bytes: 38313586
>      tx_queue_0_bytes: 3222182
>      rx_queue_0_mcast_packets: 499
>      rx_queue_0_errors: 0
>      rx_queue_0_crc_errors: 0
>      rx_queue_0_frame_errors: 0
>      rx_queue_0_length_errors: 0
>      rx_queue_0_missed_errors: 0
>      rx_queue_0_over_errors: 0
>      rx_queue_1_current: 0
>      tx_queue_1_current: 0
>      rx_queue_1_dirty: 0
>      tx_queue_1_dirty: 0
>      rx_queue_1_packets: 0
>      tx_queue_1_packets: 0
>      rx_queue_1_bytes: 0
>      tx_queue_1_bytes: 0
>      rx_queue_1_mcast_packets: 0
>      rx_queue_1_errors: 0
>      rx_queue_1_crc_errors: 0
>      rx_queue_1_frame_errors: 0
>      rx_queue_1_length_errors: 0
>      rx_queue_1_missed_errors: 0
>      rx_queue_1_over_errors: 0
> 
> Cheers,
> Biju


^ permalink raw reply	[flat|nested] 59+ messages in thread

* RE: [PATCH net-next v2 8/8] ravb: Add tx_drop_cntrs to struct ravb_hw_info
  2021-08-04 20:50   ` Sergei Shtylyov
@ 2021-08-17 15:47     ` Biju Das
  2021-08-17 16:30       ` Sergey Shtylyov
  0 siblings, 1 reply; 59+ messages in thread
From: Biju Das @ 2021-08-17 15:47 UTC (permalink / raw)
  To: Sergei Shtylyov, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hi Sergei,

Thanks for the feedback.

> Subject: Re: [PATCH net-next v2 8/8] ravb: Add tx_drop_cntrs to struct
> ravb_hw_info
> 
> On 8/2/21 1:26 PM, Biju Das wrote:
> 
> > The register for retrieving TX drop counters is present only on R-Car
> > Gen3 and RZ/G2L; it is not present on R-Car Gen2.
> >
> > Add the tx_drop_cntrs hw feature bit to struct ravb_hw_info, to enable
> > this feature specifically for R-Car Gen3 now and later extend it to
> RZ/G2L.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > v2:
> >  * Incorporated Andrew and Sergei's review comments for making it
> smaller patch
> >    and provided detailed description.
> > ---
> >  drivers/net/ethernet/renesas/ravb.h      | 1 +
> >  drivers/net/ethernet/renesas/ravb_main.c | 4 +++-
> >  2 files changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/renesas/ravb.h
> > b/drivers/net/ethernet/renesas/ravb.h
> > index 0d640dbe1eed..35fbb9f60ba8 100644
> > --- a/drivers/net/ethernet/renesas/ravb.h
> > +++ b/drivers/net/ethernet/renesas/ravb.h
> > @@ -1001,6 +1001,7 @@ struct ravb_hw_info {
> >
> >  	/* hardware features */
> >  	unsigned internal_delay:1;	/* RAVB has internal delays */
> > +	unsigned tx_drop_cntrs:1;	/* RAVB has TX error counters */
> 
>    I suggest 'tx_counters' -- this name comes from the sh_eth driver for
> the same regs (but negated meaning). And please don't call the hardware
> RAVB. :-)

Agreed. Will change it to 'tx_counters' on next version and comment it as
/* AVB-DMAC has TX counters */

Cheers,
Biju

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 8/8] ravb: Add tx_drop_cntrs to struct ravb_hw_info
  2021-08-17 15:47     ` Biju Das
@ 2021-08-17 16:30       ` Sergey Shtylyov
  2021-08-17 16:33         ` Biju Das
  0 siblings, 1 reply; 59+ messages in thread
From: Sergey Shtylyov @ 2021-08-17 16:30 UTC (permalink / raw)
  To: Biju Das, Sergei Shtylyov, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hello!

On 8/17/21 6:47 PM, Biju Das wrote:

[...]
>>> The register for retrieving TX drop counters is present only on R-Car
>>> Gen3 and RZ/G2L; it is not present on R-Car Gen2.
>>>
>>> Add the tx_drop_cntrs hw feature bit to struct ravb_hw_info, to enable
>>> this feature specifically for R-Car Gen3 now and later extend it to
>> RZ/G2L.
>>>
>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>> ---
>>> v2:
>>>  * Incorporated Andrew and Sergei's review comments for making it
>> smaller patch
>>>    and provided detailed description.
>>> ---
>>>  drivers/net/ethernet/renesas/ravb.h      | 1 +
>>>  drivers/net/ethernet/renesas/ravb_main.c | 4 +++-
>>>  2 files changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ethernet/renesas/ravb.h
>>> b/drivers/net/ethernet/renesas/ravb.h
>>> index 0d640dbe1eed..35fbb9f60ba8 100644
>>> --- a/drivers/net/ethernet/renesas/ravb.h
>>> +++ b/drivers/net/ethernet/renesas/ravb.h
>>> @@ -1001,6 +1001,7 @@ struct ravb_hw_info {
>>>
>>>  	/* hardware features */
>>>  	unsigned internal_delay:1;	/* RAVB has internal delays */
>>> +	unsigned tx_drop_cntrs:1;	/* RAVB has TX error counters */
>>
>>    I suggest 'tx_counters' -- this name comes from the sh_eth driver for
>> the same regs (but negated meaning). And please don't call the hardware
>> RAVB. :-)
> 
> Agreed. Will change it to 'tx_counters' on next version and comment it as
> /* AVB-DMAC has TX counters */

   The counters belong to E-MAC, not AVB-DMAC.

> Cheers,
> Biju

MBR, Sergey

^ permalink raw reply	[flat|nested] 59+ messages in thread

* RE: [PATCH net-next v2 8/8] ravb: Add tx_drop_cntrs to struct ravb_hw_info
  2021-08-17 16:30       ` Sergey Shtylyov
@ 2021-08-17 16:33         ` Biju Das
  0 siblings, 0 replies; 59+ messages in thread
From: Biju Das @ 2021-08-17 16:33 UTC (permalink / raw)
  To: Sergey Shtylyov, Sergei Shtylyov, David S. Miller, Jakub Kicinski
  Cc: Geert Uytterhoeven, Sergey Shtylyov, Adam Ford, Andrew Lunn,
	Yuusuke Ashizuka, Yoshihiro Shimoda, netdev, linux-renesas-soc,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hi Sergei,

> Subject: Re: [PATCH net-next v2 8/8] ravb: Add tx_drop_cntrs to struct
> ravb_hw_info
> 
> Hello!
> 
> On 8/17/21 6:47 PM, Biju Das wrote:
> 
> [...]
> >>> The register for retrieving TX drop counters is present only on
> >>> R-Car
> >>> Gen3 and RZ/G2L; it is not present on R-Car Gen2.
> >>>
> >>> Add the tx_drop_cntrs hw feature bit to struct ravb_hw_info, to
> >>> enable this feature specifically for R-Car Gen3 now and later extend
> >>> it to
> >> RZ/G2L.
> >>>
> >>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>> ---
> >>> v2:
> >>>  * Incorporated Andrew and Sergei's review comments for making it
> >> smaller patch
> >>>    and provided detailed description.
> >>> ---
> >>>  drivers/net/ethernet/renesas/ravb.h      | 1 +
> >>>  drivers/net/ethernet/renesas/ravb_main.c | 4 +++-
> >>>  2 files changed, 4 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/net/ethernet/renesas/ravb.h
> >>> b/drivers/net/ethernet/renesas/ravb.h
> >>> index 0d640dbe1eed..35fbb9f60ba8 100644
> >>> --- a/drivers/net/ethernet/renesas/ravb.h
> >>> +++ b/drivers/net/ethernet/renesas/ravb.h
> >>> @@ -1001,6 +1001,7 @@ struct ravb_hw_info {
> >>>
> >>>  	/* hardware features */
> >>>  	unsigned internal_delay:1;	/* RAVB has internal delays */
> >>> +	unsigned tx_drop_cntrs:1;	/* RAVB has TX error counters */
> >>
> >>    I suggest 'tx_counters' -- this name comes from the sh_eth driver
> >> for the same regs (but negated meaning). And please don't call the
> >> hardware RAVB. :-)
> >
> > Agreed. Will change it to 'tx_counters' on next version and comment it
> > as
> > /* AVB-DMAC has TX counters */
> 
>    The counters belong to E-MAC, not AVB-DMAC.

You are correct, it is at offset 0x700 on E-MAC block.

Cheers,
Biju

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to driver data
  2021-08-17 11:24           ` Biju Das
@ 2021-08-17 20:11             ` Sergey Shtylyov
  2021-08-18  6:29               ` Biju Das
  0 siblings, 1 reply; 59+ messages in thread
From: Sergey Shtylyov @ 2021-08-17 20:11 UTC (permalink / raw)
  To: Biju Das, Geert Uytterhoeven, Sergey Shtylyov
  Cc: David S. Miller, Jakub Kicinski, Sergei Shtylyov, Adam Ford,
	Andrew Lunn, Yuusuke Ashizuka, Yoshihiro Shimoda, netdev,
	Linux-Renesas, Chris Paterson, Biju Das, Prabhakar Mahadev Lad

On 8/17/21 2:24 PM, Biju Das wrote:

[...]
>>>>> -----Original Message-----
>>>>> On Mon, Aug 2, 2021 at 12:27 PM Biju Das
>>>>> <biju.das.jz@bp.renesas.com>
>>>>> wrote:
>>>>>> The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L
>>>>>> SoC are similar to the R-Car Ethernet AVB IP. With a few changes
>>>>>> in the driver we can support both IPs.
>>>>>>
>>>>>> Currently a runtime decision based on the chip type is used to
>>>>>> distinguish the HW differences between the SoC families.
>>>>>>
>>>>>> The number of TX descriptors for R-Car Gen3 is 1 whereas on
>>>>>> R-Car
>>>>>> Gen2 and RZ/G2L it is 2. For cases like this it is better to
>>>>>> select the number of TX descriptors by using a structure with a
>>>>>> value, rather than a runtime decision based on the chip type.
>>>>>>
>>>>>> This patch adds the num_tx_desc variable to struct ravb_hw_info
>>>>>> and also replaces the driver data chip type with struct
>>>>>> ravb_hw_info by moving chip type to it.
>>>>>>
>>>>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>>>>> Reviewed-by: Lad Prabhakar
>>>>>> <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>>>>
>>>>> Thanks for your patch!
>>>>>
>>>>>> --- a/drivers/net/ethernet/renesas/ravb.h
>>>>>> +++ b/drivers/net/ethernet/renesas/ravb.h
>>>>>> @@ -988,6 +988,11 @@ enum ravb_chip_id {
>>>>>>         RCAR_GEN3,
>>>>>>  };
>>>>>>
>>>>>> +struct ravb_hw_info {
>>>>>> +       enum ravb_chip_id chip_id;
>>>>>> +       int num_tx_desc;
>>>>>
>>>>> Why not "unsigned int"? ...
>>>>> This comment applies to a few more subsequent patches.
>>>>
>>>> To avoid signed and unsigned comparison warnings.
>>>>
>>>>>
>>>>>> +};
>>>>>> +
>>>>>>  struct ravb_private {
>>>>>>         struct net_device *ndev;
>>>>>>         struct platform_device *pdev; @@ -1040,6 +1045,8 @@
>>>>>> struct ravb_private {
>>>>>>         unsigned txcidm:1;              /* TX Clock Internal Delay
>>> Mode
>>>>> */
>>>>>>         unsigned rgmii_override:1;      /* Deprecated rgmii-*id
>>> behavior
>>>>> */
>>>>>>         int num_tx_desc;                /* TX descriptors per
>> packet
>>> */
>>>>>
>>>>> ... oh, here's the original culprit.
>>>>
>>>> Exactly, this the reason.
>>>>
>>>> Do you want me to change this into unsigned int? Please let me know.
>>>
>>> Up to you (or the maintainer? ;-)
>>>
>>> For new fields (in the other patches), I would use unsigned for all
>>> unsigned values.  Signed values have more pitfalls related to
>>> undefined behavior.
>>
>> Sergei, What is your thoughts here? Please let me know.
> 
> Here is my plan.
> 
> I will split this patch into two as Andrew suggested and 

   If you mran changing the ravb_private::num_tx_desc to *unsigned*, it'll be
a good cleanup. What's would be the 2nd part tho?

> Then on the second patch will add as info->unaligned_tx as Sergei suggested.

   OK.

> Now the only open point is related to the data type of "int num_tx_desc"
> and to align with sh_eth driver I will keep int.

   The sh_eth driver simply doesn't have this -- it always use 1 descriptor.

> Regards,
> Biju

MBR, Sergey

^ permalink raw reply	[flat|nested] 59+ messages in thread

* RE: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to driver data
  2021-08-17 20:11             ` Sergey Shtylyov
@ 2021-08-18  6:29               ` Biju Das
  2021-08-18 10:11                 ` Sergey Shtylyov
  0 siblings, 1 reply; 59+ messages in thread
From: Biju Das @ 2021-08-18  6:29 UTC (permalink / raw)
  To: Sergey Shtylyov, Geert Uytterhoeven, Sergey Shtylyov
  Cc: David S. Miller, Jakub Kicinski, Sergei Shtylyov, Adam Ford,
	Andrew Lunn, Yuusuke Ashizuka, Yoshihiro Shimoda, netdev,
	Linux-Renesas, Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hi Sergei,

Thanks for the feedback.

> Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to
> driver data
> 
> On 8/17/21 2:24 PM, Biju Das wrote:
> 
> [...]
> >>>>> -----Original Message-----
> >>>>> On Mon, Aug 2, 2021 at 12:27 PM Biju Das
> >>>>> <biju.das.jz@bp.renesas.com>
> >>>>> wrote:
> >>>>>> The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L
> >>>>>> SoC are similar to the R-Car Ethernet AVB IP. With a few changes
> >>>>>> in the driver we can support both IPs.
> >>>>>>
> >>>>>> Currently a runtime decision based on the chip type is used to
> >>>>>> distinguish the HW differences between the SoC families.
> >>>>>>
> >>>>>> The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car
> >>>>>> Gen2 and RZ/G2L it is 2. For cases like this it is better to
> >>>>>> select the number of TX descriptors by using a structure with a
> >>>>>> value, rather than a runtime decision based on the chip type.
> >>>>>>
> >>>>>> This patch adds the num_tx_desc variable to struct ravb_hw_info
> >>>>>> and also replaces the driver data chip type with struct
> >>>>>> ravb_hw_info by moving chip type to it.
> >>>>>>
> >>>>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >>>>>> Reviewed-by: Lad Prabhakar
> >>>>>> <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>>>>
> >>>>> Thanks for your patch!
> >>>>>
> >>>>>> --- a/drivers/net/ethernet/renesas/ravb.h
> >>>>>> +++ b/drivers/net/ethernet/renesas/ravb.h
> >>>>>> @@ -988,6 +988,11 @@ enum ravb_chip_id {
> >>>>>>         RCAR_GEN3,
> >>>>>>  };
> >>>>>>
> >>>>>> +struct ravb_hw_info {
> >>>>>> +       enum ravb_chip_id chip_id;
> >>>>>> +       int num_tx_desc;
> >>>>>
> >>>>> Why not "unsigned int"? ...
> >>>>> This comment applies to a few more subsequent patches.
> >>>>
> >>>> To avoid signed and unsigned comparison warnings.
> >>>>
> >>>>>
> >>>>>> +};
> >>>>>> +
> >>>>>>  struct ravb_private {
> >>>>>>         struct net_device *ndev;
> >>>>>>         struct platform_device *pdev; @@ -1040,6 +1045,8 @@
> >>>>>> struct ravb_private {
> >>>>>>         unsigned txcidm:1;              /* TX Clock Internal Delay
> >>> Mode
> >>>>> */
> >>>>>>         unsigned rgmii_override:1;      /* Deprecated rgmii-*id
> >>> behavior
> >>>>> */
> >>>>>>         int num_tx_desc;                /* TX descriptors per
> >> packet
> >>> */
> >>>>>
> >>>>> ... oh, here's the original culprit.
> >>>>
> >>>> Exactly, this the reason.
> >>>>
> >>>> Do you want me to change this into unsigned int? Please let me know.
> >>>
> >>> Up to you (or the maintainer? ;-)
> >>>
> >>> For new fields (in the other patches), I would use unsigned for all
> >>> unsigned values.  Signed values have more pitfalls related to
> >>> undefined behavior.
> >>
> >> Sergei, What is your thoughts here? Please let me know.
> >
> > Here is my plan.
> >
> > I will split this patch into two as Andrew suggested and
> 
>    If you mran changing the ravb_private::num_tx_desc to *unsigned*, it'll
> be a good cleanup. What's would be the 2nd part tho?

OK in that case, I will split this patch into 3.

First patch for adding struct ravb_hw_info to driver data and replace 
driver data chip type with struct ravb_hw_info

Second patch for changing ravb_private::num_tx_desc from int to unsigned int.

Third patch for adding aligned_tx to struct ravb_hw_info.

Regards,
Biju

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to driver data
  2021-08-18  6:29               ` Biju Das
@ 2021-08-18 10:11                 ` Sergey Shtylyov
  2021-08-18 10:23                   ` Biju Das
  0 siblings, 1 reply; 59+ messages in thread
From: Sergey Shtylyov @ 2021-08-18 10:11 UTC (permalink / raw)
  To: Biju Das, Geert Uytterhoeven, Sergey Shtylyov
  Cc: David S. Miller, Jakub Kicinski, Sergei Shtylyov, Adam Ford,
	Andrew Lunn, Yuusuke Ashizuka, Yoshihiro Shimoda, netdev,
	Linux-Renesas, Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hello!

On 18.08.2021 9:29, Biju Das wrote:

[...]
>>>>>>> -----Original Message-----
>>>>>>> On Mon, Aug 2, 2021 at 12:27 PM Biju Das
>>>>>>> <biju.das.jz@bp.renesas.com>
>>>>>>> wrote:
>>>>>>>> The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L
>>>>>>>> SoC are similar to the R-Car Ethernet AVB IP. With a few changes
>>>>>>>> in the driver we can support both IPs.
>>>>>>>>
>>>>>>>> Currently a runtime decision based on the chip type is used to
>>>>>>>> distinguish the HW differences between the SoC families.
>>>>>>>>
>>>>>>>> The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car
>>>>>>>> Gen2 and RZ/G2L it is 2. For cases like this it is better to
>>>>>>>> select the number of TX descriptors by using a structure with a
>>>>>>>> value, rather than a runtime decision based on the chip type.
>>>>>>>>
>>>>>>>> This patch adds the num_tx_desc variable to struct ravb_hw_info
>>>>>>>> and also replaces the driver data chip type with struct
>>>>>>>> ravb_hw_info by moving chip type to it.
>>>>>>>>
>>>>>>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>>>>>>> Reviewed-by: Lad Prabhakar
>>>>>>>> <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>>>>>>
>>>>>>> Thanks for your patch!
>>>>>>>
>>>>>>>> --- a/drivers/net/ethernet/renesas/ravb.h
>>>>>>>> +++ b/drivers/net/ethernet/renesas/ravb.h
>>>>>>>> @@ -988,6 +988,11 @@ enum ravb_chip_id {
>>>>>>>>          RCAR_GEN3,
>>>>>>>>   };
>>>>>>>>
>>>>>>>> +struct ravb_hw_info {
>>>>>>>> +       enum ravb_chip_id chip_id;
>>>>>>>> +       int num_tx_desc;
>>>>>>>
>>>>>>> Why not "unsigned int"? ...
>>>>>>> This comment applies to a few more subsequent patches.
>>>>>>
>>>>>> To avoid signed and unsigned comparison warnings.
>>>>>>
>>>>>>>
>>>>>>>> +};
>>>>>>>> +
>>>>>>>>   struct ravb_private {
>>>>>>>>          struct net_device *ndev;
>>>>>>>>          struct platform_device *pdev; @@ -1040,6 +1045,8 @@
>>>>>>>> struct ravb_private {
>>>>>>>>          unsigned txcidm:1;              /* TX Clock Internal Delay
>>>>> Mode
>>>>>>> */
>>>>>>>>          unsigned rgmii_override:1;      /* Deprecated rgmii-*id
>>>>> behavior
>>>>>>> */
>>>>>>>>          int num_tx_desc;                /* TX descriptors per
>>>> packet
>>>>> */
>>>>>>>
>>>>>>> ... oh, here's the original culprit.
>>>>>>
>>>>>> Exactly, this the reason.
>>>>>>
>>>>>> Do you want me to change this into unsigned int? Please let me know.
>>>>>
>>>>> Up to you (or the maintainer? ;-)
>>>>>
>>>>> For new fields (in the other patches), I would use unsigned for all
>>>>> unsigned values.  Signed values have more pitfalls related to
>>>>> undefined behavior.
>>>>
>>>> Sergei, What is your thoughts here? Please let me know.
>>>
>>> Here is my plan.
>>>
>>> I will split this patch into two as Andrew suggested and
>>
>>     If you mran changing the ravb_private::num_tx_desc to *unsigned*, it'll
>> be a good cleanup. What's would be the 2nd part tho?
> 
> OK in that case, I will split this patch into 3.
> 
> First patch for adding struct ravb_hw_info to driver data and replace
> driver data chip type with struct ravb_hw_info

    Couldn't this be a 2nd patch?..

> Second patch for changing ravb_private::num_tx_desc from int to unsigned int.

    ... and this one the 1st?

> Third patch for adding aligned_tx to struct ravb_hw_info.
> 
> Regards,
> Biju

MBR, Sergey

^ permalink raw reply	[flat|nested] 59+ messages in thread

* RE: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to driver data
  2021-08-18 10:11                 ` Sergey Shtylyov
@ 2021-08-18 10:23                   ` Biju Das
  0 siblings, 0 replies; 59+ messages in thread
From: Biju Das @ 2021-08-18 10:23 UTC (permalink / raw)
  To: Sergey Shtylyov, Geert Uytterhoeven, Sergey Shtylyov
  Cc: David S. Miller, Jakub Kicinski, Sergei Shtylyov, Adam Ford,
	Andrew Lunn, Yuusuke Ashizuka, Yoshihiro Shimoda, netdev,
	Linux-Renesas, Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hi Sergei,

> -----Original Message-----
> Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to
> driver data
> 
> Hello!
> 
> On 18.08.2021 9:29, Biju Das wrote:
> 
> [...]
> >>>>>>> -----Original Message-----
> >>>>>>> On Mon, Aug 2, 2021 at 12:27 PM Biju Das
> >>>>>>> <biju.das.jz@bp.renesas.com>
> >>>>>>> wrote:
> >>>>>>>> The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L
> >>>>>>>> SoC are similar to the R-Car Ethernet AVB IP. With a few
> >>>>>>>> changes in the driver we can support both IPs.
> >>>>>>>>
> >>>>>>>> Currently a runtime decision based on the chip type is used to
> >>>>>>>> distinguish the HW differences between the SoC families.
> >>>>>>>>
> >>>>>>>> The number of TX descriptors for R-Car Gen3 is 1 whereas on
> >>>>>>>> R-Car
> >>>>>>>> Gen2 and RZ/G2L it is 2. For cases like this it is better to
> >>>>>>>> select the number of TX descriptors by using a structure with a
> >>>>>>>> value, rather than a runtime decision based on the chip type.
> >>>>>>>>
> >>>>>>>> This patch adds the num_tx_desc variable to struct ravb_hw_info
> >>>>>>>> and also replaces the driver data chip type with struct
> >>>>>>>> ravb_hw_info by moving chip type to it.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >>>>>>>> Reviewed-by: Lad Prabhakar
> >>>>>>>> <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>>>>>>
> >>>>>>> Thanks for your patch!
> >>>>>>>
> >>>>>>>> --- a/drivers/net/ethernet/renesas/ravb.h
> >>>>>>>> +++ b/drivers/net/ethernet/renesas/ravb.h
> >>>>>>>> @@ -988,6 +988,11 @@ enum ravb_chip_id {
> >>>>>>>>          RCAR_GEN3,
> >>>>>>>>   };
> >>>>>>>>
> >>>>>>>> +struct ravb_hw_info {
> >>>>>>>> +       enum ravb_chip_id chip_id;
> >>>>>>>> +       int num_tx_desc;
> >>>>>>>
> >>>>>>> Why not "unsigned int"? ...
> >>>>>>> This comment applies to a few more subsequent patches.
> >>>>>>
> >>>>>> To avoid signed and unsigned comparison warnings.
> >>>>>>
> >>>>>>>
> >>>>>>>> +};
> >>>>>>>> +
> >>>>>>>>   struct ravb_private {
> >>>>>>>>          struct net_device *ndev;
> >>>>>>>>          struct platform_device *pdev; @@ -1040,6 +1045,8 @@
> >>>>>>>> struct ravb_private {
> >>>>>>>>          unsigned txcidm:1;              /* TX Clock Internal
> Delay
> >>>>> Mode
> >>>>>>> */
> >>>>>>>>          unsigned rgmii_override:1;      /* Deprecated rgmii-*id
> >>>>> behavior
> >>>>>>> */
> >>>>>>>>          int num_tx_desc;                /* TX descriptors per
> >>>> packet
> >>>>> */
> >>>>>>>
> >>>>>>> ... oh, here's the original culprit.
> >>>>>>
> >>>>>> Exactly, this the reason.
> >>>>>>
> >>>>>> Do you want me to change this into unsigned int? Please let me
> know.
> >>>>>
> >>>>> Up to you (or the maintainer? ;-)
> >>>>>
> >>>>> For new fields (in the other patches), I would use unsigned for
> >>>>> all unsigned values.  Signed values have more pitfalls related to
> >>>>> undefined behavior.
> >>>>
> >>>> Sergei, What is your thoughts here? Please let me know.
> >>>
> >>> Here is my plan.
> >>>
> >>> I will split this patch into two as Andrew suggested and
> >>
> >>     If you mran changing the ravb_private::num_tx_desc to *unsigned*,
> >> it'll be a good cleanup. What's would be the 2nd part tho?
> >
> > OK in that case, I will split this patch into 3.
> >
> > First patch for adding struct ravb_hw_info to driver data and replace
> > driver data chip type with struct ravb_hw_info
> 
>     Couldn't this be a 2nd patch?..
> 
> > Second patch for changing ravb_private::num_tx_desc from int to unsigned
> int.
> 
>     ... and this one the 1st?
> 
> > Third patch for adding aligned_tx to struct ravb_hw_info.

Sure. Will do.

Cheers,
Biju

^ permalink raw reply	[flat|nested] 59+ messages in thread

end of thread, other threads:[~2021-08-18 10:23 UTC | newest]

Thread overview: 59+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-02 10:26 [PATCH net-next v2 0/8] Add Gigabit Ethernet driver support Biju Das
2021-08-02 10:26 ` [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to driver data Biju Das
2021-08-02 15:02   ` Andrew Lunn
2021-08-02 20:42   ` Sergei Shtylyov
2021-08-03  5:57     ` Biju Das
2021-08-03  6:36       ` Biju Das
2021-08-04 19:27       ` Sergei Shtylyov
2021-08-04 20:27         ` Sergei Shtylyov
2021-08-09 12:06   ` Geert Uytterhoeven
2021-08-12  7:26     ` Biju Das
2021-08-12  7:58       ` Geert Uytterhoeven
2021-08-12  8:13         ` Biju Das
2021-08-17 11:24           ` Biju Das
2021-08-17 20:11             ` Sergey Shtylyov
2021-08-18  6:29               ` Biju Das
2021-08-18 10:11                 ` Sergey Shtylyov
2021-08-18 10:23                   ` Biju Das
2021-08-02 10:26 ` [PATCH net-next v2 2/8] ravb: Add skb_sz to struct ravb_hw_info Biju Das
2021-08-02 15:08   ` Andrew Lunn
2021-08-02 20:54   ` Sergei Shtylyov
2021-08-03  7:13     ` Biju Das
2021-08-02 10:26 ` [PATCH net-next v2 3/8] ravb: Add num_gstat_queue " Biju Das
2021-08-02 15:09   ` Andrew Lunn
2021-08-03 18:21   ` Sergei Shtylyov
2021-08-03 19:13     ` Biju Das
2021-08-03 19:22       ` Sergei Shtylyov
2021-08-03 19:47         ` Biju Das
2021-08-17 15:08           ` Biju Das
2021-08-02 10:26 ` [PATCH net-next v2 4/8] ravb: Add stats_len " Biju Das
2021-08-03 18:35   ` Sergei Shtylyov
2021-08-03 18:47     ` Biju Das
2021-08-03 19:20       ` Sergei Shtylyov
2021-08-02 10:26 ` [PATCH net-next v2 5/8] ravb: Add gstrings_stats and gstrings_size " Biju Das
2021-08-02 15:11   ` Andrew Lunn
2021-08-04 20:36   ` Sergei Shtylyov
2021-08-02 10:26 ` [PATCH net-next v2 6/8] ravb: Add net_features and net_hw_features " Biju Das
2021-08-02 15:12   ` Andrew Lunn
2021-08-05 19:07   ` Sergei Shtylyov
2021-08-05 19:18     ` Biju Das
2021-08-06 20:20       ` Sergei Shtylyov
2021-08-12  7:35         ` Biju Das
2021-08-06 20:31   ` Sergei Shtylyov
2021-08-02 10:26 ` [PATCH net-next v2 7/8] ravb: Add internal delay hw feature " Biju Das
2021-08-02 15:13   ` Andrew Lunn
2021-08-03 21:06   ` Sergei Shtylyov
2021-08-04  6:19     ` Biju Das
2021-08-03 21:12   ` Sergei Shtylyov
2021-08-04  5:13     ` Biju Das
2021-08-04  9:51       ` Sergey Shtylyov
2021-08-04 10:08         ` Biju Das
2021-08-04 10:34           ` Sergei Shtylyov
2021-08-04 10:20       ` Sergei Shtylyov
2021-08-04 10:32         ` Biju Das
2021-08-02 10:26 ` [PATCH net-next v2 8/8] ravb: Add tx_drop_cntrs " Biju Das
2021-08-02 15:14   ` Andrew Lunn
2021-08-04 20:50   ` Sergei Shtylyov
2021-08-17 15:47     ` Biju Das
2021-08-17 16:30       ` Sergey Shtylyov
2021-08-17 16:33         ` Biju Das

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).