[PATCH net-next 00/13] Add Factorisation code to support Gigabit Ethernet driver

[PATCH net-next 00/13] Add Factorisation code to support Gigabit Ethernet driver

[Biju Das]

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

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

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

This patch series aims to add factorisation code to support RZ/G2L SoC,
hardware feature bits for gPTP feature, Multiple irq feature and 
optional reset support.

Ref:-
 * https://lore.kernel.org/linux-renesas-soc/TYCPR01MB59334319695607A2683C1A5E86E59@TYCPR01MB5933.jpnprd01.prod.outlook.com/T/#t

Biju Das (13):
  ravb: Remove the macros NUM_TX_DESC_GEN[23]
  ravb: Add multi_irq to struct ravb_hw_info
  ravb: Add no_ptp_cfg_active to struct ravb_hw_info
  ravb: Add ptp_cfg_active to struct ravb_hw_info
  ravb: Factorise ravb_ring_free function
  ravb: Factorise ravb_ring_format function
  ravb: Factorise ravb_ring_init function
  ravb: Factorise ravb_rx function
  ravb: Factorise ravb_adjust_link function
  ravb: Factorise ravb_set_features
  ravb: Factorise ravb_dmac_init function
  ravb: Factorise ravb_emac_init function
  ravb: Add reset support

 drivers/net/ethernet/renesas/ravb.h      |  23 +-
 drivers/net/ethernet/renesas/ravb_main.c | 272 ++++++++++++++++-------
 drivers/net/ethernet/renesas/ravb_ptp.c  |   8 +-
 3 files changed, 204 insertions(+), 99 deletions(-)

-- 
2.17.1

[PATCH net-next 01/13] ravb: Remove the macros NUM_TX_DESC_GEN[23]

[Biju Das]

For addressing 4 bytes alignment restriction on transmission
buffer for R-Car Gen2 we use 2 descriptors whereas it is a single
descriptor for other cases.
Replace the macros NUM_TX_DESC_GEN[23] with magic number and
add a comment to explain it.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Suggested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/net/ethernet/renesas/ravb.h      | 4 ----
 drivers/net/ethernet/renesas/ravb_main.c | 8 ++++++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 37ad0f8aaf3c..84700a82a41c 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -956,10 +956,6 @@ enum RAVB_QUEUE {
 
 #define RX_BUF_SZ	(2048 - ETH_FCS_LEN + sizeof(__sum16))
 
-/* TX descriptors per packet */
-#define NUM_TX_DESC_GEN2	2
-#define NUM_TX_DESC_GEN3	1
-
 struct ravb_tstamp_skb {
 	struct list_head list;
 	struct sk_buff *skb;
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 02842b980a7f..073e690ab830 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -2160,8 +2160,12 @@ 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 = info->aligned_tx ?
-		NUM_TX_DESC_GEN2 : NUM_TX_DESC_GEN3;
+	/* FIXME: R-Car Gen2 has 4byte alignment restriction for tx buffer
+	 * Use two descriptor to handle such situation. First descriptor to
+	 * handle aligned data buffer and second descriptor to handle the
+	 * overflow data because of alignment.
+	 */
+	priv->num_tx_desc = info->aligned_tx ? 2 : 1;
 
 	/* Set function */
 	ndev->netdev_ops = &ravb_netdev_ops;
-- 
2.17.1

[PATCH net-next 02/13] ravb: Add multi_irq to struct ravb_hw_info

[Biju Das]

R-Car Gen3 supports separate interrupts for E-MAC and DMA queues,
whereas R-Car Gen2 and RZ/G2L have a single interrupt instead.

Add a multi_irq 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>
---
 drivers/net/ethernet/renesas/ravb.h      |  1 +
 drivers/net/ethernet/renesas/ravb_main.c | 22 ++++++++++++++--------
 drivers/net/ethernet/renesas/ravb_ptp.c  |  8 +++++---
 3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 84700a82a41c..da486e06b322 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -997,6 +997,7 @@ struct ravb_hw_info {
 	/* hardware features */
 	unsigned internal_delay:1;	/* AVB-DMAC has internal delays */
 	unsigned tx_counters:1;		/* E-MAC has TX counters */
+	unsigned multi_irqs:1;		/* AVB-DMAC and E-MAC has multiple irqs */
 };
 
 struct ravb_private {
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 073e690ab830..28b8dcae57a8 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -427,6 +427,7 @@ static void ravb_emac_init(struct net_device *ndev)
 static int ravb_dmac_init(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
 	int error;
 
 	/* Set CONFIG mode */
@@ -458,7 +459,7 @@ static int ravb_dmac_init(struct net_device *ndev)
 	ravb_write(ndev, TCCR_TFEN, TCCR);
 
 	/* Interrupt init: */
-	if (priv->chip_id == RCAR_GEN3) {
+	if (info->multi_irqs) {
 		/* Clear DIL.DPLx */
 		ravb_write(ndev, 0, DIL);
 		/* Set queue specific interrupt */
@@ -758,6 +759,7 @@ static void ravb_error_interrupt(struct net_device *ndev)
 static bool ravb_queue_interrupt(struct net_device *ndev, int q)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
 	u32 ris0 = ravb_read(ndev, RIS0);
 	u32 ric0 = ravb_read(ndev, RIC0);
 	u32 tis  = ravb_read(ndev, TIS);
@@ -766,7 +768,7 @@ static bool ravb_queue_interrupt(struct net_device *ndev, int q)
 	if (((ris0 & ric0) & BIT(q)) || ((tis  & tic)  & BIT(q))) {
 		if (napi_schedule_prep(&priv->napi[q])) {
 			/* Mask RX and TX interrupts */
-			if (priv->chip_id == RCAR_GEN2) {
+			if (!info->multi_irqs) {
 				ravb_write(ndev, ric0 & ~BIT(q), RIC0);
 				ravb_write(ndev, tic & ~BIT(q), TIC);
 			} else {
@@ -909,6 +911,7 @@ static int ravb_poll(struct napi_struct *napi, int budget)
 {
 	struct net_device *ndev = napi->dev;
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
 	unsigned long flags;
 	int q = napi - priv->napi;
 	int mask = BIT(q);
@@ -932,7 +935,7 @@ static int ravb_poll(struct napi_struct *napi, int budget)
 
 	/* Re-enable RX/TX interrupts */
 	spin_lock_irqsave(&priv->lock, flags);
-	if (priv->chip_id == RCAR_GEN2) {
+	if (!info->multi_irqs) {
 		ravb_modify(ndev, RIC0, mask, mask);
 		ravb_modify(ndev, TIC,  mask, mask);
 	} else {
@@ -1338,6 +1341,7 @@ static inline int ravb_hook_irq(unsigned int irq, irq_handler_t handler,
 static int ravb_open(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
 	struct platform_device *pdev = priv->pdev;
 	struct device *dev = &pdev->dev;
 	int error;
@@ -1345,7 +1349,7 @@ static int ravb_open(struct net_device *ndev)
 	napi_enable(&priv->napi[RAVB_BE]);
 	napi_enable(&priv->napi[RAVB_NC]);
 
-	if (priv->chip_id == RCAR_GEN2) {
+	if (!info->multi_irqs) {
 		error = request_irq(ndev->irq, ravb_interrupt, IRQF_SHARED,
 				    ndev->name, ndev);
 		if (error) {
@@ -1403,7 +1407,7 @@ static int ravb_open(struct net_device *ndev)
 	if (priv->chip_id == RCAR_GEN2)
 		ravb_ptp_stop(ndev);
 out_free_irq_nc_tx:
-	if (priv->chip_id == RCAR_GEN2)
+	if (!info->multi_irqs)
 		goto out_free_irq;
 	free_irq(priv->tx_irqs[RAVB_NC], ndev);
 out_free_irq_nc_rx:
@@ -1680,6 +1684,7 @@ static int ravb_close(struct net_device *ndev)
 {
 	struct device_node *np = ndev->dev.parent->of_node;
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
 	struct ravb_tstamp_skb *ts_skb, *ts_skb2;
 
 	netif_tx_stop_all_queues(ndev);
@@ -1713,7 +1718,7 @@ static int ravb_close(struct net_device *ndev)
 			of_phy_deregister_fixed_link(np);
 	}
 
-	if (priv->chip_id != RCAR_GEN2) {
+	if (info->multi_irqs) {
 		free_irq(priv->tx_irqs[RAVB_NC], ndev);
 		free_irq(priv->rx_irqs[RAVB_NC], ndev);
 		free_irq(priv->tx_irqs[RAVB_BE], ndev);
@@ -1939,6 +1944,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.max_rx_len = RX_BUF_SZ + RAVB_ALIGN - 1,
 	.internal_delay = 1,
 	.tx_counters = 1,
+	.multi_irqs = 1,
 };
 
 static const struct ravb_hw_info ravb_gen2_hw_info = {
@@ -2077,7 +2083,7 @@ static int ravb_probe(struct platform_device *pdev)
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_get_sync(&pdev->dev);
 
-	if (info->chip_id == RCAR_GEN3)
+	if (info->multi_irqs)
 		irq = platform_get_irq_byname(pdev, "ch22");
 	else
 		irq = platform_get_irq(pdev, 0);
@@ -2117,7 +2123,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 (info->chip_id == RCAR_GEN3) {
+	if (info->multi_irqs) {
 		irq = platform_get_irq_byname(pdev, "ch24");
 		if (irq < 0) {
 			error = irq;
diff --git a/drivers/net/ethernet/renesas/ravb_ptp.c b/drivers/net/ethernet/renesas/ravb_ptp.c
index 6984bd5b7da9..c099656dd75b 100644
--- a/drivers/net/ethernet/renesas/ravb_ptp.c
+++ b/drivers/net/ethernet/renesas/ravb_ptp.c
@@ -179,6 +179,7 @@ static int ravb_ptp_extts(struct ptp_clock_info *ptp,
 {
 	struct ravb_private *priv = container_of(ptp, struct ravb_private,
 						 ptp.info);
+	const struct ravb_hw_info *info = priv->info;
 	struct net_device *ndev = priv->ndev;
 	unsigned long flags;
 
@@ -197,7 +198,7 @@ static int ravb_ptp_extts(struct ptp_clock_info *ptp,
 	priv->ptp.extts[req->index] = on;
 
 	spin_lock_irqsave(&priv->lock, flags);
-	if (priv->chip_id == RCAR_GEN2)
+	if (!info->multi_irqs)
 		ravb_modify(ndev, GIC, GIC_PTCE, on ? GIC_PTCE : 0);
 	else if (on)
 		ravb_write(ndev, GIE_PTCS, GIE);
@@ -213,6 +214,7 @@ static int ravb_ptp_perout(struct ptp_clock_info *ptp,
 {
 	struct ravb_private *priv = container_of(ptp, struct ravb_private,
 						 ptp.info);
+	const struct ravb_hw_info *info = priv->info;
 	struct net_device *ndev = priv->ndev;
 	struct ravb_ptp_perout *perout;
 	unsigned long flags;
@@ -252,7 +254,7 @@ static int ravb_ptp_perout(struct ptp_clock_info *ptp,
 		error = ravb_ptp_update_compare(priv, (u32)start_ns);
 		if (!error) {
 			/* Unmask interrupt */
-			if (priv->chip_id == RCAR_GEN2)
+			if (!info->multi_irqs)
 				ravb_modify(ndev, GIC, GIC_PTME, GIC_PTME);
 			else
 				ravb_write(ndev, GIE_PTMS0, GIE);
@@ -264,7 +266,7 @@ static int ravb_ptp_perout(struct ptp_clock_info *ptp,
 		perout->period = 0;
 
 		/* Mask interrupt */
-		if (priv->chip_id == RCAR_GEN2)
+		if (!info->multi_irqs)
 			ravb_modify(ndev, GIC, GIC_PTME, 0);
 		else
 			ravb_write(ndev, GID_PTMD0, GID);
-- 
2.17.1

[PATCH net-next 03/13] ravb: Add no_ptp_cfg_active to struct ravb_hw_info

[Biju Das]

There are some H/W differences for the gPTP feature between
R-Car Gen3, R-Car Gen2, and RZ/G2L as below.

1) On R-Car Gen2, gPTP support is not active in config mode.
2) On R-Car Gen3, gPTP support is active in config mode.
3) RZ/G2L does not support the gPTP feature.

Add a no_ptp_cfg_active hw feature bit to struct ravb_hw_info for
handling gPTP for R-Car Gen2.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/net/ethernet/renesas/ravb.h      |  1 +
 drivers/net/ethernet/renesas/ravb_main.c | 20 ++++++++++++--------
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index da486e06b322..9ecf1a8c3ca8 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -998,6 +998,7 @@ struct ravb_hw_info {
 	unsigned internal_delay:1;	/* AVB-DMAC has internal delays */
 	unsigned tx_counters:1;		/* E-MAC has TX counters */
 	unsigned multi_irqs:1;		/* AVB-DMAC and E-MAC has multiple irqs */
+	unsigned no_ptp_cfg_active:1;	/* AVB-DMAC does not support gPTP active in config mode */
 };
 
 struct ravb_private {
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 28b8dcae57a8..e33b836218f0 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1205,6 +1205,7 @@ static int ravb_set_ringparam(struct net_device *ndev,
 			      struct ethtool_ringparam *ring)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
 	int error;
 
 	if (ring->tx_pending > BE_TX_RING_MAX ||
@@ -1218,7 +1219,7 @@ static int ravb_set_ringparam(struct net_device *ndev,
 	if (netif_running(ndev)) {
 		netif_device_detach(ndev);
 		/* Stop PTP Clock driver */
-		if (priv->chip_id == RCAR_GEN2)
+		if (info->no_ptp_cfg_active)
 			ravb_ptp_stop(ndev);
 		/* Wait for DMA stopping */
 		error = ravb_stop_dma(ndev);
@@ -1250,7 +1251,7 @@ static int ravb_set_ringparam(struct net_device *ndev,
 		ravb_emac_init(ndev);
 
 		/* Initialise PTP Clock driver */
-		if (priv->chip_id == RCAR_GEN2)
+		if (info->no_ptp_cfg_active)
 			ravb_ptp_init(ndev, priv->pdev);
 
 		netif_device_attach(ndev);
@@ -1390,7 +1391,7 @@ static int ravb_open(struct net_device *ndev)
 	ravb_emac_init(ndev);
 
 	/* Initialise PTP Clock driver */
-	if (priv->chip_id == RCAR_GEN2)
+	if (info->no_ptp_cfg_active)
 		ravb_ptp_init(ndev, priv->pdev);
 
 	netif_tx_start_all_queues(ndev);
@@ -1404,7 +1405,7 @@ static int ravb_open(struct net_device *ndev)
 
 out_ptp_stop:
 	/* Stop PTP Clock driver */
-	if (priv->chip_id == RCAR_GEN2)
+	if (info->no_ptp_cfg_active)
 		ravb_ptp_stop(ndev);
 out_free_irq_nc_tx:
 	if (!info->multi_irqs)
@@ -1445,13 +1446,14 @@ static void ravb_tx_timeout_work(struct work_struct *work)
 {
 	struct ravb_private *priv = container_of(work, struct ravb_private,
 						 work);
+	const struct ravb_hw_info *info = priv->info;
 	struct net_device *ndev = priv->ndev;
 	int error;
 
 	netif_tx_stop_all_queues(ndev);
 
 	/* Stop PTP Clock driver */
-	if (priv->chip_id == RCAR_GEN2)
+	if (info->no_ptp_cfg_active)
 		ravb_ptp_stop(ndev);
 
 	/* Wait for DMA stopping */
@@ -1486,7 +1488,7 @@ static void ravb_tx_timeout_work(struct work_struct *work)
 
 out:
 	/* Initialise PTP Clock driver */
-	if (priv->chip_id == RCAR_GEN2)
+	if (info->no_ptp_cfg_active)
 		ravb_ptp_init(ndev, priv->pdev);
 
 	netif_tx_start_all_queues(ndev);
@@ -1695,7 +1697,7 @@ static int ravb_close(struct net_device *ndev)
 	ravb_write(ndev, 0, TIC);
 
 	/* Stop PTP Clock driver */
-	if (priv->chip_id == RCAR_GEN2)
+	if (info->no_ptp_cfg_active)
 		ravb_ptp_stop(ndev);
 
 	/* Set the config mode to stop the AVB-DMAC's processes */
@@ -1956,6 +1958,7 @@ static const struct ravb_hw_info ravb_gen2_hw_info = {
 	.stats_len = ARRAY_SIZE(ravb_gstrings_stats),
 	.max_rx_len = RX_BUF_SZ + RAVB_ALIGN - 1,
 	.aligned_tx = 1,
+	.no_ptp_cfg_active = 1,
 };
 
 static const struct of_device_id ravb_match_table[] = {
@@ -1996,8 +1999,9 @@ static int ravb_set_gti(struct net_device *ndev)
 static void ravb_set_config_mode(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
 
-	if (priv->chip_id == RCAR_GEN2) {
+	if (info->no_ptp_cfg_active) {
 		ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_CONFIG);
 		/* Set CSEL value */
 		ravb_modify(ndev, CCC, CCC_CSEL, CCC_CSEL_HPB);
-- 
2.17.1

[PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct ravb_hw_info

[Biju Das]

There are some H/W differences for the gPTP feature between
R-Car Gen3, R-Car Gen2, and RZ/G2L as below.

1) On R-Car Gen3, gPTP support is active in config mode.
2) On R-Car Gen2, gPTP support is not active in config mode.
3) RZ/G2L does not support the gPTP feature.

Add a ptp_cfg_active hw feature bit to struct ravb_hw_info for
supporting gPTP active in config mode for R-Car Gen3.
This patch also removes enum ravb_chip_id, chip_id from both
struct ravb_hw_info and struct ravb_private, as it is unused.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/net/ethernet/renesas/ravb.h      |  8 +-------
 drivers/net/ethernet/renesas/ravb_main.c | 12 +++++-------
 2 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 9ecf1a8c3ca8..209e030935aa 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -979,17 +979,11 @@ struct ravb_ptp {
 	struct ravb_ptp_perout perout[N_PER_OUT];
 };
 
-enum ravb_chip_id {
-	RCAR_GEN2,
-	RCAR_GEN3,
-};
-
 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 stats_len;
 	size_t max_rx_len;
 	unsigned aligned_tx: 1;
@@ -999,6 +993,7 @@ struct ravb_hw_info {
 	unsigned tx_counters:1;		/* E-MAC has TX counters */
 	unsigned multi_irqs:1;		/* AVB-DMAC and E-MAC has multiple irqs */
 	unsigned no_ptp_cfg_active:1;	/* AVB-DMAC does not support gPTP active in config mode */
+	unsigned ptp_cfg_active:1;	/* AVB-DMAC has gPTP support active in config mode */
 };
 
 struct ravb_private {
@@ -1042,7 +1037,6 @@ struct ravb_private {
 	int msg_enable;
 	int speed;
 	int emac_irq;
-	enum ravb_chip_id chip_id;
 	int rx_irqs[NUM_RX_QUEUE];
 	int tx_irqs[NUM_TX_QUEUE];
 
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index e33b836218f0..883db1049882 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1941,12 +1941,12 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.gstrings_size = sizeof(ravb_gstrings_stats),
 	.net_hw_features = NETIF_F_RXCSUM,
 	.net_features = NETIF_F_RXCSUM,
-	.chip_id = RCAR_GEN3,
 	.stats_len = ARRAY_SIZE(ravb_gstrings_stats),
 	.max_rx_len = RX_BUF_SZ + RAVB_ALIGN - 1,
 	.internal_delay = 1,
 	.tx_counters = 1,
 	.multi_irqs = 1,
+	.ptp_cfg_active = 1,
 };
 
 static const struct ravb_hw_info ravb_gen2_hw_info = {
@@ -1954,7 +1954,6 @@ static const struct ravb_hw_info ravb_gen2_hw_info = {
 	.gstrings_size = sizeof(ravb_gstrings_stats),
 	.net_hw_features = NETIF_F_RXCSUM,
 	.net_features = NETIF_F_RXCSUM,
-	.chip_id = RCAR_GEN2,
 	.stats_len = ARRAY_SIZE(ravb_gstrings_stats),
 	.max_rx_len = RX_BUF_SZ + RAVB_ALIGN - 1,
 	.aligned_tx = 1,
@@ -2152,8 +2151,6 @@ static int ravb_probe(struct platform_device *pdev)
 		}
 	}
 
-	priv->chip_id = info->chip_id;
-
 	priv->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(priv->clk)) {
 		error = PTR_ERR(priv->clk);
@@ -2216,7 +2213,7 @@ static int ravb_probe(struct platform_device *pdev)
 	INIT_LIST_HEAD(&priv->ts_skb_list);
 
 	/* Initialise PTP Clock driver */
-	if (info->chip_id != RCAR_GEN2)
+	if (info->ptp_cfg_active)
 		ravb_ptp_init(ndev, pdev);
 
 	/* Debug message level */
@@ -2264,7 +2261,7 @@ static int ravb_probe(struct platform_device *pdev)
 			  priv->desc_bat_dma);
 
 	/* Stop PTP Clock driver */
-	if (info->chip_id != RCAR_GEN2)
+	if (info->ptp_cfg_active)
 		ravb_ptp_stop(ndev);
 out_disable_refclk:
 	clk_disable_unprepare(priv->refclk);
@@ -2280,9 +2277,10 @@ static int ravb_remove(struct platform_device *pdev)
 {
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
 
 	/* Stop PTP Clock driver */
-	if (priv->chip_id != RCAR_GEN2)
+	if (info->ptp_cfg_active)
 		ravb_ptp_stop(ndev);
 
 	clk_disable_unprepare(priv->refclk);
-- 
2.17.1

[PATCH net-next 05/13] ravb: Factorise ravb_ring_free function

[Biju Das]

R-Car uses extended descriptor in RX, whereas RZ/G2L uses normal
descriptor. Factorise ravb_ring_free function so that it can
support later SoC.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/net/ethernet/renesas/ravb.h      |  1 +
 drivers/net/ethernet/renesas/ravb_main.c | 47 +++++++++++++++---------
 2 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 209e030935aa..7cb30319524a 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -980,6 +980,7 @@ struct ravb_ptp {
 };
 
 struct ravb_hw_info {
+	void (*rx_ring_free)(struct net_device *ndev, int q);
 	const char (*gstrings_stats)[ETH_GSTRING_LEN];
 	size_t gstrings_size;
 	netdev_features_t net_hw_features;
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 883db1049882..dc388a32496a 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -216,31 +216,42 @@ static int ravb_tx_free(struct net_device *ndev, int q, bool free_txed_only)
 	return free_num;
 }
 
+static void ravb_rx_ring_free(struct net_device *ndev, int q)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	unsigned int ring_size;
+	unsigned int i;
+
+	if (!priv->rx_ring[q])
+		return;
+
+	for (i = 0; i < priv->num_rx_ring[q]; i++) {
+		struct ravb_ex_rx_desc *desc = &priv->rx_ring[q][i];
+
+		if (!dma_mapping_error(ndev->dev.parent,
+				       le32_to_cpu(desc->dptr)))
+			dma_unmap_single(ndev->dev.parent,
+					 le32_to_cpu(desc->dptr),
+					 RX_BUF_SZ,
+					 DMA_FROM_DEVICE);
+	}
+	ring_size = sizeof(struct ravb_ex_rx_desc) *
+		    (priv->num_rx_ring[q] + 1);
+	dma_free_coherent(ndev->dev.parent, ring_size, priv->rx_ring[q],
+			  priv->rx_desc_dma[q]);
+	priv->rx_ring[q] = NULL;
+}
+
 /* Free skb's and DMA buffers for Ethernet AVB */
 static void ravb_ring_free(struct net_device *ndev, int q)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
 	unsigned int num_tx_desc = priv->num_tx_desc;
 	unsigned int ring_size;
 	unsigned int i;
 
-	if (priv->rx_ring[q]) {
-		for (i = 0; i < priv->num_rx_ring[q]; i++) {
-			struct ravb_ex_rx_desc *desc = &priv->rx_ring[q][i];
-
-			if (!dma_mapping_error(ndev->dev.parent,
-					       le32_to_cpu(desc->dptr)))
-				dma_unmap_single(ndev->dev.parent,
-						 le32_to_cpu(desc->dptr),
-						 RX_BUF_SZ,
-						 DMA_FROM_DEVICE);
-		}
-		ring_size = sizeof(struct ravb_ex_rx_desc) *
-			    (priv->num_rx_ring[q] + 1);
-		dma_free_coherent(ndev->dev.parent, ring_size, priv->rx_ring[q],
-				  priv->rx_desc_dma[q]);
-		priv->rx_ring[q] = NULL;
-	}
+	info->rx_ring_free(ndev, q);
 
 	if (priv->tx_ring[q]) {
 		ravb_tx_free(ndev, q, false);
@@ -1937,6 +1948,7 @@ static int ravb_mdio_release(struct ravb_private *priv)
 }
 
 static const struct ravb_hw_info ravb_gen3_hw_info = {
+	.rx_ring_free = ravb_rx_ring_free,
 	.gstrings_stats = ravb_gstrings_stats,
 	.gstrings_size = sizeof(ravb_gstrings_stats),
 	.net_hw_features = NETIF_F_RXCSUM,
@@ -1950,6 +1962,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
 };
 
 static const struct ravb_hw_info ravb_gen2_hw_info = {
+	.rx_ring_free = ravb_rx_ring_free,
 	.gstrings_stats = ravb_gstrings_stats,
 	.gstrings_size = sizeof(ravb_gstrings_stats),
 	.net_hw_features = NETIF_F_RXCSUM,
-- 
2.17.1

[PATCH net-next 06/13] ravb: Factorise ravb_ring_format function

[Biju Das]

The ravb_ring_format function uses an extended descriptor in RX
for R-Car compared to the normal descriptor for RZ/G2L. Factorise
RX ring buffer buildup to extend the support for later SoC.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/net/ethernet/renesas/ravb.h      |  1 +
 drivers/net/ethernet/renesas/ravb_main.c | 35 ++++++++++++++++--------
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 7cb30319524a..dbf114d2ceef 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -981,6 +981,7 @@ struct ravb_ptp {
 
 struct ravb_hw_info {
 	void (*rx_ring_free)(struct net_device *ndev, int q);
+	void (*rx_ring_format)(struct net_device *ndev, int q);
 	const char (*gstrings_stats)[ETH_GSTRING_LEN];
 	size_t gstrings_size;
 	netdev_features_t net_hw_features;
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index dc388a32496a..e52e36ccd1c6 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -282,25 +282,14 @@ static void ravb_ring_free(struct net_device *ndev, int q)
 	priv->tx_skb[q] = NULL;
 }
 
-/* Format skb and descriptor buffer for Ethernet AVB */
-static void ravb_ring_format(struct net_device *ndev, int q)
+static void ravb_rx_ring_format(struct net_device *ndev, int q)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
-	unsigned int num_tx_desc = priv->num_tx_desc;
 	struct ravb_ex_rx_desc *rx_desc;
-	struct ravb_tx_desc *tx_desc;
-	struct ravb_desc *desc;
 	unsigned int rx_ring_size = sizeof(*rx_desc) * priv->num_rx_ring[q];
-	unsigned int tx_ring_size = sizeof(*tx_desc) * priv->num_tx_ring[q] *
-				    num_tx_desc;
 	dma_addr_t dma_addr;
 	unsigned int i;
 
-	priv->cur_rx[q] = 0;
-	priv->cur_tx[q] = 0;
-	priv->dirty_rx[q] = 0;
-	priv->dirty_tx[q] = 0;
-
 	memset(priv->rx_ring[q], 0, rx_ring_size);
 	/* Build RX ring buffer */
 	for (i = 0; i < priv->num_rx_ring[q]; i++) {
@@ -321,6 +310,26 @@ static void ravb_ring_format(struct net_device *ndev, int q)
 	rx_desc = &priv->rx_ring[q][i];
 	rx_desc->dptr = cpu_to_le32((u32)priv->rx_desc_dma[q]);
 	rx_desc->die_dt = DT_LINKFIX; /* type */
+}
+
+/* Format skb and descriptor buffer for Ethernet AVB */
+static void ravb_ring_format(struct net_device *ndev, int q)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
+	unsigned int num_tx_desc = priv->num_tx_desc;
+	struct ravb_tx_desc *tx_desc;
+	struct ravb_desc *desc;
+	unsigned int tx_ring_size = sizeof(*tx_desc) * priv->num_tx_ring[q] *
+				    num_tx_desc;
+	unsigned int i;
+
+	priv->cur_rx[q] = 0;
+	priv->cur_tx[q] = 0;
+	priv->dirty_rx[q] = 0;
+	priv->dirty_tx[q] = 0;
+
+	info->rx_ring_format(ndev, q);
 
 	memset(priv->tx_ring[q], 0, tx_ring_size);
 	/* Build TX ring buffer */
@@ -1949,6 +1958,7 @@ static int ravb_mdio_release(struct ravb_private *priv)
 
 static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.rx_ring_free = ravb_rx_ring_free,
+	.rx_ring_format = ravb_rx_ring_format,
 	.gstrings_stats = ravb_gstrings_stats,
 	.gstrings_size = sizeof(ravb_gstrings_stats),
 	.net_hw_features = NETIF_F_RXCSUM,
@@ -1963,6 +1973,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
 
 static const struct ravb_hw_info ravb_gen2_hw_info = {
 	.rx_ring_free = ravb_rx_ring_free,
+	.rx_ring_format = ravb_rx_ring_format,
 	.gstrings_stats = ravb_gstrings_stats,
 	.gstrings_size = sizeof(ravb_gstrings_stats),
 	.net_hw_features = NETIF_F_RXCSUM,
-- 
2.17.1

[PATCH net-next 07/13] ravb: Factorise ravb_ring_init function

[Biju Das]

The ravb_ring_init function uses an extended descriptor in RX for
R-Car and normal descriptor for RZ/G2L. Add a helper function
for RX ring buffer allocation to support later SoC.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/net/ethernet/renesas/ravb.h      |  1 +
 drivers/net/ethernet/renesas/ravb_main.c | 21 ++++++++++++++++-----
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index dbf114d2ceef..39df045b1a0b 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -982,6 +982,7 @@ struct ravb_ptp {
 struct ravb_hw_info {
 	void (*rx_ring_free)(struct net_device *ndev, int q);
 	void (*rx_ring_format)(struct net_device *ndev, int q);
+	void *(*alloc_rx_desc)(struct net_device *ndev, int q);
 	const char (*gstrings_stats)[ETH_GSTRING_LEN];
 	size_t gstrings_size;
 	netdev_features_t net_hw_features;
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index e52e36ccd1c6..148c974499b4 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -355,6 +355,19 @@ static void ravb_ring_format(struct net_device *ndev, int q)
 	desc->dptr = cpu_to_le32((u32)priv->tx_desc_dma[q]);
 }
 
+static void *ravb_alloc_rx_desc(struct net_device *ndev, int q)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	unsigned int ring_size;
+
+	ring_size = sizeof(struct ravb_ex_rx_desc) * (priv->num_rx_ring[q] + 1);
+
+	priv->rx_ring[q] = dma_alloc_coherent(ndev->dev.parent, ring_size,
+					      &priv->rx_desc_dma[q],
+					      GFP_KERNEL);
+	return priv->rx_ring[q];
+}
+
 /* Init skb and descriptor buffer for Ethernet AVB */
 static int ravb_ring_init(struct net_device *ndev, int q)
 {
@@ -390,11 +403,7 @@ static int ravb_ring_init(struct net_device *ndev, int q)
 	}
 
 	/* Allocate all RX descriptors. */
-	ring_size = sizeof(struct ravb_ex_rx_desc) * (priv->num_rx_ring[q] + 1);
-	priv->rx_ring[q] = dma_alloc_coherent(ndev->dev.parent, ring_size,
-					      &priv->rx_desc_dma[q],
-					      GFP_KERNEL);
-	if (!priv->rx_ring[q])
+	if (!info->alloc_rx_desc(ndev, q))
 		goto error;
 
 	priv->dirty_rx[q] = 0;
@@ -1959,6 +1968,7 @@ static int ravb_mdio_release(struct ravb_private *priv)
 static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.rx_ring_free = ravb_rx_ring_free,
 	.rx_ring_format = ravb_rx_ring_format,
+	.alloc_rx_desc = ravb_alloc_rx_desc,
 	.gstrings_stats = ravb_gstrings_stats,
 	.gstrings_size = sizeof(ravb_gstrings_stats),
 	.net_hw_features = NETIF_F_RXCSUM,
@@ -1974,6 +1984,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
 static const struct ravb_hw_info ravb_gen2_hw_info = {
 	.rx_ring_free = ravb_rx_ring_free,
 	.rx_ring_format = ravb_rx_ring_format,
+	.alloc_rx_desc = ravb_alloc_rx_desc,
 	.gstrings_stats = ravb_gstrings_stats,
 	.gstrings_size = sizeof(ravb_gstrings_stats),
 	.net_hw_features = NETIF_F_RXCSUM,
-- 
2.17.1

[PATCH net-next 08/13] ravb: Factorise ravb_rx function

[Biju Das]

R-Car uses an extended descriptor in RX whereas, RZ/G2L uses
normal descriptor in RX. Factorise the ravb_rx function to
support the later SoC.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/net/ethernet/renesas/ravb.h      |  1 +
 drivers/net/ethernet/renesas/ravb_main.c | 14 ++++++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 39df045b1a0b..00f2d67b4dad 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -983,6 +983,7 @@ struct ravb_hw_info {
 	void (*rx_ring_free)(struct net_device *ndev, int q);
 	void (*rx_ring_format)(struct net_device *ndev, int q);
 	void *(*alloc_rx_desc)(struct net_device *ndev, int q);
+	bool (*receive)(struct net_device *ndev, int *quota, int q);
 	const char (*gstrings_stats)[ETH_GSTRING_LEN];
 	size_t gstrings_size;
 	netdev_features_t net_hw_features;
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 148c974499b4..1cace5324261 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -562,8 +562,7 @@ static void ravb_rx_csum(struct sk_buff *skb)
 	skb_trim(skb, skb->len - sizeof(__sum16));
 }
 
-/* Packet receive function for Ethernet AVB */
-static bool ravb_rx(struct net_device *ndev, int *quota, int q)
+static bool ravb_rcar_rx(struct net_device *ndev, int *quota, int q)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
 	const struct ravb_hw_info *info = priv->info;
@@ -677,6 +676,15 @@ static bool ravb_rx(struct net_device *ndev, int *quota, int q)
 	return boguscnt <= 0;
 }
 
+/* Packet receive function for Ethernet AVB */
+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;
+
+	return info->receive(ndev, quota, q);
+}
+
 static void ravb_rcv_snd_disable(struct net_device *ndev)
 {
 	/* Disable TX and RX */
@@ -1969,6 +1977,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.rx_ring_free = ravb_rx_ring_free,
 	.rx_ring_format = ravb_rx_ring_format,
 	.alloc_rx_desc = ravb_alloc_rx_desc,
+	.receive = ravb_rcar_rx,
 	.gstrings_stats = ravb_gstrings_stats,
 	.gstrings_size = sizeof(ravb_gstrings_stats),
 	.net_hw_features = NETIF_F_RXCSUM,
@@ -1985,6 +1994,7 @@ static const struct ravb_hw_info ravb_gen2_hw_info = {
 	.rx_ring_free = ravb_rx_ring_free,
 	.rx_ring_format = ravb_rx_ring_format,
 	.alloc_rx_desc = ravb_alloc_rx_desc,
+	.receive = ravb_rcar_rx,
 	.gstrings_stats = ravb_gstrings_stats,
 	.gstrings_size = sizeof(ravb_gstrings_stats),
 	.net_hw_features = NETIF_F_RXCSUM,
-- 
2.17.1

[PATCH net-next 09/13] ravb: Factorise ravb_adjust_link function

[Biju Das]

R-Car supports 100 and 1000 Mbps transfer speed whereas RZ/G2L
in addition support 10Mbps. Factorise ravb_adjust_link function
in order to support 10Mbps speed.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 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 00f2d67b4dad..9879690c5cd8 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -984,6 +984,7 @@ struct ravb_hw_info {
 	void (*rx_ring_format)(struct net_device *ndev, int q);
 	void *(*alloc_rx_desc)(struct net_device *ndev, int q);
 	bool (*receive)(struct net_device *ndev, int *quota, int q);
+	void (*set_rate)(struct net_device *ndev);
 	const char (*gstrings_stats)[ETH_GSTRING_LEN];
 	size_t gstrings_size;
 	netdev_features_t net_hw_features;
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 1cace5324261..1f9d9f54bf1b 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -996,6 +996,7 @@ static int ravb_poll(struct napi_struct *napi, int budget)
 static void ravb_adjust_link(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
 	struct phy_device *phydev = ndev->phydev;
 	bool new_state = false;
 	unsigned long flags;
@@ -1010,7 +1011,7 @@ static void ravb_adjust_link(struct net_device *ndev)
 		if (phydev->speed != priv->speed) {
 			new_state = true;
 			priv->speed = phydev->speed;
-			ravb_set_rate(ndev);
+			info->set_rate(ndev);
 		}
 		if (!priv->link) {
 			ravb_modify(ndev, ECMR, ECMR_TXF, 0);
@@ -1978,6 +1979,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.rx_ring_format = ravb_rx_ring_format,
 	.alloc_rx_desc = ravb_alloc_rx_desc,
 	.receive = ravb_rcar_rx,
+	.set_rate = ravb_set_rate,
 	.gstrings_stats = ravb_gstrings_stats,
 	.gstrings_size = sizeof(ravb_gstrings_stats),
 	.net_hw_features = NETIF_F_RXCSUM,
@@ -1995,6 +1997,7 @@ static const struct ravb_hw_info ravb_gen2_hw_info = {
 	.rx_ring_format = ravb_rx_ring_format,
 	.alloc_rx_desc = ravb_alloc_rx_desc,
 	.receive = ravb_rcar_rx,
+	.set_rate = ravb_set_rate,
 	.gstrings_stats = ravb_gstrings_stats,
 	.gstrings_size = sizeof(ravb_gstrings_stats),
 	.net_hw_features = NETIF_F_RXCSUM,
-- 
2.17.1

[PATCH net-next 10/13] ravb: Factorise ravb_set_features

[Biju Das]

RZ/G2L supports HW checksum on RX and TX whereas R-Car supports on RX.
Factorise ravb_set_features to support this feature.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/net/ethernet/renesas/ravb.h      |  1 +
 drivers/net/ethernet/renesas/ravb_main.c | 15 +++++++++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 9879690c5cd8..9e284238ed83 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -985,6 +985,7 @@ struct ravb_hw_info {
 	void *(*alloc_rx_desc)(struct net_device *ndev, int q);
 	bool (*receive)(struct net_device *ndev, int *quota, int q);
 	void (*set_rate)(struct net_device *ndev);
+	int (*set_rx_csum_feature)(struct net_device *ndev, netdev_features_t features);
 	const char (*gstrings_stats)[ETH_GSTRING_LEN];
 	size_t gstrings_size;
 	netdev_features_t net_hw_features;
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 1f9d9f54bf1b..1789309c4c03 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1901,8 +1901,8 @@ static void ravb_set_rx_csum(struct net_device *ndev, bool enable)
 	spin_unlock_irqrestore(&priv->lock, flags);
 }
 
-static int ravb_set_features(struct net_device *ndev,
-			     netdev_features_t features)
+static int ravb_set_features_rx_csum(struct net_device *ndev,
+				     netdev_features_t features)
 {
 	netdev_features_t changed = ndev->features ^ features;
 
@@ -1914,6 +1914,15 @@ static int ravb_set_features(struct net_device *ndev,
 	return 0;
 }
 
+static int ravb_set_features(struct net_device *ndev,
+			     netdev_features_t features)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
+
+	return info->set_rx_csum_feature(ndev, features);
+}
+
 static const struct net_device_ops ravb_netdev_ops = {
 	.ndo_open		= ravb_open,
 	.ndo_stop		= ravb_close,
@@ -1980,6 +1989,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.alloc_rx_desc = ravb_alloc_rx_desc,
 	.receive = ravb_rcar_rx,
 	.set_rate = ravb_set_rate,
+	.set_rx_csum_feature = ravb_set_features_rx_csum,
 	.gstrings_stats = ravb_gstrings_stats,
 	.gstrings_size = sizeof(ravb_gstrings_stats),
 	.net_hw_features = NETIF_F_RXCSUM,
@@ -1998,6 +2008,7 @@ static const struct ravb_hw_info ravb_gen2_hw_info = {
 	.alloc_rx_desc = ravb_alloc_rx_desc,
 	.receive = ravb_rcar_rx,
 	.set_rate = ravb_set_rate,
+	.set_rx_csum_feature = ravb_set_features_rx_csum,
 	.gstrings_stats = ravb_gstrings_stats,
 	.gstrings_size = sizeof(ravb_gstrings_stats),
 	.net_hw_features = NETIF_F_RXCSUM,
-- 
2.17.1

[PATCH net-next 11/13] ravb: Factorise ravb_dmac_init function

[Biju Das]

The DMAC IP on the R-Car AVB module has different initialization
parameters for RCR, TGC, TCCR, RIC0, RIC2, and TIC compared to
DMAC IP on the RZ/G2L Gigabit Ethernet module. Factorise the
ravb_dmac_init function to support the later SoC.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/net/ethernet/renesas/ravb.h      |  1 +
 drivers/net/ethernet/renesas/ravb_main.c | 52 ++++++++++++++----------
 2 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 9e284238ed83..24a3abd00053 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -986,6 +986,7 @@ struct ravb_hw_info {
 	bool (*receive)(struct net_device *ndev, int *quota, int q);
 	void (*set_rate)(struct net_device *ndev);
 	int (*set_rx_csum_feature)(struct net_device *ndev, netdev_features_t features);
+	void (*dmac_init)(struct net_device *ndev);
 	const char (*gstrings_stats)[ETH_GSTRING_LEN];
 	size_t gstrings_size;
 	netdev_features_t net_hw_features;
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 1789309c4c03..391e7927ea08 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -452,30 +452,10 @@ static void ravb_emac_init(struct net_device *ndev)
 	ravb_write(ndev, ECSIPR_ICDIP | ECSIPR_MPDIP | ECSIPR_LCHNGIP, ECSIPR);
 }
 
-/* Device init function for Ethernet AVB */
-static int ravb_dmac_init(struct net_device *ndev)
+static void ravb_rcar_dmac_init(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
 	const struct ravb_hw_info *info = priv->info;
-	int error;
-
-	/* Set CONFIG mode */
-	error = ravb_config(ndev);
-	if (error)
-		return error;
-
-	error = ravb_ring_init(ndev, RAVB_BE);
-	if (error)
-		return error;
-	error = ravb_ring_init(ndev, RAVB_NC);
-	if (error) {
-		ravb_ring_free(ndev, RAVB_BE);
-		return error;
-	}
-
-	/* Descriptor format */
-	ravb_ring_format(ndev, RAVB_BE);
-	ravb_ring_format(ndev, RAVB_NC);
 
 	/* Set AVB RX */
 	ravb_write(ndev,
@@ -502,6 +482,34 @@ static int ravb_dmac_init(struct net_device *ndev)
 	ravb_write(ndev, RIC2_QFE0 | RIC2_QFE1 | RIC2_RFFE, RIC2);
 	/* Frame transmitted, timestamp FIFO updated */
 	ravb_write(ndev, TIC_FTE0 | TIC_FTE1 | TIC_TFUE, TIC);
+}
+
+/* Device init function for Ethernet AVB */
+static int ravb_dmac_init(struct net_device *ndev)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
+	int error;
+
+	/* Set CONFIG mode */
+	error = ravb_config(ndev);
+	if (error)
+		return error;
+
+	error = ravb_ring_init(ndev, RAVB_BE);
+	if (error)
+		return error;
+	error = ravb_ring_init(ndev, RAVB_NC);
+	if (error) {
+		ravb_ring_free(ndev, RAVB_BE);
+		return error;
+	}
+
+	/* Descriptor format */
+	ravb_ring_format(ndev, RAVB_BE);
+	ravb_ring_format(ndev, RAVB_NC);
+
+	info->dmac_init(ndev);
 
 	/* Setting the control will start the AVB-DMAC process. */
 	ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_OPERATION);
@@ -1990,6 +1998,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.receive = ravb_rcar_rx,
 	.set_rate = ravb_set_rate,
 	.set_rx_csum_feature = ravb_set_features_rx_csum,
+	.dmac_init = ravb_rcar_dmac_init,
 	.gstrings_stats = ravb_gstrings_stats,
 	.gstrings_size = sizeof(ravb_gstrings_stats),
 	.net_hw_features = NETIF_F_RXCSUM,
@@ -2009,6 +2018,7 @@ static const struct ravb_hw_info ravb_gen2_hw_info = {
 	.receive = ravb_rcar_rx,
 	.set_rate = ravb_set_rate,
 	.set_rx_csum_feature = ravb_set_features_rx_csum,
+	.dmac_init = ravb_rcar_dmac_init,
 	.gstrings_stats = ravb_gstrings_stats,
 	.gstrings_size = sizeof(ravb_gstrings_stats),
 	.net_hw_features = NETIF_F_RXCSUM,
-- 
2.17.1

[PATCH net-next 12/13] ravb: Factorise ravb_emac_init function

[Biju Das]

The E-MAC IP on the R-Car AVB module has different initialization
parameters for RX frame size, duplex settings, different offset
for transfer speed setting and has magic packet detection support
compared to E-MAC on RZ/G2L Gigabit Ethernet module. Factorise
the ravb_emac_init function to support the later SoC.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/net/ethernet/renesas/ravb.h      |  1 +
 drivers/net/ethernet/renesas/ravb_main.c | 14 ++++++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 24a3abd00053..117eb22349c5 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -987,6 +987,7 @@ struct ravb_hw_info {
 	void (*set_rate)(struct net_device *ndev);
 	int (*set_rx_csum_feature)(struct net_device *ndev, netdev_features_t features);
 	void (*dmac_init)(struct net_device *ndev);
+	void (*emac_init)(struct net_device *ndev);
 	const char (*gstrings_stats)[ETH_GSTRING_LEN];
 	size_t gstrings_size;
 	netdev_features_t net_hw_features;
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 391e7927ea08..7a144b45e41d 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -425,8 +425,7 @@ static int ravb_ring_init(struct net_device *ndev, int q)
 	return -ENOMEM;
 }
 
-/* E-MAC init function */
-static void ravb_emac_init(struct net_device *ndev)
+static void ravb_rcar_emac_init(struct net_device *ndev)
 {
 	/* Receive frame limit set register */
 	ravb_write(ndev, ndev->mtu + ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN, RFLR);
@@ -452,6 +451,15 @@ static void ravb_emac_init(struct net_device *ndev)
 	ravb_write(ndev, ECSIPR_ICDIP | ECSIPR_MPDIP | ECSIPR_LCHNGIP, ECSIPR);
 }
 
+/* E-MAC init function */
+static void ravb_emac_init(struct net_device *ndev)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_hw_info *info = priv->info;
+
+	info->emac_init(ndev);
+}
+
 static void ravb_rcar_dmac_init(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
@@ -1999,6 +2007,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = {
 	.set_rate = ravb_set_rate,
 	.set_rx_csum_feature = ravb_set_features_rx_csum,
 	.dmac_init = ravb_rcar_dmac_init,
+	.emac_init = ravb_rcar_emac_init,
 	.gstrings_stats = ravb_gstrings_stats,
 	.gstrings_size = sizeof(ravb_gstrings_stats),
 	.net_hw_features = NETIF_F_RXCSUM,
@@ -2019,6 +2028,7 @@ static const struct ravb_hw_info ravb_gen2_hw_info = {
 	.set_rate = ravb_set_rate,
 	.set_rx_csum_feature = ravb_set_features_rx_csum,
 	.dmac_init = ravb_rcar_dmac_init,
+	.emac_init = ravb_rcar_emac_init,
 	.gstrings_stats = ravb_gstrings_stats,
 	.gstrings_size = sizeof(ravb_gstrings_stats),
 	.net_hw_features = NETIF_F_RXCSUM,
-- 
2.17.1

[PATCH net-next 13/13] ravb: Add reset support

[Biju Das]

Reset support is present on R-Car. Let's support it, if it is
available.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/net/ethernet/renesas/ravb.h      |  1 +
 drivers/net/ethernet/renesas/ravb_main.c | 11 +++++++++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 117eb22349c5..47c5377e4f42 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -1057,6 +1057,7 @@ struct ravb_private {
 	unsigned int num_tx_desc;	/* TX descriptors per packet */
 
 	const struct ravb_hw_info *info;
+	struct reset_control *rstc;
 };
 
 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 7a144b45e41d..0f85f2d97b18 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -29,6 +29,7 @@
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/sys_soc.h>
+#include <linux/reset.h>
 
 #include <asm/div64.h>
 
@@ -2140,6 +2141,7 @@ static int ravb_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
 	const struct ravb_hw_info *info;
+	struct reset_control *rstc;
 	struct ravb_private *priv;
 	struct net_device *ndev;
 	int error, irq, q;
@@ -2152,6 +2154,11 @@ static int ravb_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	rstc = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
+	if (IS_ERR(rstc))
+		return dev_err_probe(&pdev->dev, PTR_ERR(rstc),
+				     "failed to get cpg reset\n");
+
 	ndev = alloc_etherdev_mqs(sizeof(struct ravb_private),
 				  NUM_TX_QUEUE, NUM_RX_QUEUE);
 	if (!ndev)
@@ -2162,6 +2169,7 @@ static int ravb_probe(struct platform_device *pdev)
 	ndev->features = info->net_features;
 	ndev->hw_features = info->net_hw_features;
 
+	reset_control_deassert(rstc);
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_get_sync(&pdev->dev);
 
@@ -2179,6 +2187,7 @@ static int ravb_probe(struct platform_device *pdev)
 
 	priv = netdev_priv(ndev);
 	priv->info = info;
+	priv->rstc = rstc;
 	priv->ndev = ndev;
 	priv->pdev = pdev;
 	priv->num_tx_ring[RAVB_BE] = BE_TX_RING_SIZE;
@@ -2349,6 +2358,7 @@ static int ravb_probe(struct platform_device *pdev)
 
 	pm_runtime_put(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
+	reset_control_assert(rstc);
 	return error;
 }
 
@@ -2374,6 +2384,7 @@ static int ravb_remove(struct platform_device *pdev)
 	netif_napi_del(&priv->napi[RAVB_BE]);
 	ravb_mdio_release(priv);
 	pm_runtime_disable(&pdev->dev);
+	reset_control_assert(priv->rstc);
 	free_netdev(ndev);
 	platform_set_drvdata(pdev, NULL);
 
-- 
2.17.1

Re: [PATCH net-next 00/13] Add Factorisation code to support Gigabit Ethernet driver

[patchwork-bot+netdevbpf]

Hello:

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

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

Here is the summary with links:
  - [net-next,01/13] ravb: Remove the macros NUM_TX_DESC_GEN[23]
    https://git.kernel.org/netdev/net-next/c/c81d894226b9
  - [net-next,02/13] ravb: Add multi_irq to struct ravb_hw_info
    https://git.kernel.org/netdev/net-next/c/6de19fa0e9f7
  - [net-next,03/13] ravb: Add no_ptp_cfg_active to struct ravb_hw_info
    https://git.kernel.org/netdev/net-next/c/8f27219a6191
  - [net-next,04/13] ravb: Add ptp_cfg_active to struct ravb_hw_info
    https://git.kernel.org/netdev/net-next/c/a69a3d094de3
  - [net-next,05/13] ravb: Factorise ravb_ring_free function
    https://git.kernel.org/netdev/net-next/c/bf46b7578404
  - [net-next,06/13] ravb: Factorise ravb_ring_format function
    https://git.kernel.org/netdev/net-next/c/1ae22c19e75c
  - [net-next,07/13] ravb: Factorise ravb_ring_init function
    https://git.kernel.org/netdev/net-next/c/7870a41848ab
  - [net-next,08/13] ravb: Factorise ravb_rx function
    https://git.kernel.org/netdev/net-next/c/d5d95c11365b
  - [net-next,09/13] ravb: Factorise ravb_adjust_link function
    https://git.kernel.org/netdev/net-next/c/cb21104f2c35
  - [net-next,10/13] ravb: Factorise ravb_set_features
    https://git.kernel.org/netdev/net-next/c/80f35a0df086
  - [net-next,11/13] ravb: Factorise ravb_dmac_init function
    https://git.kernel.org/netdev/net-next/c/eb4fd127448b
  - [net-next,12/13] ravb: Factorise ravb_emac_init function
    https://git.kernel.org/netdev/net-next/c/511d74d9d86c
  - [net-next,13/13] ravb: Add reset support
    https://git.kernel.org/netdev/net-next/c/0d13a1a464a0

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

Re: [PATCH net-next 00/13] Add Factorisation code to support Gigabit Ethernet driver

[Sergey Shtylyov]

Hello!

On 25.08.2021 13:30, patchwork-bot+netdevbpf@kernel.org wrote:

 > This series was applied to netdev/net-next.git (refs/heads/master):
 >
 > On Wed, 25 Aug 2021 08:01:41 +0100 you wrote:
    Now this is super fast -- I didn't even have the time to promise 
reviewing... :-/

 >> The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC are
 >> similar to the R-Car Ethernet AVB IP.
 >>
 >> The Gigabit Ethernet IP consists of Ethernet controller (E-MAC), Internal
 >> TCP/IP Offload Engine (TOE)  and Dedicated Direct memory access controller
 >> (DMAC).
 >>
 >> [...]
 >
 > Here is the summary with links:
 >    - [net-next,01/13] ravb: Remove the macros NUM_TX_DESC_GEN[23]
 >      https://git.kernel.org/netdev/net-next/c/c81d894226b9
 >    - [net-next,02/13] ravb: Add multi_irq to struct ravb_hw_info
 >      https://git.kernel.org/netdev/net-next/c/6de19fa0e9f7
 >    - [net-next,03/13] ravb: Add no_ptp_cfg_active to struct ravb_hw_info
 >      https://git.kernel.org/netdev/net-next/c/8f27219a6191
 >    - [net-next,04/13] ravb: Add ptp_cfg_active to struct ravb_hw_info
 >      https://git.kernel.org/netdev/net-next/c/a69a3d094de3
 >    - [net-next,05/13] ravb: Factorise ravb_ring_free function
 >      https://git.kernel.org/netdev/net-next/c/bf46b7578404
 >    - [net-next,06/13] ravb: Factorise ravb_ring_format function
 >      https://git.kernel.org/netdev/net-next/c/1ae22c19e75c
 >    - [net-next,07/13] ravb: Factorise ravb_ring_init function
 >      https://git.kernel.org/netdev/net-next/c/7870a41848ab
 >    - [net-next,08/13] ravb: Factorise ravb_rx function
 >      https://git.kernel.org/netdev/net-next/c/d5d95c11365b
 >    - [net-next,09/13] ravb: Factorise ravb_adjust_link function
 >      https://git.kernel.org/netdev/net-next/c/cb21104f2c35
 >    - [net-next,10/13] ravb: Factorise ravb_set_features
 >      https://git.kernel.org/netdev/net-next/c/80f35a0df086
 >    - [net-next,11/13] ravb: Factorise ravb_dmac_init function
 >      https://git.kernel.org/netdev/net-next/c/eb4fd127448b
 >    - [net-next,12/13] ravb: Factorise ravb_emac_init function
 >      https://git.kernel.org/netdev/net-next/c/511d74d9d86c
 >    - [net-next,13/13] ravb: Add reset support
 >      https://git.kernel.org/netdev/net-next/c/0d13a1a464a0

    Will have to do a post-merge review again. And I expect more issues here 
than in a previous patch set...

 > You are awesome, thank you!

MBR, Sergey

Re: [PATCH net-next 00/13] Add Factorisation code to support Gigabit Ethernet driver

[Andrew Lunn]

On Wed, Aug 25, 2021 at 01:57:55PM +0300, Sergey Shtylyov wrote:
> Hello!
> 
> On 25.08.2021 13:30, patchwork-bot+netdevbpf@kernel.org wrote:
> 
> > This series was applied to netdev/net-next.git (refs/heads/master):
> >
> > On Wed, 25 Aug 2021 08:01:41 +0100 you wrote:
>    Now this is super fast -- I didn't even have the time to promise
> reviewing... :-/

2 hours 30 minutes, i think.

Seems like reviews are no longer wanted in netdev.

      Andrew

Re: [PATCH net-next 00/13] Add Factorisation code to support Gigabit Ethernet driver

[Sergey Shtylyov]

On 8/25/21 4:46 PM, Andrew Lunn wrote:

[...]
>>> This series was applied to netdev/net-next.git (refs/heads/master):
>>>
>>> On Wed, 25 Aug 2021 08:01:41 +0100 you wrote:
>>    Now this is super fast -- I didn't even have the time to promise
>> reviewing... :-/
> 
> 2 hours 30 minutes, i think.

   Took 3 hours 30 mins on my side. :-)

> Seems like reviews are no longer wanted in netdev.

   At least with merge window coming close? 

>       Andrew

MBR, Sergey

Re: [PATCH net-next 01/13] ravb: Remove the macros NUM_TX_DESC_GEN[23]

[Sergey Shtylyov]

Hello!

On 8/25/21 10:01 AM, Biju Das wrote:

> For addressing 4 bytes alignment restriction on transmission
> buffer for R-Car Gen2 we use 2 descriptors whereas it is a single
> descriptor for other cases.
> Replace the macros NUM_TX_DESC_GEN[23] with magic number and
> add a comment to explain it.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Suggested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
[...]
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 02842b980a7f..073e690ab830 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -2160,8 +2160,12 @@ 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 = info->aligned_tx ?
> -		NUM_TX_DESC_GEN2 : NUM_TX_DESC_GEN3;
> +	/* FIXME: R-Car Gen2 has 4byte alignment restriction for tx buffer

   Mhm, what are you going to fix here?

> +	 * Use two descriptor to handle such situation. First descriptor to
> +	 * handle aligned data buffer and second descriptor to handle the
> +	 * overflow data because of alignment.
> +	 */
> +	priv->num_tx_desc = info->aligned_tx ? 2 : 1;
>  
>  	/* Set function */
>  	ndev->netdev_ops = &ravb_netdev_ops;

   Other than that:

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

[...]

MBR, Sergey

Re: [PATCH net-next 02/13] ravb: Add multi_irq to struct ravb_hw_info

[Sergey Shtylyov]

On 8/25/21 10:01 AM, Biju Das wrote:

> R-Car Gen3 supports separate interrupts for E-MAC and DMA queues,
> whereas R-Car Gen2 and RZ/G2L have a single interrupt instead.
> 
> Add a multi_irq hw feature bit to struct ravb_hw_info to enable

   So you have 'multi_irq' in the patch subject/description but 'multi_irqs'?
Not very consistent...

> 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>
> ---
>  drivers/net/ethernet/renesas/ravb.h      |  1 +
>  drivers/net/ethernet/renesas/ravb_main.c | 22 ++++++++++++++--------
>  drivers/net/ethernet/renesas/ravb_ptp.c  |  8 +++++---
>  3 files changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
> index 84700a82a41c..da486e06b322 100644
> --- a/drivers/net/ethernet/renesas/ravb.h
> +++ b/drivers/net/ethernet/renesas/ravb.h
> @@ -997,6 +997,7 @@ struct ravb_hw_info {
>  	/* hardware features */
>  	unsigned internal_delay:1;	/* AVB-DMAC has internal delays */
>  	unsigned tx_counters:1;		/* E-MAC has TX counters */
> +	unsigned multi_irqs:1;		/* AVB-DMAC and E-MAC has multiple irqs */

   It's generally written IRQs but we can live with that. :-)

[...]
> diff --git a/drivers/net/ethernet/renesas/ravb_ptp.c b/drivers/net/ethernet/renesas/ravb_ptp.c
> index 6984bd5b7da9..c099656dd75b 100644
> --- a/drivers/net/ethernet/renesas/ravb_ptp.c
> +++ b/drivers/net/ethernet/renesas/ravb_ptp.c
[...]
> @@ -252,7 +254,7 @@ static int ravb_ptp_perout(struct ptp_clock_info *ptp,
>  		error = ravb_ptp_update_compare(priv, (u32)start_ns);
>  		if (!error) {
>  			/* Unmask interrupt */
> -			if (priv->chip_id == RCAR_GEN2)
> +			if (!info->multi_irqs)
>  				ravb_modify(ndev, GIC, GIC_PTME, GIC_PTME);
>  			else
>  				ravb_write(ndev, GIE_PTMS0, GIE);
> @@ -264,7 +266,7 @@ static int ravb_ptp_perout(struct ptp_clock_info *ptp,
>  		perout->period = 0;
>  
>  		/* Mask interrupt */
> -		if (priv->chip_id == RCAR_GEN2)
> +		if (!info->multi_irqs)
>  			ravb_modify(ndev, GIC, GIC_PTME, 0);
>  		else
>  			ravb_write(ndev, GID_PTMD0, GID);

   Hm... Let's assume GIE/GID are a part of multi-IRQ feature...

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

[...]

MBR, Sergey

Re: [PATCH net-next 02/13] ravb: Add multi_irq to struct ravb_hw_info

[Sergey Shtylyov]

On 8/25/21 9:49 PM, Sergey Shtylyov wrote:
> On 8/25/21 10:01 AM, Biju Das wrote:
> 
>> R-Car Gen3 supports separate interrupts for E-MAC and DMA queues,
>> whereas R-Car Gen2 and RZ/G2L have a single interrupt instead.
>>
>> Add a multi_irq hw feature bit to struct ravb_hw_info to enable
> 
>    So you have 'multi_irq' in the patch subject/description but 'multi_irqs'?

   "in the patch diff", I meant to type.

> Not very consistent...
> 
>> 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>
[...]

MBR, Sergey

Re: [PATCH net-next 03/13] ravb: Add no_ptp_cfg_active to struct ravb_hw_info

[Sergey Shtylyov]

On 8/25/21 10:01 AM, Biju Das wrote:

> There are some H/W differences for the gPTP feature between
> R-Car Gen3, R-Car Gen2, and RZ/G2L as below.
> 
> 1) On R-Car Gen2, gPTP support is not active in config mode.
> 2) On R-Car Gen3, gPTP support is active in config mode.
> 3) RZ/G2L does not support the gPTP feature.
> 
> Add a no_ptp_cfg_active hw feature bit to struct ravb_hw_info for
> handling gPTP for R-Car Gen2.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  drivers/net/ethernet/renesas/ravb.h      |  1 +
>  drivers/net/ethernet/renesas/ravb_main.c | 20 ++++++++++++--------
>  2 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
> index da486e06b322..9ecf1a8c3ca8 100644
> --- a/drivers/net/ethernet/renesas/ravb.h
> +++ b/drivers/net/ethernet/renesas/ravb.h
> @@ -998,6 +998,7 @@ struct ravb_hw_info {
>  	unsigned internal_delay:1;	/* AVB-DMAC has internal delays */
>  	unsigned tx_counters:1;		/* E-MAC has TX counters */
>  	unsigned multi_irqs:1;		/* AVB-DMAC and E-MAC has multiple irqs */
> +	unsigned no_ptp_cfg_active:1;	/* AVB-DMAC does not support gPTP active in config mode */

   Isn't this better to name it 'ptp_active_cfg' -- positive, instead of negative?

[...]

MBR, Sergey

Re: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct ravb_hw_info

[Sergey Shtylyov]

On 8/25/21 10:01 AM, Biju Das wrote:

> There are some H/W differences for the gPTP feature between
> R-Car Gen3, R-Car Gen2, and RZ/G2L as below.
> 
> 1) On R-Car Gen3, gPTP support is active in config mode.
> 2) On R-Car Gen2, gPTP support is not active in config mode.
> 3) RZ/G2L does not support the gPTP feature.
> 
> Add a ptp_cfg_active hw feature bit to struct ravb_hw_info for
> supporting gPTP active in config mode for R-Car Gen3.

   Wait, we've just done this ion the previous patch!

> This patch also removes enum ravb_chip_id, chip_id from both
> struct ravb_hw_info and struct ravb_private, as it is unused.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  drivers/net/ethernet/renesas/ravb.h      |  8 +-------
>  drivers/net/ethernet/renesas/ravb_main.c | 12 +++++-------
>  2 files changed, 6 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
> index 9ecf1a8c3ca8..209e030935aa 100644
> --- a/drivers/net/ethernet/renesas/ravb.h
> +++ b/drivers/net/ethernet/renesas/ravb.h
> @@ -979,17 +979,11 @@ struct ravb_ptp {
>  	struct ravb_ptp_perout perout[N_PER_OUT];
>  };
>  
> -enum ravb_chip_id {
> -	RCAR_GEN2,
> -	RCAR_GEN3,
> -};
> -
>  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 stats_len;
>  	size_t max_rx_len;

   I would put the above in a spearte patch...

>  	unsigned aligned_tx: 1;
> @@ -999,6 +993,7 @@ struct ravb_hw_info {
>  	unsigned tx_counters:1;		/* E-MAC has TX counters */
>  	unsigned multi_irqs:1;		/* AVB-DMAC and E-MAC has multiple irqs */
>  	unsigned no_ptp_cfg_active:1;	/* AVB-DMAC does not support gPTP active in config mode */
> +	unsigned ptp_cfg_active:1;	/* AVB-DMAC has gPTP support active in config mode */

   Huh?

>  };
>  
>  struct ravb_private {
[...]
> @@ -2216,7 +2213,7 @@ static int ravb_probe(struct platform_device *pdev)
>  	INIT_LIST_HEAD(&priv->ts_skb_list);
>  
>  	/* Initialise PTP Clock driver */
> -	if (info->chip_id != RCAR_GEN2)
> +	if (info->ptp_cfg_active)
>  		ravb_ptp_init(ndev, pdev);

   What's that? Didn't you touch this lie in patch #3?

   This seems lie a NAK bait... :-(

MBR, Sergey

Re: [PATCH net-next 05/13] ravb: Factorise ravb_ring_free function

[Sergey Shtylyov]

On 8/25/21 10:01 AM, Biju Das wrote:

> R-Car uses extended descriptor in RX, whereas RZ/G2L uses normal
> descriptor. Factorise ravb_ring_free function so that it can
> support later SoC.
> 
> 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 209e030935aa..7cb30319524a 100644
> --- a/drivers/net/ethernet/renesas/ravb.h
> +++ b/drivers/net/ethernet/renesas/ravb.h
[...]
> index 883db1049882..dc388a32496a 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -216,31 +216,42 @@ static int ravb_tx_free(struct net_device *ndev, int q, bool free_txed_only)
>  	return free_num;
>  }
>  
> +static void ravb_rx_ring_free(struct net_device *ndev, int q)
> +{
> +	struct ravb_private *priv = netdev_priv(ndev);
> +	unsigned int ring_size;
> +	unsigned int i;
> +
> +	if (!priv->rx_ring[q])
> +		return;
> +
> +	for (i = 0; i < priv->num_rx_ring[q]; i++) {
> +		struct ravb_ex_rx_desc *desc = &priv->rx_ring[q][i];
> +
> +		if (!dma_mapping_error(ndev->dev.parent,
> +				       le32_to_cpu(desc->dptr)))
> +			dma_unmap_single(ndev->dev.parent,
> +					 le32_to_cpu(desc->dptr),
> +					 RX_BUF_SZ,
> +					 DMA_FROM_DEVICE);

   I think we could reflow this argument list, so that it takes less lines...

> +	}
> +	ring_size = sizeof(struct ravb_ex_rx_desc) *
> +		    (priv->num_rx_ring[q] + 1);

   Here as well...

[...]

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

MBR, Sergey

RE: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct ravb_hw_info

[Biju Das]

Hi Sergei,

Thanks for the feedback.

> Subject: Re: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct
> ravb_hw_info
> 
> On 8/25/21 10:01 AM, Biju Das wrote:
> 
> > There are some H/W differences for the gPTP feature between R-Car
> > Gen3, R-Car Gen2, and RZ/G2L as below.
> >
> > 1) On R-Car Gen3, gPTP support is active in config mode.
> > 2) On R-Car Gen2, gPTP support is not active in config mode.
> > 3) RZ/G2L does not support the gPTP feature.
> >
> > Add a ptp_cfg_active hw feature bit to struct ravb_hw_info for
> > supporting gPTP active in config mode for R-Car Gen3.
> 
>    Wait, we've just done this ion the previous patch!
> 
> > This patch also removes enum ravb_chip_id, chip_id from both struct
> > ravb_hw_info and struct ravb_private, as it is unused.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  drivers/net/ethernet/renesas/ravb.h      |  8 +-------
> >  drivers/net/ethernet/renesas/ravb_main.c | 12 +++++-------
> >  2 files changed, 6 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/renesas/ravb.h
> > b/drivers/net/ethernet/renesas/ravb.h
> > index 9ecf1a8c3ca8..209e030935aa 100644
> > --- a/drivers/net/ethernet/renesas/ravb.h
> > +++ b/drivers/net/ethernet/renesas/ravb.h
> > @@ -979,17 +979,11 @@ struct ravb_ptp {
> >  	struct ravb_ptp_perout perout[N_PER_OUT];  };
> >
> > -enum ravb_chip_id {
> > -	RCAR_GEN2,
> > -	RCAR_GEN3,
> > -};
> > -
> >  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 stats_len;
> >  	size_t max_rx_len;
> 
>    I would put the above in a spearte patch...
> 
> >  	unsigned aligned_tx: 1;
> > @@ -999,6 +993,7 @@ struct ravb_hw_info {
> >  	unsigned tx_counters:1;		/* E-MAC has TX counters */
> >  	unsigned multi_irqs:1;		/* AVB-DMAC and E-MAC has multiple
> irqs */
> >  	unsigned no_ptp_cfg_active:1;	/* AVB-DMAC does not support gPTP
> active in config mode */
> > +	unsigned ptp_cfg_active:1;	/* AVB-DMAC has gPTP support active in
> config mode */
> 
>    Huh?
> 
> >  };
> >
> >  struct ravb_private {
> [...]
> > @@ -2216,7 +2213,7 @@ static int ravb_probe(struct platform_device
> *pdev)
> >  	INIT_LIST_HEAD(&priv->ts_skb_list);
> >
> >  	/* Initialise PTP Clock driver */
> > -	if (info->chip_id != RCAR_GEN2)
> > +	if (info->ptp_cfg_active)
> >  		ravb_ptp_init(ndev, pdev);
> 
>    What's that? Didn't you touch this lie in patch #3?
> 
>    This seems lie a NAK bait... :-(

Please refer the original patch[1] which introduced gPTP support active in config mode.
I am sure this will clear all your doubts.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/net/ethernet/renesas/ravb_main.c?h=next-20210825&id=f5d7837f96e53a8c9b6c49e1bc95cf0ae88b99e8

Regards,
Biju

> 
> MBR, Sergey

Re: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct ravb_hw_info

[Sergey Shtylyov]

On 26.08.2021 9:20, Biju Das wrote:

[...]
>>> There are some H/W differences for the gPTP feature between R-Car
>>> Gen3, R-Car Gen2, and RZ/G2L as below.
>>>
>>> 1) On R-Car Gen3, gPTP support is active in config mode.
>>> 2) On R-Car Gen2, gPTP support is not active in config mode.
>>> 3) RZ/G2L does not support the gPTP feature.
>>>
>>> Add a ptp_cfg_active hw feature bit to struct ravb_hw_info for
>>> supporting gPTP active in config mode for R-Car Gen3.
>>
>>     Wait, we've just done this ion the previous patch!
>>
>>> This patch also removes enum ravb_chip_id, chip_id from both struct
>>> ravb_hw_info and struct ravb_private, as it is unused.
>>>
>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>> ---
>>>   drivers/net/ethernet/renesas/ravb.h      |  8 +-------
>>>   drivers/net/ethernet/renesas/ravb_main.c | 12 +++++-------
>>>   2 files changed, 6 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/renesas/ravb.h
>>> b/drivers/net/ethernet/renesas/ravb.h
>>> index 9ecf1a8c3ca8..209e030935aa 100644
>>> --- a/drivers/net/ethernet/renesas/ravb.h
>>> +++ b/drivers/net/ethernet/renesas/ravb.h
>>> @@ -979,17 +979,11 @@ struct ravb_ptp {
>>>   	struct ravb_ptp_perout perout[N_PER_OUT];  };
>>>
>>> -enum ravb_chip_id {
>>> -	RCAR_GEN2,
>>> -	RCAR_GEN3,
>>> -};
>>> -
>>>   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 stats_len;
>>>   	size_t max_rx_len;
>>
>>     I would put the above in a spearte patch...

    Separate. :-)

>>>   	unsigned aligned_tx: 1;
>>> @@ -999,6 +993,7 @@ struct ravb_hw_info {
>>>   	unsigned tx_counters:1;		/* E-MAC has TX counters */
>>>   	unsigned multi_irqs:1;		/* AVB-DMAC and E-MAC has multiple
>> irqs */
>>>   	unsigned no_ptp_cfg_active:1;	/* AVB-DMAC does not support gPTP
>> active in config mode */
>>> +	unsigned ptp_cfg_active:1;	/* AVB-DMAC has gPTP support active in
>> config mode */
>>
>>     Huh?
>>
>>>   };
>>>
>>>   struct ravb_private {
>> [...]
>>> @@ -2216,7 +2213,7 @@ static int ravb_probe(struct platform_device
>> *pdev)
>>>   	INIT_LIST_HEAD(&priv->ts_skb_list);
>>>
>>>   	/* Initialise PTP Clock driver */
>>> -	if (info->chip_id != RCAR_GEN2)
>>> +	if (info->ptp_cfg_active)
>>>   		ravb_ptp_init(ndev, pdev);
>>
>>     What's that? Didn't you touch this lie in patch #3?
>>
>>     This seems lie a NAK bait... :-(
> 
> Please refer the original patch[1] which introduced gPTP support active in config mode.
> I am sure this will clear all your doubts.

    It hasn't. Why do we need 2 bit fields (1 "positive" and 1 "negative") for 
the same feature is beyond me.

> [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/net/ethernet/renesas/ravb_main.c?h=next-20210825&id=f5d7837f96e53a8c9b6c49e1bc95cf0ae88b99e8
> 
> Regards,
> Biju

MBR, Sergey

RE: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct ravb_hw_info

[Biju Das]

Hi Sergei,

> Subject: Re: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct
> ravb_hw_info
> 
> On 26.08.2021 9:20, Biju Das wrote:
> 
> [...]
> >>> There are some H/W differences for the gPTP feature between R-Car
> >>> Gen3, R-Car Gen2, and RZ/G2L as below.
> >>>
> >>> 1) On R-Car Gen3, gPTP support is active in config mode.
> >>> 2) On R-Car Gen2, gPTP support is not active in config mode.
> >>> 3) RZ/G2L does not support the gPTP feature.
> >>>
> >>> Add a ptp_cfg_active hw feature bit to struct ravb_hw_info for
> >>> supporting gPTP active in config mode for R-Car Gen3.
> >>
> >>     Wait, we've just done this ion the previous patch!
> >>
> >>> This patch also removes enum ravb_chip_id, chip_id from both struct
> >>> ravb_hw_info and struct ravb_private, as it is unused.
> >>>
> >>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>> ---
> >>>   drivers/net/ethernet/renesas/ravb.h      |  8 +-------
> >>>   drivers/net/ethernet/renesas/ravb_main.c | 12 +++++-------
> >>>   2 files changed, 6 insertions(+), 14 deletions(-)
> >>>
> >>> diff --git a/drivers/net/ethernet/renesas/ravb.h
> >>> b/drivers/net/ethernet/renesas/ravb.h
> >>> index 9ecf1a8c3ca8..209e030935aa 100644
> >>> --- a/drivers/net/ethernet/renesas/ravb.h
> >>> +++ b/drivers/net/ethernet/renesas/ravb.h
> >>> @@ -979,17 +979,11 @@ struct ravb_ptp {
> >>>   	struct ravb_ptp_perout perout[N_PER_OUT];  };
> >>>
> >>> -enum ravb_chip_id {
> >>> -	RCAR_GEN2,
> >>> -	RCAR_GEN3,
> >>> -};
> >>> -
> >>>   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 stats_len;
> >>>   	size_t max_rx_len;
> >>
> >>     I would put the above in a spearte patch...
> 
>     Separate. :-)
> 
> >>>   	unsigned aligned_tx: 1;
> >>> @@ -999,6 +993,7 @@ struct ravb_hw_info {
> >>>   	unsigned tx_counters:1;		/* E-MAC has TX counters */
> >>>   	unsigned multi_irqs:1;		/* AVB-DMAC and E-MAC has
> multiple
> >> irqs */
> >>>   	unsigned no_ptp_cfg_active:1;	/* AVB-DMAC does not support
> gPTP
> >> active in config mode */
> >>> +	unsigned ptp_cfg_active:1;	/* AVB-DMAC has gPTP support active in
> >> config mode */
> >>
> >>     Huh?
> >>
> >>>   };
> >>>
> >>>   struct ravb_private {
> >> [...]
> >>> @@ -2216,7 +2213,7 @@ static int ravb_probe(struct platform_device
> >> *pdev)
> >>>   	INIT_LIST_HEAD(&priv->ts_skb_list);
> >>>
> >>>   	/* Initialise PTP Clock driver */
> >>> -	if (info->chip_id != RCAR_GEN2)
> >>> +	if (info->ptp_cfg_active)
> >>>   		ravb_ptp_init(ndev, pdev);
> >>
> >>     What's that? Didn't you touch this lie in patch #3?
> >>
> >>     This seems lie a NAK bait... :-(
> >
> > Please refer the original patch[1] which introduced gPTP support active
> in config mode.
> > I am sure this will clear all your doubts.
> 
>     It hasn't. Why do we need 2 bit fields (1 "positive" and 1 "negative")
> for the same feature is beyond me.

The reason is mentioned in commit description, Do you agree 1, 2 and 3 mutually exclusive?

1) On R-Car Gen3, gPTP support is active in config mode.
2) On R-Car Gen2, gPTP support is not active in config mode.
3) RZ/G2L does not support the gPTP feature.

Regards,
Biju

> 
> > [1]
> > https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.
> > kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fnext%2Flinux-next.git%
> > 2Fcommit%2Fdrivers%2Fnet%2Fethernet%2Frenesas%2Fravb_main.c%3Fh%3Dnext
> > -20210825%26id%3Df5d7837f96e53a8c9b6c49e1bc95cf0ae88b99e8&amp;data=04%
> > 7C01%7Cbiju.das.jz%40bp.renesas.com%7C142c7f172b4141617e7008d9687c7881
> > %7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C637655706082218315%7CUnk
> > nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWw
> > iLCJXVCI6Mn0%3D%7C1000&amp;sdata=Miy2q4JGEZpwLHlkGxlEBK8P0XAPzuHUX6xib
> > FS8nDs%3D&amp;reserved=0
> >
> > Regards,
> > Biju
> 
> MBR, Sergey

Re: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct ravb_hw_info

[Sergey Shtylyov]

On 26.08.2021 13:34, Biju Das wrote:

[...]
>>>>> There are some H/W differences for the gPTP feature between R-Car
>>>>> Gen3, R-Car Gen2, and RZ/G2L as below.
>>>>>
>>>>> 1) On R-Car Gen3, gPTP support is active in config mode.
>>>>> 2) On R-Car Gen2, gPTP support is not active in config mode.
>>>>> 3) RZ/G2L does not support the gPTP feature.
>>>>>
>>>>> Add a ptp_cfg_active hw feature bit to struct ravb_hw_info for
>>>>> supporting gPTP active in config mode for R-Car Gen3.
>>>>
>>>>      Wait, we've just done this ion the previous patch!
>>>>
>>>>> This patch also removes enum ravb_chip_id, chip_id from both struct
>>>>> ravb_hw_info and struct ravb_private, as it is unused.
>>>>>
>>>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>>>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>>>> ---
>>>>>    drivers/net/ethernet/renesas/ravb.h      |  8 +-------
>>>>>    drivers/net/ethernet/renesas/ravb_main.c | 12 +++++-------
>>>>>    2 files changed, 6 insertions(+), 14 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/renesas/ravb.h
>>>>> b/drivers/net/ethernet/renesas/ravb.h
>>>>> index 9ecf1a8c3ca8..209e030935aa 100644
>>>>> --- a/drivers/net/ethernet/renesas/ravb.h
>>>>> +++ b/drivers/net/ethernet/renesas/ravb.h
>>>>> @@ -979,17 +979,11 @@ struct ravb_ptp {
>>>>>    	struct ravb_ptp_perout perout[N_PER_OUT];  };
>>>>>
>>>>> -enum ravb_chip_id {
>>>>> -	RCAR_GEN2,
>>>>> -	RCAR_GEN3,
>>>>> -};
>>>>> -
>>>>>    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 stats_len;
>>>>>    	size_t max_rx_len;
>>>>
>>>>      I would put the above in a spearte patch...
>>
>>      Separate. :-)
>>
>>>>>    	unsigned aligned_tx: 1;
>>>>> @@ -999,6 +993,7 @@ struct ravb_hw_info {
>>>>>    	unsigned tx_counters:1;		/* E-MAC has TX counters */
>>>>>    	unsigned multi_irqs:1;		/* AVB-DMAC and E-MAC has
>> multiple
>>>> irqs */
>>>>>    	unsigned no_ptp_cfg_active:1;	/* AVB-DMAC does not support
>> gPTP
>>>> active in config mode */
>>>>> +	unsigned ptp_cfg_active:1;	/* AVB-DMAC has gPTP support active in
>>>> config mode */
>>>>
>>>>      Huh?
>>>>
>>>>>    };
>>>>>
>>>>>    struct ravb_private {
>>>> [...]
>>>>> @@ -2216,7 +2213,7 @@ static int ravb_probe(struct platform_device
>>>> *pdev)
>>>>>    	INIT_LIST_HEAD(&priv->ts_skb_list);
>>>>>
>>>>>    	/* Initialise PTP Clock driver */
>>>>> -	if (info->chip_id != RCAR_GEN2)
>>>>> +	if (info->ptp_cfg_active)
>>>>>    		ravb_ptp_init(ndev, pdev);
>>>>
>>>>      What's that? Didn't you touch this lie in patch #3?
>>>>
>>>>      This seems lie a NAK bait... :-(
>>>
>>> Please refer the original patch[1] which introduced gPTP support active
>> in config mode.
>>> I am sure this will clear all your doubts.
>>
>>      It hasn't. Why do we need 2 bit fields (1 "positive" and 1 "negative")
>> for the same feature is beyond me.
> 
> The reason is mentioned in commit description, Do you agree 1, 2 and 3 mutually exclusive?
> 
> 1) On R-Car Gen3, gPTP support is active in config mode.
> 2) On R-Car Gen2, gPTP support is not active in config mode.
> 3) RZ/G2L does not support the gPTP feature.

    No, (1) includes (2).

[...]

> Regards,
> Biju

[...]

MBR, Sergey

RE: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct ravb_hw_info

[Biju Das]

Hi Sergei,

> Subject: Re: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct
> ravb_hw_info
> 
> On 26.08.2021 13:34, Biju Das wrote:
> 
> [...]
> >>>>> There are some H/W differences for the gPTP feature between R-Car
> >>>>> Gen3, R-Car Gen2, and RZ/G2L as below.
> >>>>>
> >>>>> 1) On R-Car Gen3, gPTP support is active in config mode.
> >>>>> 2) On R-Car Gen2, gPTP support is not active in config mode.
> >>>>> 3) RZ/G2L does not support the gPTP feature.
> >>>>>
> >>>>> Add a ptp_cfg_active hw feature bit to struct ravb_hw_info for
> >>>>> supporting gPTP active in config mode for R-Car Gen3.
> >>>>
> >>>>      Wait, we've just done this ion the previous patch!
> >>>>
> >>>>> This patch also removes enum ravb_chip_id, chip_id from both
> >>>>> struct ravb_hw_info and struct ravb_private, as it is unused.
> >>>>>
> >>>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >>>>> Reviewed-by: Lad Prabhakar
> >>>>> <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>>>> ---
> >>>>>    drivers/net/ethernet/renesas/ravb.h      |  8 +-------
> >>>>>    drivers/net/ethernet/renesas/ravb_main.c | 12 +++++-------
> >>>>>    2 files changed, 6 insertions(+), 14 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/net/ethernet/renesas/ravb.h
> >>>>> b/drivers/net/ethernet/renesas/ravb.h
> >>>>> index 9ecf1a8c3ca8..209e030935aa 100644
> >>>>> --- a/drivers/net/ethernet/renesas/ravb.h
> >>>>> +++ b/drivers/net/ethernet/renesas/ravb.h
> >>>>> @@ -979,17 +979,11 @@ struct ravb_ptp {
> >>>>>    	struct ravb_ptp_perout perout[N_PER_OUT];  };
> >>>>>
> >>>>> -enum ravb_chip_id {
> >>>>> -	RCAR_GEN2,
> >>>>> -	RCAR_GEN3,
> >>>>> -};
> >>>>> -
> >>>>>    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 stats_len;
> >>>>>    	size_t max_rx_len;
> >>>>
> >>>>      I would put the above in a spearte patch...
> >>
> >>      Separate. :-)
> >>
> >>>>>    	unsigned aligned_tx: 1;
> >>>>> @@ -999,6 +993,7 @@ struct ravb_hw_info {
> >>>>>    	unsigned tx_counters:1;		/* E-MAC has TX counters */
> >>>>>    	unsigned multi_irqs:1;		/* AVB-DMAC and E-MAC has
> >> multiple
> >>>> irqs */
> >>>>>    	unsigned no_ptp_cfg_active:1;	/* AVB-DMAC does not support
> >> gPTP
> >>>> active in config mode */
> >>>>> +	unsigned ptp_cfg_active:1;	/* AVB-DMAC has gPTP support
> active in
> >>>> config mode */
> >>>>
> >>>>      Huh?
> >>>>
> >>>>>    };
> >>>>>
> >>>>>    struct ravb_private {
> >>>> [...]
> >>>>> @@ -2216,7 +2213,7 @@ static int ravb_probe(struct platform_device
> >>>> *pdev)
> >>>>>    	INIT_LIST_HEAD(&priv->ts_skb_list);
> >>>>>
> >>>>>    	/* Initialise PTP Clock driver */
> >>>>> -	if (info->chip_id != RCAR_GEN2)
> >>>>> +	if (info->ptp_cfg_active)
> >>>>>    		ravb_ptp_init(ndev, pdev);
> >>>>
> >>>>      What's that? Didn't you touch this lie in patch #3?
> >>>>
> >>>>      This seems lie a NAK bait... :-(
> >>>
> >>> Please refer the original patch[1] which introduced gPTP support
> >>> active
> >> in config mode.
> >>> I am sure this will clear all your doubts.
> >>
> >>      It hasn't. Why do we need 2 bit fields (1 "positive" and 1
> >> "negative") for the same feature is beyond me.
> >
> > The reason is mentioned in commit description, Do you agree 1, 2 and 3
> mutually exclusive?
> >
> > 1) On R-Car Gen3, gPTP support is active in config mode.
> > 2) On R-Car Gen2, gPTP support is not active in config mode.
> > 3) RZ/G2L does not support the gPTP feature.
> 
>     No, (1) includes (2).

patch[1] is for supporting gPTP support active in config mode.

Do you agree GAC register(gPTP active in Config) bit in AVB-DMAC mode register(CCC) present only in R-Car Gen3?

[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/net/ethernet/renesas/ravb_main.c?h=next-20210825&id=f5d7837f96e53a8c9b6c49e1bc95cf0ae88b99e8

Regards,
Biju

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

Re: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct ravb_hw_info

[Sergei Shtylyov]

On 8/26/21 1:52 PM, Biju Das wrote:
[...]
>>>>>>> There are some H/W differences for the gPTP feature between R-Car
>>>>>>> Gen3, R-Car Gen2, and RZ/G2L as below.
>>>>>>>
>>>>>>> 1) On R-Car Gen3, gPTP support is active in config mode.
>>>>>>> 2) On R-Car Gen2, gPTP support is not active in config mode.
>>>>>>> 3) RZ/G2L does not support the gPTP feature.
>>>>>>>
>>>>>>> Add a ptp_cfg_active hw feature bit to struct ravb_hw_info for
>>>>>>> supporting gPTP active in config mode for R-Car Gen3.
>>>>>>
>>>>>>      Wait, we've just done this ion the previous patch!
>>>>>>
>>>>>>> This patch also removes enum ravb_chip_id, chip_id from both
>>>>>>> struct ravb_hw_info and struct ravb_private, as it is unused.
>>>>>>>
>>>>>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>>>>>> Reviewed-by: Lad Prabhakar
>>>>>>> <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>>>>>> ---
>>>>>>>    drivers/net/ethernet/renesas/ravb.h      |  8 +-------
>>>>>>>    drivers/net/ethernet/renesas/ravb_main.c | 12 +++++-------
>>>>>>>    2 files changed, 6 insertions(+), 14 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/net/ethernet/renesas/ravb.h
>>>>>>> b/drivers/net/ethernet/renesas/ravb.h
>>>>>>> index 9ecf1a8c3ca8..209e030935aa 100644
>>>>>>> --- a/drivers/net/ethernet/renesas/ravb.h
>>>>>>> +++ b/drivers/net/ethernet/renesas/ravb.h
>>>>>>> @@ -979,17 +979,11 @@ struct ravb_ptp {
>>>>>>>    	struct ravb_ptp_perout perout[N_PER_OUT];  };
>>>>>>>
>>>>>>> -enum ravb_chip_id {
>>>>>>> -	RCAR_GEN2,
>>>>>>> -	RCAR_GEN3,
>>>>>>> -};
>>>>>>> -
>>>>>>>    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 stats_len;
>>>>>>>    	size_t max_rx_len;
>>>>>>
>>>>>>      I would put the above in a spearte patch...
>>>>
>>>>      Separate. :-)
>>>>
>>>>>>>    	unsigned aligned_tx: 1;
>>>>>>> @@ -999,6 +993,7 @@ struct ravb_hw_info {
>>>>>>>    	unsigned tx_counters:1;		/* E-MAC has TX counters */
>>>>>>>    	unsigned multi_irqs:1;		/* AVB-DMAC and E-MAC has
>>>> multiple
>>>>>> irqs */
>>>>>>>    	unsigned no_ptp_cfg_active:1;	/* AVB-DMAC does not support
>>>> gPTP
>>>>>> active in config mode */
>>>>>>> +	unsigned ptp_cfg_active:1;	/* AVB-DMAC has gPTP support
>> active in
>>>>>> config mode */
>>>>>>
>>>>>>      Huh?
>>>>>>
>>>>>>>    };
>>>>>>>
>>>>>>>    struct ravb_private {
>>>>>> [...]
>>>>>>> @@ -2216,7 +2213,7 @@ static int ravb_probe(struct platform_device
>>>>>> *pdev)
>>>>>>>    	INIT_LIST_HEAD(&priv->ts_skb_list);
>>>>>>>
>>>>>>>    	/* Initialise PTP Clock driver */
>>>>>>> -	if (info->chip_id != RCAR_GEN2)
>>>>>>> +	if (info->ptp_cfg_active)
>>>>>>>    		ravb_ptp_init(ndev, pdev);
>>>>>>
>>>>>>      What's that? Didn't you touch this lie in patch #3?
>>>>>>
>>>>>>      This seems lie a NAK bait... :-(
>>>>>
>>>>> Please refer the original patch[1] which introduced gPTP support
>>>>> active
>>>> in config mode.
>>>>> I am sure this will clear all your doubts.
>>>>
>>>>      It hasn't. Why do we need 2 bit fields (1 "positive" and 1
>>>> "negative") for the same feature is beyond me.
>>>
>>> The reason is mentioned in commit description, Do you agree 1, 2 and 3
>> mutually exclusive?
>>>
>>> 1) On R-Car Gen3, gPTP support is active in config mode.
>>> 2) On R-Car Gen2, gPTP support is not active in config mode.
>>> 3) RZ/G2L does not support the gPTP feature.
>>
>>     No, (1) includes (2).
> 
> patch[1] is for supporting gPTP support active in config mode.

   Yes.

> Do you agree GAC register(gPTP active in Config) bit in AVB-DMAC mode register(CCC) present only in R-Car Gen3?

   Yes.
   But you feature naming is totally misguiding, nevertheless...

> [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/net/ethernet/renesas/ravb_main.c?h=next-20210825&id=f5d7837f96e53a8c9b6c49e1bc95cf0ae88b99e8
> 
> Regards,
> Biju

[...]

MBR, Sergey

Re: [PATCH net-next 06/13] ravb: Factorise ravb_ring_format function

[Sergey Shtylyov]

On 8/25/21 10:01 AM, Biju Das wrote:

> The ravb_ring_format function uses an extended descriptor in RX
> for R-Car compared to the normal descriptor for RZ/G2L. Factorise
> RX ring buffer buildup to extend the support for later SoC.
> 
> 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_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index dc388a32496a..e52e36ccd1c6 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
[...]
> @@ -321,6 +310,26 @@ static void ravb_ring_format(struct net_device *ndev, int q)
>  	rx_desc = &priv->rx_ring[q][i];
>  	rx_desc->dptr = cpu_to_le32((u32)priv->rx_desc_dma[q]);
>  	rx_desc->die_dt = DT_LINKFIX; /* type */
> +}
> +
> +/* Format skb and descriptor buffer for Ethernet AVB */
> +static void ravb_ring_format(struct net_device *ndev, int q)
> +{
> +	struct ravb_private *priv = netdev_priv(ndev);
> +	const struct ravb_hw_info *info = priv->info;
> +	unsigned int num_tx_desc = priv->num_tx_desc;
> +	struct ravb_tx_desc *tx_desc;
> +	struct ravb_desc *desc;
> +	unsigned int tx_ring_size = sizeof(*tx_desc) * priv->num_tx_ring[q] *
> +				    num_tx_desc;
> +	unsigned int i;
> +
> +	priv->cur_rx[q] = 0;
> +	priv->cur_tx[q] = 0;
> +	priv->dirty_rx[q] = 0;
> +	priv->dirty_tx[q] = 0;
> +
> +	info->rx_ring_format(ndev, q);
>  
>  	memset(priv->tx_ring[q], 0, tx_ring_size);
>  	/* Build TX ring buffer */

   That's all fine but the fragment that sets up TX descriptor ring base address was left in ravb_rx_ring_formet()...

[...]

MBR, Sergey

Re: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct ravb_hw_info

[Andrew Lunn]

> > Do you agree GAC register(gPTP active in Config) bit in AVB-DMAC mode register(CCC) present only in R-Car Gen3?
> 
>    Yes.
>    But you feature naming is totally misguiding, nevertheless...

It can still be changed. Just suggest a new name.

   Andrew

Re: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct ravb_hw_info

[Sergey Shtylyov]

On 8/26/21 9:57 PM, Andrew Lunn wrote:

>>> Do you agree GAC register(gPTP active in Config) bit in AVB-DMAC mode register(CCC) present only in R-Car Gen3?
>>
>>    Yes.
>>    But you feature naming is totally misguiding, nevertheless...
> 
> It can still be changed.

    Thank goodness, yea!

> Just suggest a new name.

    I'd prolly go with 'gptp' for the gPTP support and 'ccc_gac' for the gPTP working also in CONFIG mode
(CCC.GAC controls this feature).

>    Andrew

MBR, Sergey

Re: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct ravb_hw_info

[Andrew Lunn]

On Thu, Aug 26, 2021 at 10:02:07PM +0300, Sergey Shtylyov wrote:
> On 8/26/21 9:57 PM, Andrew Lunn wrote:
> 
> >>> Do you agree GAC register(gPTP active in Config) bit in AVB-DMAC mode register(CCC) present only in R-Car Gen3?
> >>
> >>    Yes.
> >>    But you feature naming is totally misguiding, nevertheless...
> > 
> > It can still be changed.
> 
>     Thank goodness, yea!

We have to live with the first version of this in the git history, but
we can add more patches fixing up whatever is broken in the unreviewed
code which got merged.

> > Just suggest a new name.
> 
>     I'd prolly go with 'gptp' for the gPTP support and 'ccc_gac' for the gPTP working also in CONFIG mode
> (CCC.GAC controls this feature).

Biju, please could you work on a couple of patches to change the names.

I also suggest you post further refactoring patches as RFC. We might
get a chance to review them then.

    Andrew

RE: [PATCH net-next 06/13] ravb: Factorise ravb_ring_format function

[Biju Das]

Hi Sergei,

> Subject: Re: [PATCH net-next 06/13] ravb: Factorise ravb_ring_format
> function
> 
> On 8/25/21 10:01 AM, Biju Das wrote:
> 
> > The ravb_ring_format function uses an extended descriptor in RX for
> > R-Car compared to the normal descriptor for RZ/G2L. Factorise RX ring
> > buffer buildup to extend the support for later SoC.
> >
> > 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_main.c
> > b/drivers/net/ethernet/renesas/ravb_main.c
> > index dc388a32496a..e52e36ccd1c6 100644
> > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> [...]
> > @@ -321,6 +310,26 @@ static void ravb_ring_format(struct net_device
> *ndev, int q)
> >  	rx_desc = &priv->rx_ring[q][i];
> >  	rx_desc->dptr = cpu_to_le32((u32)priv->rx_desc_dma[q]);
> >  	rx_desc->die_dt = DT_LINKFIX; /* type */
> > +}
> > +
> > +/* Format skb and descriptor buffer for Ethernet AVB */ static void
> > +ravb_ring_format(struct net_device *ndev, int q) {
> > +	struct ravb_private *priv = netdev_priv(ndev);
> > +	const struct ravb_hw_info *info = priv->info;
> > +	unsigned int num_tx_desc = priv->num_tx_desc;
> > +	struct ravb_tx_desc *tx_desc;
> > +	struct ravb_desc *desc;
> > +	unsigned int tx_ring_size = sizeof(*tx_desc) * priv->num_tx_ring[q]
> *
> > +				    num_tx_desc;
> > +	unsigned int i;
> > +
> > +	priv->cur_rx[q] = 0;
> > +	priv->cur_tx[q] = 0;
> > +	priv->dirty_rx[q] = 0;
> > +	priv->dirty_tx[q] = 0;
> > +
> > +	info->rx_ring_format(ndev, q);
> >
> >  	memset(priv->tx_ring[q], 0, tx_ring_size);
> >  	/* Build TX ring buffer */
> 
>    That's all fine but the fragment that sets up TX descriptor ring base
> address was left in ravb_rx_ring_formet()...

Can you please clarify this? Which fragment in [1]? Do you see any problems with that?

[1] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/drivers/net/ethernet/renesas/ravb_main.c#n286

Regards,
Biju


Regards,
Biju

RE: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct ravb_hw_info

[Biju Das]

Hi Andrew,

Thanks for the feedback.

> Subject: Re: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct
> ravb_hw_info
> 
> On Thu, Aug 26, 2021 at 10:02:07PM +0300, Sergey Shtylyov wrote:
> > On 8/26/21 9:57 PM, Andrew Lunn wrote:
> >
> > >>> Do you agree GAC register(gPTP active in Config) bit in AVB-DMAC
> mode register(CCC) present only in R-Car Gen3?
> > >>
> > >>    Yes.
> > >>    But you feature naming is totally misguiding, nevertheless...
> > >
> > > It can still be changed.
> >
> >     Thank goodness, yea!
> 
> We have to live with the first version of this in the git history, but we
> can add more patches fixing up whatever is broken in the unreviewed code
> which got merged.
> 
> > > Just suggest a new name.
> >
> >     I'd prolly go with 'gptp' for the gPTP support and 'ccc_gac' for
> > the gPTP working also in CONFIG mode (CCC.GAC controls this feature).
> 
> Biju, please could you work on a couple of patches to change the names.

Yes. Will work on the patches to change the names as suggested. 

> 
> I also suggest you post further refactoring patches as RFC. We might get a
> chance to review them then.

Agreed.

Cheers,
Biju

Re: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct ravb_hw_info

[Sergey Shtylyov]

On 8/26/21 10:37 PM, Biju Das wrote:
[...]

>>>>>> Do you agree GAC register(gPTP active in Config) bit in AVB-DMAC
>> mode register(CCC) present only in R-Car Gen3?
>>>>>
>>>>>    Yes.
>>>>>    But you feature naming is totally misguiding, nevertheless...
>>>>
>>>> It can still be changed.
>>>
>>>     Thank goodness, yea!
>>
>> We have to live with the first version of this in the git history, but we
>> can add more patches fixing up whatever is broken in the unreviewed code
>> which got merged.
>>
>>>> Just suggest a new name.
>>>
>>>     I'd prolly go with 'gptp' for the gPTP support and 'ccc_gac' for
>>> the gPTP working also in CONFIG mode (CCC.GAC controls this feature).
>>
>> Biju, please could you work on a couple of patches to change the names.
> 
> Yes. Will work on the patches to change the names as suggested. 

   TIA!
   After some more thinking, 'no_gptp' seems to suit better for the 1st case
Might need to invert the checks tho...

[...]

> Cheers,
> Biju

MBR, Sergey

Re: [PATCH net-next 07/13] ravb: Factorise ravb_ring_init function

[Sergey Shtylyov]

On 8/25/21 10:01 AM, Biju Das wrote:

> The ravb_ring_init function uses an extended descriptor in RX for
> R-Car and normal descriptor for RZ/G2L. Add a helper function
> for RX ring buffer allocation to support later SoC.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
   Here as well...
[...]

   Seems sane, so:

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

MBR, Sergey

Re: [PATCH net-next 08/13] ravb: Factorise ravb_rx function

[Sergey Shtylyov]

On 8/25/21 10:01 AM, Biju Das wrote:

> R-Car uses an extended descriptor in RX whereas, RZ/G2L uses
> normal descriptor in RX. Factorise the ravb_rx function to
> support the later SoC.
> 
> 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_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 148c974499b4..1cace5324261 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -562,8 +562,7 @@ static void ravb_rx_csum(struct sk_buff *skb)
>  	skb_trim(skb, skb->len - sizeof(__sum16));
>  }
>  
> -/* Packet receive function for Ethernet AVB */
> -static bool ravb_rx(struct net_device *ndev, int *quota, int q)
> +static bool ravb_rcar_rx(struct net_device *ndev, int *quota, int q)

   Mhm, isn't this too large a function to duplicate it all for RZ-G2?

[...]

MBR, Sergey

RE: [PATCH net-next 08/13] ravb: Factorise ravb_rx function

[Biju Das]

Hi Sergei,

> Subject: Re: [PATCH net-next 08/13] ravb: Factorise ravb_rx function
> 
> On 8/25/21 10:01 AM, Biju Das wrote:
> 
> > R-Car uses an extended descriptor in RX whereas, RZ/G2L uses normal
> > descriptor in RX. Factorise the ravb_rx function to support the later
> > SoC.
> >
> > 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_main.c
> > b/drivers/net/ethernet/renesas/ravb_main.c
> > index 148c974499b4..1cace5324261 100644
> > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> > @@ -562,8 +562,7 @@ static void ravb_rx_csum(struct sk_buff *skb)
> >  	skb_trim(skb, skb->len - sizeof(__sum16));  }
> >
> > -/* Packet receive function for Ethernet AVB */ -static bool
> > ravb_rx(struct net_device *ndev, int *quota, int q)
> > +static bool ravb_rcar_rx(struct net_device *ndev, int *quota, int q)
> 
>    Mhm, isn't this too large a function to duplicate it all for RZ-G2?

For your reference, it is a large change. See [1].
[1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20210722141351.13668-18-biju.das.jz@bp.renesas.com/

Currently I am working on the name change and next RFC patchset for started adding
Initial support for RZ/G2. I am expecting your valuable suggestion on that granular
RFC Patch for RZ/G2(ravb_rx) function.

Regards,
Biju

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

RE: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct ravb_hw_info

[Biju Das]

Hi Sergei,

Thanks for the feedback.

> Subject: Re: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct
> ravb_hw_info
> 
> On 8/26/21 10:37 PM, Biju Das wrote:
> [...]
> 
> >>>>>> Do you agree GAC register(gPTP active in Config) bit in AVB-DMAC
> >> mode register(CCC) present only in R-Car Gen3?
> >>>>>
> >>>>>    Yes.
> >>>>>    But you feature naming is totally misguiding, nevertheless...
> >>>>
> >>>> It can still be changed.
> >>>
> >>>     Thank goodness, yea!
> >>
> >> We have to live with the first version of this in the git history,
> >> but we can add more patches fixing up whatever is broken in the
> >> unreviewed code which got merged.
> >>
> >>>> Just suggest a new name.
> >>>
> >>>     I'd prolly go with 'gptp' for the gPTP support and 'ccc_gac' for
> >>> the gPTP working also in CONFIG mode (CCC.GAC controls this feature).
> >>
> >> Biju, please could you work on a couple of patches to change the names.
> >
> > Yes. Will work on the patches to change the names as suggested.
> 
>    TIA!
>    After some more thinking, 'no_gptp' seems to suit better for the 1st
> case Might need to invert the checks tho...

OK, Will do with invert checks.

So just to conclude,

'no_gptp' and 'ccc_gac' are the suggested names changes for the previous patch
and current patch.

Cheers,
Biju

Re: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct ravb_hw_info

[Sergey Shtylyov]

On 27.08.2021 9:36, Biju Das wrote:

[...]

>>>>>>>> Do you agree GAC register(gPTP active in Config) bit in AVB-DMAC
>>>> mode register(CCC) present only in R-Car Gen3?
>>>>>>>
>>>>>>>     Yes.
>>>>>>>     But you feature naming is totally misguiding, nevertheless...
>>>>>>
>>>>>> It can still be changed.
>>>>>
>>>>>      Thank goodness, yea!
>>>>
>>>> We have to live with the first version of this in the git history,
>>>> but we can add more patches fixing up whatever is broken in the
>>>> unreviewed code which got merged.
>>>>
>>>>>> Just suggest a new name.
>>>>>
>>>>>      I'd prolly go with 'gptp' for the gPTP support and 'ccc_gac' for
>>>>> the gPTP working also in CONFIG mode (CCC.GAC controls this feature).
>>>>
>>>> Biju, please could you work on a couple of patches to change the names.
>>>
>>> Yes. Will work on the patches to change the names as suggested.
>>
>>     TIA!
>>     After some more thinking, 'no_gptp' seems to suit better for the 1st
>> case Might need to invert the checks tho...
> 
> OK, Will do with invert checks.
> 
> So just to conclude,
> 
> 'no_gptp' and 'ccc_gac' are the suggested names changes for the previous patch
> and current patch.

     Your patches have been merged already. Might try to encompass all gPTP 
features with one patch (just a thought)...

> Cheers,
> Biju

MBR, Sergey

RE: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct ravb_hw_info

[Biju Das]

Hi Sergei,

Thanks for the feedback.

> Subject: Re: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct
> ravb_hw_info
> 
> On 27.08.2021 9:36, Biju Das wrote:
> 
> [...]
> 
> >>>>>>>> Do you agree GAC register(gPTP active in Config) bit in
> >>>>>>>> AVB-DMAC
> >>>> mode register(CCC) present only in R-Car Gen3?
> >>>>>>>
> >>>>>>>     Yes.
> >>>>>>>     But you feature naming is totally misguiding, nevertheless...
> >>>>>>
> >>>>>> It can still be changed.
> >>>>>
> >>>>>      Thank goodness, yea!
> >>>>
> >>>> We have to live with the first version of this in the git history,
> >>>> but we can add more patches fixing up whatever is broken in the
> >>>> unreviewed code which got merged.
> >>>>
> >>>>>> Just suggest a new name.
> >>>>>
> >>>>>      I'd prolly go with 'gptp' for the gPTP support and 'ccc_gac'
> >>>>> for the gPTP working also in CONFIG mode (CCC.GAC controls this
> feature).
> >>>>
> >>>> Biju, please could you work on a couple of patches to change the
> names.
> >>>
> >>> Yes. Will work on the patches to change the names as suggested.
> >>
> >>     TIA!
> >>     After some more thinking, 'no_gptp' seems to suit better for the
> >> 1st case Might need to invert the checks tho...
> >
> > OK, Will do with invert checks.
> >
> > So just to conclude,
> >
> > 'no_gptp' and 'ccc_gac' are the suggested names changes for the
> > previous patch and current patch.
> 
>      Your patches have been merged already. Might try to encompass all
> gPTP features with one patch (just a thought)...

OK, in that case it will be taken care in next RFC patch set.

Regards,
Biju

Re: [PATCH net-next 10/13] ravb: Factorise ravb_set_features

[Sergey Shtylyov]

On 8/25/21 10:01 AM, Biju Das wrote:

> RZ/G2L supports HW checksum on RX and TX whereas R-Car supports on RX.
> Factorise ravb_set_features to support this feature.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  drivers/net/ethernet/renesas/ravb.h      |  1 +
>  drivers/net/ethernet/renesas/ravb_main.c | 15 +++++++++++++--
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
[...]
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 1f9d9f54bf1b..1789309c4c03 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -1901,8 +1901,8 @@ static void ravb_set_rx_csum(struct net_device *ndev, bool enable)
>  	spin_unlock_irqrestore(&priv->lock, flags);
>  }
>  
> -static int ravb_set_features(struct net_device *ndev,
> -			     netdev_features_t features)
> +static int ravb_set_features_rx_csum(struct net_device *ndev,
> +				     netdev_features_t features)

   How about ravb_set_features_rcar() or s/th alike?

[...]

   Other than that:

Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
   Let's see the TOC code now...

MBR, Sergey

Re: [PATCH net-next 11/13] ravb: Factorise ravb_dmac_init function

[Sergey Shtylyov]

On 8/25/21 10:01 AM, Biju Das wrote:

> The DMAC IP on the R-Car AVB module has different initialization
> parameters for RCR, TGC, TCCR, RIC0, RIC2, and TIC compared to
> DMAC IP on the RZ/G2L Gigabit Ethernet module. Factorise the
> ravb_dmac_init function to support the later SoC.

   Couldn't we resolve these differencies like the sh_eth driver does,
by adding the register values into the *struct* ravb_hw_info?

> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
[...]

MBR, Sergey

Re: [PATCH net-next 12/13] ravb: Factorise ravb_emac_init function

[Sergey Shtylyov]

On 8/25/21 10:01 AM, Biju Das wrote:

> The E-MAC IP on the R-Car AVB module has different initialization
> parameters for RX frame size, duplex settings, different offset
> for transfer speed setting and has magic packet detection support
> compared to E-MAC on RZ/G2L Gigabit Ethernet module. Factorise
> the ravb_emac_init function to support the later SoC.

   Again, couldn't we resolve these differencies like the sh_eth driver does,
by adding the register values into the *struct* ravb_hw_info?

> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
[...]

MBR, Sergey

Re: [PATCH net-next 13/13] ravb: Add reset support

[Sergey Shtylyov]

On 8/25/21 10:01 AM, Biju Das wrote:

> Reset support is present on R-Car. Let's support it, if it is
> available.
> 
> 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_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 7a144b45e41d..0f85f2d97b18 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
[...]

> @@ -2349,6 +2358,7 @@ static int ravb_probe(struct platform_device *pdev)
>  
>  	pm_runtime_put(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
> +	reset_control_assert(rstc);
>  	return error;
>  }
>  
> @@ -2374,6 +2384,7 @@ static int ravb_remove(struct platform_device *pdev)
>  	netif_napi_del(&priv->napi[RAVB_BE]);
>  	ravb_mdio_release(priv);
>  	pm_runtime_disable(&pdev->dev);
> +	reset_control_assert(priv->rstc);
>  	free_netdev(ndev);
>  	platform_set_drvdata(pdev, NULL);
>  

   Is it possible to get into/out of reset in open()/close() methods?
   Otherwise, looks good (I'm not much into reset h/w)

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

MBR, Sergey

RE: [PATCH net-next 10/13] ravb: Factorise ravb_set_features

[Biju Das]

> -----Original Message-----
> From: Sergey Shtylyov <s.shtylyov@omp.ru>
> Sent: 27 August 2021 20:17
> To: Biju Das <biju.das.jz@bp.renesas.com>; David S. Miller
> <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>
> Cc: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>;
> Andrew Lunn <andrew@lunn.ch>; Sergei Shtylyov <sergei.shtylyov@gmail.com>;
> Geert Uytterhoeven <geert+renesas@glider.be>; Adam Ford
> <aford173@gmail.com>; Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com>; netdev@vger.kernel.org; linux-renesas-
> soc@vger.kernel.org; Chris Paterson <Chris.Paterson2@renesas.com>; Biju
> Das <biju.das@bp.renesas.com>
> Subject: Re: [PATCH net-next 10/13] ravb: Factorise ravb_set_features
> 
> On 8/25/21 10:01 AM, Biju Das wrote:
> 
> > RZ/G2L supports HW checksum on RX and TX whereas R-Car supports on RX.
> > Factorise ravb_set_features to support this feature.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  drivers/net/ethernet/renesas/ravb.h      |  1 +
> >  drivers/net/ethernet/renesas/ravb_main.c | 15 +++++++++++++--
> >  2 files changed, 14 insertions(+), 2 deletions(-)
> >
> [...]
> > diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> > b/drivers/net/ethernet/renesas/ravb_main.c
> > index 1f9d9f54bf1b..1789309c4c03 100644
> > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> > @@ -1901,8 +1901,8 @@ static void ravb_set_rx_csum(struct net_device
> *ndev, bool enable)
> >  	spin_unlock_irqrestore(&priv->lock, flags);  }
> >
> > -static int ravb_set_features(struct net_device *ndev,
> > -			     netdev_features_t features)
> > +static int ravb_set_features_rx_csum(struct net_device *ndev,
> > +				     netdev_features_t features)
> 
>    How about ravb_set_features_rcar() or s/th alike?

What about

ravb_rcar_set_features_csum()?

and

ravb_rgeth_set_features_csum()?


If you are ok with this name change I will incorporate this changes in next - RFC patchset?

If you still want ravb_set_features_rcar() and ravb_set_features_rgeth(), I am ok with that as well.

Please let me know, which name you like.

Regards,
Biju



> [...]
> 
>    Other than that:
> 
> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
>    Let's see the TOC code now...
> 
> MBR, Sergey

RE: [PATCH net-next 11/13] ravb: Factorise ravb_dmac_init function

[Biju Das]

Hi Sergei,

Thanks for the feedback.

> Subject: Re: [PATCH net-next 11/13] ravb: Factorise ravb_dmac_init
> function
> 
> On 8/25/21 10:01 AM, Biju Das wrote:
> 
> > The DMAC IP on the R-Car AVB module has different initialization
> > parameters for RCR, TGC, TCCR, RIC0, RIC2, and TIC compared to DMAC IP
> > on the RZ/G2L Gigabit Ethernet module. Factorise the ravb_dmac_init
> > function to support the later SoC.
> 
>    Couldn't we resolve these differencies like the sh_eth driver does, by
> adding the register values into the *struct* ravb_hw_info?

I will evaluate your proposal in terms of code size and data size
And with the current code and share the details in next RFC patchset
for supporting RZ/G2L with dmac_init function.
Based on the RFC discussion, we can conclude it.

Currently by looking at your proposal, I am seeing duplication of
Data in R-Car Gen3 and R-Car Gen2.

If statement for adding RIC3 register for RZ/G2L, which involves
Exposing another hwinfo bit.

Regards,
Biju

> 
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> [...]
> 
> MBR, Sergey

RE: [PATCH net-next 12/13] ravb: Factorise ravb_emac_init function

[Biju Das]

Hi Sergei,

Thanks for the feedback.

> Subject: Re: [PATCH net-next 12/13] ravb: Factorise ravb_emac_init
> function
> 
> On 8/25/21 10:01 AM, Biju Das wrote:
> 
> > The E-MAC IP on the R-Car AVB module has different initialization
> > parameters for RX frame size, duplex settings, different offset for
> > transfer speed setting and has magic packet detection support compared
> > to E-MAC on RZ/G2L Gigabit Ethernet module. Factorise the
> > ravb_emac_init function to support the later SoC.
> 
>    Again, couldn't we resolve these differencies like the sh_eth driver
> does, by adding the register values into the *struct* ravb_hw_info?


I will evaluate your proposal in terms of code size and data size
And with the current code and share the details in next RFC patchset
for supporting RZ/G2L with emac_init function.
Based on the RFC discussion, we can conclude it.

Currently by looking at your proposal, I am seeing duplication of
Data in R-Car Gen3 and R-Car Gen2.

Multiple if statement for handling duplex, initialising CSR0(Checksum operating mode register),
CXR31(In-band status register)


Regards,
Biju


> 
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> [...]
> 
> MBR, Sergey

RE: [PATCH net-next 13/13] ravb: Add reset support

[Biju Das]

Hi Sergei,

Thanks for the feedback.

> Subject: Re: [PATCH net-next 13/13] ravb: Add reset support
> 
> On 8/25/21 10:01 AM, Biju Das wrote:
> 
> > Reset support is present on R-Car. Let's support it, if it is
> > available.
> >
> > 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_main.c
> > b/drivers/net/ethernet/renesas/ravb_main.c
> > index 7a144b45e41d..0f85f2d97b18 100644
> > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> [...]
> 
> > @@ -2349,6 +2358,7 @@ static int ravb_probe(struct platform_device
> > *pdev)
> >
> >  	pm_runtime_put(&pdev->dev);
> >  	pm_runtime_disable(&pdev->dev);
> > +	reset_control_assert(rstc);
> >  	return error;
> >  }
> >
> > @@ -2374,6 +2384,7 @@ static int ravb_remove(struct platform_device
> *pdev)
> >  	netif_napi_del(&priv->napi[RAVB_BE]);
> >  	ravb_mdio_release(priv);
> >  	pm_runtime_disable(&pdev->dev);
> > +	reset_control_assert(priv->rstc);
> >  	free_netdev(ndev);
> >  	platform_set_drvdata(pdev, NULL);
> >
> 
>    Is it possible to get into/out of reset in open()/close() methods?

No, Reason, Normally reset will be called

	ravb_mdio_release(priv);
	pm_runtime_disable(&pdev->dev);
	reset_control_assert(priv->rstc);

After reset assert, We should not access any RAVB registers, otherwise system will hang.
There is a high chance that other users(for eg:- mdio) may access ravb registers and system hangs.

Regards,
Biju


>    Otherwise, looks good (I'm not much into reset h/w)
> 
> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> 
> MBR, Sergey

Re: [PATCH net-next 10/13] ravb: Factorise ravb_set_features

[Sergey Shtylyov]

On 28.08.2021 12:20, Biju Das wrote:

[...]
>>> RZ/G2L supports HW checksum on RX and TX whereas R-Car supports on RX.
>>> Factorise ravb_set_features to support this feature.
>>>
>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>> ---
>>>   drivers/net/ethernet/renesas/ravb.h      |  1 +
>>>   drivers/net/ethernet/renesas/ravb_main.c | 15 +++++++++++++--
>>>   2 files changed, 14 insertions(+), 2 deletions(-)
>>>
>> [...]
>>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c
>>> b/drivers/net/ethernet/renesas/ravb_main.c
>>> index 1f9d9f54bf1b..1789309c4c03 100644
>>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>>> @@ -1901,8 +1901,8 @@ static void ravb_set_rx_csum(struct net_device
>> *ndev, bool enable)
>>>   	spin_unlock_irqrestore(&priv->lock, flags);  }
>>>
>>> -static int ravb_set_features(struct net_device *ndev,
>>> -			     netdev_features_t features)
>>> +static int ravb_set_features_rx_csum(struct net_device *ndev,
>>> +				     netdev_features_t features)
>>
>>     How about ravb_set_features_rcar() or s/th alike?
> 
> What about
> 
> ravb_rcar_set_features_csum()?
> 
> and
> 
> ravb_rgeth_set_features_csum()?
 >
> If you are ok with this name change I will incorporate this changes in next - RFC patchset?
> 
> If you still want ravb_set_features_rcar() and ravb_set_features_rgeth(), I am ok with that as well.
> 
> Please let me know, which name you like.

    Looking back at sh_eth, my variant seems to fit better...

> Regards,
> Biju

[...]

MBR, Sergey

RE: [PATCH net-next 10/13] ravb: Factorise ravb_set_features

[Biju Das]

Hi Sergei,

> Subject: Re: [PATCH net-next 10/13] ravb: Factorise ravb_set_features
> 
> On 28.08.2021 12:20, Biju Das wrote:
> 
> [...]
> >>> RZ/G2L supports HW checksum on RX and TX whereas R-Car supports on RX.
> >>> Factorise ravb_set_features to support this feature.
> >>>
> >>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>> ---
> >>>   drivers/net/ethernet/renesas/ravb.h      |  1 +
> >>>   drivers/net/ethernet/renesas/ravb_main.c | 15 +++++++++++++--
> >>>   2 files changed, 14 insertions(+), 2 deletions(-)
> >>>
> >> [...]
> >>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> >>> b/drivers/net/ethernet/renesas/ravb_main.c
> >>> index 1f9d9f54bf1b..1789309c4c03 100644
> >>> --- a/drivers/net/ethernet/renesas/ravb_main.c
> >>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> >>> @@ -1901,8 +1901,8 @@ static void ravb_set_rx_csum(struct net_device
> >> *ndev, bool enable)
> >>>   	spin_unlock_irqrestore(&priv->lock, flags);  }
> >>>
> >>> -static int ravb_set_features(struct net_device *ndev,
> >>> -			     netdev_features_t features)
> >>> +static int ravb_set_features_rx_csum(struct net_device *ndev,
> >>> +				     netdev_features_t features)
> >>
> >>     How about ravb_set_features_rcar() or s/th alike?
> >
> > What about
> >
> > ravb_rcar_set_features_csum()?
> >
> > and
> >
> > ravb_rgeth_set_features_csum()?
>  >
> > If you are ok with this name change I will incorporate this changes in
> next - RFC patchset?
> >
> > If you still want ravb_set_features_rcar() and
> ravb_set_features_rgeth(), I am ok with that as well.
> >
> > Please let me know, which name you like.
> 
>     Looking back at sh_eth, my variant seems to fit better...

OK, will take care this name change in next RFC- patchset which includes RZ/G2L support.

Regards,
Biju