[PATCH net-next 00/18] Add Gigabit Ethernet driver support

STALE1851d

Revision v1 of 6 in this series.

42 messages, 6 authors, 2021-08-20 · open the first message on its own page

[PATCH net-next 00/18] Add Gigabit Ethernet driver support

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-22 14:14:02

The DMAC and EMAC blocks of Gigabit Ethernet IP is almost similar to Ethernet AVB.

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

With few changes in driver, we can support Gigabit ethernet driver as well.

This patch series is aims to support the same

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

Biju Das (18):
  dt-bindings: net: renesas,etheravb: Document Gigabit Ethernet IP
  drivers: clk: renesas: rzg2l-cpg: Add support to handle MUX clocks
  drivers: clk: renesas: r9a07g044-cpg: Add ethernet clock sources
  drivers: clk: renesas: r9a07g044-cpg: Add GbEthernet clock/reset
  ravb: Replace chip type with a structure for driver data
  ravb: Factorise ptp feature
  ravb: Add features specific to R-Car Gen3
  ravb: Add R-Car common features
  ravb: Factorise ravb_ring_free function
  ravb: Factorise ravb_ring_format function
  ravb: Factorise ravb_ring_init function
  ravb: Factorise {emac,dmac} init function
  ravb: Factorise ravb_rx function
  ravb: Factorise ravb_adjust_link function
  ravb: Factorise ravb_set_features
  ravb: Add reset support
  ravb: Add GbEthernet driver support
  arm64: dts: renesas: r9a07g044: Add GbEther nodes

 .../bindings/net/renesas,etheravb.yaml        |  57 +-
 arch/arm64/boot/dts/renesas/r9a07g044.dtsi    |  42 +
 drivers/clk/renesas/r9a07g044-cpg.c           |  27 +
 drivers/clk/renesas/rzg2l-cpg.c               |  24 +
 drivers/clk/renesas/rzg2l-cpg.h               |  15 +
 drivers/net/ethernet/renesas/ravb.h           | 112 ++-
 drivers/net/ethernet/renesas/ravb_main.c      | 922 +++++++++++++++---
 7 files changed, 1031 insertions(+), 168 deletions(-)

-- 
2.17.1

[PATCH net-next 07/18] ravb: Add features specific to R-Car Gen3

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-22 14:14:37

Multiple irqs, internal delay and tx drop counter is present only
in R-Car Gen3. Add feature bits to support the same.

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_main.c | 36 ++++++++++++++++--------
 1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index e966b76df32c..b3c99f974632 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -42,10 +42,18 @@
 
 #define RAVB_PTP_CONFIG_ACTIVE		BIT(0)
 #define RAVB_PTP_CONFIG_INACTIVE	BIT(1)
+#define RAVB_MULTI_IRQS			BIT(2)
+#define RAVB_INTERNAL_DELAY		BIT(3)
+#define RAVB_TX_DROP_COUNTER		BIT(4)
 
 #define RAVB_PTP	(RAVB_PTP_CONFIG_ACTIVE | RAVB_PTP_CONFIG_INACTIVE)
 
-#define RAVB_RCAR_GEN3_FEATURES	RAVB_PTP_CONFIG_ACTIVE
+#define RAVB_RCAR_GEN3_FEATURES \
+		(RAVB_PTP_CONFIG_ACTIVE		| \
+		 RAVB_MULTI_IRQS		| \
+		 RAVB_INTERNAL_DELAY		| \
+		 RAVB_TX_DROP_COUNTER)
+
 #define RAVB_RCAR_GEN2_FEATURES	RAVB_PTP_CONFIG_INACTIVE
 
 static const char *ravb_rx_irqs[NUM_RX_QUEUE] = {
@@ -435,6 +443,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_drv_data *info = priv->info;
 	int error;
 
 	/* Set CONFIG mode */
@@ -466,7 +475,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->features & RAVB_MULTI_IRQS) {
 		/* Clear DIL.DPLx */
 		ravb_write(ndev, 0, DIL);
 		/* Set queue specific interrupt */
@@ -767,6 +776,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_drv_data *info = priv->info;
 	u32 ris0 = ravb_read(ndev, RIS0);
 	u32 ric0 = ravb_read(ndev, RIC0);
 	u32 tis  = ravb_read(ndev, TIS);
@@ -775,7 +785,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->features & RAVB_MULTI_IRQS)) {
 				ravb_write(ndev, ric0 & ~BIT(q), RIC0);
 				ravb_write(ndev, tic & ~BIT(q), TIC);
 			} else {
@@ -919,6 +929,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_drv_data *info = priv->info;
 	unsigned long flags;
 	int q = napi - priv->napi;
 	int mask = BIT(q);
@@ -942,7 +953,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->features & RAVB_MULTI_IRQS)) {
 		ravb_modify(ndev, RIC0, mask, mask);
 		ravb_modify(ndev, TIC,  mask, mask);
 	} else {
@@ -1360,7 +1371,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->features & RAVB_MULTI_IRQS)) {
 		error = request_irq(ndev->irq, ravb_interrupt, IRQF_SHARED,
 				    ndev->name, ndev);
 		if (error) {
@@ -1418,7 +1429,7 @@ static int ravb_open(struct net_device *ndev)
 	if (info->features & RAVB_PTP_CONFIG_INACTIVE)
 		ravb_ptp_stop(ndev);
 out_free_irq_nc_tx:
-	if (priv->chip_id == RCAR_GEN2)
+	if (!(info->features & RAVB_MULTI_IRQS))
 		goto out_free_irq;
 	free_irq(priv->tx_irqs[RAVB_NC], ndev);
 out_free_irq_nc_rx:
@@ -1648,13 +1659,14 @@ static u16 ravb_select_queue(struct net_device *ndev, struct sk_buff *skb,
 static struct net_device_stats *ravb_get_stats(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
 	struct net_device_stats *nstats, *stats0, *stats1;
 
 	nstats = &ndev->stats;
 	stats0 = &priv->stats[RAVB_BE];
 	stats1 = &priv->stats[RAVB_NC];
 
-	if (priv->chip_id == RCAR_GEN3) {
+	if (info->features & RAVB_TX_DROP_COUNTER) {
 		nstats->tx_dropped += ravb_read(ndev, TROCR);
 		ravb_write(ndev, 0, TROCR);	/* (write clear) */
 	}
@@ -1729,7 +1741,7 @@ static int ravb_close(struct net_device *ndev)
 			of_phy_deregister_fixed_link(np);
 	}
 
-	if (priv->chip_id != RCAR_GEN2) {
+	if (info->features & RAVB_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);
@@ -2113,7 +2125,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->features & RAVB_MULTI_IRQS)
 		irq = platform_get_irq_byname(pdev, "ch22");
 	else
 		irq = platform_get_irq(pdev, 0);
@@ -2153,7 +2165,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->features & RAVB_MULTI_IRQS) {
 		irq = platform_get_irq_byname(pdev, "ch24");
 		if (irq < 0) {
 			error = irq;
@@ -2215,7 +2227,7 @@ static int ravb_probe(struct platform_device *pdev)
 		ravb_modify(ndev, GCCR, GCCR_LTI, GCCR_LTI);
 	}
 
-	if (priv->chip_id != RCAR_GEN2) {
+	if (info->features & RAVB_INTERNAL_DELAY) {
 		ravb_parse_delay_mode(np, ndev);
 		ravb_set_delay_mode(ndev);
 	}
@@ -2414,7 +2426,7 @@ static int __maybe_unused ravb_resume(struct device *dev)
 		ravb_modify(ndev, GCCR, GCCR_LTI, GCCR_LTI);
 	}
 
-	if (priv->chip_id != RCAR_GEN2)
+	if (info->features & RAVB_INTERNAL_DELAY)
 		ravb_set_delay_mode(ndev);
 
 	/* Restore descriptor base address table */
-- 
2.17.1

[PATCH net-next 06/18] ravb: Factorise ptp feature

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-22 14:14:37

Gptp is active in CONFIG mode for R-Car Gen3, where as it is not
active in CONFIG mode for R-Car Gen2. Add feature bits to handle
both cases.

RZ/G2L does not support ptp feature. Factorise ptp feature
specific to R-Car.

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 | 81 ++++++++++++++++--------
 2 files changed, 56 insertions(+), 26 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 0ed21262f26b..a474ed68db22 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -998,6 +998,7 @@ struct ravb_drv_data {
 	size_t skb_sz;
 	u8 num_tx_desc;
 	enum ravb_chip_id chip_id;
+	u32 features;
 };
 
 struct ravb_private {
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 84ebd6fef711..e966b76df32c 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -40,6 +40,14 @@
 		 NETIF_MSG_RX_ERR | \
 		 NETIF_MSG_TX_ERR)
 
+#define RAVB_PTP_CONFIG_ACTIVE		BIT(0)
+#define RAVB_PTP_CONFIG_INACTIVE	BIT(1)
+
+#define RAVB_PTP	(RAVB_PTP_CONFIG_ACTIVE | RAVB_PTP_CONFIG_INACTIVE)
+
+#define RAVB_RCAR_GEN3_FEATURES	RAVB_PTP_CONFIG_ACTIVE
+#define RAVB_RCAR_GEN2_FEATURES	RAVB_PTP_CONFIG_INACTIVE
+
 static const char *ravb_rx_irqs[NUM_RX_QUEUE] = {
 	"ch0", /* RAVB_BE */
 	"ch1", /* RAVB_NC */
@@ -804,6 +812,7 @@ static irqreturn_t ravb_interrupt(int irq, void *dev_id)
 {
 	struct net_device *ndev = dev_id;
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
 	irqreturn_t result = IRQ_NONE;
 	u32 iss;
 
@@ -839,7 +848,7 @@ static irqreturn_t ravb_interrupt(int irq, void *dev_id)
 	}
 
 	/* gPTP interrupt status summary */
-	if (iss & ISS_CGIS) {
+	if ((info->features & RAVB_PTP) && (iss & ISS_CGIS)) {
 		ravb_ptp_interrupt(ndev);
 		result = IRQ_HANDLED;
 	}
@@ -1204,6 +1213,7 @@ static int ravb_set_ringparam(struct net_device *ndev,
 			      struct ethtool_ringparam *ring)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
 	int error;
 
 	if (ring->tx_pending > BE_TX_RING_MAX ||
@@ -1217,7 +1227,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->features & RAVB_PTP_CONFIG_INACTIVE)
 			ravb_ptp_stop(ndev);
 		/* Wait for DMA stopping */
 		error = ravb_stop_dma(ndev);
@@ -1249,7 +1259,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->features & RAVB_PTP_CONFIG_INACTIVE)
 			ravb_ptp_init(ndev, priv->pdev);
 
 		netif_device_attach(ndev);
@@ -1262,6 +1272,7 @@ static int ravb_get_ts_info(struct net_device *ndev,
 			    struct ethtool_ts_info *info)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *data = priv->info;
 
 	info->so_timestamping =
 		SOF_TIMESTAMPING_TX_SOFTWARE |
@@ -1275,7 +1286,8 @@ static int ravb_get_ts_info(struct net_device *ndev,
 		(1 << HWTSTAMP_FILTER_NONE) |
 		(1 << HWTSTAMP_FILTER_PTP_V2_L2_EVENT) |
 		(1 << HWTSTAMP_FILTER_ALL);
-	info->phc_index = ptp_clock_index(priv->ptp.clock);
+	if (data->features & RAVB_PTP)
+		info->phc_index = ptp_clock_index(priv->ptp.clock);
 
 	return 0;
 }
@@ -1340,6 +1352,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_drv_data *info = priv->info;
 	struct platform_device *pdev = priv->pdev;
 	struct device *dev = &pdev->dev;
 	int error;
@@ -1388,7 +1401,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->features & RAVB_PTP_CONFIG_INACTIVE)
 		ravb_ptp_init(ndev, priv->pdev);
 
 	netif_tx_start_all_queues(ndev);
@@ -1402,7 +1415,7 @@ static int ravb_open(struct net_device *ndev)
 
 out_ptp_stop:
 	/* Stop PTP Clock driver */
-	if (priv->chip_id == RCAR_GEN2)
+	if (info->features & RAVB_PTP_CONFIG_INACTIVE)
 		ravb_ptp_stop(ndev);
 out_free_irq_nc_tx:
 	if (priv->chip_id == RCAR_GEN2)
@@ -1443,13 +1456,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_drv_data *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->features & RAVB_PTP_CONFIG_INACTIVE)
 		ravb_ptp_stop(ndev);
 
 	/* Wait for DMA stopping */
@@ -1484,7 +1498,7 @@ static void ravb_tx_timeout_work(struct work_struct *work)
 
 out:
 	/* Initialise PTP Clock driver */
-	if (priv->chip_id == RCAR_GEN2)
+	if (info->features & RAVB_PTP_CONFIG_INACTIVE)
 		ravb_ptp_init(ndev, priv->pdev);
 
 	netif_tx_start_all_queues(ndev);
@@ -1681,6 +1695,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_drv_data *info = priv->info;
 	struct ravb_tstamp_skb *ts_skb, *ts_skb2;
 
 	netif_tx_stop_all_queues(ndev);
@@ -1691,7 +1706,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->features & RAVB_PTP_CONFIG_INACTIVE)
 		ravb_ptp_stop(ndev);
 
 	/* Set the config mode to stop the AVB-DMAC's processes */
@@ -1940,6 +1955,7 @@ static const struct ravb_drv_data ravb_gen3_data = {
 	.skb_sz = RX_BUF_SZ + RAVB_ALIGN - 1,
 	.num_tx_desc = NUM_TX_DESC_GEN3,
 	.chip_id = RCAR_GEN3,
+	.features = RAVB_RCAR_GEN3_FEATURES,
 };
 
 static const struct ravb_drv_data ravb_gen2_data = {
@@ -1952,6 +1968,7 @@ static const struct ravb_drv_data ravb_gen2_data = {
 	.skb_sz = RX_BUF_SZ + RAVB_ALIGN - 1,
 	.num_tx_desc = NUM_TX_DESC_GEN2,
 	.chip_id = RCAR_GEN2,
+	.features = RAVB_RCAR_GEN2_FEATURES,
 };
 
 static const struct of_device_id ravb_match_table[] = {
@@ -1992,14 +2009,20 @@ 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_drv_data *info = priv->info;
 
-	if (priv->chip_id == RCAR_GEN2) {
+	switch (info->features & RAVB_PTP) {
+	case RAVB_PTP_CONFIG_INACTIVE:
 		ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_CONFIG);
 		/* Set CSEL value */
 		ravb_modify(ndev, CCC, CCC_CSEL, CCC_CSEL_HPB);
-	} else {
+		break;
+	case RAVB_PTP_CONFIG_ACTIVE:
 		ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_CONFIG |
 			    CCC_GAC | CCC_CSEL_HPB);
+		break;
+	default:
+		ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_CONFIG);
 	}
 }
 
@@ -2182,13 +2205,15 @@ static int ravb_probe(struct platform_device *pdev)
 	/* Set AVB config mode */
 	ravb_set_config_mode(ndev);
 
-	/* Set GTI value */
-	error = ravb_set_gti(ndev);
-	if (error)
-		goto out_disable_refclk;
+	if (info->features & RAVB_PTP) {
+		/* Set GTI value */
+		error = ravb_set_gti(ndev);
+		if (error)
+			goto out_disable_refclk;
 
-	/* Request GTI loading */
-	ravb_modify(ndev, GCCR, GCCR_LTI, GCCR_LTI);
+		/* Request GTI loading */
+		ravb_modify(ndev, GCCR, GCCR_LTI, GCCR_LTI);
+	}
 
 	if (priv->chip_id != RCAR_GEN2) {
 		ravb_parse_delay_mode(np, ndev);
@@ -2214,7 +2239,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->features & RAVB_PTP_CONFIG_ACTIVE)
 		ravb_ptp_init(ndev, pdev);
 
 	/* Debug message level */
@@ -2262,7 +2287,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->features & RAVB_PTP_CONFIG_ACTIVE)
 		ravb_ptp_stop(ndev);
 out_disable_refclk:
 	clk_disable_unprepare(priv->refclk);
@@ -2278,9 +2303,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_drv_data *info = priv->info;
 
 	/* Stop PTP Clock driver */
-	if (priv->chip_id != RCAR_GEN2)
+	if (info->features & RAVB_PTP_CONFIG_ACTIVE)
 		ravb_ptp_stop(ndev);
 
 	clk_disable_unprepare(priv->refclk);
@@ -2363,6 +2389,7 @@ static int __maybe_unused ravb_resume(struct device *dev)
 {
 	struct net_device *ndev = dev_get_drvdata(dev);
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
 	int ret = 0;
 
 	/* If WoL is enabled set reset mode to rearm the WoL logic */
@@ -2377,13 +2404,15 @@ static int __maybe_unused ravb_resume(struct device *dev)
 	/* Set AVB config mode */
 	ravb_set_config_mode(ndev);
 
-	/* Set GTI value */
-	ret = ravb_set_gti(ndev);
-	if (ret)
-		return ret;
+	if (info->features & RAVB_PTP) {
+		/* Set GTI value */
+		ret = ravb_set_gti(ndev);
+		if (ret)
+			return ret;
 
-	/* Request GTI loading */
-	ravb_modify(ndev, GCCR, GCCR_LTI, GCCR_LTI);
+		/* Request GTI loading */
+		ravb_modify(ndev, GCCR, GCCR_LTI, GCCR_LTI);
+	}
 
 	if (priv->chip_id != RCAR_GEN2)
 		ravb_set_delay_mode(ndev);
-- 
2.17.1

[PATCH net-next 01/18] dt-bindings: net: renesas,etheravb: Document Gigabit Ethernet IP

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-22 14:14:42

Document Gigabit Ethernet IP found on RZ/G2L SoC.

Gigabit Ethernet Interface includes Ethernet controller (E-MAC),
Internal TCP/IP Offload Engine (TOE) and Dedicated Direct memory
access controller (DMAC) for transferring transmitted Ethernet
frames to and received Ethernet frames from respective storage
areas in the URAM at high speed.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 .../bindings/net/renesas,etheravb.yaml        | 57 +++++++++++++++----
 1 file changed, 45 insertions(+), 12 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/renesas,etheravb.yaml b/Documentation/devicetree/bindings/net/renesas,etheravb.yaml
index 005868f703a6..5e12a759004f 100644
--- a/Documentation/devicetree/bindings/net/renesas,etheravb.yaml
+++ b/Documentation/devicetree/bindings/net/renesas,etheravb.yaml
@@ -43,23 +43,20 @@ properties:
               - renesas,etheravb-r8a779a0     # R-Car V3U
           - const: renesas,etheravb-rcar-gen3 # R-Car Gen3 and RZ/G2
 
+      - items:
+          - enum:
+              - renesas,r9a07g044-gbeth # RZ/G2{L,LC}
+          - const: renesas,rzg2l-gbeth  # RZ/G2L
+
   reg: true
 
   interrupts: true
 
   interrupt-names: true
 
-  clocks:
-    minItems: 1
-    items:
-      - description: AVB functional clock
-      - description: Optional TXC reference clock
+  clocks: true
 
-  clock-names:
-    minItems: 1
-    items:
-      - const: fck
-      - const: refclk
+  clock-names: true
 
   iommus:
     maxItems: 1
@@ -145,14 +142,20 @@ allOf:
       properties:
         compatible:
           contains:
-            const: renesas,etheravb-rcar-gen2
+            enum:
+              - renesas,etheravb-rcar-gen2
+              - renesas,rzg2l-gbeth
     then:
       properties:
         interrupts:
-          maxItems: 1
+          minItems: 1
+          maxItems: 3
         interrupt-names:
+          minItems: 1
           items:
             - const: mux
+            - const: int_fil_n
+            - const: int_arp_ns_n
         rx-internal-delay-ps: false
     else:
       properties:
@@ -208,6 +211,36 @@ allOf:
         tx-internal-delay-ps:
           const: 2000
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,rzg2l-gbeth
+    then:
+      properties:
+        clocks:
+          items:
+            - description: Main clock
+            - description: Register access clock
+            - description: Reference clock for RGMII
+        clock-names:
+          items:
+            - const: axi
+            - const: chi
+            - const: refclk
+    else:
+      properties:
+        clocks:
+          minItems: 1
+          items:
+            - description: AVB functional clock
+            - description: Optional TXC reference clock
+        clock-names:
+          minItems: 1
+          items:
+            - const: fck
+            - const: refclk
+
 additionalProperties: false
 
 examples:
-- 
2.17.1

[PATCH net-next 05/18] ravb: Replace chip type with a structure for driver data

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-22 14:14:42

The DMAC and EMAC blocks of Gigabit Ethernet IP is almost similar to
Ethernet AVB. With few changes in driver we can support both the IP.

This patch is in preparation for supporting the same by replacing chip
type by a structure with values, feature bits and function pointers.

Currently only values is added to structure and later patches will add
features and function pointers.

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      | 14 +++++
 drivers/net/ethernet/renesas/ravb_main.c | 76 +++++++++++++++++-------
 2 files changed, 67 insertions(+), 23 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 80e62ca2e3d3..0ed21262f26b 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -988,6 +988,18 @@ enum ravb_chip_id {
 	RCAR_GEN3,
 };
 
+struct ravb_drv_data {
+	netdev_features_t net_features;
+	netdev_features_t net_hw_features;
+	const char (*gstrings_stats)[ETH_GSTRING_LEN];
+	size_t gstrings_size;
+	size_t stats_len;
+	u32 num_gstat_queue;
+	size_t skb_sz;
+	u8 num_tx_desc;
+	enum ravb_chip_id chip_id;
+};
+
 struct ravb_private {
 	struct net_device *ndev;
 	struct platform_device *pdev;
@@ -1040,6 +1052,8 @@ struct ravb_private {
 	unsigned txcidm:1;		/* TX Clock Internal Delay Mode */
 	unsigned rgmii_override:1;	/* Deprecated rgmii-*id behavior */
 	int num_tx_desc;		/* TX descriptors per packet */
+
+	const struct ravb_drv_data *info;
 };
 
 static inline u32 ravb_read(struct net_device *ndev, enum ravb_reg reg)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 805397088850..84ebd6fef711 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -339,6 +339,7 @@ static void ravb_ring_format(struct net_device *ndev, int q)
 static int ravb_ring_init(struct net_device *ndev, int q)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
 	int num_tx_desc = priv->num_tx_desc;
 	struct sk_buff *skb;
 	int ring_size;
@@ -353,7 +354,7 @@ static int ravb_ring_init(struct net_device *ndev, int q)
 		goto error;
 
 	for (i = 0; i < priv->num_rx_ring[q]; i++) {
-		skb = netdev_alloc_skb(ndev, RX_BUF_SZ + RAVB_ALIGN - 1);
+		skb = netdev_alloc_skb(ndev, info->skb_sz);
 		if (!skb)
 			goto error;
 		ravb_set_buffer_align(skb);
@@ -1133,13 +1134,14 @@ static const char ravb_gstrings_stats[][ETH_GSTRING_LEN] = {
 	"rx_queue_1_over_errors",
 };
 
-#define RAVB_STATS_LEN	ARRAY_SIZE(ravb_gstrings_stats)
-
 static int ravb_get_sset_count(struct net_device *netdev, int sset)
 {
+	struct ravb_private *priv = netdev_priv(netdev);
+	const struct ravb_drv_data *info = priv->info;
+
 	switch (sset) {
 	case ETH_SS_STATS:
-		return RAVB_STATS_LEN;
+		return info->stats_len;
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -1149,11 +1151,12 @@ static void ravb_get_ethtool_stats(struct net_device *ndev,
 				   struct ethtool_stats *estats, u64 *data)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
 	int i = 0;
 	int q;
 
 	/* Device-specific stats */
-	for (q = RAVB_BE; q < NUM_RX_QUEUE; q++) {
+	for (q = RAVB_BE; q < info->num_gstat_queue; q++) {
 		struct net_device_stats *stats = &priv->stats[q];
 
 		data[i++] = priv->cur_rx[q];
@@ -1176,9 +1179,12 @@ static void ravb_get_ethtool_stats(struct net_device *ndev,
 
 static void ravb_get_strings(struct net_device *ndev, u32 stringset, u8 *data)
 {
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
+
 	switch (stringset) {
 	case ETH_SS_STATS:
-		memcpy(data, ravb_gstrings_stats, sizeof(ravb_gstrings_stats));
+		memcpy(data, info->gstrings_stats, info->gstrings_size);
 		break;
 	}
 }
@@ -1924,12 +1930,36 @@ static int ravb_mdio_release(struct ravb_private *priv)
 	return 0;
 }
 
+static const struct ravb_drv_data ravb_gen3_data = {
+	.net_features = NETIF_F_RXCSUM,
+	.net_hw_features = NETIF_F_RXCSUM,
+	.gstrings_stats = ravb_gstrings_stats,
+	.gstrings_size = sizeof(ravb_gstrings_stats),
+	.stats_len = ARRAY_SIZE(ravb_gstrings_stats),
+	.num_gstat_queue = NUM_RX_QUEUE,
+	.skb_sz = RX_BUF_SZ + RAVB_ALIGN - 1,
+	.num_tx_desc = NUM_TX_DESC_GEN3,
+	.chip_id = RCAR_GEN3,
+};
+
+static const struct ravb_drv_data ravb_gen2_data = {
+	.net_features = NETIF_F_RXCSUM,
+	.net_hw_features = NETIF_F_RXCSUM,
+	.gstrings_stats = ravb_gstrings_stats,
+	.gstrings_size = sizeof(ravb_gstrings_stats),
+	.stats_len = ARRAY_SIZE(ravb_gstrings_stats),
+	.num_gstat_queue = NUM_RX_QUEUE,
+	.skb_sz = RX_BUF_SZ + RAVB_ALIGN - 1,
+	.num_tx_desc = NUM_TX_DESC_GEN2,
+	.chip_id = RCAR_GEN2,
+};
+
 static const struct of_device_id ravb_match_table[] = {
-	{ .compatible = "renesas,etheravb-r8a7790", .data = (void *)RCAR_GEN2 },
-	{ .compatible = "renesas,etheravb-r8a7794", .data = (void *)RCAR_GEN2 },
-	{ .compatible = "renesas,etheravb-rcar-gen2", .data = (void *)RCAR_GEN2 },
-	{ .compatible = "renesas,etheravb-r8a7795", .data = (void *)RCAR_GEN3 },
-	{ .compatible = "renesas,etheravb-rcar-gen3", .data = (void *)RCAR_GEN3 },
+	{ .compatible = "renesas,etheravb-r8a7790", .data = &ravb_gen2_data },
+	{ .compatible = "renesas,etheravb-r8a7794", .data = &ravb_gen2_data },
+	{ .compatible = "renesas,etheravb-rcar-gen2", .data = &ravb_gen2_data },
+	{ .compatible = "renesas,etheravb-r8a7795", .data = &ravb_gen3_data },
+	{ .compatible = "renesas,etheravb-rcar-gen3", .data = &ravb_gen3_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ravb_match_table);
@@ -2034,8 +2064,8 @@ static void ravb_set_delay_mode(struct net_device *ndev)
 static int ravb_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
+	const struct ravb_drv_data *info;
 	struct ravb_private *priv;
-	enum ravb_chip_id chip_id;
 	struct net_device *ndev;
 	int error, irq, q;
 	struct resource *res;
@@ -2052,15 +2082,15 @@ static int ravb_probe(struct platform_device *pdev)
 	if (!ndev)
 		return -ENOMEM;
 
-	ndev->features = NETIF_F_RXCSUM;
-	ndev->hw_features = NETIF_F_RXCSUM;
+	info = of_device_get_match_data(&pdev->dev);
+
+	ndev->features = info->net_features;
+	ndev->hw_features = info->net_hw_features;
 
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_get_sync(&pdev->dev);
 
-	chip_id = (enum ravb_chip_id)of_device_get_match_data(&pdev->dev);
-
-	if (chip_id == RCAR_GEN3)
+	if (info->chip_id == RCAR_GEN3)
 		irq = platform_get_irq_byname(pdev, "ch22");
 	else
 		irq = platform_get_irq(pdev, 0);
@@ -2073,6 +2103,7 @@ static int ravb_probe(struct platform_device *pdev)
 	SET_NETDEV_DEV(ndev, &pdev->dev);
 
 	priv = netdev_priv(ndev);
+	priv->info = info;
 	priv->ndev = ndev;
 	priv->pdev = pdev;
 	priv->num_tx_ring[RAVB_BE] = BE_TX_RING_SIZE;
@@ -2099,7 +2130,7 @@ static int ravb_probe(struct platform_device *pdev)
 	priv->avb_link_active_low =
 		of_property_read_bool(np, "renesas,ether-link-active-low");
 
-	if (chip_id == RCAR_GEN3) {
+	if (info->chip_id == RCAR_GEN3) {
 		irq = platform_get_irq_byname(pdev, "ch24");
 		if (irq < 0) {
 			error = irq;
@@ -2124,7 +2155,7 @@ static int ravb_probe(struct platform_device *pdev)
 		}
 	}
 
-	priv->chip_id = chip_id;
+	priv->chip_id = info->chip_id;
 
 	priv->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(priv->clk)) {
@@ -2142,8 +2173,7 @@ static int ravb_probe(struct platform_device *pdev)
 	ndev->max_mtu = 2048 - (ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN);
 	ndev->min_mtu = ETH_MIN_MTU;
 
-	priv->num_tx_desc = chip_id == RCAR_GEN2 ?
-		NUM_TX_DESC_GEN2 : NUM_TX_DESC_GEN3;
+	priv->num_tx_desc = info->num_tx_desc;
 
 	/* Set function */
 	ndev->netdev_ops = &ravb_netdev_ops;
@@ -2184,7 +2214,7 @@ static int ravb_probe(struct platform_device *pdev)
 	INIT_LIST_HEAD(&priv->ts_skb_list);
 
 	/* Initialise PTP Clock driver */
-	if (chip_id != RCAR_GEN2)
+	if (info->chip_id != RCAR_GEN2)
 		ravb_ptp_init(ndev, pdev);
 
 	/* Debug message level */
@@ -2232,7 +2262,7 @@ static int ravb_probe(struct platform_device *pdev)
 			  priv->desc_bat_dma);
 
 	/* Stop PTP Clock driver */
-	if (chip_id != RCAR_GEN2)
+	if (info->chip_id != RCAR_GEN2)
 		ravb_ptp_stop(ndev);
 out_disable_refclk:
 	clk_disable_unprepare(priv->refclk);
-- 
2.17.1

[PATCH net-next 08/18] ravb: Add R-Car common features

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-22 14:15:00

The below features are supported by both R-Car Gen2 and Gen3.

1) magic packet detection
2) multiple TSRQ support
3) extended descriptor in rx
4) No half duplex support
5) override mtu change

Add features bits to support the same.

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_main.c | 110 +++++++++++++++--------
 1 file changed, 71 insertions(+), 39 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index b3c99f974632..4ef2565534d2 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -45,16 +45,30 @@
 #define RAVB_MULTI_IRQS			BIT(2)
 #define RAVB_INTERNAL_DELAY		BIT(3)
 #define RAVB_TX_DROP_COUNTER		BIT(4)
+#define RAVB_MAGIC			BIT(5)
+#define RAVB_MULTI_TSRQ			BIT(6)
+#define RAVB_NO_HALF_DUPLEX		BIT(7)
+#define RAVB_OVERRIDE_MTU_CHANGE	BIT(8)
+#define RAVB_EX_RX_DESC			BIT(9)
 
 #define RAVB_PTP	(RAVB_PTP_CONFIG_ACTIVE | RAVB_PTP_CONFIG_INACTIVE)
+#define RAVB_RCAR_COMMON \
+		(RAVB_MAGIC			| \
+		 RAVB_MULTI_TSRQ		| \
+		 RAVB_NO_HALF_DUPLEX		| \
+		 RAVB_OVERRIDE_MTU_CHANGE	| \
+		 RAVB_EX_RX_DESC)
 
 #define RAVB_RCAR_GEN3_FEATURES \
 		(RAVB_PTP_CONFIG_ACTIVE		| \
 		 RAVB_MULTI_IRQS		| \
 		 RAVB_INTERNAL_DELAY		| \
-		 RAVB_TX_DROP_COUNTER)
+		 RAVB_TX_DROP_COUNTER		| \
+		 RAVB_RCAR_COMMON)
 
-#define RAVB_RCAR_GEN2_FEATURES	RAVB_PTP_CONFIG_INACTIVE
+#define RAVB_RCAR_GEN2_FEATURES \
+		(RAVB_PTP_CONFIG_INACTIVE	| \
+		 RAVB_RCAR_COMMON)
 
 static const char *ravb_rx_irqs[NUM_RX_QUEUE] = {
 	"ch0", /* RAVB_BE */
@@ -680,11 +694,14 @@ static void ravb_rcv_snd_enable(struct net_device *ndev)
 /* function for waiting dma process finished */
 static int ravb_stop_dma(struct net_device *ndev)
 {
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
 	int error;
 
 	/* Wait for stopping the hardware TX process */
-	error = ravb_wait(ndev, TCCR,
-			  TCCR_TSRQ0 | TCCR_TSRQ1 | TCCR_TSRQ2 | TCCR_TSRQ3, 0);
+	if (info->features & RAVB_MULTI_TSRQ)
+		error = ravb_wait(ndev, TCCR,
+				  TCCR_TSRQ0 | TCCR_TSRQ1 | TCCR_TSRQ2 | TCCR_TSRQ3, 0);
 	if (error)
 		return error;
 
@@ -709,12 +726,13 @@ static int ravb_stop_dma(struct net_device *ndev)
 static void ravb_emac_interrupt_unlocked(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
 	u32 ecsr, psr;
 
 	ecsr = ravb_read(ndev, ECSR);
 	ravb_write(ndev, ecsr, ECSR);	/* clear interrupt */
 
-	if (ecsr & ECSR_MPD)
+	if ((info->features & RAVB_MAGIC) && (ecsr & ECSR_MPD))
 		pm_wakeup_event(&priv->pdev->dev, 0);
 	if (ecsr & ECSR_ICD)
 		ndev->stats.tx_carrier_errors++;
@@ -808,11 +826,14 @@ static bool ravb_queue_interrupt(struct net_device *ndev, int q)
 
 static bool ravb_timestamp_interrupt(struct net_device *ndev)
 {
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
 	u32 tis = ravb_read(ndev, TIS);
 
 	if (tis & TIS_TFUF) {
 		ravb_write(ndev, ~(TIS_TFUF | TIS_RESERVED), TIS);
-		ravb_get_tx_tstamp(ndev);
+		if (info->features & RAVB_EX_RX_DESC)
+			ravb_get_tx_tstamp(ndev);
 		return true;
 	}
 	return false;
@@ -1024,6 +1045,7 @@ static int ravb_phy_init(struct net_device *ndev)
 {
 	struct device_node *np = ndev->dev.parent->of_node;
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
 	struct phy_device *phydev;
 	struct device_node *pn;
 	phy_interface_t iface;
@@ -1069,15 +1091,17 @@ static int ravb_phy_init(struct net_device *ndev)
 		netdev_info(ndev, "limited PHY to 100Mbit/s\n");
 	}
 
-	/* 10BASE, Pause and Asym Pause is not supported */
-	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
-	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT);
-	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Pause_BIT);
-	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Asym_Pause_BIT);
+	if (info->features & RAVB_NO_HALF_DUPLEX) {
+		/* 10BASE, Pause and Asym Pause is not supported */
+		phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
+		phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT);
+		phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Pause_BIT);
+		phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Asym_Pause_BIT);
 
-	/* Half Duplex is not supported */
-	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
-	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_100baseT_Half_BIT);
+		/* Half Duplex is not supported */
+		phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
+		phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_100baseT_Half_BIT);
+	}
 
 	phy_attached_info(phydev);
 
@@ -1314,8 +1338,9 @@ static void ravb_get_wol(struct net_device *ndev, struct ethtool_wolinfo *wol)
 static int ravb_set_wol(struct net_device *ndev, struct ethtool_wolinfo *wol)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
 
-	if (wol->wolopts & ~WAKE_MAGIC)
+	if ((wol->wolopts & ~WAKE_MAGIC) || (!(info->features & RAVB_MAGIC)))
 		return -EOPNOTSUPP;
 
 	priv->wol_enabled = !!(wol->wolopts & WAKE_MAGIC);
@@ -1519,6 +1544,7 @@ static void ravb_tx_timeout_work(struct work_struct *work)
 static netdev_tx_t ravb_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
 	int num_tx_desc = priv->num_tx_desc;
 	u16 q = skb_get_queue_mapping(skb);
 	struct ravb_tstamp_skb *ts_skb;
@@ -1595,28 +1621,30 @@ static netdev_tx_t ravb_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 	desc->dptr = cpu_to_le32(dma_addr);
 
 	/* TX timestamp required */
-	if (q == RAVB_NC) {
-		ts_skb = kmalloc(sizeof(*ts_skb), GFP_ATOMIC);
-		if (!ts_skb) {
-			if (num_tx_desc > 1) {
-				desc--;
-				dma_unmap_single(ndev->dev.parent, dma_addr,
-						 len, DMA_TO_DEVICE);
+	if (info->features & RAVB_EX_RX_DESC) {
+		if (q == RAVB_NC) {
+			ts_skb = kmalloc(sizeof(*ts_skb), GFP_ATOMIC);
+			if (!ts_skb) {
+				if (num_tx_desc > 1) {
+					desc--;
+					dma_unmap_single(ndev->dev.parent, dma_addr,
+							 len, DMA_TO_DEVICE);
+				}
+				goto unmap;
 			}
-			goto unmap;
+			ts_skb->skb = skb_get(skb);
+			ts_skb->tag = priv->ts_skb_tag++;
+			priv->ts_skb_tag &= 0x3ff;
+			list_add_tail(&ts_skb->list, &priv->ts_skb_list);
+
+			/* TAG and timestamp required flag */
+			skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
+			desc->tagh_tsr = (ts_skb->tag >> 4) | TX_TSR;
+			desc->ds_tagl |= cpu_to_le16(ts_skb->tag << 12);
 		}
-		ts_skb->skb = skb_get(skb);
-		ts_skb->tag = priv->ts_skb_tag++;
-		priv->ts_skb_tag &= 0x3ff;
-		list_add_tail(&ts_skb->list, &priv->ts_skb_list);
 
-		/* TAG and timestamp required flag */
-		skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
-		desc->tagh_tsr = (ts_skb->tag >> 4) | TX_TSR;
-		desc->ds_tagl |= cpu_to_le16(ts_skb->tag << 12);
+		skb_tx_timestamp(skb);
 	}
-
-	skb_tx_timestamp(skb);
 	/* Descriptor type must be set after all the above writes */
 	dma_wmb();
 	if (num_tx_desc > 1) {
@@ -1727,10 +1755,12 @@ static int ravb_close(struct net_device *ndev)
 			   "device will be stopped after h/w processes are done.\n");
 
 	/* Clear the timestamp list */
-	list_for_each_entry_safe(ts_skb, ts_skb2, &priv->ts_skb_list, list) {
-		list_del(&ts_skb->list);
-		kfree_skb(ts_skb->skb);
-		kfree(ts_skb);
+	if (info->features & RAVB_EX_RX_DESC) {
+		list_for_each_entry_safe(ts_skb, ts_skb2, &priv->ts_skb_list, list) {
+			list_del(&ts_skb->list);
+			kfree_skb(ts_skb->skb);
+			kfree(ts_skb);
+		}
 	}
 
 	/* PHY disconnect */
@@ -2205,8 +2235,10 @@ static int ravb_probe(struct platform_device *pdev)
 	}
 	clk_prepare_enable(priv->refclk);
 
-	ndev->max_mtu = 2048 - (ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN);
-	ndev->min_mtu = ETH_MIN_MTU;
+	if (info->features & RAVB_OVERRIDE_MTU_CHANGE) {
+		ndev->max_mtu = 2048 - (ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN);
+		ndev->min_mtu = ETH_MIN_MTU;
+	}
 
 	priv->num_tx_desc = info->num_tx_desc;
 
-- 
2.17.1

[PATCH net-next 10/18] ravb: Factorise ravb_ring_format function

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-22 14:15:00

The ravb_ring_format function uses extended descriptor in rx for
R-Car where as it use 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 | 34 +++++++++++++++---------
 2 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 3a9cf6e8671a..a3258c5d0c3d 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -990,6 +990,7 @@ enum ravb_chip_id {
 
 struct ravb_ops {
 	void (*ring_free)(struct net_device *ndev, int q);
+	void (*ring_format)(struct net_device *ndev, int q);
 };
 
 struct ravb_drv_data {
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index a3b8b243fd54..c23f0d420c70 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -311,26 +311,15 @@ static void ravb_ring_free(struct net_device *ndev, int q)
 }
 
 /* Format skb and descriptor buffer for Ethernet AVB */
-static void ravb_ring_format(struct net_device *ndev, int q)
+static void ravb_ring_format_rx(struct net_device *ndev, int q)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
-	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;
 	int rx_ring_size = sizeof(*rx_desc) * priv->num_rx_ring[q];
-	int tx_ring_size = sizeof(*tx_desc) * priv->num_tx_ring[q] *
-			   num_tx_desc;
 	dma_addr_t dma_addr;
 	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++) {
 		/* RX descriptor */
 		rx_desc = &priv->rx_ring[q][i];
@@ -349,6 +338,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 */
+}
+
+static void ravb_ring_format(struct net_device *ndev, int q)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
+	int num_tx_desc = priv->num_tx_desc;
+	struct ravb_tx_desc *tx_desc;
+	struct ravb_desc *desc;
+	int tx_ring_size = sizeof(*tx_desc) * priv->num_tx_ring[q] *
+			   num_tx_desc;
+	int i;
+
+	priv->cur_rx[q] = 0;
+	priv->cur_tx[q] = 0;
+	priv->dirty_rx[q] = 0;
+	priv->dirty_tx[q] = 0;
+
+	/* Build RX ring buffer */
+	info->ravb_ops->ring_format(ndev, q);
 
 	memset(priv->tx_ring[q], 0, tx_ring_size);
 	/* Build TX ring buffer */
@@ -1998,6 +2007,7 @@ static int ravb_mdio_release(struct ravb_private *priv)
 
 static const struct ravb_ops ravb_gen3_ops = {
 	.ring_free = ravb_ring_free_rx,
+	.ring_format = ravb_ring_format_rx,
 };
 
 static const struct ravb_drv_data ravb_gen3_data = {
-- 
2.17.1

[PATCH net-next 11/18] ravb: Factorise ravb_ring_init function

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-22 14:15:02

The ravb_ring_init function uses extended descriptor in rx for
R-Car and normal descriptor for RZ/G2L. Factorise rx ring buffer
allocation 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 | 20 +++++++++++++++-----
 2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index a3258c5d0c3d..d82bfa6e57c1 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -991,6 +991,7 @@ enum ravb_chip_id {
 struct ravb_ops {
 	void (*ring_free)(struct net_device *ndev, int q);
 	void (*ring_format)(struct net_device *ndev, int q);
+	bool (*alloc_rx_desc)(struct net_device *ndev, int q);
 };
 
 struct ravb_drv_data {
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index c23f0d420c70..3d0f6598b936 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -384,6 +384,19 @@ static void ravb_ring_format(struct net_device *ndev, int q)
 }
 
 /* Init skb and descriptor buffer for Ethernet AVB */
+static bool ravb_alloc_rx_desc(struct net_device *ndev, int q)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	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];
+}
+
 static int ravb_ring_init(struct net_device *ndev, int q)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
@@ -418,11 +431,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->ravb_ops->alloc_rx_desc(ndev, q))
 		goto error;
 
 	priv->dirty_rx[q] = 0;
@@ -2008,6 +2017,7 @@ static int ravb_mdio_release(struct ravb_private *priv)
 static const struct ravb_ops ravb_gen3_ops = {
 	.ring_free = ravb_ring_free_rx,
 	.ring_format = ravb_ring_format_rx,
+	.alloc_rx_desc = ravb_alloc_rx_desc,
 };
 
 static const struct ravb_drv_data ravb_gen3_data = {
-- 
2.17.1

[PATCH net-next 09/18] ravb: Factorise ravb_ring_free function

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-22 14:15:03

Extended descriptor support in RX is available for R-Car where as it
is a normal descriptor for RZ/G2L. 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      |  5 +++
 drivers/net/ethernet/renesas/ravb_main.c | 49 ++++++++++++++++--------
 2 files changed, 37 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index a474ed68db22..3a9cf6e8671a 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -988,7 +988,12 @@ enum ravb_chip_id {
 	RCAR_GEN3,
 };
 
+struct ravb_ops {
+	void (*ring_free)(struct net_device *ndev, int q);
+};
+
 struct ravb_drv_data {
+	const struct ravb_ops *ravb_ops;
 	netdev_features_t net_features;
 	netdev_features_t net_hw_features;
 	const char (*gstrings_stats)[ETH_GSTRING_LEN];
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 4ef2565534d2..a3b8b243fd54 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -247,30 +247,39 @@ static int ravb_tx_free(struct net_device *ndev, int q, bool free_txed_only)
 }
 
 /* Free skb's and DMA buffers for Ethernet AVB */
-static void ravb_ring_free(struct net_device *ndev, int q)
+static void ravb_ring_free_rx(struct net_device *ndev, int q)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
-	int num_tx_desc = priv->num_tx_desc;
 	int ring_size;
 	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];
+	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;
+		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;
+}
+
+static void ravb_ring_free(struct net_device *ndev, int q)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
+	int num_tx_desc = priv->num_tx_desc;
+	int ring_size;
+	int i;
+
+	if (priv->rx_ring[q])
+		info->ravb_ops->ring_free(ndev, q);
 
 	if (priv->tx_ring[q]) {
 		ravb_tx_free(ndev, q, false);
@@ -1987,7 +1996,12 @@ static int ravb_mdio_release(struct ravb_private *priv)
 	return 0;
 }
 
+static const struct ravb_ops ravb_gen3_ops = {
+	.ring_free = ravb_ring_free_rx,
+};
+
 static const struct ravb_drv_data ravb_gen3_data = {
+	.ravb_ops = &ravb_gen3_ops,
 	.net_features = NETIF_F_RXCSUM,
 	.net_hw_features = NETIF_F_RXCSUM,
 	.gstrings_stats = ravb_gstrings_stats,
@@ -2001,6 +2015,7 @@ static const struct ravb_drv_data ravb_gen3_data = {
 };
 
 static const struct ravb_drv_data ravb_gen2_data = {
+	.ravb_ops = &ravb_gen3_ops,
 	.net_features = NETIF_F_RXCSUM,
 	.net_hw_features = NETIF_F_RXCSUM,
 	.gstrings_stats = ravb_gstrings_stats,
-- 
2.17.1

[PATCH net-next 12/18] ravb: Factorise {emac,dmac} init function

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-22 14:15:04

The R-Car AVB module has Magic packet detection, multiple irq's and
timestamp enable features which is not present on RZ/G2L Gigabit
Ethernet module. Factorise emac and dmac initialization 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      |  2 +
 drivers/net/ethernet/renesas/ravb_main.c | 58 ++++++++++++++++--------
 2 files changed, 40 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index d82bfa6e57c1..4d5910dcda86 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -992,6 +992,8 @@ struct ravb_ops {
 	void (*ring_free)(struct net_device *ndev, int q);
 	void (*ring_format)(struct net_device *ndev, int q);
 	bool (*alloc_rx_desc)(struct net_device *ndev, int q);
+	void (*emac_init)(struct net_device *ndev);
+	void (*dmac_init)(struct net_device *ndev);
 };
 
 struct ravb_drv_data {
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 3d0f6598b936..e200114376e4 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -454,7 +454,7 @@ static int ravb_ring_init(struct net_device *ndev, int q)
 }
 
 /* E-MAC init function */
-static void ravb_emac_init(struct net_device *ndev)
+static void ravb_emac_init_ex(struct net_device *ndev)
 {
 	/* Receive frame limit set register */
 	ravb_write(ndev, ndev->mtu + ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN, RFLR);
@@ -480,30 +480,19 @@ 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_emac_init(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
 	const struct ravb_drv_data *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;
-	}
+	info->ravb_ops->emac_init(ndev);
+}
 
-	/* Descriptor format */
-	ravb_ring_format(ndev, RAVB_BE);
-	ravb_ring_format(ndev, RAVB_NC);
+/* Device init function for Ethernet AVB */
+static void ravb_dmac_init_ex(struct net_device *ndev)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
 
 	/* Set AVB RX */
 	ravb_write(ndev,
@@ -530,6 +519,33 @@ 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);
+}
+
+static int ravb_dmac_init(struct net_device *ndev)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *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->ravb_ops->dmac_init(ndev);
 
 	/* Setting the control will start the AVB-DMAC process. */
 	ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_OPERATION);
@@ -2018,6 +2034,8 @@ static const struct ravb_ops ravb_gen3_ops = {
 	.ring_free = ravb_ring_free_rx,
 	.ring_format = ravb_ring_format_rx,
 	.alloc_rx_desc = ravb_alloc_rx_desc,
+	.emac_init = ravb_emac_init_ex,
+	.dmac_init = ravb_dmac_init_ex,
 };
 
 static const struct ravb_drv_data ravb_gen3_data = {
-- 
2.17.1

[PATCH net-next 15/18] ravb: Factorise ravb_set_features

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-22 14:15:07

RZ/G2L supports HW checksum. 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 | 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 4725be4e85d0..f1de095f21d9 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -996,6 +996,7 @@ struct ravb_ops {
 	void (*dmac_init)(struct net_device *ndev);
 	bool (*receive)(struct net_device *ndev, int *quota, int q);
 	void (*set_rate)(struct net_device *ndev);
+	int (*set_features)(struct net_device *ndev, netdev_features_t features);
 };
 
 struct ravb_drv_data {
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 9d5ccd8f9bb1..5a375ac962a0 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1966,8 +1966,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;
 
@@ -1979,6 +1979,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_drv_data *info = priv->info;
+
+	return info->ravb_ops->set_features(ndev, features);
+}
+
 static const struct net_device_ops ravb_netdev_ops = {
 	.ndo_open		= ravb_open,
 	.ndo_stop		= ravb_close,
@@ -2047,6 +2056,7 @@ static const struct ravb_ops ravb_gen3_ops = {
 	.dmac_init = ravb_dmac_init_ex,
 	.receive = ravb_ex_rx,
 	.set_rate = ravb_set_rate,
+	.set_features = ravb_set_features_rx_csum,
 };
 
 static const struct ravb_drv_data ravb_gen3_data = {
-- 
2.17.1

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

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-22 14:15:08

R-Car uses extended descriptor where as RZ/G2L uses normal
descriptor. Factorise ravb_rx function to support the later.

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, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 4d5910dcda86..8a35b0ca1183 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -994,6 +994,7 @@ struct ravb_ops {
 	bool (*alloc_rx_desc)(struct net_device *ndev, int q);
 	void (*emac_init)(struct net_device *ndev);
 	void (*dmac_init)(struct net_device *ndev);
+	bool (*receive)(struct net_device *ndev, int *quota, int q);
 };
 
 struct ravb_drv_data {
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index e200114376e4..a0f19c6f8833 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -607,7 +607,7 @@ static void ravb_rx_csum(struct sk_buff *skb)
 }
 
 /* Packet receive function for Ethernet AVB */
-static bool ravb_rx(struct net_device *ndev, int *quota, int q)
+static bool ravb_ex_rx(struct net_device *ndev, int *quota, int q)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
 	int entry = priv->cur_rx[q] % priv->num_rx_ring[q];
@@ -722,6 +722,14 @@ static bool ravb_rx(struct net_device *ndev, int *quota, int q)
 	return boguscnt <= 0;
 }
 
+static bool ravb_rx(struct net_device *ndev, int *quota, int q)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
+
+	return info->ravb_ops->receive(ndev, quota, q);
+}
+
 static void ravb_rcv_snd_disable(struct net_device *ndev)
 {
 	/* Disable TX and RX */
@@ -2036,6 +2044,7 @@ static const struct ravb_ops ravb_gen3_ops = {
 	.alloc_rx_desc = ravb_alloc_rx_desc,
 	.emac_init = ravb_emac_init_ex,
 	.dmac_init = ravb_dmac_init_ex,
+	.receive = ravb_ex_rx,
 };
 
 static const struct ravb_drv_data ravb_gen3_data = {
-- 
2.17.1

[PATCH net-next 14/18] ravb: Factorise ravb_adjust_link function

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-22 14:15:15

R-Car supports 100 and 1000 Mbs transfer speed where as 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 | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 8a35b0ca1183..4725be4e85d0 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -995,6 +995,7 @@ struct ravb_ops {
 	void (*emac_init)(struct net_device *ndev);
 	void (*dmac_init)(struct net_device *ndev);
 	bool (*receive)(struct net_device *ndev, int *quota, int q);
+	void (*set_rate)(struct net_device *ndev);
 };
 
 struct ravb_drv_data {
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index a0f19c6f8833..9d5ccd8f9bb1 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1049,6 +1049,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_drv_data *info = priv->info;
 	struct phy_device *phydev = ndev->phydev;
 	bool new_state = false;
 	unsigned long flags;
@@ -1063,7 +1064,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->ravb_ops->set_rate(ndev);
 		}
 		if (!priv->link) {
 			ravb_modify(ndev, ECMR, ECMR_TXF, 0);
@@ -2045,6 +2046,7 @@ static const struct ravb_ops ravb_gen3_ops = {
 	.emac_init = ravb_emac_init_ex,
 	.dmac_init = ravb_dmac_init_ex,
 	.receive = ravb_ex_rx,
+	.set_rate = ravb_set_rate,
 };
 
 static const struct ravb_drv_data ravb_gen3_data = {
-- 
2.17.1

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

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-22 14:15:15

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 f1de095f21d9..af06e849db47 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -1067,6 +1067,7 @@ struct ravb_private {
 	int num_tx_desc;		/* TX descriptors per packet */
 
 	const struct ravb_drv_data *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 5a375ac962a0..5a83dd83c635 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>
 
@@ -2204,6 +2205,7 @@ static int ravb_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
 	const struct ravb_drv_data *info;
+	struct reset_control *rstc;
 	struct ravb_private *priv;
 	struct net_device *ndev;
 	int error, irq, q;
@@ -2216,6 +2218,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)
@@ -2226,6 +2233,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);
 
@@ -2243,6 +2251,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;
@@ -2414,6 +2423,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;
 }
 
@@ -2439,6 +2449,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

[PATCH net-next 17/18] ravb: Add GbEthernet driver support

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-22 14:15:16

Add Gigabit Ethernet driver support found on RZ/G2L.

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

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      |  84 ++++-
 drivers/net/ethernet/renesas/ravb_main.c | 436 ++++++++++++++++++++++-
 2 files changed, 513 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index af06e849db47..6d730f479a37 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -81,6 +81,7 @@ enum ravb_reg {
 	RQC3	= 0x00A0,
 	RQC4	= 0x00A4,
 	RPC	= 0x00B0,
+	RTC	= 0x00B4,	/* RZ/G2L only */
 	UFCW	= 0x00BC,
 	UFCS	= 0x00C0,
 	UFCV0	= 0x00C4,
@@ -156,6 +157,7 @@ enum ravb_reg {
 	TIS	= 0x037C,
 	ISS	= 0x0380,
 	CIE	= 0x0384,	/* R-Car Gen3 only */
+	RIC3	= 0x0388,	/* RZ/G2L only */
 	GCCR	= 0x0390,
 	GMTT	= 0x0394,
 	GPTC	= 0x0398,
@@ -187,19 +189,28 @@ enum ravb_reg {
 	PIR	= 0x0520,
 	PSR	= 0x0528,
 	PIPR	= 0x052c,
+	CXR31	= 0x0530,	/* Documented for RZ/G2L only */
+	CXR35	= 0x0540,	/* Documented for RZ/G2L only */
 	MPR	= 0x0558,
 	PFTCR	= 0x055c,
 	PFRCR	= 0x0560,
 	GECMR	= 0x05b0,
 	MAHR	= 0x05c0,
 	MALR	= 0x05c8,
-	TROCR	= 0x0700,	/* R-Car Gen3 only */
+	TROCR	= 0x0700,	/* R-Car Gen3 and RZ/G2L only */
+	CXR41	= 0x0708,	/* Documented for RZ/G2L only */
+	CXR42	= 0x0710,	/* Documented for RZ/G2L only */
 	CEFCR	= 0x0740,
 	FRECR	= 0x0748,
 	TSFRCR	= 0x0750,
 	TLFRCR	= 0x0758,
 	RFCR	= 0x0760,
+	CXR55	= 0x0768,	/* Documented for RZ/G2L only */
+	CXR56	= 0x0770,	/* Documented for RZ/G2L only */
 	MAFCR	= 0x0778,
+	CSR0     = 0x0800,	/* Documented for RZ/G2L only */
+	CSR1     = 0x0804,	/* Documented for RZ/G2L only */
+	CSR2     = 0x0808,	/* Documented for RZ/G2L only */
 };
 
 
@@ -804,16 +815,21 @@ enum TID_BIT {
 enum ECMR_BIT {
 	ECMR_PRM	= 0x00000001,
 	ECMR_DM		= 0x00000002,
+	ECMR_LPM	= 0x00000010,	/* Documented for RZ/G2L only */
 	ECMR_TE		= 0x00000020,
 	ECMR_RE		= 0x00000040,
 	ECMR_MPDE	= 0x00000200,
+	ECMR_CER	= 0x00001000,	/* Documented for RZ/G2L only */
 	ECMR_TXF	= 0x00010000,	/* Documented for R-Car Gen3 only */
 	ECMR_RXF	= 0x00020000,
 	ECMR_PFR	= 0x00040000,
 	ECMR_ZPF	= 0x00080000,	/* Documented for R-Car Gen3 only */
 	ECMR_RZPF	= 0x00100000,
 	ECMR_DPAD	= 0x00200000,
+	ECMR_CXSER	= 0x00400000,	/* Documented for RZ/G2L only */
 	ECMR_RCSC	= 0x00800000,
+	ECMR_TCPT	= 0x01000000,	/* Documented for RZ/G2L only */
+	ECMR_RCPT	= 0x02000000,	/* Documented for RZ/G2L only */
 	ECMR_TRCCM	= 0x04000000,
 };
 
@@ -823,6 +839,7 @@ enum ECSR_BIT {
 	ECSR_MPD	= 0x00000002,
 	ECSR_LCHNG	= 0x00000004,
 	ECSR_PHYI	= 0x00000008,
+	ECSR_PFRI	= 0x00000010,
 };
 
 /* ECSIPR */
@@ -857,9 +874,13 @@ enum MPR_BIT {
 
 /* GECMR */
 enum GECMR_BIT {
-	GECMR_SPEED	= 0x00000001,
-	GECMR_SPEED_100	= 0x00000000,
-	GECMR_SPEED_1000 = 0x00000001,
+	GECMR_SPEED		= 0x00000001,
+	GECMR_SPEED_100		= 0x00000000,
+	GECMR_SPEED_1000	= 0x00000001,
+	RGETH_GECMR_SPEED	= 0x00000030,
+	RGETH_GECMR_SPEED_10	= 0x00000000,
+	RGETH_GECMR_SPEED_100	= 0x00000010,
+	RGETH_GECMR_SPEED_1000	= 0x00000020,
 };
 
 /* The Ethernet AVB descriptor definitions. */
@@ -949,6 +970,54 @@ enum RAVB_QUEUE {
 	RAVB_NC,	/* Network Control Queue */
 };
 
+enum CXR31_BIT {
+	CXR31_SEL_LINK0	= 0x00000001,
+	CXR31_SEL_LINK1	= 0x00000008,
+};
+
+enum CXR35_BIT {
+	CXR35_SEL_MODIN	= 0x00000100,
+};
+
+enum CSR0_BIT {
+	CSR0_CCM	= 0x00000001,
+	CSR0_TPE	= 0x00000010,
+	CSR0_RPE	= 0x00000020,
+	CSR0_TBP	= 0x00000100,
+	CSR0_RBP	= 0x00000200,
+	CSR0_FIFOCAP	= 0x00003000,
+};
+
+enum CSR1_BIT {
+	CSR1_TIP4	= 0x00000001,
+	CSR1_TTCP4	= 0x00000010,
+	CSR1_TUDP4	= 0x00000020,
+	CSR1_TICMP4	= 0x00000040,
+	CSR1_TTCP6	= 0x00100000,
+	CSR1_TUDP6	= 0x00200000,
+	CSR1_TICMP6	= 0x00400000,
+	CSR1_THOP	= 0x01000000,
+	CSR1_TROUT	= 0x02000000,
+	CSR1_TAHD	= 0x04000000,
+	CSR1_TDHD	= 0x08000000,
+	CSR1_ALL	= 0x0F700071,
+};
+
+enum CSR2_BIT {
+	CSR2_RIP4	= 0x00000001,
+	CSR2_RTCP4	= 0x00000010,
+	CSR2_RUDP4	= 0x00000020,
+	CSR2_RICMP4	= 0x00000040,
+	CSR2_RTCP6	= 0x00100000,
+	CSR2_RUDP6	= 0x00200000,
+	CSR2_RICMP6	= 0x00400000,
+	CSR2_RHOP	= 0x01000000,
+	CSR2_RROUT	= 0x02000000,
+	CSR2_RAHD	= 0x04000000,
+	CSR2_RDHD	= 0x08000000,
+	CSR2_ALL	= 0x0F700071,
+};
+
 #define DBAT_ENTRY_NUM	22
 #define RX_QUEUE_OFFSET	4
 #define NUM_RX_QUEUE	2
@@ -956,6 +1025,9 @@ enum RAVB_QUEUE {
 
 #define RX_BUF_SZ	(2048 - ETH_FCS_LEN + sizeof(__sum16))
 
+#define RGETH_RX_BUFF_MAX 8192
+#define RGETH_RX_DESC_DATA_SIZE 4080
+
 /* TX descriptors per packet */
 #define NUM_TX_DESC_GEN2	2
 #define NUM_TX_DESC_GEN3	1
@@ -1068,6 +1140,10 @@ struct ravb_private {
 
 	const struct ravb_drv_data *info;
 	struct reset_control *rstc;
+
+	int duplex;
+	struct ravb_rx_desc *rgeth_rx_ring[NUM_RX_QUEUE];
+	struct sk_buff *rxtop_skb;
 };
 
 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 5a83dd83c635..0378b2d26b8c 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -51,6 +51,8 @@
 #define RAVB_NO_HALF_DUPLEX		BIT(7)
 #define RAVB_OVERRIDE_MTU_CHANGE	BIT(8)
 #define RAVB_EX_RX_DESC			BIT(9)
+#define RAVB_MII_RGMII_SELECTION	BIT(10)
+#define RAVB_CARRIER_COUNTER		BIT(11)
 
 #define RAVB_PTP	(RAVB_PTP_CONFIG_ACTIVE | RAVB_PTP_CONFIG_INACTIVE)
 #define RAVB_RCAR_COMMON \
@@ -71,6 +73,10 @@
 		(RAVB_PTP_CONFIG_INACTIVE	| \
 		 RAVB_RCAR_COMMON)
 
+#define RAVB_RZ_G2L_FEATURES \
+		(RAVB_MII_RGMII_SELECTION	| \
+		 RAVB_CARRIER_COUNTER)
+
 static const char *ravb_rx_irqs[NUM_RX_QUEUE] = {
 	"ch0", /* RAVB_BE */
 	"ch1", /* RAVB_NC */
@@ -113,6 +119,23 @@ static int ravb_config(struct net_device *ndev)
 	return error;
 }
 
+static void ravb_set_rate_rgeth(struct net_device *ndev)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+
+	switch (priv->speed) {
+	case 10:                /* 10BASE */
+		ravb_write(ndev, RGETH_GECMR_SPEED_10, GECMR);
+		break;
+	case 100:               /* 100BASE */
+		ravb_write(ndev, RGETH_GECMR_SPEED_100, GECMR);
+		break;
+	case 1000:              /* 1000BASE */
+		ravb_write(ndev, RGETH_GECMR_SPEED_1000, GECMR);
+		break;
+	}
+}
+
 static void ravb_set_rate(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
@@ -248,6 +271,28 @@ static int ravb_tx_free(struct net_device *ndev, int q, bool free_txed_only)
 }
 
 /* Free skb's and DMA buffers for Ethernet AVB */
+static void ravb_ring_free_rx_rgeth(struct net_device *ndev, int q)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	int ring_size;
+	int i;
+
+	for (i = 0; i < priv->num_rx_ring[q]; i++) {
+		struct ravb_rx_desc *desc = &priv->rgeth_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),
+					 RGETH_RX_BUFF_MAX,
+					 DMA_FROM_DEVICE);
+	}
+	ring_size = sizeof(struct ravb_rx_desc) * (priv->num_rx_ring[q] + 1);
+	dma_free_coherent(ndev->dev.parent, ring_size, priv->rgeth_rx_ring[q],
+			  priv->rx_desc_dma[q]);
+	priv->rgeth_rx_ring[q] = NULL;
+}
+
 static void ravb_ring_free_rx(struct net_device *ndev, int q)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
@@ -279,7 +324,7 @@ static void ravb_ring_free(struct net_device *ndev, int q)
 	int ring_size;
 	int i;
 
-	if (priv->rx_ring[q])
+	if (priv->rx_ring[q] || priv->rgeth_rx_ring[q])
 		info->ravb_ops->ring_free(ndev, q);
 
 	if (priv->tx_ring[q]) {
@@ -312,6 +357,36 @@ static void ravb_ring_free(struct net_device *ndev, int q)
 }
 
 /* Format skb and descriptor buffer for Ethernet AVB */
+static void ravb_ring_format_rx_rgeth(struct net_device *ndev, int q)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	struct ravb_rx_desc *rx_desc;
+	int rx_ring_size = sizeof(*rx_desc) * priv->num_rx_ring[q];
+	dma_addr_t dma_addr;
+	int i;
+
+	memset(priv->rgeth_rx_ring[q], 0, rx_ring_size);
+	/* Build RX ring buffer */
+	for (i = 0; i < priv->num_rx_ring[q]; i++) {
+		/* RX descriptor */
+		rx_desc = &priv->rgeth_rx_ring[q][i];
+		rx_desc->ds_cc = cpu_to_le16(RGETH_RX_DESC_DATA_SIZE);
+		dma_addr = dma_map_single(ndev->dev.parent, priv->rx_skb[q][i]->data,
+					  RGETH_RX_BUFF_MAX,
+					  DMA_FROM_DEVICE);
+		/* We just set the data size to 0 for a failed mapping which
+		 * should prevent DMA from happening...
+		 */
+		if (dma_mapping_error(ndev->dev.parent, dma_addr))
+			rx_desc->ds_cc = cpu_to_le16(0);
+		rx_desc->dptr = cpu_to_le32(dma_addr);
+		rx_desc->die_dt = DT_FEMPTY;
+	}
+	rx_desc = &priv->rgeth_rx_ring[q][i];
+	rx_desc->dptr = cpu_to_le32((u32)priv->rx_desc_dma[q]);
+	rx_desc->die_dt = DT_LINKFIX; /* type */
+}
+
 static void ravb_ring_format_rx(struct net_device *ndev, int q)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
@@ -385,6 +460,19 @@ static void ravb_ring_format(struct net_device *ndev, int q)
 }
 
 /* Init skb and descriptor buffer for Ethernet AVB */
+static bool ravb_alloc_rx_desc_rgeth(struct net_device *ndev, int q)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	int ring_size;
+
+	ring_size = sizeof(struct ravb_rx_desc) * (priv->num_rx_ring[q] + 1);
+
+	priv->rgeth_rx_ring[q] = dma_alloc_coherent(ndev->dev.parent, ring_size,
+						    &priv->rx_desc_dma[q],
+						    GFP_KERNEL);
+	return priv->rgeth_rx_ring[q];
+}
+
 static bool ravb_alloc_rx_desc(struct net_device *ndev, int q)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
@@ -455,6 +543,37 @@ static int ravb_ring_init(struct net_device *ndev, int q)
 }
 
 /* E-MAC init function */
+static void ravb_emac_init_rgeth(struct net_device *ndev)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+
+	/* Receive frame limit set register */
+	ravb_write(ndev, RGETH_RX_BUFF_MAX + ETH_FCS_LEN, RFLR);
+
+	/* PAUSE prohibition */
+	ravb_write(ndev, ECMR_ZPF | ((priv->duplex > 0) ? ECMR_DM : 0) |
+			 ECMR_TE | ECMR_RE | ECMR_RCPT |
+			 ECMR_TXF | ECMR_RXF | ECMR_PRM, ECMR);
+
+	ravb_set_rate_rgeth(ndev);
+
+	/* Set MAC address */
+	ravb_write(ndev,
+		   (ndev->dev_addr[0] << 24) | (ndev->dev_addr[1] << 16) |
+		   (ndev->dev_addr[2] << 8)  | (ndev->dev_addr[3]), MAHR);
+	ravb_write(ndev, (ndev->dev_addr[4] << 8)  | (ndev->dev_addr[5]), MALR);
+
+	/* E-MAC status register clear */
+	ravb_write(ndev, ECSR_ICD | ECSR_LCHNG | ECSR_PFRI, ECSR);
+	ravb_write(ndev, CSR0_TPE | CSR0_RPE, CSR0);
+
+	/* E-MAC interrupt enable register */
+	ravb_write(ndev, ECSIPR_ICDIP, ECSIPR);
+
+	ravb_write(ndev, ravb_read(ndev, CXR31) & ~CXR31_SEL_LINK1, CXR31);
+	ravb_write(ndev, ravb_read(ndev, CXR31) | CXR31_SEL_LINK0, CXR31);
+}
+
 static void ravb_emac_init_ex(struct net_device *ndev)
 {
 	/* Receive frame limit set register */
@@ -490,6 +609,31 @@ static void ravb_emac_init(struct net_device *ndev)
 }
 
 /* Device init function for Ethernet AVB */
+static void ravb_dmac_init_rgeth(struct net_device *ndev)
+{
+	/* Set AVB RX */
+	ravb_write(ndev, 0x60000000, RCR);
+
+	/* Set Max Frame Length (RTC) */
+	ravb_write(ndev, 0x7ffc0000 | RGETH_RX_BUFF_MAX, RTC);
+
+	/* Set FIFO size */
+	ravb_write(ndev, 0x00222200, TGC);
+
+	ravb_write(ndev, 0, TCCR);
+
+	/* Frame receive */
+	ravb_write(ndev, RIC0_FRE0, RIC0);
+	/* Disable FIFO full warning */
+	ravb_write(ndev, 0x0, RIC1);
+	/* Receive FIFO full error, descriptor empty */
+	ravb_write(ndev, RIC2_QFE0 | RIC2_RFFE, RIC2);
+
+	ravb_write(ndev, 0x0, RIC3);
+
+	ravb_write(ndev, TIC_FTE0, TIC);
+}
+
 static void ravb_dmac_init_ex(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
@@ -592,6 +736,23 @@ static void ravb_get_tx_tstamp(struct net_device *ndev)
 	}
 }
 
+static void ravb_rx_csum_rgeth(struct sk_buff *skb)
+{
+	u8 *hw_csum;
+
+	/* The hardware checksum is contained in sizeof(__sum16) (2) bytes
+	 * appended to packet data
+	 */
+	if (unlikely(skb->len < sizeof(__sum16)))
+		return;
+	hw_csum = skb_tail_pointer(skb) - sizeof(__sum16);
+
+	if (*hw_csum == 0)
+		skb->ip_summed = CHECKSUM_UNNECESSARY;
+	else
+		skb->ip_summed = CHECKSUM_NONE;
+}
+
 static void ravb_rx_csum(struct sk_buff *skb)
 {
 	u8 *hw_csum;
@@ -608,6 +769,148 @@ static void ravb_rx_csum(struct sk_buff *skb)
 }
 
 /* Packet receive function for Ethernet AVB */
+static struct sk_buff *ravb_get_skb_rgeth(struct net_device *ndev,  int q,
+					  int entry, struct ravb_rx_desc *desc)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	struct sk_buff *skb;
+
+	skb = priv->rx_skb[q][entry];
+	priv->rx_skb[q][entry] = NULL;
+	dma_unmap_single(ndev->dev.parent, le32_to_cpu(desc->dptr),
+			 ALIGN(RGETH_RX_BUFF_MAX, 16), DMA_FROM_DEVICE);
+
+	return skb;
+}
+
+static bool ravb_rx_rgeth(struct net_device *ndev, int *quota, int q)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	int entry = priv->cur_rx[q] % priv->num_rx_ring[q];
+	int boguscnt = priv->dirty_rx[q] + priv->num_rx_ring[q] - priv->cur_rx[q];
+	struct net_device_stats *stats = &priv->stats[q];
+	struct ravb_rx_desc *desc;
+	struct sk_buff *skb;
+	dma_addr_t dma_addr;
+	u8  desc_status;
+	u8  die_dt;
+	u16 pkt_len;
+	int limit;
+
+	boguscnt = min(boguscnt, *quota);
+	limit = boguscnt;
+	desc = &priv->rgeth_rx_ring[q][entry];
+	while (desc->die_dt != DT_FEMPTY) {
+		/* Descriptor type must be checked before all other reads */
+		dma_rmb();
+		desc_status = desc->msc;
+		pkt_len = le16_to_cpu(desc->ds_cc) & RX_DS;
+
+		if (--boguscnt < 0)
+			break;
+
+		/* We use 0-byte descriptors to mark the DMA mapping errors */
+		if (!pkt_len)
+			continue;
+
+		if (desc_status & MSC_MC)
+			stats->multicast++;
+
+		if (desc_status & (MSC_CRC | MSC_RFE | MSC_RTSF | MSC_RTLF | MSC_CEEF)) {
+			stats->rx_errors++;
+			if (desc_status & MSC_CRC)
+				stats->rx_crc_errors++;
+			if (desc_status & MSC_RFE)
+				stats->rx_frame_errors++;
+			if (desc_status & (MSC_RTLF | MSC_RTSF))
+				stats->rx_length_errors++;
+			if (desc_status & MSC_CEEF)
+				stats->rx_missed_errors++;
+		} else {
+			die_dt = desc->die_dt & 0xF0;
+			switch (die_dt) {
+			case DT_FSINGLE:
+				skb = ravb_get_skb_rgeth(ndev, q, entry, desc);
+				skb_put(skb, pkt_len);
+				skb->protocol = eth_type_trans(skb, ndev);
+				if (ndev->features & NETIF_F_RXCSUM)
+					ravb_rx_csum_rgeth(skb);
+				napi_gro_receive(&priv->napi[q], skb);
+				stats->rx_packets++;
+				stats->rx_bytes += pkt_len;
+				break;
+			case DT_FSTART:
+				priv->rxtop_skb = ravb_get_skb_rgeth(ndev, q, entry, desc);
+				skb_put(priv->rxtop_skb, pkt_len);
+				break;
+			case DT_FMID:
+				skb = ravb_get_skb_rgeth(ndev, q, entry, desc);
+				skb_copy_to_linear_data_offset(priv->rxtop_skb,
+							       priv->rxtop_skb->len,
+							       skb->data,
+							       pkt_len);
+				skb_put(priv->rxtop_skb, pkt_len);
+				dev_kfree_skb(skb);
+				break;
+			case DT_FEND:
+				skb = ravb_get_skb_rgeth(ndev, q, entry, desc);
+				skb_copy_to_linear_data_offset(priv->rxtop_skb,
+							       priv->rxtop_skb->len,
+							       skb->data,
+							       pkt_len);
+				skb_put(priv->rxtop_skb, pkt_len);
+				dev_kfree_skb(skb);
+				priv->rxtop_skb->protocol =
+					eth_type_trans(priv->rxtop_skb, ndev);
+				if (ndev->features & NETIF_F_RXCSUM)
+					ravb_rx_csum_rgeth(skb);
+				napi_gro_receive(&priv->napi[q],
+						 priv->rxtop_skb);
+				stats->rx_packets++;
+				stats->rx_bytes += priv->rxtop_skb->len;
+				break;
+			}
+		}
+
+		entry = (++priv->cur_rx[q]) % priv->num_rx_ring[q];
+		desc = &priv->rgeth_rx_ring[q][entry];
+	}
+
+	/* Refill the RX ring buffers. */
+	for (; priv->cur_rx[q] - priv->dirty_rx[q] > 0; priv->dirty_rx[q]++) {
+		entry = priv->dirty_rx[q] % priv->num_rx_ring[q];
+		desc = &priv->rgeth_rx_ring[q][entry];
+		desc->ds_cc = cpu_to_le16(RGETH_RX_DESC_DATA_SIZE);
+
+		if (!priv->rx_skb[q][entry]) {
+			skb = netdev_alloc_skb(ndev,
+					       RGETH_RX_BUFF_MAX + RAVB_ALIGN - 1);
+			if (!skb)
+				break;
+			ravb_set_buffer_align(skb);
+			dma_addr = dma_map_single(ndev->dev.parent,
+						  skb->data,
+						  le16_to_cpu(RGETH_RX_BUFF_MAX),
+						  DMA_FROM_DEVICE);
+			skb_checksum_none_assert(skb);
+			/* We just set the data size to 0 for a failed mapping
+			 * which should prevent DMA  from happening...
+			 */
+			if (dma_mapping_error(ndev->dev.parent, dma_addr))
+				desc->ds_cc = cpu_to_le16(0);
+			desc->dptr = cpu_to_le32(dma_addr);
+			priv->rx_skb[q][entry] = skb;
+		}
+		/* Descriptor type must be set after all the above writes */
+		dma_wmb();
+		desc->die_dt = DT_FEMPTY;
+	}
+
+	*quota -= limit - (++boguscnt);
+
+	return boguscnt <= 0;
+}
+
 static bool ravb_ex_rx(struct net_device *ndev, int *quota, int q)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
@@ -754,6 +1057,9 @@ static int ravb_stop_dma(struct net_device *ndev)
 	if (info->features & RAVB_MULTI_TSRQ)
 		error = ravb_wait(ndev, TCCR,
 				  TCCR_TSRQ0 | TCCR_TSRQ1 | TCCR_TSRQ2 | TCCR_TSRQ3, 0);
+	else
+		error = ravb_wait(ndev, TCCR, TCCR_TSRQ0, 0);
+
 	if (error)
 		return error;
 
@@ -1003,16 +1309,24 @@ 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_drv_data *info = priv->info;
+	struct ravb_rx_desc *desc;
 	unsigned long flags;
 	int q = napi - priv->napi;
 	int mask = BIT(q);
 	int quota = budget;
+	int entry;
 
+	if (!(info->features & RAVB_EX_RX_DESC)) {
+		entry = priv->cur_rx[q] % priv->num_rx_ring[q];
+		desc = &priv->rgeth_rx_ring[q][entry];
+	}
 	/* Processing RX Descriptor Ring */
 	/* Clear RX interrupt */
 	ravb_write(ndev, ~(mask | RIS0_RESERVED), RIS0);
-	if (ravb_rx(ndev, &quota, q))
-		goto out;
+	if ((info->features & RAVB_EX_RX_DESC) || desc->die_dt != DT_FEMPTY) {
+		if (ravb_rx(ndev, &quota, q))
+			goto out;
+	}
 
 	/* Processing TX Descriptor Ring */
 	spin_lock_irqsave(&priv->lock, flags);
@@ -1046,6 +1360,18 @@ static int ravb_poll(struct napi_struct *napi, int budget)
 	return budget - quota;
 }
 
+static void ravb_set_duplex_rgeth(struct net_device *ndev)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	u32 ecmr = ravb_read(ndev, ECMR);
+
+	if (priv->duplex > 0)	/* Full */
+		ecmr |=  ECMR_DM;
+	else			/* Half */
+		ecmr &= ~ECMR_DM;
+	ravb_write(ndev, ecmr, ECMR);
+}
+
 /* PHY state control function */
 static void ravb_adjust_link(struct net_device *ndev)
 {
@@ -1062,6 +1388,12 @@ static void ravb_adjust_link(struct net_device *ndev)
 		ravb_rcv_snd_disable(ndev);
 
 	if (phydev->link) {
+		if (!(info->features & RAVB_NO_HALF_DUPLEX) && phydev->duplex != priv->duplex) {
+			new_state = true;
+			priv->duplex = phydev->duplex;
+			ravb_set_duplex_rgeth(ndev);
+		}
+
 		if (phydev->speed != priv->speed) {
 			new_state = true;
 			priv->speed = phydev->speed;
@@ -1076,6 +1408,8 @@ static void ravb_adjust_link(struct net_device *ndev)
 		new_state = true;
 		priv->link = 0;
 		priv->speed = 0;
+		if (!(info->features & RAVB_NO_HALF_DUPLEX))
+			priv->duplex = -1;
 	}
 
 	/* Enable TX and RX right over here, if E-MAC change is ignored */
@@ -1106,6 +1440,7 @@ static int ravb_phy_init(struct net_device *ndev)
 
 	priv->link = 0;
 	priv->speed = 0;
+	priv->duplex = -1;
 
 	/* Try connecting to PHY */
 	pn = of_parse_phandle(np, "phy-handle", 0);
@@ -1144,6 +1479,9 @@ static int ravb_phy_init(struct net_device *ndev)
 		netdev_info(ndev, "limited PHY to 100Mbit/s\n");
 	}
 
+	if (info->features & RAVB_MII_RGMII_SELECTION)
+		ravb_write(ndev, ravb_read(ndev, CXR35) | CXR35_SEL_MODIN, CXR35);
+
 	if (info->features & RAVB_NO_HALF_DUPLEX) {
 		/* 10BASE, Pause and Asym Pause is not supported */
 		phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
@@ -1197,6 +1535,24 @@ static void ravb_set_msglevel(struct net_device *ndev, u32 value)
 	priv->msg_enable = value;
 }
 
+static const char ravb_gstrings_stats_rgeth[][ETH_GSTRING_LEN] = {
+	"rx_queue_0_current",
+	"tx_queue_0_current",
+	"rx_queue_0_dirty",
+	"tx_queue_0_dirty",
+	"rx_queue_0_packets",
+	"tx_queue_0_packets",
+	"rx_queue_0_bytes",
+	"tx_queue_0_bytes",
+	"rx_queue_0_mcast_packets",
+	"rx_queue_0_errors",
+	"rx_queue_0_crc_errors",
+	"rx_queue_0_frame_errors",
+	"rx_queue_0_length_errors",
+	"rx_queue_0_csum_offload_errors",
+	"rx_queue_0_over_errors",
+};
+
 static const char ravb_gstrings_stats[][ETH_GSTRING_LEN] = {
 	"rx_queue_0_current",
 	"tx_queue_0_current",
@@ -1752,6 +2108,18 @@ static struct net_device_stats *ravb_get_stats(struct net_device *ndev)
 		ravb_write(ndev, 0, TROCR);	/* (write clear) */
 	}
 
+	if (info->features & RAVB_CARRIER_COUNTER) {
+		nstats->collisions += ravb_read(ndev, CXR41);
+		ravb_write(ndev, 0, CXR41);	/* (write clear) */
+		nstats->tx_carrier_errors += ravb_read(ndev, CXR42);
+		ravb_write(ndev, 0, CXR42);	/* (write clear) */
+
+		nstats->tx_carrier_errors += ravb_read(ndev, CXR55);
+		ravb_write(ndev, 0, CXR55);	/* (write clear) */
+		nstats->tx_carrier_errors += ravb_read(ndev, CXR56);
+		ravb_write(ndev, 0, CXR56);	/* (write clear) */
+	}
+
 	nstats->rx_packets = stats0->rx_packets + stats1->rx_packets;
 	nstats->tx_packets = stats0->tx_packets + stats1->tx_packets;
 	nstats->rx_bytes = stats0->rx_bytes + stats1->rx_bytes;
@@ -1967,6 +2335,44 @@ static void ravb_set_rx_csum(struct net_device *ndev, bool enable)
 	spin_unlock_irqrestore(&priv->lock, flags);
 }
 
+static int ravb_set_features_rx_csum_rgeth(struct net_device *ndev,
+					   netdev_features_t features)
+{
+	netdev_features_t changed = features ^ ndev->features;
+	unsigned int reg;
+	int error;
+
+	reg = ravb_read(ndev, CSR0);
+
+	ravb_write(ndev, reg & ~(CSR0_RPE | CSR0_TPE), CSR0);
+	error = ravb_wait(ndev, CSR0, CSR0_RPE | CSR0_TPE, 0);
+	if (error) {
+		ravb_write(ndev, reg, CSR0);
+		return error;
+	}
+
+	if (changed & NETIF_F_RXCSUM) {
+		if (features & NETIF_F_RXCSUM)
+			ravb_write(ndev, CSR2_ALL, CSR2);
+		else
+			ravb_write(ndev, 0, CSR2);
+	}
+
+	if (changed & NETIF_F_HW_CSUM) {
+		if (features & NETIF_F_HW_CSUM) {
+			ravb_write(ndev, CSR1_ALL, CSR1);
+			ndev->features |= NETIF_F_CSUM_MASK;
+		} else {
+			ravb_write(ndev, 0, CSR1);
+		}
+	}
+	ravb_write(ndev, reg, CSR0);
+
+	ndev->features = features;
+
+	return 0;
+}
+
 static int ravb_set_features_rx_csum(struct net_device *ndev,
 				     netdev_features_t features)
 {
@@ -2060,6 +2466,17 @@ static const struct ravb_ops ravb_gen3_ops = {
 	.set_features = ravb_set_features_rx_csum,
 };
 
+static const struct ravb_ops ravb_ops_rgeth = {
+	.ring_free = ravb_ring_free_rx_rgeth,
+	.ring_format = ravb_ring_format_rx_rgeth,
+	.alloc_rx_desc = ravb_alloc_rx_desc_rgeth,
+	.emac_init = ravb_emac_init_rgeth,
+	.dmac_init = ravb_dmac_init_rgeth,
+	.receive = ravb_rx_rgeth,
+	.set_rate = ravb_set_rate_rgeth,
+	.set_features = ravb_set_features_rx_csum_rgeth,
+};
+
 static const struct ravb_drv_data ravb_gen3_data = {
 	.ravb_ops = &ravb_gen3_ops,
 	.net_features = NETIF_F_RXCSUM,
@@ -2088,12 +2505,25 @@ static const struct ravb_drv_data ravb_gen2_data = {
 	.features = RAVB_RCAR_GEN2_FEATURES,
 };
 
+static const struct ravb_drv_data rgeth_data = {
+	.ravb_ops = &ravb_ops_rgeth,
+	.net_hw_features = (NETIF_F_HW_CSUM | NETIF_F_RXCSUM),
+	.gstrings_stats = ravb_gstrings_stats_rgeth,
+	.gstrings_size = sizeof(ravb_gstrings_stats_rgeth),
+	.stats_len = ARRAY_SIZE(ravb_gstrings_stats_rgeth),
+	.num_gstat_queue = 1,
+	.skb_sz = RGETH_RX_BUFF_MAX + RAVB_ALIGN - 1,
+	.num_tx_desc = 2,
+	.features = RAVB_RZ_G2L_FEATURES,
+};
+
 static const struct of_device_id ravb_match_table[] = {
 	{ .compatible = "renesas,etheravb-r8a7790", .data = &ravb_gen2_data },
 	{ .compatible = "renesas,etheravb-r8a7794", .data = &ravb_gen2_data },
 	{ .compatible = "renesas,etheravb-rcar-gen2", .data = &ravb_gen2_data },
 	{ .compatible = "renesas,etheravb-r8a7795", .data = &ravb_gen3_data },
 	{ .compatible = "renesas,etheravb-rcar-gen3", .data = &ravb_gen3_data },
+	{ .compatible = "renesas,rzg2l-gbeth", .data = &rgeth_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ravb_match_table);
-- 
2.17.1

Re: [PATCH net-next 05/18] ravb: Replace chip type with a structure for driver data

From: Sergei Shtylyov <hidden>
Date: 2021-07-22 20:42:26

Hello!

On 7/22/21 5:13 PM, Biju Das wrote:
quoted hunk
The DMAC and EMAC blocks of Gigabit Ethernet IP is almost similar to
Ethernet AVB. With few changes in driver we can support both the IP.

This patch is in preparation for supporting the same by replacing chip
type by a structure with values, feature bits and function pointers.

Currently only values is added to structure and later patches will add
features and function pointers.

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      | 14 +++++
 drivers/net/ethernet/renesas/ravb_main.c | 76 +++++++++++++++++-------
 2 files changed, 67 insertions(+), 23 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 80e62ca2e3d3..0ed21262f26b 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -988,6 +988,18 @@ enum ravb_chip_id {
 	RCAR_GEN3,
 };
 
+struct ravb_drv_data {
   I'd rather suggest *struct* ravb_hw_info... This is hardly a driver data, more like
hwrdware's one. :-)
+	netdev_features_t net_features;
+	netdev_features_t net_hw_features;
+	const char (*gstrings_stats)[ETH_GSTRING_LEN];
+	size_t gstrings_size;
+	size_t stats_len;
+	u32 num_gstat_queue;
+	size_t skb_sz;
+	u8 num_tx_desc;
+	enum ravb_chip_id chip_id;
   Mhm, I'd expect that chip_id is no longer needed with the feature structs... 

[...]
quoted hunk
@@ -1040,6 +1052,8 @@ struct ravb_private {
 	unsigned txcidm:1;		/* TX Clock Internal Delay Mode */
 	unsigned rgmii_override:1;	/* Deprecated rgmii-*id behavior */
 	int num_tx_desc;		/* TX descriptors per packet */
+
+	const struct ravb_drv_data *info;
   So data or info? :-)

[...]
quoted hunk
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 805397088850..84ebd6fef711 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
[...]
quoted hunk
@@ -1176,9 +1179,12 @@ static void ravb_get_ethtool_stats(struct net_device *ndev,
 
 static void ravb_get_strings(struct net_device *ndev, u32 stringset, u8 *data)
 {
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
+
 	switch (stringset) {
 	case ETH_SS_STATS:
-		memcpy(data, ravb_gstrings_stats, sizeof(ravb_gstrings_stats));
+		memcpy(data, info->gstrings_stats, info->gstrings_size);
 		break;
 	}
 }
@@ -1924,12 +1930,36 @@ static int ravb_mdio_release(struct ravb_private *priv)
 	return 0;
 }
 
+static const struct ravb_drv_data ravb_gen3_data = {
+	.net_features = NETIF_F_RXCSUM,
+	.net_hw_features = NETIF_F_RXCSUM,
+	.gstrings_stats = ravb_gstrings_stats,
+	.gstrings_size = sizeof(ravb_gstrings_stats),
+	.stats_len = ARRAY_SIZE(ravb_gstrings_stats),
+	.num_gstat_queue = NUM_RX_QUEUE,
+	.skb_sz = RX_BUF_SZ + RAVB_ALIGN - 1,
+	.num_tx_desc = NUM_TX_DESC_GEN3,
+	.chip_id = RCAR_GEN3,
+};
+
+static const struct ravb_drv_data ravb_gen2_data = {
+	.net_features = NETIF_F_RXCSUM,
+	.net_hw_features = NETIF_F_RXCSUM,
   Mhm, why have the fields that don't change from SoC to SoC anyway?
I do think they should be added when a new SoC support is added...
+	.gstrings_stats = ravb_gstrings_stats,
+	.gstrings_size = sizeof(ravb_gstrings_stats),
+	.stats_len = ARRAY_SIZE(ravb_gstrings_stats),
   Same question...
+	.num_gstat_queue = NUM_RX_QUEUE,
+	.skb_sz = RX_BUF_SZ + RAVB_ALIGN - 1,
   Again why?
+	.num_tx_desc = NUM_TX_DESC_GEN2,
+	.chip_id = RCAR_GEN2,
+};
+
[...]
quoted hunk
@@ -2052,15 +2082,15 @@ static int ravb_probe(struct platform_device *pdev)
 	if (!ndev)
 		return -ENOMEM;
 
-	ndev->features = NETIF_F_RXCSUM;
-	ndev->hw_features = NETIF_F_RXCSUM;
+	info = of_device_get_match_data(&pdev->dev);
+
+	ndev->features = info->net_features;
+	ndev->hw_features = info->net_hw_features;
 
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_get_sync(&pdev->dev);
 
-	chip_id = (enum ravb_chip_id)of_device_get_match_data(&pdev->dev);
-
-	if (chip_id == RCAR_GEN3)
+	if (info->chip_id == RCAR_GEN3)
   Ugh...
 		irq = platform_get_irq_byname(pdev, "ch22");
 	else
 		irq = platform_get_irq(pdev, 0);
[...]
quoted hunk
@@ -2099,7 +2130,7 @@ static int ravb_probe(struct platform_device *pdev)
 	priv->avb_link_active_low =
 		of_property_read_bool(np, "renesas,ether-link-active-low");
 
-	if (chip_id == RCAR_GEN3) {
+	if (info->chip_id == RCAR_GEN3) {
   Ugh...
 		irq = platform_get_irq_byname(pdev, "ch24");
 		if (irq < 0) {
 			error = irq;
[...]
quoted hunk
@@ -2184,7 +2214,7 @@ static int ravb_probe(struct platform_device *pdev)
 	INIT_LIST_HEAD(&priv->ts_skb_list);
 
 	/* Initialise PTP Clock driver */
-	if (chip_id != RCAR_GEN2)
+	if (info->chip_id != RCAR_GEN2)
 		ravb_ptp_init(ndev, pdev);
   Ugh...
quoted hunk
 
 	/* Debug message level */
@@ -2232,7 +2262,7 @@ static int ravb_probe(struct platform_device *pdev)
 			  priv->desc_bat_dma);
 
 	/* Stop PTP Clock driver */
-	if (chip_id != RCAR_GEN2)
+	if (info->chip_id != RCAR_GEN2)
 		ravb_ptp_stop(ndev);
  Ugh...

 out_disable_refclk:
 	clk_disable_unprepare(priv->refclk);
MBR, Sergei

Re: [PATCH net-next 00/18] Add Gigabit Ethernet driver support

From: Sergei Shtylyov <hidden>
Date: 2021-07-22 20:55:32

On 7/22/21 5:13 PM, Biju Das wrote:
The DMAC and EMAC blocks of Gigabit Ethernet IP is almost similar to Ethernet AVB.

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

With few changes in driver, we can support Gigabit ethernet driver as well.

This patch series is aims to support the same

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

Biju Das (18):
  dt-bindings: net: renesas,etheravb: Document Gigabit Ethernet IP
  drivers: clk: renesas: rzg2l-cpg: Add support to handle MUX clocks
  drivers: clk: renesas: r9a07g044-cpg: Add ethernet clock sources
  drivers: clk: renesas: r9a07g044-cpg: Add GbEthernet clock/reset

   It's not a good idea to have the patch to the defferent subsystems lumped
all together in a single series...
  ravb: Replace chip type with a structure for driver data
   I was expecting some real changes on how the gen2/3 diff. features in this patch,
but I only saw new info having no real changes where they were needed and having the
changes that did not need to be converted yet...
   Anwyay, I have stopped here for today.
  ravb: Factorise ptp feature
  ravb: Add features specific to R-Car Gen3
  ravb: Add R-Car common features
  ravb: Factorise ravb_ring_free function
  ravb: Factorise ravb_ring_format function
  ravb: Factorise ravb_ring_init function
  ravb: Factorise {emac,dmac} init function
  ravb: Factorise ravb_rx function
  ravb: Factorise ravb_adjust_link function
  ravb: Factorise ravb_set_features
  ravb: Add reset support
  ravb: Add GbEthernet driver support
  arm64: dts: renesas: r9a07g044: Add GbEther nodes
[...]

MBR, Sergei

Re: [PATCH net-next 00/18] Add Gigabit Ethernet driver support

From: Andrew Lunn <andrew@lunn.ch>
Date: 2021-07-22 21:07:37

On Thu, Jul 22, 2021 at 11:53:59PM +0300, Sergei Shtylyov wrote:
On 7/22/21 5:13 PM, Biju Das wrote:
quoted
The DMAC and EMAC blocks of Gigabit Ethernet IP is almost similar to Ethernet AVB.

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

With few changes in driver, we can support Gigabit ethernet driver as well.

This patch series is aims to support the same

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

Biju Das (18):
  dt-bindings: net: renesas,etheravb: Document Gigabit Ethernet IP
  drivers: clk: renesas: rzg2l-cpg: Add support to handle MUX clocks
  drivers: clk: renesas: r9a07g044-cpg: Add ethernet clock sources
  drivers: clk: renesas: r9a07g044-cpg: Add GbEthernet clock/reset

   It's not a good idea to have the patch to the defferent subsystems lumped
all together in a single series...
Agreed.

Are these changes inseparable? If so, you need to be up front on this,
and you need an agreement with the subsystem maintainers how the
patches are going to be merged? Through which tree. And you need
Acked-by from the other tree maintainers.

Ideally you submit multiple patchsets. This assumes all sets will
compile independently.

	Andrew

RE: [PATCH net-next 05/18] ravb: Replace chip type with a structure for driver data

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-23 06:08:23

Hi Sergei,

Thanks for the feedback.
Biju Das [off-list ref]; Prabhakar Mahadev Lad
[off-list ref]
Subject: Re: [PATCH net-next 05/18] ravb: Replace chip type with a
structure for driver data

Hello!

On 7/22/21 5:13 PM, Biju Das wrote:
quoted
The DMAC and EMAC blocks of Gigabit Ethernet IP is almost similar to
Ethernet AVB. With few changes in driver we can support both the IP.

This patch is in preparation for supporting the same by replacing chip
type by a structure with values, feature bits and function pointers.

Currently only values is added to structure and later patches will add
features and function pointers.

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      | 14 +++++
 drivers/net/ethernet/renesas/ravb_main.c | 76
+++++++++++++++++-------
 2 files changed, 67 insertions(+), 23 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h
b/drivers/net/ethernet/renesas/ravb.h
index 80e62ca2e3d3..0ed21262f26b 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -988,6 +988,18 @@ enum ravb_chip_id {
 	RCAR_GEN3,
 };

+struct ravb_drv_data {
   I'd rather suggest *struct* ravb_hw_info... This is hardly a driver
data, more like hwrdware's one. :-)
OK.
quoted
+	netdev_features_t net_features;
+	netdev_features_t net_hw_features;
+	const char (*gstrings_stats)[ETH_GSTRING_LEN];
+	size_t gstrings_size;
+	size_t stats_len;
+	u32 num_gstat_queue;
+	size_t skb_sz;
+	u8 num_tx_desc;
+	enum ravb_chip_id chip_id;
   Mhm, I'd expect that chip_id is no longer needed with the feature
structs...
Yes,If you see the subsequent 3 patches, chip_id usage is completely removed from
ravb_main.c. but it is still required for ravb_ptp.c. Please let me know do you want
me take out from there as well. Then as part of [1], I can take out chipid completely.

[1]
https://patchwork.kernel.org/project/linux-renesas-soc/patch/20210722141351.13668-7-biju.das.jz@bp.renesas.com/

[...]
quoted
@@ -1040,6 +1052,8 @@ struct ravb_private {
 	unsigned txcidm:1;		/* TX Clock Internal Delay Mode */
 	unsigned rgmii_override:1;	/* Deprecated rgmii-*id behavior */
 	int num_tx_desc;		/* TX descriptors per packet */
+
+	const struct ravb_drv_data *info;
   So data or info? :-)
As we are going to use ravb_hw_info, it will be info.
[...]
quoted
diff --git a/drivers/net/ethernet/renesas/ravb_main.c
b/drivers/net/ethernet/renesas/ravb_main.c
index 805397088850..84ebd6fef711 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
[...]
quoted
@@ -1176,9 +1179,12 @@ static void ravb_get_ethtool_stats(struct
net_device *ndev,

 static void ravb_get_strings(struct net_device *ndev, u32 stringset,
u8 *data)  {
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
+
 	switch (stringset) {
 	case ETH_SS_STATS:
-		memcpy(data, ravb_gstrings_stats,
sizeof(ravb_gstrings_stats));
quoted
+		memcpy(data, info->gstrings_stats, info->gstrings_size);
 		break;
 	}
 }
@@ -1924,12 +1930,36 @@ static int ravb_mdio_release(struct ravb_private
*priv)
quoted
 	return 0;
 }

+static const struct ravb_drv_data ravb_gen3_data = {
+	.net_features = NETIF_F_RXCSUM,
+	.net_hw_features = NETIF_F_RXCSUM,
+	.gstrings_stats = ravb_gstrings_stats,
+	.gstrings_size = sizeof(ravb_gstrings_stats),
+	.stats_len = ARRAY_SIZE(ravb_gstrings_stats),
+	.num_gstat_queue = NUM_RX_QUEUE,
+	.skb_sz = RX_BUF_SZ + RAVB_ALIGN - 1,
+	.num_tx_desc = NUM_TX_DESC_GEN3,
+	.chip_id = RCAR_GEN3,
+};
+
+static const struct ravb_drv_data ravb_gen2_data = {
+	.net_features = NETIF_F_RXCSUM,
+	.net_hw_features = NETIF_F_RXCSUM,
   Mhm, why have the fields that don't change from SoC to SoC anyway?
I do think they should be added when a new SoC support is added...
This is a preparation patch for supporting RZ/G2L and there is a difference w.r.to RZ/G2L
See [2]. That is the reason to isolate the new SoC changes w.r.to existing one it is added
Here. Andrew also suggested to make smaller changes.

[2]
https://patchwork.kernel.org/project/linux-renesas-soc/patch/20210722141351.13668-18-biju.das.jz@bp.renesas.com/
quoted
+	.gstrings_stats = ravb_gstrings_stats,
+	.gstrings_size = sizeof(ravb_gstrings_stats),
+	.stats_len = ARRAY_SIZE(ravb_gstrings_stats),
   Same question...
Same as above.
quoted
+	.num_gstat_queue = NUM_RX_QUEUE,
+	.skb_sz = RX_BUF_SZ + RAVB_ALIGN - 1,
   Again why?
Same as above.
quoted
+	.num_tx_desc = NUM_TX_DESC_GEN2,
+	.chip_id = RCAR_GEN2,
+};
+
[...]
quoted
@@ -2052,15 +2082,15 @@ static int ravb_probe(struct platform_device
*pdev)
quoted
 	if (!ndev)
 		return -ENOMEM;

-	ndev->features = NETIF_F_RXCSUM;
-	ndev->hw_features = NETIF_F_RXCSUM;
+	info = of_device_get_match_data(&pdev->dev);
+
+	ndev->features = info->net_features;
+	ndev->hw_features = info->net_hw_features;

 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_get_sync(&pdev->dev);

-	chip_id = (enum ravb_chip_id)of_device_get_match_data(&pdev->dev);
-
-	if (chip_id == RCAR_GEN3)
+	if (info->chip_id == RCAR_GEN3)
   Ugh...
This chip id is replaced in subsequent patch [3]

[3]
https://patchwork.kernel.org/project/linux-renesas-soc/patch/20210722141351.13668-8-biju.das.jz@bp.renesas.com/
quoted
 		irq = platform_get_irq_byname(pdev, "ch22");
 	else
 		irq = platform_get_irq(pdev, 0);
[...]
quoted
@@ -2099,7 +2130,7 @@ static int ravb_probe(struct platform_device
*pdev)
quoted
 	priv->avb_link_active_low =
 		of_property_read_bool(np, "renesas,ether-link-active-low");

-	if (chip_id == RCAR_GEN3) {
+	if (info->chip_id == RCAR_GEN3) {
   Ugh...
Same as above.
quoted
 		irq = platform_get_irq_byname(pdev, "ch24");
 		if (irq < 0) {
 			error = irq;
[...]
quoted
@@ -2184,7 +2214,7 @@ static int ravb_probe(struct platform_device
*pdev)
quoted
 	INIT_LIST_HEAD(&priv->ts_skb_list);

 	/* Initialise PTP Clock driver */
-	if (chip_id != RCAR_GEN2)
+	if (info->chip_id != RCAR_GEN2)
 		ravb_ptp_init(ndev, pdev);
   Ugh...
Same as above.
quoted
 	/* Debug message level */
@@ -2232,7 +2262,7 @@ static int ravb_probe(struct platform_device
*pdev)
quoted
 			  priv->desc_bat_dma);

 	/* Stop PTP Clock driver */
-	if (chip_id != RCAR_GEN2)
+	if (info->chip_id != RCAR_GEN2)
 		ravb_ptp_stop(ndev);
  Ugh...
Same as above.

Regards,
Biju
quoted
 out_disable_refclk:
 	clk_disable_unprepare(priv->refclk);
MBR, Sergei

RE: [PATCH net-next 00/18] Add Gigabit Ethernet driver support

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-23 06:23:44

Hi Sergei,

Thanks for the feedback.
Subject: Re: [PATCH net-next 00/18] Add Gigabit Ethernet driver support

On 7/22/21 5:13 PM, Biju Das wrote:
quoted
The DMAC and EMAC blocks of Gigabit Ethernet IP is almost similar to
Ethernet AVB.
quoted
The Gigabit Etherner IP consists of Ethernet controller (E-MAC),
Internal TCP/IP Offload Engine (TOE) and Dedicated Direct memory access
controller (DMAC).
quoted
With few changes in driver, we can support Gigabit ethernet driver as
well.
quoted
This patch series is aims to support the same

RFC->V1
  * Incorporated feedback from Andrew, Sergei, Geert and Prabhakar
  *
https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
hwork.kernel.org%2Fproject%2Flinux-renesas-soc%2Flist%2F%3Fseries%3D51
5525&amp;data=04%7C01%7Cbiju.das.jz%40bp.renesas.com%7Cb01d51eb4442476
149d608d94d52d7c9%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C6376258
40484693261%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMz
IiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=M6biLbregS1y2R%2BMNb
b5PRRvgQxympZfHZkbuH0ZrXI%3D&amp;reserved=0

Biju Das (18):
  dt-bindings: net: renesas,etheravb: Document Gigabit Ethernet IP
  drivers: clk: renesas: rzg2l-cpg: Add support to handle MUX clocks
  drivers: clk: renesas: r9a07g044-cpg: Add ethernet clock sources
  drivers: clk: renesas: r9a07g044-cpg: Add GbEthernet clock/reset

   It's not a good idea to have the patch to the defferent subsystems
lumped all together in a single series...
quoted
  ravb: Replace chip type with a structure for driver data
   I was expecting some real changes on how the gen2/3 diff. features in
this patch, but I only saw new info having no real changes where they were
needed and having the changes that did not need to be converted yet...
   Anwyay, I have stopped here for today.
This patch is a preparation patch. On the subsequent patches[1] [2] and [3] you will see the real hw changes wr.to gen2/gen3. Feature bit is not added in this patch, but on the subsequent patches. I believe you haven't reviewed that patches yet.

PTP feature and the diff between gen2/gen3
------------------------------------------
[1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20210722141351.13668-7-biju.das.jz@bp.renesas.com/


Features specific to R-Car Gen3
--------------------------------
[2] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20210722141351.13668-8-biju.das.jz@bp.renesas.com/


R-Car common features
---------------------

[3] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20210722141351.13668-9-biju.das.jz@bp.renesas.com/

There is a review comment for making smaller changes. 
what do you recommend after going through [1],[2] and [3]. 
Please let us know.

Regards,
Biju

quoted
  ravb: Factorise ptp feature
  ravb: Add features specific to R-Car Gen3
  ravb: Add R-Car common features
  ravb: Factorise ravb_ring_free function
  ravb: Factorise ravb_ring_format function
  ravb: Factorise ravb_ring_init function
  ravb: Factorise {emac,dmac} init function
  ravb: Factorise ravb_rx function
  ravb: Factorise ravb_adjust_link function
  ravb: Factorise ravb_set_features
  ravb: Add reset support
  ravb: Add GbEthernet driver support
  arm64: dts: renesas: r9a07g044: Add GbEther nodes
[...]

MBR, Sergei

RE: [PATCH net-next 00/18] Add Gigabit Ethernet driver support

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-23 06:28:53

Hi Andrew and Sergei,

Subject: Re: [PATCH net-next 00/18] Add Gigabit Ethernet driver support

On Thu, Jul 22, 2021 at 11:53:59PM +0300, Sergei Shtylyov wrote:
quoted
On 7/22/21 5:13 PM, Biju Das wrote:
quoted
The DMAC and EMAC blocks of Gigabit Ethernet IP is almost similar to
Ethernet AVB.
quoted
quoted
The Gigabit Etherner IP consists of Ethernet controller (E-MAC),
Internal TCP/IP Offload Engine (TOE) and Dedicated Direct memory access
controller (DMAC).
quoted
quoted
With few changes in driver, we can support Gigabit ethernet driver as
well.
quoted
quoted
This patch series is aims to support the same

RFC->V1
  * Incorporated feedback from Andrew, Sergei, Geert and Prabhakar
  *
https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa
tchwork.kernel.org%2Fproject%2Flinux-renesas-soc%2Flist%2F%3Fseries%
3D515525&amp;data=04%7C01%7Cbiju.das.jz%40bp.renesas.com%7C6fe3922cc
35d4178cb1d08d94d54bc75%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7
C637625848601442706%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQ
IjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=bOpIqV1g
lMUXqz9rsX0UK3Oqap2J1cY86TGVOJvzYe4%3D&amp;reserved=0

Biju Das (18):
  dt-bindings: net: renesas,etheravb: Document Gigabit Ethernet IP
  drivers: clk: renesas: rzg2l-cpg: Add support to handle MUX clocks
  drivers: clk: renesas: r9a07g044-cpg: Add ethernet clock sources
  drivers: clk: renesas: r9a07g044-cpg: Add GbEthernet clock/reset

   It's not a good idea to have the patch to the defferent subsystems
lumped all together in a single series...
Agreed.

Are these changes inseparable? If so, you need to be up front on this, and
you need an agreement with the subsystem maintainers how the patches are
going to be merged? Through which tree. And you need Acked-by from the
other tree maintainers.

Ideally you submit multiple patchsets. This assumes all sets will compile
independently.
Agreed. Will split this patch series in 3 patchsets

1) single binding patch 

2) Clock patchset

3) ravb driver patchset.

Cheers,
Biju

Re: [PATCH net-next 06/18] ravb: Factorise ptp feature

From: Sergei Shtylyov <hidden>
Date: 2021-07-23 20:57:00

HEllo!

On 7/22/21 5:13 PM, Biju Das wrote:
Gptp is active in CONFIG mode for R-Car Gen3, where as it is not
   It's gPTP, the manuals say. :-)
active in CONFIG mode for R-Car Gen2. Add feature bits to handle
both cases.
   I have no idea why this single diff requires 2 fetaure bits....
RZ/G2L does not support ptp feature.
   Ah, that explains it. :-)
   It doesn't explain why we should bother with the 2nd bit in the same patch tho...
quoted hunk
Factorise ptp feature
specific to R-Car.

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 | 81 ++++++++++++++++--------
 2 files changed, 56 insertions(+), 26 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 0ed21262f26b..a474ed68db22 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -998,6 +998,7 @@ struct ravb_drv_data {
 	size_t skb_sz;
 	u8 num_tx_desc;
 	enum ravb_chip_id chip_id;
+	u32 features;
   You didn't like bitfelds (in sh_eth) so much? :-)

[...]
quoted hunk
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 84ebd6fef711..e966b76df32c 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -40,6 +40,14 @@
 		 NETIF_MSG_RX_ERR | \
 		 NETIF_MSG_TX_ERR)
 
+#define RAVB_PTP_CONFIG_ACTIVE		BIT(0)
+#define RAVB_PTP_CONFIG_INACTIVE	BIT(1)
   If both bits are 0, it means GbEth?
+
+#define RAVB_PTP	(RAVB_PTP_CONFIG_ACTIVE | RAVB_PTP_CONFIG_INACTIVE)
   Hm?
+
+#define RAVB_RCAR_GEN3_FEATURES	RAVB_PTP_CONFIG_ACTIVE
+#define RAVB_RCAR_GEN2_FEATURES	RAVB_PTP_CONFIG_INACTIVE
   Not sure whtehr these are necessary...

[...]
 	}
 
 	/* gPTP interrupt status summary */
-	if (iss & ISS_CGIS) {
+	if ((info->features & RAVB_PTP) && (iss & ISS_CGIS)) {
   This is not a transparent change -- the fearture check came fromn nownere...
 		ravb_ptp_interrupt(ndev);
 		result = IRQ_HANDLED;
 	}
[...]
quoted hunk
@@ -1275,7 +1286,8 @@ static int ravb_get_ts_info(struct net_device *ndev,
 		(1 << HWTSTAMP_FILTER_NONE) |
 		(1 << HWTSTAMP_FILTER_PTP_V2_L2_EVENT) |
 		(1 << HWTSTAMP_FILTER_ALL);
-	info->phc_index = ptp_clock_index(priv->ptp.clock);
+	if (data->features & RAVB_PTP)
   Again, not transparent...
+		info->phc_index = ptp_clock_index(priv->ptp.clock);
 
 	return 0;
 }
[...]
quoted hunk
@@ -1992,14 +2009,20 @@ 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_drv_data *info = priv->info;
 
-	if (priv->chip_id == RCAR_GEN2) {
+	switch (info->features & RAVB_PTP) {
+	case RAVB_PTP_CONFIG_INACTIVE:
 		ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_CONFIG);
 		/* Set CSEL value */
 		ravb_modify(ndev, CCC, CCC_CSEL, CCC_CSEL_HPB);
-	} else {
+		break;
+	case RAVB_PTP_CONFIG_ACTIVE:
 		ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_CONFIG |
 			    CCC_GAC | CCC_CSEL_HPB);
+		break;
+	default:
+		ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_CONFIG);
   Not trasparent again...

[...]
quoted hunk
@@ -2182,13 +2205,15 @@ static int ravb_probe(struct platform_device *pdev)
 	/* Set AVB config mode */
 	ravb_set_config_mode(ndev);
 
-	/* Set GTI value */
-	error = ravb_set_gti(ndev);
-	if (error)
-		goto out_disable_refclk;
+	if (info->features & RAVB_PTP) {
   Not transparent enough yet again...
+		/* Set GTI value */
+		error = ravb_set_gti(ndev);
+		if (error)
+			goto out_disable_refclk;
 
-	/* Request GTI loading */
-	ravb_modify(ndev, GCCR, GCCR_LTI, GCCR_LTI);
+		/* Request GTI loading */
+		ravb_modify(ndev, GCCR, GCCR_LTI, GCCR_LTI);
+	}
 
 	if (priv->chip_id != RCAR_GEN2) {
 		ravb_parse_delay_mode(np, ndev);
[...]
quoted hunk
@@ -2377,13 +2404,15 @@ static int __maybe_unused ravb_resume(struct device *dev)
 	/* Set AVB config mode */
 	ravb_set_config_mode(ndev);
 
-	/* Set GTI value */
-	ret = ravb_set_gti(ndev);
-	if (ret)
-		return ret;
+	if (info->features & RAVB_PTP) {
   Not transparent enough again...
+		/* Set GTI value */
+		ret = ravb_set_gti(ndev);
+		if (ret)
+			return ret;
 
-	/* Request GTI loading */
-	ravb_modify(ndev, GCCR, GCCR_LTI, GCCR_LTI);
+		/* Request GTI loading */
+		ravb_modify(ndev, GCCR, GCCR_LTI, GCCR_LTI);
+	}
 
 	if (priv->chip_id != RCAR_GEN2)
 		ravb_set_delay_mode(ndev);
MBR, Sergei

RE: [PATCH net-next 06/18] ravb: Factorise ptp feature

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-26 09:01:35

Hi Sergei,

Thanks for the feedback.
Subject: Re: [PATCH net-next 06/18] ravb: Factorise ptp feature

HEllo!

On 7/22/21 5:13 PM, Biju Das wrote:
quoted
Gptp is active in CONFIG mode for R-Car Gen3, where as it is not
   It's gPTP, the manuals say. :-)
Ok.
quoted
active in CONFIG mode for R-Car Gen2. Add feature bits to handle both
cases.
   I have no idea why this single diff requires 2 fetaure bits....
Basically this is a HW feature.

1) for R-Car Gen3, gPTP is active in config mode (R-Car Gen3)
2) for R-Car Gen2, gPTP is not active in config mode (R-Car Gen2)
3) RZ/G2L does not support ptp feature.
quoted
RZ/G2L does not support ptp feature.
   Ah, that explains it. :-)
   It doesn't explain why we should bother with the 2nd bit in the same
patch tho...
See above it is HW feature diff between R-Car Gen3 and R-Car Gen2.
quoted
Factorise ptp feature
specific to R-Car.

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 | 81
++++++++++++++++--------
 2 files changed, 56 insertions(+), 26 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h
b/drivers/net/ethernet/renesas/ravb.h
index 0ed21262f26b..a474ed68db22 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -998,6 +998,7 @@ struct ravb_drv_data {
 	size_t skb_sz;
 	u8 num_tx_desc;
 	enum ravb_chip_id chip_id;
+	u32 features;
   You didn't like bitfelds (in sh_eth) so much? :-)
OK. I will change to bit fields, if there is no objection.
[...]
quoted
diff --git a/drivers/net/ethernet/renesas/ravb_main.c
b/drivers/net/ethernet/renesas/ravb_main.c
index 84ebd6fef711..e966b76df32c 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -40,6 +40,14 @@
 		 NETIF_MSG_RX_ERR | \
 		 NETIF_MSG_TX_ERR)

+#define RAVB_PTP_CONFIG_ACTIVE		BIT(0)
+#define RAVB_PTP_CONFIG_INACTIVE	BIT(1)
   If both bits are 0, it means GbEth?
Yes that is correct. ptp is not supported in GbEth.
quoted
+
+#define RAVB_PTP	(RAVB_PTP_CONFIG_ACTIVE |
RAVB_PTP_CONFIG_INACTIVE)

   Hm?
quoted
+
+#define RAVB_RCAR_GEN3_FEATURES	RAVB_PTP_CONFIG_ACTIVE
+#define RAVB_RCAR_GEN2_FEATURES	RAVB_PTP_CONFIG_INACTIVE
   Not sure whtehr these are necessary...
If there is no objection for using bit fields, then the above definitions not required.
[...]
quoted
 	}

 	/* gPTP interrupt status summary */
-	if (iss & ISS_CGIS) {
+	if ((info->features & RAVB_PTP) && (iss & ISS_CGIS)) {
   This is not a transparent change -- the fearture check came fromn
nownere...
I have added commit statement to make it clear, "RZ/G2L does not support ptp feature. Factorise ptp feature
specific to R-Car".

Do you see any issues with this? If needed we can factorize this portion of the code again to make 
It simpler. First patch is ptp feature for r-car differences(R-Car Gen3 and R-Car Gen2) and second patch( with "transparent" comments you have mentioned in this patch)

Please let me know.
quoted
 		ravb_ptp_interrupt(ndev);
 		result = IRQ_HANDLED;
 	}
[...]
quoted
@@ -1275,7 +1286,8 @@ static int ravb_get_ts_info(struct net_device
*ndev,
quoted
 		(1 << HWTSTAMP_FILTER_NONE) |
 		(1 << HWTSTAMP_FILTER_PTP_V2_L2_EVENT) |
 		(1 << HWTSTAMP_FILTER_ALL);
-	info->phc_index = ptp_clock_index(priv->ptp.clock);
+	if (data->features & RAVB_PTP)
   Again, not transparent...
See above.
quoted
+		info->phc_index = ptp_clock_index(priv->ptp.clock);

 	return 0;
 }
[...]
quoted
@@ -1992,14 +2009,20 @@ 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_drv_data *info = priv->info;

-	if (priv->chip_id == RCAR_GEN2) {
+	switch (info->features & RAVB_PTP) {
+	case RAVB_PTP_CONFIG_INACTIVE:
 		ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_CONFIG);
 		/* Set CSEL value */
 		ravb_modify(ndev, CCC, CCC_CSEL, CCC_CSEL_HPB);
-	} else {
+		break;
+	case RAVB_PTP_CONFIG_ACTIVE:
 		ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_CONFIG |
 			    CCC_GAC | CCC_CSEL_HPB);
+		break;
+	default:
+		ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_CONFIG);
   Not trasparent again...
See above.

First Case is for R-Car Gen3 where ptp is active in config mode.
Second Case is for R-Car Gen2 where ptp is not active in config mode.
Third Case is default for RZ/G2L where ptp is not present.

Do you see any issues with this?
[...]
quoted
@@ -2182,13 +2205,15 @@ static int ravb_probe(struct platform_device
*pdev)
quoted
 	/* Set AVB config mode */
 	ravb_set_config_mode(ndev);

-	/* Set GTI value */
-	error = ravb_set_gti(ndev);
-	if (error)
-		goto out_disable_refclk;
+	if (info->features & RAVB_PTP) {
   Not transparent enough yet again...
See above.
quoted
+		/* Set GTI value */
+		error = ravb_set_gti(ndev);
+		if (error)
+			goto out_disable_refclk;

-	/* Request GTI loading */
-	ravb_modify(ndev, GCCR, GCCR_LTI, GCCR_LTI);
+		/* Request GTI loading */
+		ravb_modify(ndev, GCCR, GCCR_LTI, GCCR_LTI);
+	}

 	if (priv->chip_id != RCAR_GEN2) {
 		ravb_parse_delay_mode(np, ndev);
[...]
quoted
@@ -2377,13 +2404,15 @@ static int __maybe_unused ravb_resume(struct
device *dev)
quoted
 	/* Set AVB config mode */
 	ravb_set_config_mode(ndev);

-	/* Set GTI value */
-	ret = ravb_set_gti(ndev);
-	if (ret)
-		return ret;
+	if (info->features & RAVB_PTP) {
   Not transparent enough again...
See above.

Cheers,
Biju

Re: [PATCH net-next 00/18] Add Gigabit Ethernet driver support

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2021-07-26 10:55:54

Hi Biju,

On Fri, Jul 23, 2021 at 8:28 AM Biju Das [off-list ref] wrote:
quoted
Subject: Re: [PATCH net-next 00/18] Add Gigabit Ethernet driver support

On Thu, Jul 22, 2021 at 11:53:59PM +0300, Sergei Shtylyov wrote:
quoted
On 7/22/21 5:13 PM, Biju Das wrote:
quoted
The DMAC and EMAC blocks of Gigabit Ethernet IP is almost similar to
Ethernet AVB.
quoted
quoted
The Gigabit Etherner IP consists of Ethernet controller (E-MAC),
Internal TCP/IP Offload Engine (TOE) and Dedicated Direct memory access
controller (DMAC).
quoted
quoted
With few changes in driver, we can support Gigabit ethernet driver as
well.
quoted
quoted
This patch series is aims to support the same

RFC->V1
  * Incorporated feedback from Andrew, Sergei, Geert and Prabhakar
  *
https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa
tchwork.kernel.org%2Fproject%2Flinux-renesas-soc%2Flist%2F%3Fseries%
3D515525&amp;data=04%7C01%7Cbiju.das.jz%40bp.renesas.com%7C6fe3922cc
35d4178cb1d08d94d54bc75%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7
C637625848601442706%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQ
IjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=bOpIqV1g
lMUXqz9rsX0UK3Oqap2J1cY86TGVOJvzYe4%3D&amp;reserved=0

Biju Das (18):
  dt-bindings: net: renesas,etheravb: Document Gigabit Ethernet IP
  drivers: clk: renesas: rzg2l-cpg: Add support to handle MUX clocks
  drivers: clk: renesas: r9a07g044-cpg: Add ethernet clock sources
  drivers: clk: renesas: r9a07g044-cpg: Add GbEthernet clock/reset

   It's not a good idea to have the patch to the defferent subsystems
lumped all together in a single series...
Agreed.

Are these changes inseparable? If so, you need to be up front on this, and
you need an agreement with the subsystem maintainers how the patches are
going to be merged? Through which tree. And you need Acked-by from the
other tree maintainers.

Ideally you submit multiple patchsets. This assumes all sets will compile
independently.
Agreed. Will split this patch series in 3 patchsets

1) single binding patch

2) Clock patchset

3) ravb driver patchset.
4) dts part.

Part 2 should pass through renesas-clk.
Part 4 should pass through renesas-devel.

Gr{oetje,eeting}s,

                        Geert

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

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

Re: [PATCH net-next 06/18] ravb: Factorise ptp feature

From: Andrew Lunn <andrew@lunn.ch>
Date: 2021-07-26 13:09:12

On Mon, Jul 26, 2021 at 09:01:29AM +0000, Biju Das wrote:
Hi Sergei,

Thanks for the feedback.
quoted
Subject: Re: [PATCH net-next 06/18] ravb: Factorise ptp feature

HEllo!

On 7/22/21 5:13 PM, Biju Das wrote:
quoted
Gptp is active in CONFIG mode for R-Car Gen3, where as it is not
   It's gPTP, the manuals say. :-)
Ok.
quoted
quoted
active in CONFIG mode for R-Car Gen2. Add feature bits to handle both
cases.
   I have no idea why this single diff requires 2 fetaure bits....
Basically this is a HW feature.

1) for R-Car Gen3, gPTP is active in config mode (R-Car Gen3)
2) for R-Car Gen2, gPTP is not active in config mode (R-Car Gen2)
3) RZ/G2L does not support ptp feature.
This is useful information to put in he commit message. The commit
message should answer the question "Why is this change being made?",
since what is being changed should be obvious from the patch.

      Andrew

RE: [PATCH net-next 06/18] ravb: Factorise ptp feature

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-26 13:41:24

Hi Andrew,

Thanks for the feedback.
Subject: Re: [PATCH net-next 06/18] ravb: Factorise ptp feature

On Mon, Jul 26, 2021 at 09:01:29AM +0000, Biju Das wrote:
quoted
Hi Sergei,

Thanks for the feedback.
quoted
Subject: Re: [PATCH net-next 06/18] ravb: Factorise ptp feature

HEllo!

On 7/22/21 5:13 PM, Biju Das wrote:
quoted
Gptp is active in CONFIG mode for R-Car Gen3, where as it is not
   It's gPTP, the manuals say. :-)
Ok.
quoted
quoted
active in CONFIG mode for R-Car Gen2. Add feature bits to handle
both cases.
   I have no idea why this single diff requires 2 fetaure bits....
Basically this is a HW feature.

1) for R-Car Gen3, gPTP is active in config mode (R-Car Gen3)
2) for R-Car Gen2, gPTP is not active in config mode (R-Car Gen2)
3) RZ/G2L does not support ptp feature.
This is useful information to put in he commit message. The commit message
should answer the question "Why is this change being made?", since what is
being changed should be obvious from the patch.
OK. will add this in commit description.

Regards,
Biju
      Andrew

Re: [PATCH net-next 00/18] Add Gigabit Ethernet driver support

From: Arnd Bergmann <arnd@kernel.org>
Date: 2021-07-26 13:50:10

On Mon, Jul 26, 2021 at 12:56 PM Geert Uytterhoeven
[off-list ref] wrote:
On Fri, Jul 23, 2021 at 8:28 AM Biju Das [off-list ref] wrote:
quoted
quoted
Are these changes inseparable? If so, you need to be up front on this, and
you need an agreement with the subsystem maintainers how the patches are
going to be merged? Through which tree. And you need Acked-by from the
other tree maintainers.

Ideally you submit multiple patchsets. This assumes all sets will compile
independently.
Agreed. Will split this patch series in 3 patchsets

1) single binding patch

2) Clock patchset

3) ravb driver patchset.
4) dts part.

Part 2 should pass through renesas-clk.
Part 4 should pass through renesas-devel.
Sounds good. To clarify: the changes should not just compile, but
each branch should be usable independently with no loss of
functionality. Using the feature depends on having all branches
merged, but they should not introduced regressions when applied
into some other tree.

I hope this was already obvious to everyone involved.

       Arnd

Re: [PATCH net-next 08/18] ravb: Add R-Car common features

From: Sergei Shtylyov <hidden>
Date: 2021-07-27 20:49:05

Hello!

On 7/22/21 5:13 PM, Biju Das wrote:
The below features are supported by both R-Car Gen2 and Gen3.

1) magic packet detection
2) multiple TSRQ support
3) extended descriptor in rx
   I think this one should better be called timestamping...
4) No half duplex support
   Couldn't we avoid the "negative" features?
5) override mtu change
   Hm, I'd vote for the individual patches covering only single feature...
quoted hunk
Add features bits to support the same.

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_main.c | 110 +++++++++++++++--------
 1 file changed, 71 insertions(+), 39 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index b3c99f974632..4ef2565534d2 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
[...]
quoted hunk
@@ -680,11 +694,14 @@ static void ravb_rcv_snd_enable(struct net_device *ndev)
 /* function for waiting dma process finished */
 static int ravb_stop_dma(struct net_device *ndev)
 {
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
 	int error;
 
 	/* Wait for stopping the hardware TX process */
-	error = ravb_wait(ndev, TCCR,
-			  TCCR_TSRQ0 | TCCR_TSRQ1 | TCCR_TSRQ2 | TCCR_TSRQ3, 0);
+	if (info->features & RAVB_MULTI_TSRQ)
+		error = ravb_wait(ndev, TCCR,
+				  TCCR_TSRQ0 | TCCR_TSRQ1 | TCCR_TSRQ2 | TCCR_TSRQ3, 0);
 	if (error)
   What if the above *if* skips the ravb_wait() call -- didn't you get a complaint from gcc
about the unnintialized variable?

[...]
quoted hunk
@@ -808,11 +826,14 @@ static bool ravb_queue_interrupt(struct net_device *ndev, int q)
 
 static bool ravb_timestamp_interrupt(struct net_device *ndev)
 {
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
 	u32 tis = ravb_read(ndev, TIS);
 
 	if (tis & TIS_TFUF) {
 		ravb_write(ndev, ~(TIS_TFUF | TIS_RESERVED), TIS);
-		ravb_get_tx_tstamp(ndev);
+		if (info->features & RAVB_EX_RX_DESC)
   Yeah, definitely a bad feature name...
+			ravb_get_tx_tstamp(ndev);
 		return true;
 	}
 	return false;
[...]
quoted hunk
@@ -1069,15 +1091,17 @@ static int ravb_phy_init(struct net_device *ndev)
 		netdev_info(ndev, "limited PHY to 100Mbit/s\n");
 	}
 
-	/* 10BASE, Pause and Asym Pause is not supported */
-	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
-	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT);
-	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Pause_BIT);
-	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Asym_Pause_BIT);
+	if (info->features & RAVB_NO_HALF_DUPLEX) {
+		/* 10BASE, Pause and Asym Pause is not supported */
+		phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
+		phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT);
+		phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Pause_BIT);
+		phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Asym_Pause_BIT);
 
-	/* Half Duplex is not supported */
-	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
-	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_100baseT_Half_BIT);
+		/* Half Duplex is not supported */
+		phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
+		phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_100baseT_Half_BIT);
    Mhm? Some of the half-duplex modes sre unsupported still?

[...]
quoted hunk
@@ -1314,8 +1338,9 @@ static void ravb_get_wol(struct net_device *ndev, struct ethtool_wolinfo *wol)
 static int ravb_set_wol(struct net_device *ndev, struct ethtool_wolinfo *wol)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
 
-	if (wol->wolopts & ~WAKE_MAGIC)
+	if ((wol->wolopts & ~WAKE_MAGIC) || (!(info->features & RAVB_MAGIC)))
   Parens about !x not needed. And I think the second operand should come first instead...
 		return -EOPNOTSUPP;
 
 	priv->wol_enabled = !!(wol->wolopts & WAKE_MAGIC);
[...]
quoted hunk
@@ -1595,28 +1621,30 @@ static netdev_tx_t ravb_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 	desc->dptr = cpu_to_le32(dma_addr);
 
 	/* TX timestamp required */
-	if (q == RAVB_NC) {
-		ts_skb = kmalloc(sizeof(*ts_skb), GFP_ATOMIC);
-		if (!ts_skb) {
-			if (num_tx_desc > 1) {
-				desc--;
-				dma_unmap_single(ndev->dev.parent, dma_addr,
-						 len, DMA_TO_DEVICE);
+	if (info->features & RAVB_EX_RX_DESC) {
   Definitely a bad name...

[...]
quoted hunk
@@ -2205,8 +2235,10 @@ static int ravb_probe(struct platform_device *pdev)
 	}
 	clk_prepare_enable(priv->refclk);
 
-	ndev->max_mtu = 2048 - (ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN);
-	ndev->min_mtu = ETH_MIN_MTU;
+	if (info->features & RAVB_OVERRIDE_MTU_CHANGE) {
   Why? :-/ Could you tell me more details?
+		ndev->max_mtu = 2048 - (ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN);
+		ndev->min_mtu = ETH_MIN_MTU;
+	}
 
 	priv->num_tx_desc = info->num_tx_desc;
 
MBR, Sergei

RE: [PATCH net-next 08/18] ravb: Add R-Car common features

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-28 10:13:52

Hi Sergei,

Thanks for the feedback.
Subject: Re: [PATCH net-next 08/18] ravb: Add R-Car common features

Hello!

On 7/22/21 5:13 PM, Biju Das wrote:
quoted
The below features are supported by both R-Car Gen2 and Gen3.

1) magic packet detection
2) multiple TSRQ support
3) extended descriptor in rx
   I think this one should better be called timestamping...
OK. Will change it to timestamp.
quoted
4) No half duplex support
   Couldn't we avoid the "negative" features?
10 Mbps/Half duplex(As per the comment below) is not supported in R-Car. May be it should be a "positive" feature for
RZ/G2L. probably we need to add this as part of RZ/G2L feature.
quoted
5) override mtu change
   Hm, I'd vote for the individual patches covering only single feature...
OK. That will make patches again simpler.
quoted
Add features bits to support the same.

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_main.c | 110
+++++++++++++++--------
 1 file changed, 71 insertions(+), 39 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c
b/drivers/net/ethernet/renesas/ravb_main.c
index b3c99f974632..4ef2565534d2 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
[...]
quoted
@@ -680,11 +694,14 @@ static void ravb_rcv_snd_enable(struct
net_device *ndev)
 /* function for waiting dma process finished */  static int
ravb_stop_dma(struct net_device *ndev)  {
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
 	int error;

 	/* Wait for stopping the hardware TX process */
-	error = ravb_wait(ndev, TCCR,
-			  TCCR_TSRQ0 | TCCR_TSRQ1 | TCCR_TSRQ2 | TCCR_TSRQ3, 0);
+	if (info->features & RAVB_MULTI_TSRQ)
+		error = ravb_wait(ndev, TCCR,
+				  TCCR_TSRQ0 | TCCR_TSRQ1 | TCCR_TSRQ2 |
TCCR_TSRQ3, 0);
quoted
 	if (error)
   What if the above *if* skips the ravb_wait() call -- didn't you get a
complaint from gcc about the unnintialized variable?
Good catch will initialize with error = 0 and later patch when we add RZ/G2L, will remove this unnecessary initialization.
[...]
quoted
@@ -808,11 +826,14 @@ static bool ravb_queue_interrupt(struct
net_device *ndev, int q)

 static bool ravb_timestamp_interrupt(struct net_device *ndev)  {
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
 	u32 tis = ravb_read(ndev, TIS);

 	if (tis & TIS_TFUF) {
 		ravb_write(ndev, ~(TIS_TFUF | TIS_RESERVED), TIS);
-		ravb_get_tx_tstamp(ndev);
+		if (info->features & RAVB_EX_RX_DESC)
   Yeah, definitely a bad feature name...
OK. AS you suggested, will change the macro RAVB_EX_RX_DESC to ravb_timestamp feature bit like sh_eth.
quoted
+			ravb_get_tx_tstamp(ndev);
 		return true;
 	}
 	return false;
[...]
quoted
@@ -1069,15 +1091,17 @@ static int ravb_phy_init(struct net_device
*ndev)
quoted
 		netdev_info(ndev, "limited PHY to 100Mbit/s\n");
 	}

-	/* 10BASE, Pause and Asym Pause is not supported */
-	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
-	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT);
-	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Pause_BIT);
-	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Asym_Pause_BIT);
+	if (info->features & RAVB_NO_HALF_DUPLEX) {
+		/* 10BASE, Pause and Asym Pause is not supported */
+		phy_remove_link_mode(phydev,
ETHTOOL_LINK_MODE_10baseT_Half_BIT);
quoted
+		phy_remove_link_mode(phydev,
ETHTOOL_LINK_MODE_10baseT_Full_BIT);
quoted
+		phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Pause_BIT);
+		phy_remove_link_mode(phydev,
ETHTOOL_LINK_MODE_Asym_Pause_BIT);
quoted
-	/* Half Duplex is not supported */
-	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
-	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_100baseT_Half_BIT);
+		/* Half Duplex is not supported */
+		phy_remove_link_mode(phydev,
ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
quoted
+		phy_remove_link_mode(phydev,
ETHTOOL_LINK_MODE_100baseT_Half_BIT);

    Mhm? Some of the half-duplex modes sre unsupported still?

[...]
quoted
@@ -1314,8 +1338,9 @@ static void ravb_get_wol(struct net_device
*ndev, struct ethtool_wolinfo *wol)  static int ravb_set_wol(struct
net_device *ndev, struct ethtool_wolinfo *wol)  {
 	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;

-	if (wol->wolopts & ~WAKE_MAGIC)
+	if ((wol->wolopts & ~WAKE_MAGIC) || (!(info->features &
+RAVB_MAGIC)))
   Parens about !x not needed. And I think the second operand should come
first instead...
OK.
quoted
 		return -EOPNOTSUPP;

 	priv->wol_enabled = !!(wol->wolopts & WAKE_MAGIC);
[...]
quoted
@@ -1595,28 +1621,30 @@ static netdev_tx_t ravb_start_xmit(struct
sk_buff *skb, struct net_device *ndev)
quoted
 	desc->dptr = cpu_to_le32(dma_addr);

 	/* TX timestamp required */
-	if (q == RAVB_NC) {
-		ts_skb = kmalloc(sizeof(*ts_skb), GFP_ATOMIC);
-		if (!ts_skb) {
-			if (num_tx_desc > 1) {
-				desc--;
-				dma_unmap_single(ndev->dev.parent, dma_addr,
-						 len, DMA_TO_DEVICE);
+	if (info->features & RAVB_EX_RX_DESC) {
   Definitely a bad name...
OK.
[...]
quoted
@@ -2205,8 +2235,10 @@ static int ravb_probe(struct platform_device
*pdev)
quoted
 	}
 	clk_prepare_enable(priv->refclk);

-	ndev->max_mtu = 2048 - (ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN);
-	ndev->min_mtu = ETH_MIN_MTU;
+	if (info->features & RAVB_OVERRIDE_MTU_CHANGE) {
   Why? :-/ Could you tell me more details?
RX buff size = 2048 for R-Car where as it is 8K for RZ/G2L.
So Do you think we should use the same max_mtu for RZ/G2L as well?

Please let us know your recommendation here in this case?  The original bsp code used default "eth_mtu_change"
which is now deprecated.

Regards,
Biju
quoted
+		ndev->max_mtu = 2048 - (ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN);
+		ndev->min_mtu = ETH_MIN_MTU;
+	}

 	priv->num_tx_desc = info->num_tx_desc;
MBR, Sergei

Re: [PATCH net-next 08/18] ravb: Add R-Car common features

From: Andrew Lunn <andrew@lunn.ch>
Date: 2021-07-28 13:47:07

quoted
quoted
@@ -2205,8 +2235,10 @@ static int ravb_probe(struct platform_device
*pdev)
quoted
 	}
 	clk_prepare_enable(priv->refclk);

-	ndev->max_mtu = 2048 - (ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN);
-	ndev->min_mtu = ETH_MIN_MTU;
+	if (info->features & RAVB_OVERRIDE_MTU_CHANGE) {
   Why? :-/ Could you tell me more details?
RX buff size = 2048 for R-Car where as it is 8K for RZ/G2L.
RAVB_OVERRIDE_MTU_CHANGE is not the most descriptive name. You are not
overriding, you are setting the correct value for the hardware
variant.

Maybe name the feature RAVB_8K_BUFFERS or RAVB_2K_BUFFERS.

Also, putting more details in the commit message will help, and lots
of small patches, each patch doing one thing. It is much better to
have 40 simple, well documented, obviously correct patches, than 20
hard to understand patches. But please do submit them in small
batches, no more than 15 at once.

	 Andrew


RE: [PATCH net-next 08/18] ravb: Add R-Car common features

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-29 15:10:58

Hi Andrew,

Thanks for the feedback.
Subject: Re: [PATCH net-next 08/18] ravb: Add R-Car common features
quoted
quoted
quoted
@@ -2205,8 +2235,10 @@ static int ravb_probe(struct
platform_device
*pdev)
quoted
 	}
 	clk_prepare_enable(priv->refclk);

-	ndev->max_mtu = 2048 - (ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN);
-	ndev->min_mtu = ETH_MIN_MTU;
+	if (info->features & RAVB_OVERRIDE_MTU_CHANGE) {
   Why? :-/ Could you tell me more details?
RX buff size = 2048 for R-Car where as it is 8K for RZ/G2L.
RAVB_OVERRIDE_MTU_CHANGE is not the most descriptive name. You are not
overriding, you are setting the correct value for the hardware variant.
Thanks for correcting me.
Maybe name the feature RAVB_8K_BUFFERS or RAVB_2K_BUFFERS.
OK.
Also, putting more details in the commit message will help, and lots of
small patches, each patch doing one thing. 
Agreed. Will send smaller patches with more details on commit message.

It is much better to have 40
simple, well documented, obviously correct patches, than 20 hard to
understand patches. But please do submit them in small batches, no more
than 15 at once.
OK. 

Cheers,
Biju

Re: [PATCH net-next 09/18] ravb: Factorise ravb_ring_free function

From: Sergei Shtylyov <hidden>
Date: 2021-07-29 18:02:53

Hello!

On 7/22/21 5:13 PM, Biju Das wrote:
quoted hunk
Extended descriptor support in RX is available for R-Car where as it
is a normal descriptor for RZ/G2L. 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      |  5 +++
 drivers/net/ethernet/renesas/ravb_main.c | 49 ++++++++++++++++--------
 2 files changed, 37 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index a474ed68db22..3a9cf6e8671a 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -988,7 +988,12 @@ enum ravb_chip_id {
 	RCAR_GEN3,
 };
 
+struct ravb_ops {
+	void (*ring_free)(struct net_device *ndev, int q);
   Hmm, why not store it right in the *struct* ravb_drv_data?
quoted hunk
+};
+
 struct ravb_drv_data {
+	const struct ravb_ops *ravb_ops;
 	netdev_features_t net_features;
 	netdev_features_t net_hw_features;
 	const char (*gstrings_stats)[ETH_GSTRING_LEN];
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 4ef2565534d2..a3b8b243fd54 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -247,30 +247,39 @@ static int ravb_tx_free(struct net_device *ndev, int q, bool free_txed_only)
 }
 
 /* Free skb's and DMA buffers for Ethernet AVB */
-static void ravb_ring_free(struct net_device *ndev, int q)
+static void ravb_ring_free_rx(struct net_device *ndev, int q)
   How about ravb_rx_ring_free() instead?
 
 {
 	struct ravb_private *priv = netdev_priv(ndev);
-	int num_tx_desc = priv->num_tx_desc;
 	int ring_size;
 	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];
+	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;
+		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;
   Couldn't this be moved into the new ravb_ring_free(), like the initial NULL check?
+}
+
+static void ravb_ring_free(struct net_device *ndev, int q)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
+	int num_tx_desc = priv->num_tx_desc;
+	int ring_size;
+	int i;
+
+	if (priv->rx_ring[q])
+		info->ravb_ops->ring_free(ndev, q);
   ... here?

[...]

MBR, Sergei

Re: [PATCH net-next 10/18] ravb: Factorise ravb_ring_format function

From: Sergei Shtylyov <hidden>
Date: 2021-07-29 18:30:24

On 7/22/21 5:13 PM, Biju Das wrote:
quoted hunk
The ravb_ring_format function uses extended descriptor in rx for
R-Car where as it use 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 | 34 +++++++++++++++---------
 2 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 3a9cf6e8671a..a3258c5d0c3d 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -990,6 +990,7 @@ enum ravb_chip_id {
 
 struct ravb_ops {
 	void (*ring_free)(struct net_device *ndev, int q);
+	void (*ring_format)(struct net_device *ndev, int q);
   Like I said, we don't need another indirection.... also both ops are for RX.

[...]
quoted hunk
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index a3b8b243fd54..c23f0d420c70 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -311,26 +311,15 @@ static void ravb_ring_free(struct net_device *ndev, int q)
 }
 
 /* Format skb and descriptor buffer for Ethernet AVB */
-static void ravb_ring_format(struct net_device *ndev, int q)
+static void ravb_ring_format_rx(struct net_device *ndev, int rxq)
   How about ravb_rx_ring_format(struct net_device *ndev, int q)?

[...]

MBR, Sergei

Re: [PATCH net-next 11/18] ravb: Factorise ravb_ring_init function

From: Sergei Shtylyov <hidden>
Date: 2021-07-29 18:53:56

On 7/22/21 5:13 PM, Biju Das wrote:
The ravb_ring_init function uses extended descriptor in rx for
R-Car and normal descriptor for RZ/G2L. Factorise rx ring buffer
allocation so that it can support later SoC.
   In this case I think you factored out the function in question... but my ENglish is possibly
too weak. :-)
quoted hunk
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, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index a3258c5d0c3d..d82bfa6e57c1 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -991,6 +991,7 @@ enum ravb_chip_id {
 struct ravb_ops {
 	void (*ring_free)(struct net_device *ndev, int q);
 	void (*ring_format)(struct net_device *ndev, int q);
+	bool (*alloc_rx_desc)(struct net_device *ndev, int q);
  Aha, rx_ appears at last! :-)

quoted hunk
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index c23f0d420c70..3d0f6598b936 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -384,6 +384,19 @@ static void ravb_ring_format(struct net_device *ndev, int q)
 }
 
 /* Init skb and descriptor buffer for Ethernet AVB */
+static bool ravb_alloc_rx_desc(struct net_device *ndev, int q)
    Why *bool*? I think we shold just return a pointer allocated.
quoted hunk
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	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];
+}
+
 static int ravb_ring_init(struct net_device *ndev, int q)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
@@ -418,11 +431,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->ravb_ops->alloc_rx_desc(ndev, q))
 		goto error;
 
 	priv->dirty_rx[q] = 0;
@@ -2008,6 +2017,7 @@ static int ravb_mdio_release(struct ravb_private *priv)
 static const struct ravb_ops ravb_gen3_ops = {
 	.ring_free = ravb_ring_free_rx,
 	.ring_format = ravb_ring_format_rx,
+	.alloc_rx_desc = ravb_alloc_rx_desc,
 };
 
 static const struct ravb_drv_data ravb_gen3_data = {

RE: [PATCH net-next 09/18] ravb: Factorise ravb_ring_free function

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-30 06:21:52

Hi Sergei,

Thanks for the feedback.
Subject: Re: [PATCH net-next 09/18] ravb: Factorise ravb_ring_free
function

Hello!

On 7/22/21 5:13 PM, Biju Das wrote:
quoted
Extended descriptor support in RX is available for R-Car where as it
is a normal descriptor for RZ/G2L. 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      |  5 +++
 drivers/net/ethernet/renesas/ravb_main.c | 49
++++++++++++++++--------
 2 files changed, 37 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h
b/drivers/net/ethernet/renesas/ravb.h
index a474ed68db22..3a9cf6e8671a 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -988,7 +988,12 @@ enum ravb_chip_id {
 	RCAR_GEN3,
 };

+struct ravb_ops {
+	void (*ring_free)(struct net_device *ndev, int q);
   Hmm, why not store it right in the *struct* ravb_drv_data?
OK.
quoted
+};
+
 struct ravb_drv_data {
+	const struct ravb_ops *ravb_ops;
 	netdev_features_t net_features;
 	netdev_features_t net_hw_features;
 	const char (*gstrings_stats)[ETH_GSTRING_LEN];
diff --git a/drivers/net/ethernet/renesas/ravb_main.c
b/drivers/net/ethernet/renesas/ravb_main.c
index 4ef2565534d2..a3b8b243fd54 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -247,30 +247,39 @@ static int ravb_tx_free(struct net_device *ndev,
int q, bool free_txed_only)  }

 /* Free skb's and DMA buffers for Ethernet AVB */ -static void
ravb_ring_free(struct net_device *ndev, int q)
+static void ravb_ring_free_rx(struct net_device *ndev, int q)
   How about ravb_rx_ring_free() instead?
Agreed.
quoted
 {
 	struct ravb_private *priv = netdev_priv(ndev);
-	int num_tx_desc = priv->num_tx_desc;
 	int ring_size;
 	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];
+	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;
+		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;
   Couldn't this be moved into the new ravb_ring_free(), like the initial
NULL check?
For RZ/G2L, it is priv->rgeth_rx_ring, that is the reason.

I can move the initial NULL check here, so the generic ravb_ring_free does not differentiate between
priv->rx_ring and priv->rgeth_rx_ring. See below.
quoted
+}
+
+static void ravb_ring_free(struct net_device *ndev, int q) {
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
+	int num_tx_desc = priv->num_tx_desc;
+	int ring_size;
+	int i;
+
+	if (priv->rx_ring[q])
+		info->ravb_ops->ring_free(ndev, q);
   ... here?
     It will be just  info->ravb_ops->ring_free(ndev, q);
And NULL check will be handled respective rx helper function.

What do you think?

Cheers,
Biju
[...]

MBR, Sergei

RE: [PATCH net-next 10/18] ravb: Factorise ravb_ring_format function

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-30 06:24:16

HI Sergei,

Thanks for the feedback.
Subject: Re: [PATCH net-next 10/18] ravb: Factorise ravb_ring_format
function

On 7/22/21 5:13 PM, Biju Das wrote:
quoted
The ravb_ring_format function uses extended descriptor in rx for R-Car
where as it use 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 | 34
+++++++++++++++---------
 2 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h
b/drivers/net/ethernet/renesas/ravb.h
index 3a9cf6e8671a..a3258c5d0c3d 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -990,6 +990,7 @@ enum ravb_chip_id {

 struct ravb_ops {
 	void (*ring_free)(struct net_device *ndev, int q);
+	void (*ring_format)(struct net_device *ndev, int q);
   Like I said, we don't need another indirection.... also both ops are
for RX.
OK. Will add this as part of ravb_drv_data.
[...]
quoted
diff --git a/drivers/net/ethernet/renesas/ravb_main.c
b/drivers/net/ethernet/renesas/ravb_main.c
index a3b8b243fd54..c23f0d420c70 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -311,26 +311,15 @@ static void ravb_ring_free(struct net_device
*ndev, int q)  }

 /* Format skb and descriptor buffer for Ethernet AVB */ -static void
ravb_ring_format(struct net_device *ndev, int q)
+static void ravb_ring_format_rx(struct net_device *ndev, int rxq)
   How about ravb_rx_ring_format(struct net_device *ndev, int q)?
Agreed.

Cheers,
Biju
[...]

MBR, Sergei

RE: [PATCH net-next 11/18] ravb: Factorise ravb_ring_init function

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-07-30 06:54:48

Hi Sergei,

Thanks for the feedback.
Subject: Re: [PATCH net-next 11/18] ravb: Factorise ravb_ring_init
function

On 7/22/21 5:13 PM, Biju Das wrote:
quoted
The ravb_ring_init function uses extended descriptor in rx for R-Car
and normal descriptor for RZ/G2L. Factorise rx ring buffer allocation
so that it can support later SoC.
   In this case I think you factored out the function in question... but
my ENglish is possibly too weak. :-)
OK. I meant add a helper function for rx ring buffer allocation.
quoted
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, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h
b/drivers/net/ethernet/renesas/ravb.h
index a3258c5d0c3d..d82bfa6e57c1 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -991,6 +991,7 @@ enum ravb_chip_id {  struct ravb_ops {
 	void (*ring_free)(struct net_device *ndev, int q);
 	void (*ring_format)(struct net_device *ndev, int q);
+	bool (*alloc_rx_desc)(struct net_device *ndev, int q);
  Aha, rx_ appears at last! :-)

quoted
diff --git a/drivers/net/ethernet/renesas/ravb_main.c
b/drivers/net/ethernet/renesas/ravb_main.c
index c23f0d420c70..3d0f6598b936 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -384,6 +384,19 @@ static void ravb_ring_format(struct net_device
*ndev, int q)  }

 /* Init skb and descriptor buffer for Ethernet AVB */
+static bool ravb_alloc_rx_desc(struct net_device *ndev, int q)
    Why *bool*? I think we shold just return a pointer allocated.
Ah OK.

Cheers,
Biju
quoted
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	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];
+}
+
 static int ravb_ring_init(struct net_device *ndev, int q)  {
 	struct ravb_private *priv = netdev_priv(ndev); @@ -418,11 +431,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);
quoted
-	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->ravb_ops->alloc_rx_desc(ndev, q))
 		goto error;

 	priv->dirty_rx[q] = 0;
@@ -2008,6 +2017,7 @@ static int ravb_mdio_release(struct ravb_private
*priv)  static const struct ravb_ops ravb_gen3_ops = {
 	.ring_free = ravb_ring_free_rx,
 	.ring_format = ravb_ring_format_rx,
+	.alloc_rx_desc = ravb_alloc_rx_desc,
 };

 static const struct ravb_drv_data ravb_gen3_data = {

Re: [PATCH net-next 12/18] ravb: Factorise {emac,dmac} init function

From: Sergei Shtylyov <hidden>
Date: 2021-08-02 19:41:57

Hello!

On 7/22/21 5:13 PM, Biju Das wrote:
The R-Car AVB module has Magic packet detection, multiple irq's and
timestamp enable features which is not present on RZ/G2L Gigabit
                                   ^ are
quoted hunk
Ethernet module. Factorise emac and dmac initialization 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      |  2 +
 drivers/net/ethernet/renesas/ravb_main.c | 58 ++++++++++++++++--------
 2 files changed, 40 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index d82bfa6e57c1..4d5910dcda86 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -992,6 +992,8 @@ struct ravb_ops {
 	void (*ring_free)(struct net_device *ndev, int q);
 	void (*ring_format)(struct net_device *ndev, int q);
 	bool (*alloc_rx_desc)(struct net_device *ndev, int q);
+	void (*emac_init)(struct net_device *ndev);
+	void (*dmac_init)(struct net_device *ndev);
 };
 
 struct ravb_drv_data {
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 3d0f6598b936..e200114376e4 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -454,7 +454,7 @@ static int ravb_ring_init(struct net_device *ndev, int q)
 }
 
 /* E-MAC init function */
-static void ravb_emac_init(struct net_device *ndev)
+static void ravb_emac_init_ex(struct net_device *ndev)
 {
 	/* Receive frame limit set register */
 	ravb_write(ndev, ndev->mtu + ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN, RFLR);
@@ -480,30 +480,19 @@ 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 */
   Grr, this comment seems oudated...
-static int ravb_dmac_init(struct net_device *ndev)
+static void ravb_emac_init(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
 	const struct ravb_drv_data *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;
-	}
+	info->ravb_ops->emac_init(ndev);
+}
   The whole ravb_emac_init() now consists only of a single method call?
Why do we need it at all?
 
-	/* Descriptor format */
-	ravb_ring_format(ndev, RAVB_BE);
-	ravb_ring_format(ndev, RAVB_NC);
+/* Device init function for Ethernet AVB */
   s/Device/DMAC/. Or this comment shouldn't have been moved.
+static void ravb_dmac_init_ex(struct net_device *ndev)
   Please no _ex suffixes -- reminds me of Windoze too much. :-)
quoted hunk
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
 
 	/* Set AVB RX */
 	ravb_write(ndev,
@@ -530,6 +519,33 @@ 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);
+}
+
+static int ravb_dmac_init(struct net_device *ndev)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *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->ravb_ops->dmac_init(ndev);
 
 	/* Setting the control will start the AVB-DMAC process. */
 	ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_OPERATION);
@@ -2018,6 +2034,8 @@ static const struct ravb_ops ravb_gen3_ops = {
 	.ring_free = ravb_ring_free_rx,
 	.ring_format = ravb_ring_format_rx,
 	.alloc_rx_desc = ravb_alloc_rx_desc,
+	.emac_init = ravb_emac_init_ex,
+	.dmac_init = ravb_dmac_init_ex,
   Hmm, why not also gen2?!
 };
[...]

MBR, Sergei

RE: [PATCH net-next 09/18] ravb: Factorise ravb_ring_free function

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-08-20 15:32:21

Hi Sergei,

Subject: RE: [PATCH net-next 09/18] ravb: Factorise ravb_ring_free
function

Hi Sergei,

Thanks for the feedback.
quoted
Subject: Re: [PATCH net-next 09/18] ravb: Factorise ravb_ring_free
function

Hello!

On 7/22/21 5:13 PM, Biju Das wrote:
quoted
Extended descriptor support in RX is available for R-Car where as it
is a normal descriptor for RZ/G2L. 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      |  5 +++
 drivers/net/ethernet/renesas/ravb_main.c | 49
++++++++++++++++--------
 2 files changed, 37 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h
b/drivers/net/ethernet/renesas/ravb.h
index a474ed68db22..3a9cf6e8671a 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -988,7 +988,12 @@ enum ravb_chip_id {
 	RCAR_GEN3,
 };

+struct ravb_ops {
+	void (*ring_free)(struct net_device *ndev, int q);
   Hmm, why not store it right in the *struct* ravb_drv_data?
OK.
quoted
quoted
+};
+
 struct ravb_drv_data {
+	const struct ravb_ops *ravb_ops;
 	netdev_features_t net_features;
 	netdev_features_t net_hw_features;
 	const char (*gstrings_stats)[ETH_GSTRING_LEN];
diff --git a/drivers/net/ethernet/renesas/ravb_main.c
b/drivers/net/ethernet/renesas/ravb_main.c
index 4ef2565534d2..a3b8b243fd54 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -247,30 +247,39 @@ static int ravb_tx_free(struct net_device
*ndev, int q, bool free_txed_only)  }

 /* Free skb's and DMA buffers for Ethernet AVB */ -static void
ravb_ring_free(struct net_device *ndev, int q)
+static void ravb_ring_free_rx(struct net_device *ndev, int q)
   How about ravb_rx_ring_free() instead?
Agreed.
quoted
quoted
 {
 	struct ravb_private *priv = netdev_priv(ndev);
-	int num_tx_desc = priv->num_tx_desc;
 	int ring_size;
 	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];
+	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;
+		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;
   Couldn't this be moved into the new ravb_ring_free(), like the
initial NULL check?
For RZ/G2L, it is priv->rgeth_rx_ring, that is the reason.

I can move the initial NULL check here, so the generic ravb_ring_free does
not differentiate between
priv->rx_ring and priv->rgeth_rx_ring. See below.
quoted
quoted
+}
+
+static void ravb_ring_free(struct net_device *ndev, int q) {
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;
+	int num_tx_desc = priv->num_tx_desc;
+	int ring_size;
+	int i;
+
+	if (priv->rx_ring[q])
+		info->ravb_ops->ring_free(ndev, q);
   ... here?
     It will be just  info->ravb_ops->ring_free(ndev, q); And NULL check
will be handled respective rx helper function.
I will be sending next set of patches incorporating the above comments
within couple of days.

Regards,
Biju

RE: [PATCH net-next 12/18] ravb: Factorise {emac,dmac} init function

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-08-20 15:42:27

Hi Sergei,

Thanks for the feedback.
[off-list ref]
Subject: Re: [PATCH net-next 12/18] ravb: Factorise {emac,dmac} init
function

Hello!

On 7/22/21 5:13 PM, Biju Das wrote:
quoted
The R-Car AVB module has Magic packet detection, multiple irq's and
timestamp enable features which is not present on RZ/G2L Gigabit
                                   ^ are
OK. Will fix this in next patch set.
quoted
Ethernet module. Factorise emac and dmac initialization 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      |  2 +
 drivers/net/ethernet/renesas/ravb_main.c | 58
++++++++++++++++--------
 2 files changed, 40 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h
b/drivers/net/ethernet/renesas/ravb.h
index d82bfa6e57c1..4d5910dcda86 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -992,6 +992,8 @@ struct ravb_ops {
 	void (*ring_free)(struct net_device *ndev, int q);
 	void (*ring_format)(struct net_device *ndev, int q);
 	bool (*alloc_rx_desc)(struct net_device *ndev, int q);
+	void (*emac_init)(struct net_device *ndev);
+	void (*dmac_init)(struct net_device *ndev);
 };

 struct ravb_drv_data {
diff --git a/drivers/net/ethernet/renesas/ravb_main.c
b/drivers/net/ethernet/renesas/ravb_main.c
index 3d0f6598b936..e200114376e4 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -454,7 +454,7 @@ static int ravb_ring_init(struct net_device *ndev,
int q)  }

 /* E-MAC init function */
-static void ravb_emac_init(struct net_device *ndev)
+static void ravb_emac_init_ex(struct net_device *ndev)
 {
 	/* Receive frame limit set register */
 	ravb_write(ndev, ndev->mtu + ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN,
RFLR); @@ -480,30 +480,19 @@ static void ravb_emac_init(struct
net_device *ndev)
quoted
 	ravb_write(ndev, ECSIPR_ICDIP | ECSIPR_MPDIP | ECSIPR_LCHNGIP,
ECSIPR);  }

-/* Device init function for Ethernet AVB */
   Grr, this comment seems oudated...
OK.
quoted
-static int ravb_dmac_init(struct net_device *ndev)
+static void ravb_emac_init(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
 	const struct ravb_drv_data *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;
-	}
+	info->ravb_ops->emac_init(ndev);
+}
   The whole ravb_emac_init() now consists only of a single method call?
Why do we need it at all?
OK will assign info->emac_init with ravb_emac_init, so GbEthernet just need to
fill emac_init function. I will remove the function "ravb_emac_init_ex".

quoted
-	/* Descriptor format */
-	ravb_ring_format(ndev, RAVB_BE);
-	ravb_ring_format(ndev, RAVB_NC);
+/* Device init function for Ethernet AVB */
   s/Device/DMAC/. Or this comment shouldn't have been moved.
OK.
quoted
+static void ravb_dmac_init_ex(struct net_device *ndev)
   Please no _ex suffixes -- reminds me of Windoze too much. :-)
OK. Will change it to ravb_device_init

Regards,
Biju
quoted
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;

 	/* Set AVB RX */
 	ravb_write(ndev,
@@ -530,6 +519,33 @@ 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);
+}
+
+static int ravb_dmac_init(struct net_device *ndev) {
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *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->ravb_ops->dmac_init(ndev);

 	/* Setting the control will start the AVB-DMAC process. */
 	ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_OPERATION); @@ -2018,6
+2034,8 @@ static const struct ravb_ops ravb_gen3_ops = {
 	.ring_free = ravb_ring_free_rx,
 	.ring_format = ravb_ring_format_rx,
 	.alloc_rx_desc = ravb_alloc_rx_desc,
+	.emac_init = ravb_emac_init_ex,
+	.dmac_init = ravb_dmac_init_ex,
   Hmm, why not also gen2?!

Re: [PATCH net-next 12/18] ravb: Factorise {emac,dmac} init function

From: Sergey Shtylyov <hidden>
Date: 2021-08-20 18:57:18

On 8/20/21 6:42 PM, Biju Das wrote:

[...]
quoted
quoted
The R-Car AVB module has Magic packet detection, multiple irq's and
timestamp enable features which is not present on RZ/G2L Gigabit
                                   ^ are
OK. Will fix this in next patch set.
quoted
quoted
Ethernet module. Factorise emac and dmac initialization 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      |  2 +
 drivers/net/ethernet/renesas/ravb_main.c | 58
++++++++++++++++--------
 2 files changed, 40 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h
b/drivers/net/ethernet/renesas/ravb.h
index d82bfa6e57c1..4d5910dcda86 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -992,6 +992,8 @@ struct ravb_ops {
 	void (*ring_free)(struct net_device *ndev, int q);
 	void (*ring_format)(struct net_device *ndev, int q);
 	bool (*alloc_rx_desc)(struct net_device *ndev, int q);
+	void (*emac_init)(struct net_device *ndev);
+	void (*dmac_init)(struct net_device *ndev);
 };

 struct ravb_drv_data {
diff --git a/drivers/net/ethernet/renesas/ravb_main.c
b/drivers/net/ethernet/renesas/ravb_main.c
index 3d0f6598b936..e200114376e4 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -454,7 +454,7 @@ static int ravb_ring_init(struct net_device *ndev,
int q)  }

 /* E-MAC init function */
-static void ravb_emac_init(struct net_device *ndev)
+static void ravb_emac_init_ex(struct net_device *ndev)
 {
 	/* Receive frame limit set register */
 	ravb_write(ndev, ndev->mtu + ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN,
RFLR); @@ -480,30 +480,19 @@ static void ravb_emac_init(struct
net_device *ndev)
quoted
 	ravb_write(ndev, ECSIPR_ICDIP | ECSIPR_MPDIP | ECSIPR_LCHNGIP,
ECSIPR);  }

-/* Device init function for Ethernet AVB */
   Grr, this comment seems oudated...
OK.
   Just don't move the comment. :-)
quoted
quoted
-static int ravb_dmac_init(struct net_device *ndev)
+static void ravb_emac_init(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
 	const struct ravb_drv_data *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;
-	}
+	info->ravb_ops->emac_init(ndev);
+}
   The whole ravb_emac_init() now consists only of a single method call?
Why do we need it at all?
OK will assign info->emac_init with ravb_emac_init, so GbEthernet just need to
fill emac_init function. I will remove the function "ravb_emac_init_ex".
  Will the EMAC init methods differ so much as to we should provide 2 separate
implementations?

[...]
quoted
quoted
+static void ravb_dmac_init_ex(struct net_device *ndev)
   Please no _ex suffixes -- reminds me of Windoze too much. :-)
OK. Will change it to ravb_device_init
   Ugh! Why not leave it named ravb_dmac_init()?
Regards,
Biju
quoted
quoted
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;

 	/* Set AVB RX */
 	ravb_write(ndev,
@@ -530,6 +519,33 @@ 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);
+}
+
+static int ravb_dmac_init(struct net_device *ndev) {
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *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->ravb_ops->dmac_init(ndev);

 	/* Setting the control will start the AVB-DMAC process. */
 	ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_OPERATION); @@ -2018,6
+2034,8 @@ static const struct ravb_ops ravb_gen3_ops = {
 	.ring_free = ravb_ring_free_rx,
 	.ring_format = ravb_ring_format_rx,
 	.alloc_rx_desc = ravb_alloc_rx_desc,
+	.emac_init = ravb_emac_init_ex,
+	.dmac_init = ravb_dmac_init_ex,
   Hmm, why not also gen2?!
   The question remained unreplied?... :-/

MBR, Sergey 

RE: [PATCH net-next 12/18] ravb: Factorise {emac,dmac} init function

From: Biju Das <biju.das.jz@bp.renesas.com>
Date: 2021-08-20 19:44:14

Hi Sergei,
Subject: Re: [PATCH net-next 12/18] ravb: Factorise {emac,dmac} init
function

On 8/20/21 6:42 PM, Biju Das wrote:

[...]
quoted
quoted
quoted
The R-Car AVB module has Magic packet detection, multiple irq's and
timestamp enable features which is not present on RZ/G2L Gigabit
                                   ^ are
OK. Will fix this in next patch set.
quoted
quoted
Ethernet module. Factorise emac and dmac initialization 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      |  2 +
 drivers/net/ethernet/renesas/ravb_main.c | 58
++++++++++++++++--------
 2 files changed, 40 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h
b/drivers/net/ethernet/renesas/ravb.h
index d82bfa6e57c1..4d5910dcda86 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -992,6 +992,8 @@ struct ravb_ops {
 	void (*ring_free)(struct net_device *ndev, int q);
 	void (*ring_format)(struct net_device *ndev, int q);
 	bool (*alloc_rx_desc)(struct net_device *ndev, int q);
+	void (*emac_init)(struct net_device *ndev);
+	void (*dmac_init)(struct net_device *ndev);
 };

 struct ravb_drv_data {
diff --git a/drivers/net/ethernet/renesas/ravb_main.c
b/drivers/net/ethernet/renesas/ravb_main.c
index 3d0f6598b936..e200114376e4 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -454,7 +454,7 @@ static int ravb_ring_init(struct net_device
*ndev, int q)  }

 /* E-MAC init function */
-static void ravb_emac_init(struct net_device *ndev)
+static void ravb_emac_init_ex(struct net_device *ndev)
 {
 	/* Receive frame limit set register */
 	ravb_write(ndev, ndev->mtu + ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN,
RFLR); @@ -480,30 +480,19 @@ static void ravb_emac_init(struct
net_device *ndev)
quoted
 	ravb_write(ndev, ECSIPR_ICDIP | ECSIPR_MPDIP | ECSIPR_LCHNGIP,
ECSIPR);  }

-/* Device init function for Ethernet AVB */
   Grr, this comment seems oudated...
OK.
   Just don't move the comment. :-)
quoted
quoted
quoted
-static int ravb_dmac_init(struct net_device *ndev)
+static void ravb_emac_init(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
 	const struct ravb_drv_data *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;
-	}
+	info->ravb_ops->emac_init(ndev);
+}
   The whole ravb_emac_init() now consists only of a single method
call?
quoted
quoted
Why do we need it at all?
OK will assign info->emac_init with ravb_emac_init, so GbEthernet just
need to fill emac_init function. I will remove the function
"ravb_emac_init_ex".

  Will the EMAC init methods differ so much as to we should provide 2
separate implementations?

[...]
quoted
quoted
quoted
+static void ravb_dmac_init_ex(struct net_device *ndev)
   Please no _ex suffixes -- reminds me of Windoze too much. :-)
OK. Will change it to ravb_device_init
   Ugh! Why not leave it named ravb_dmac_init()?
Please see [1] below and also planning to send another 10 small incremental patches,
So that it is clear to every one.
quoted
Regards,
Biju
quoted
quoted
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *info = priv->info;

 	/* Set AVB RX */
 	ravb_write(ndev,
@@ -530,6 +519,33 @@ static int ravb_dmac_init(struct net_device
*ndev)
quoted
quoted
quoted
 	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);
+}
+
+static int ravb_dmac_init(struct net_device *ndev) {
+	struct ravb_private *priv = netdev_priv(ndev);
+	const struct ravb_drv_data *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->ravb_ops->dmac_init(ndev);

 	/* Setting the control will start the AVB-DMAC process. */
 	ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_OPERATION); @@ -2018,6
+2034,8 @@ static const struct ravb_ops ravb_gen3_ops = {
 	.ring_free = ravb_ring_free_rx,
 	.ring_format = ravb_ring_format_rx,
 	.alloc_rx_desc = ravb_alloc_rx_desc,
+	.emac_init = ravb_emac_init_ex,
+	.dmac_init = ravb_dmac_init_ex,
   Hmm, why not also gen2?!
   The question remained unreplied?... :-/
ravb_ops for gen3 and gen2 same. But RZ/G2L have different function pointers[1].

As agreed on next patchset, we are avoiding indirection and it will be hw_ifo.
In that case there will be 1 entry in gen3 and 1 entry in gen2. Hope this clears
Your doubts.

[1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20210722141351.13668-18-biju.das.jz@bp.renesas.com/

Cheers,
Biju
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help