From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-25 07:02:03
The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC are
similar to the R-Car Ethernet AVB IP.
The Gigabit Ethernet IP consists of Ethernet controller (E-MAC), Internal
TCP/IP Offload Engine (TOE) and Dedicated Direct memory access controller
(DMAC).
With a few changes in the driver we can support both IPs.
This patch series aims to add factorisation code to support RZ/G2L SoC,
hardware feature bits for gPTP feature, Multiple irq feature and
optional reset support.
Ref:-
* https://lore.kernel.org/linux-renesas-soc/TYCPR01MB59334319695607A2683C1A5E86E59@TYCPR01MB5933.jpnprd01.prod.outlook.com/T/#t
Biju Das (13):
ravb: Remove the macros NUM_TX_DESC_GEN[23]
ravb: Add multi_irq to struct ravb_hw_info
ravb: Add no_ptp_cfg_active to struct ravb_hw_info
ravb: Add ptp_cfg_active to struct ravb_hw_info
ravb: Factorise ravb_ring_free function
ravb: Factorise ravb_ring_format function
ravb: Factorise ravb_ring_init function
ravb: Factorise ravb_rx function
ravb: Factorise ravb_adjust_link function
ravb: Factorise ravb_set_features
ravb: Factorise ravb_dmac_init function
ravb: Factorise ravb_emac_init function
ravb: Add reset support
drivers/net/ethernet/renesas/ravb.h | 23 +-
drivers/net/ethernet/renesas/ravb_main.c | 272 ++++++++++++++++-------
drivers/net/ethernet/renesas/ravb_ptp.c | 8 +-
3 files changed, 204 insertions(+), 99 deletions(-)
--
2.17.1
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-25 07:02:26
For addressing 4 bytes alignment restriction on transmission
buffer for R-Car Gen2 we use 2 descriptors whereas it is a single
descriptor for other cases.
Replace the macros NUM_TX_DESC_GEN[23] with magic number and
add a comment to explain it.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Suggested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
drivers/net/ethernet/renesas/ravb.h | 4 ----
drivers/net/ethernet/renesas/ravb_main.c | 8 ++++++--
2 files changed, 6 insertions(+), 6 deletions(-)
@@ -2160,8 +2160,12 @@ static int ravb_probe(struct platform_device *pdev)ndev->max_mtu=2048-(ETH_HLEN+VLAN_HLEN+ETH_FCS_LEN);ndev->min_mtu=ETH_MIN_MTU;-priv->num_tx_desc=info->aligned_tx?-NUM_TX_DESC_GEN2:NUM_TX_DESC_GEN3;+/* FIXME: R-Car Gen2 has 4byte alignment restriction for tx buffer+*Usetwodescriptortohandlesuchsituation.Firstdescriptorto+*handlealigneddatabufferandseconddescriptortohandlethe+*overflowdatabecauseofalignment.+*/+priv->num_tx_desc=info->aligned_tx?2:1;/* Set function */ndev->netdev_ops=&ravb_netdev_ops;
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-25 07:02:29
R-Car Gen3 supports separate interrupts for E-MAC and DMA queues,
whereas R-Car Gen2 and RZ/G2L have a single interrupt instead.
Add a multi_irq hw feature bit to struct ravb_hw_info to enable
this only for R-Car Gen3.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
drivers/net/ethernet/renesas/ravb.h | 1 +
drivers/net/ethernet/renesas/ravb_main.c | 22 ++++++++++++++--------
drivers/net/ethernet/renesas/ravb_ptp.c | 8 +++++---
3 files changed, 20 insertions(+), 11 deletions(-)
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-25 07:02:30
There are some H/W differences for the gPTP feature between
R-Car Gen3, R-Car Gen2, and RZ/G2L as below.
1) On R-Car Gen3, gPTP support is active in config mode.
2) On R-Car Gen2, gPTP support is not active in config mode.
3) RZ/G2L does not support the gPTP feature.
Add a ptp_cfg_active hw feature bit to struct ravb_hw_info for
supporting gPTP active in config mode for R-Car Gen3.
This patch also removes enum ravb_chip_id, chip_id from both
struct ravb_hw_info and struct ravb_private, as it is unused.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
drivers/net/ethernet/renesas/ravb.h | 8 +-------
drivers/net/ethernet/renesas/ravb_main.c | 12 +++++-------
2 files changed, 6 insertions(+), 14 deletions(-)
@@ -999,6 +993,7 @@ struct ravb_hw_info {unsignedtx_counters:1;/* E-MAC has TX counters */unsignedmulti_irqs:1;/* AVB-DMAC and E-MAC has multiple irqs */unsignedno_ptp_cfg_active:1;/* AVB-DMAC does not support gPTP active in config mode */+unsignedptp_cfg_active:1;/* AVB-DMAC has gPTP support active in config mode */};structravb_private{
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-25 07:02:30
The ravb_ring_init function uses an extended descriptor in RX for
R-Car and normal descriptor for RZ/G2L. Add a helper function
for RX ring buffer allocation to support later SoC.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
drivers/net/ethernet/renesas/ravb.h | 1 +
drivers/net/ethernet/renesas/ravb_main.c | 21 ++++++++++++++++-----
2 files changed, 17 insertions(+), 5 deletions(-)
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-25 07:02:31
R-Car uses extended descriptor in RX, whereas RZ/G2L uses normal
descriptor. Factorise ravb_ring_free function so that it can
support later SoC.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
drivers/net/ethernet/renesas/ravb.h | 1 +
drivers/net/ethernet/renesas/ravb_main.c | 47 +++++++++++++++---------
2 files changed, 31 insertions(+), 17 deletions(-)
@@ -216,31 +216,42 @@ static int ravb_tx_free(struct net_device *ndev, int q, bool free_txed_only)returnfree_num;}+staticvoidravb_rx_ring_free(structnet_device*ndev,intq)+{+structravb_private*priv=netdev_priv(ndev);+unsignedintring_size;+unsignedinti;++if(!priv->rx_ring[q])+return;++for(i=0;i<priv->num_rx_ring[q];i++){+structravb_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(structravb_ex_rx_desc)*+(priv->num_rx_ring[q]+1);+dma_free_coherent(ndev->dev.parent,ring_size,priv->rx_ring[q],+priv->rx_desc_dma[q]);+priv->rx_ring[q]=NULL;+}+/* Free skb's and DMA buffers for Ethernet AVB */staticvoidravb_ring_free(structnet_device*ndev,intq){structravb_private*priv=netdev_priv(ndev);+conststructravb_hw_info*info=priv->info;unsignedintnum_tx_desc=priv->num_tx_desc;unsignedintring_size;unsignedinti;-if(priv->rx_ring[q]){-for(i=0;i<priv->num_rx_ring[q];i++){-structravb_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(structravb_ex_rx_desc)*-(priv->num_rx_ring[q]+1);-dma_free_coherent(ndev->dev.parent,ring_size,priv->rx_ring[q],-priv->rx_desc_dma[q]);-priv->rx_ring[q]=NULL;-}+info->rx_ring_free(ndev,q);if(priv->tx_ring[q]){ravb_tx_free(ndev,q,false);
@@ -1937,6 +1948,7 @@ static int ravb_mdio_release(struct ravb_private *priv)}staticconststructravb_hw_inforavb_gen3_hw_info={+.rx_ring_free=ravb_rx_ring_free,.gstrings_stats=ravb_gstrings_stats,.gstrings_size=sizeof(ravb_gstrings_stats),.net_hw_features=NETIF_F_RXCSUM,
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-25 07:02:31
There are some H/W differences for the gPTP feature between
R-Car Gen3, R-Car Gen2, and RZ/G2L as below.
1) On R-Car Gen2, gPTP support is not active in config mode.
2) On R-Car Gen3, gPTP support is active in config mode.
3) RZ/G2L does not support the gPTP feature.
Add a no_ptp_cfg_active hw feature bit to struct ravb_hw_info for
handling gPTP for R-Car Gen2.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
drivers/net/ethernet/renesas/ravb.h | 1 +
drivers/net/ethernet/renesas/ravb_main.c | 20 ++++++++++++--------
2 files changed, 13 insertions(+), 8 deletions(-)
@@ -998,6 +998,7 @@ struct ravb_hw_info {unsignedinternal_delay:1;/* AVB-DMAC has internal delays */unsignedtx_counters:1;/* E-MAC has TX counters */unsignedmulti_irqs:1;/* AVB-DMAC and E-MAC has multiple irqs */+unsignedno_ptp_cfg_active:1;/* AVB-DMAC does not support gPTP active in config mode */};structravb_private{
@@ -1695,7 +1697,7 @@ static int ravb_close(struct net_device *ndev)ravb_write(ndev,0,TIC);/* Stop PTP Clock driver */-if(priv->chip_id==RCAR_GEN2)+if(info->no_ptp_cfg_active)ravb_ptp_stop(ndev);/* Set the config mode to stop the AVB-DMAC's processes */
@@ -1996,8 +1999,9 @@ static int ravb_set_gti(struct net_device *ndev)staticvoidravb_set_config_mode(structnet_device*ndev){structravb_private*priv=netdev_priv(ndev);+conststructravb_hw_info*info=priv->info;-if(priv->chip_id==RCAR_GEN2){+if(info->no_ptp_cfg_active){ravb_modify(ndev,CCC,CCC_OPC,CCC_OPC_CONFIG);/* Set CSEL value */ravb_modify(ndev,CCC,CCC_CSEL,CCC_CSEL_HPB);
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-25 07:02:32
The ravb_ring_format function uses an extended descriptor in RX
for R-Car compared to the normal descriptor for RZ/G2L. Factorise
RX ring buffer buildup to extend the support for later SoC.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
drivers/net/ethernet/renesas/ravb.h | 1 +
drivers/net/ethernet/renesas/ravb_main.c | 35 ++++++++++++++++--------
2 files changed, 24 insertions(+), 12 deletions(-)
@@ -282,25 +282,14 @@ static void ravb_ring_free(struct net_device *ndev, int q)priv->tx_skb[q]=NULL;}-/* Format skb and descriptor buffer for Ethernet AVB */-staticvoidravb_ring_format(structnet_device*ndev,intq)+staticvoidravb_rx_ring_format(structnet_device*ndev,intq){structravb_private*priv=netdev_priv(ndev);-unsignedintnum_tx_desc=priv->num_tx_desc;structravb_ex_rx_desc*rx_desc;-structravb_tx_desc*tx_desc;-structravb_desc*desc;unsignedintrx_ring_size=sizeof(*rx_desc)*priv->num_rx_ring[q];-unsignedinttx_ring_size=sizeof(*tx_desc)*priv->num_tx_ring[q]*-num_tx_desc;dma_addr_tdma_addr;unsignedinti;-priv->cur_rx[q]=0;-priv->cur_tx[q]=0;-priv->dirty_rx[q]=0;-priv->dirty_tx[q]=0;-memset(priv->rx_ring[q],0,rx_ring_size);/* Build RX ring buffer */for(i=0;i<priv->num_rx_ring[q];i++){
@@ -321,6 +310,26 @@ static void ravb_ring_format(struct net_device *ndev, int q)rx_desc=&priv->rx_ring[q][i];rx_desc->dptr=cpu_to_le32((u32)priv->rx_desc_dma[q]);rx_desc->die_dt=DT_LINKFIX;/* type */+}++/* Format skb and descriptor buffer for Ethernet AVB */+staticvoidravb_ring_format(structnet_device*ndev,intq)+{+structravb_private*priv=netdev_priv(ndev);+conststructravb_hw_info*info=priv->info;+unsignedintnum_tx_desc=priv->num_tx_desc;+structravb_tx_desc*tx_desc;+structravb_desc*desc;+unsignedinttx_ring_size=sizeof(*tx_desc)*priv->num_tx_ring[q]*+num_tx_desc;+unsignedinti;++priv->cur_rx[q]=0;+priv->cur_tx[q]=0;+priv->dirty_rx[q]=0;+priv->dirty_tx[q]=0;++info->rx_ring_format(ndev,q);memset(priv->tx_ring[q],0,tx_ring_size);/* Build TX ring buffer */
@@ -1949,6 +1958,7 @@ static int ravb_mdio_release(struct ravb_private *priv)staticconststructravb_hw_inforavb_gen3_hw_info={.rx_ring_free=ravb_rx_ring_free,+.rx_ring_format=ravb_rx_ring_format,.gstrings_stats=ravb_gstrings_stats,.gstrings_size=sizeof(ravb_gstrings_stats),.net_hw_features=NETIF_F_RXCSUM,
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-25 07:02:32
R-Car uses an extended descriptor in RX whereas, RZ/G2L uses
normal descriptor in RX. Factorise the ravb_rx function to
support the later SoC.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
drivers/net/ethernet/renesas/ravb.h | 1 +
drivers/net/ethernet/renesas/ravb_main.c | 14 ++++++++++++--
2 files changed, 13 insertions(+), 2 deletions(-)
@@ -562,8 +562,7 @@ static void ravb_rx_csum(struct sk_buff *skb)skb_trim(skb,skb->len-sizeof(__sum16));}-/* Packet receive function for Ethernet AVB */-staticboolravb_rx(structnet_device*ndev,int*quota,intq)+staticboolravb_rcar_rx(structnet_device*ndev,int*quota,intq){structravb_private*priv=netdev_priv(ndev);conststructravb_hw_info*info=priv->info;
@@ -677,6 +676,15 @@ static bool ravb_rx(struct net_device *ndev, int *quota, int q)returnboguscnt<=0;}+/* Packet receive function for Ethernet AVB */+staticboolravb_rx(structnet_device*ndev,int*quota,intq)+{+structravb_private*priv=netdev_priv(ndev);+conststructravb_hw_info*info=priv->info;++returninfo->receive(ndev,quota,q);+}+staticvoidravb_rcv_snd_disable(structnet_device*ndev){/* Disable TX and RX */
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-25 07:02:39
R-Car supports 100 and 1000 Mbps transfer speed whereas RZ/G2L
in addition support 10Mbps. Factorise ravb_adjust_link function
in order to support 10Mbps speed.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
drivers/net/ethernet/renesas/ravb.h | 1 +
drivers/net/ethernet/renesas/ravb_main.c | 5 ++++-
2 files changed, 5 insertions(+), 1 deletion(-)
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-25 07:02:47
The DMAC IP on the R-Car AVB module has different initialization
parameters for RCR, TGC, TCCR, RIC0, RIC2, and TIC compared to
DMAC IP on the RZ/G2L Gigabit Ethernet module. Factorise the
ravb_dmac_init function to support the later SoC.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
drivers/net/ethernet/renesas/ravb.h | 1 +
drivers/net/ethernet/renesas/ravb_main.c | 52 ++++++++++++++----------
2 files changed, 32 insertions(+), 21 deletions(-)
@@ -452,30 +452,10 @@ static void ravb_emac_init(struct net_device *ndev)ravb_write(ndev,ECSIPR_ICDIP|ECSIPR_MPDIP|ECSIPR_LCHNGIP,ECSIPR);}-/* Device init function for Ethernet AVB */-staticintravb_dmac_init(structnet_device*ndev)+staticvoidravb_rcar_dmac_init(structnet_device*ndev){structravb_private*priv=netdev_priv(ndev);conststructravb_hw_info*info=priv->info;-interror;--/* Set CONFIG mode */-error=ravb_config(ndev);-if(error)-returnerror;--error=ravb_ring_init(ndev,RAVB_BE);-if(error)-returnerror;-error=ravb_ring_init(ndev,RAVB_NC);-if(error){-ravb_ring_free(ndev,RAVB_BE);-returnerror;-}--/* Descriptor format */-ravb_ring_format(ndev,RAVB_BE);-ravb_ring_format(ndev,RAVB_NC);/* Set AVB RX */ravb_write(ndev,
@@ -502,6 +482,34 @@ static int ravb_dmac_init(struct net_device *ndev)ravb_write(ndev,RIC2_QFE0|RIC2_QFE1|RIC2_RFFE,RIC2);/* Frame transmitted, timestamp FIFO updated */ravb_write(ndev,TIC_FTE0|TIC_FTE1|TIC_TFUE,TIC);+}++/* Device init function for Ethernet AVB */+staticintravb_dmac_init(structnet_device*ndev)+{+structravb_private*priv=netdev_priv(ndev);+conststructravb_hw_info*info=priv->info;+interror;++/* Set CONFIG mode */+error=ravb_config(ndev);+if(error)+returnerror;++error=ravb_ring_init(ndev,RAVB_BE);+if(error)+returnerror;+error=ravb_ring_init(ndev,RAVB_NC);+if(error){+ravb_ring_free(ndev,RAVB_BE);+returnerror;+}++/* Descriptor format */+ravb_ring_format(ndev,RAVB_BE);+ravb_ring_format(ndev,RAVB_NC);++info->dmac_init(ndev);/* Setting the control will start the AVB-DMAC process. */ravb_modify(ndev,CCC,CCC_OPC,CCC_OPC_OPERATION);
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-25 07:02:49
The E-MAC IP on the R-Car AVB module has different initialization
parameters for RX frame size, duplex settings, different offset
for transfer speed setting and has magic packet detection support
compared to E-MAC on RZ/G2L Gigabit Ethernet module. Factorise
the ravb_emac_init function to support the later SoC.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
drivers/net/ethernet/renesas/ravb.h | 1 +
drivers/net/ethernet/renesas/ravb_main.c | 14 ++++++++++++--
2 files changed, 13 insertions(+), 2 deletions(-)
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-25 07:02:53
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(+)
@@ -2140,6 +2141,7 @@ static int ravb_probe(struct platform_device *pdev){structdevice_node*np=pdev->dev.of_node;conststructravb_hw_info*info;+structreset_control*rstc;structravb_private*priv;structnet_device*ndev;interror,irq,q;
@@ -2152,6 +2154,11 @@ static int ravb_probe(struct platform_device *pdev)return-EINVAL;}+rstc=devm_reset_control_get_optional_exclusive(&pdev->dev,NULL);+if(IS_ERR(rstc))+returndev_err_probe(&pdev->dev,PTR_ERR(rstc),+"failed to get cpg reset\n");+ndev=alloc_etherdev_mqs(sizeof(structravb_private),NUM_TX_QUEUE,NUM_RX_QUEUE);if(!ndev)
@@ -2162,6 +2169,7 @@ static int ravb_probe(struct platform_device *pdev)ndev->features=info->net_features;ndev->hw_features=info->net_hw_features;+reset_control_deassert(rstc);pm_runtime_enable(&pdev->dev);pm_runtime_get_sync(&pdev->dev);
@@ -2179,6 +2187,7 @@ static int ravb_probe(struct platform_device *pdev)priv=netdev_priv(ndev);priv->info=info;+priv->rstc=rstc;priv->ndev=ndev;priv->pdev=pdev;priv->num_tx_ring[RAVB_BE]=BE_TX_RING_SIZE;
@@ -2349,6 +2358,7 @@ static int ravb_probe(struct platform_device *pdev)pm_runtime_put(&pdev->dev);pm_runtime_disable(&pdev->dev);+reset_control_assert(rstc);returnerror;}
@@ -2374,6 +2384,7 @@ static int ravb_remove(struct platform_device *pdev)netif_napi_del(&priv->napi[RAVB_BE]);ravb_mdio_release(priv);pm_runtime_disable(&pdev->dev);+reset_control_assert(priv->rstc);free_netdev(ndev);platform_set_drvdata(pdev,NULL);
Hello:
This series was applied to netdev/net-next.git (refs/heads/master):
On Wed, 25 Aug 2021 08:01:41 +0100 you wrote:
The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC are
similar to the R-Car Ethernet AVB IP.
The Gigabit Ethernet IP consists of Ethernet controller (E-MAC), Internal
TCP/IP Offload Engine (TOE) and Dedicated Direct memory access controller
(DMAC).
[...]
For addressing 4 bytes alignment restriction on transmission
buffer for R-Car Gen2 we use 2 descriptors whereas it is a single
descriptor for other cases.
Replace the macros NUM_TX_DESC_GEN[23] with magic number and
add a comment to explain it.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Suggested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
@@ -2160,8 +2160,12 @@ static int ravb_probe(struct platform_device *pdev)ndev->max_mtu=2048-(ETH_HLEN+VLAN_HLEN+ETH_FCS_LEN);ndev->min_mtu=ETH_MIN_MTU;-priv->num_tx_desc=info->aligned_tx?-NUM_TX_DESC_GEN2:NUM_TX_DESC_GEN3;+/* FIXME: R-Car Gen2 has 4byte alignment restriction for tx buffer
Mhm, what are you going to fix here?
+ * Use two descriptor to handle such situation. First descriptor to
+ * handle aligned data buffer and second descriptor to handle the
+ * overflow data because of alignment.
+ */
+ priv->num_tx_desc = info->aligned_tx ? 2 : 1;
/* Set function */
ndev->netdev_ops = &ravb_netdev_ops;
Other than that:
Reviewed-by: Sergey Shtylyov <redacted>
[...]
MBR, Sergey
R-Car Gen3 supports separate interrupts for E-MAC and DMA queues,
whereas R-Car Gen2 and RZ/G2L have a single interrupt instead.
Add a multi_irq hw feature bit to struct ravb_hw_info to enable
So you have 'multi_irq' in the patch subject/description but 'multi_irqs'?
Not very consistent...
quoted hunk
this only for R-Car Gen3.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
drivers/net/ethernet/renesas/ravb.h | 1 +
drivers/net/ethernet/renesas/ravb_main.c | 22 ++++++++++++++--------
drivers/net/ethernet/renesas/ravb_ptp.c | 8 +++++---
3 files changed, 20 insertions(+), 11 deletions(-)
R-Car Gen3 supports separate interrupts for E-MAC and DMA queues,
whereas R-Car Gen2 and RZ/G2L have a single interrupt instead.
Add a multi_irq hw feature bit to struct ravb_hw_info to enable
So you have 'multi_irq' in the patch subject/description but 'multi_irqs'?
"in the patch diff", I meant to type.
Not very consistent...
quoted
this only for R-Car Gen3.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
There are some H/W differences for the gPTP feature between
R-Car Gen3, R-Car Gen2, and RZ/G2L as below.
1) On R-Car Gen2, gPTP support is not active in config mode.
2) On R-Car Gen3, gPTP support is active in config mode.
3) RZ/G2L does not support the gPTP feature.
Add a no_ptp_cfg_active hw feature bit to struct ravb_hw_info for
handling gPTP for R-Car Gen2.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
drivers/net/ethernet/renesas/ravb.h | 1 +
drivers/net/ethernet/renesas/ravb_main.c | 20 ++++++++++++--------
2 files changed, 13 insertions(+), 8 deletions(-)
@@ -998,6 +998,7 @@ struct ravb_hw_info {unsignedinternal_delay:1;/* AVB-DMAC has internal delays */unsignedtx_counters:1;/* E-MAC has TX counters */unsignedmulti_irqs:1;/* AVB-DMAC and E-MAC has multiple irqs */+unsignedno_ptp_cfg_active:1;/* AVB-DMAC does not support gPTP active in config mode */
Isn't this better to name it 'ptp_active_cfg' -- positive, instead of negative?
[...]
MBR, Sergey
There are some H/W differences for the gPTP feature between
R-Car Gen3, R-Car Gen2, and RZ/G2L as below.
1) On R-Car Gen3, gPTP support is active in config mode.
2) On R-Car Gen2, gPTP support is not active in config mode.
3) RZ/G2L does not support the gPTP feature.
Add a ptp_cfg_active hw feature bit to struct ravb_hw_info for
supporting gPTP active in config mode for R-Car Gen3.
Wait, we've just done this ion the previous patch!
quoted hunk
This patch also removes enum ravb_chip_id, chip_id from both
struct ravb_hw_info and struct ravb_private, as it is unused.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
drivers/net/ethernet/renesas/ravb.h | 8 +-------
drivers/net/ethernet/renesas/ravb_main.c | 12 +++++-------
2 files changed, 6 insertions(+), 14 deletions(-)
@@ -999,6 +993,7 @@ struct ravb_hw_info { unsigned tx_counters:1; /* E-MAC has TX counters */ unsigned multi_irqs:1; /* AVB-DMAC and E-MAC has multiple irqs */ unsigned no_ptp_cfg_active:1; /* AVB-DMAC does not support gPTP active in config mode */+ unsigned ptp_cfg_active:1; /* AVB-DMAC has gPTP support active in config mode */
Huh?
};
struct ravb_private {
[...]
quoted hunk
@@ -2216,7 +2213,7 @@ static int ravb_probe(struct platform_device *pdev) INIT_LIST_HEAD(&priv->ts_skb_list); /* Initialise PTP Clock driver */- if (info->chip_id != RCAR_GEN2)+ if (info->ptp_cfg_active) ravb_ptp_init(ndev, pdev);
What's that? Didn't you touch this lie in patch #3?
This seems lie a NAK bait... :-(
MBR, Sergey
R-Car uses extended descriptor in RX, whereas RZ/G2L uses normal
descriptor. Factorise ravb_ring_free function so that it can
support later SoC.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
@@ -216,31 +216,42 @@ static int ravb_tx_free(struct net_device *ndev, int q, bool free_txed_only)returnfree_num;}+staticvoidravb_rx_ring_free(structnet_device*ndev,intq)+{+structravb_private*priv=netdev_priv(ndev);+unsignedintring_size;+unsignedinti;++if(!priv->rx_ring[q])+return;++for(i=0;i<priv->num_rx_ring[q];i++){+structravb_ex_rx_desc*desc=&priv->rx_ring[q][i];++if(!dma_mapping_error(ndev->dev.parent,+le32_to_cpu(desc->dptr)))+dma_unmap_single(ndev->dev.parent,+le32_to_cpu(desc->dptr),+RX_BUF_SZ,+DMA_FROM_DEVICE);
I think we could reflow this argument list, so that it takes less lines...
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-26 06:20:23
Hi Sergei,
Thanks for the feedback.
Subject: Re: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct
ravb_hw_info
On 8/25/21 10:01 AM, Biju Das wrote:
quoted
There are some H/W differences for the gPTP feature between R-Car
Gen3, R-Car Gen2, and RZ/G2L as below.
1) On R-Car Gen3, gPTP support is active in config mode.
2) On R-Car Gen2, gPTP support is not active in config mode.
3) RZ/G2L does not support the gPTP feature.
Add a ptp_cfg_active hw feature bit to struct ravb_hw_info for
supporting gPTP active in config mode for R-Car Gen3.
Wait, we've just done this ion the previous patch!
quoted
This patch also removes enum ravb_chip_id, chip_id from both struct
ravb_hw_info and struct ravb_private, as it is unused.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
drivers/net/ethernet/renesas/ravb.h | 8 +-------
drivers/net/ethernet/renesas/ravb_main.c | 12 +++++-------
2 files changed, 6 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h
b/drivers/net/ethernet/renesas/ravb.h
index 9ecf1a8c3ca8..209e030935aa 100644
There are some H/W differences for the gPTP feature between R-Car
Gen3, R-Car Gen2, and RZ/G2L as below.
1) On R-Car Gen3, gPTP support is active in config mode.
2) On R-Car Gen2, gPTP support is not active in config mode.
3) RZ/G2L does not support the gPTP feature.
Add a ptp_cfg_active hw feature bit to struct ravb_hw_info for
supporting gPTP active in config mode for R-Car Gen3.
Wait, we've just done this ion the previous patch!
quoted
This patch also removes enum ravb_chip_id, chip_id from both struct
ravb_hw_info and struct ravb_private, as it is unused.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
drivers/net/ethernet/renesas/ravb.h | 8 +-------
drivers/net/ethernet/renesas/ravb_main.c | 12 +++++-------
2 files changed, 6 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h
b/drivers/net/ethernet/renesas/ravb.h
index 9ecf1a8c3ca8..209e030935aa 100644
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-26 10:34:50
Hi Sergei,
Subject: Re: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct
ravb_hw_info
On 26.08.2021 9:20, Biju Das wrote:
[...]
quoted
quoted
quoted
There are some H/W differences for the gPTP feature between R-Car
Gen3, R-Car Gen2, and RZ/G2L as below.
1) On R-Car Gen3, gPTP support is active in config mode.
2) On R-Car Gen2, gPTP support is not active in config mode.
3) RZ/G2L does not support the gPTP feature.
Add a ptp_cfg_active hw feature bit to struct ravb_hw_info for
supporting gPTP active in config mode for R-Car Gen3.
Wait, we've just done this ion the previous patch!
quoted
This patch also removes enum ravb_chip_id, chip_id from both struct
ravb_hw_info and struct ravb_private, as it is unused.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
drivers/net/ethernet/renesas/ravb.h | 8 +-------
drivers/net/ethernet/renesas/ravb_main.c | 12 +++++-------
2 files changed, 6 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h
b/drivers/net/ethernet/renesas/ravb.h
index 9ecf1a8c3ca8..209e030935aa 100644
@@ -999,6 +993,7 @@ struct ravb_hw_info { unsigned tx_counters:1; /* E-MAC has TX counters */ unsigned multi_irqs:1; /* AVB-DMAC and E-MAC has
multiple
quoted
quoted
irqs */
quoted
unsigned no_ptp_cfg_active:1; /* AVB-DMAC does not support
gPTP
quoted
quoted
active in config mode */
quoted
+ unsigned ptp_cfg_active:1; /* AVB-DMAC has gPTP support active in
config mode */
Huh?
quoted
};
struct ravb_private {
[...]
quoted
@@ -2216,7 +2213,7 @@ static int ravb_probe(struct platform_device
*pdev)
quoted
INIT_LIST_HEAD(&priv->ts_skb_list);
/* Initialise PTP Clock driver */
- if (info->chip_id != RCAR_GEN2)
+ if (info->ptp_cfg_active)
ravb_ptp_init(ndev, pdev);
What's that? Didn't you touch this lie in patch #3?
This seems lie a NAK bait... :-(
Please refer the original patch[1] which introduced gPTP support active
in config mode.
quoted
I am sure this will clear all your doubts.
It hasn't. Why do we need 2 bit fields (1 "positive" and 1 "negative")
for the same feature is beyond me.
The reason is mentioned in commit description, Do you agree 1, 2 and 3 mutually exclusive?
1) On R-Car Gen3, gPTP support is active in config mode.
2) On R-Car Gen2, gPTP support is not active in config mode.
3) RZ/G2L does not support the gPTP feature.
Regards,
Biju
There are some H/W differences for the gPTP feature between R-Car
Gen3, R-Car Gen2, and RZ/G2L as below.
1) On R-Car Gen3, gPTP support is active in config mode.
2) On R-Car Gen2, gPTP support is not active in config mode.
3) RZ/G2L does not support the gPTP feature.
Add a ptp_cfg_active hw feature bit to struct ravb_hw_info for
supporting gPTP active in config mode for R-Car Gen3.
Wait, we've just done this ion the previous patch!
quoted
This patch also removes enum ravb_chip_id, chip_id from both struct
ravb_hw_info and struct ravb_private, as it is unused.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
drivers/net/ethernet/renesas/ravb.h | 8 +-------
drivers/net/ethernet/renesas/ravb_main.c | 12 +++++-------
2 files changed, 6 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h
b/drivers/net/ethernet/renesas/ravb.h
index 9ecf1a8c3ca8..209e030935aa 100644
@@ -999,6 +993,7 @@ struct ravb_hw_info { unsigned tx_counters:1; /* E-MAC has TX counters */ unsigned multi_irqs:1; /* AVB-DMAC and E-MAC has
multiple
quoted
quoted
irqs */
quoted
unsigned no_ptp_cfg_active:1; /* AVB-DMAC does not support
gPTP
quoted
quoted
active in config mode */
quoted
+ unsigned ptp_cfg_active:1; /* AVB-DMAC has gPTP support active in
config mode */
Huh?
quoted
};
struct ravb_private {
[...]
quoted
@@ -2216,7 +2213,7 @@ static int ravb_probe(struct platform_device
*pdev)
quoted
INIT_LIST_HEAD(&priv->ts_skb_list);
/* Initialise PTP Clock driver */
- if (info->chip_id != RCAR_GEN2)
+ if (info->ptp_cfg_active)
ravb_ptp_init(ndev, pdev);
What's that? Didn't you touch this lie in patch #3?
This seems lie a NAK bait... :-(
Please refer the original patch[1] which introduced gPTP support active
in config mode.
quoted
I am sure this will clear all your doubts.
It hasn't. Why do we need 2 bit fields (1 "positive" and 1 "negative")
for the same feature is beyond me.
The reason is mentioned in commit description, Do you agree 1, 2 and 3 mutually exclusive?
1) On R-Car Gen3, gPTP support is active in config mode.
2) On R-Car Gen2, gPTP support is not active in config mode.
3) RZ/G2L does not support the gPTP feature.
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-26 10:52:38
Hi Sergei,
Subject: Re: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct
ravb_hw_info
On 26.08.2021 13:34, Biju Das wrote:
[...]
quoted
quoted
quoted
quoted
quoted
There are some H/W differences for the gPTP feature between R-Car
Gen3, R-Car Gen2, and RZ/G2L as below.
1) On R-Car Gen3, gPTP support is active in config mode.
2) On R-Car Gen2, gPTP support is not active in config mode.
3) RZ/G2L does not support the gPTP feature.
Add a ptp_cfg_active hw feature bit to struct ravb_hw_info for
supporting gPTP active in config mode for R-Car Gen3.
Wait, we've just done this ion the previous patch!
quoted
This patch also removes enum ravb_chip_id, chip_id from both
struct ravb_hw_info and struct ravb_private, as it is unused.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar
[off-list ref]
---
drivers/net/ethernet/renesas/ravb.h | 8 +-------
drivers/net/ethernet/renesas/ravb_main.c | 12 +++++-------
2 files changed, 6 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h
b/drivers/net/ethernet/renesas/ravb.h
index 9ecf1a8c3ca8..209e030935aa 100644
@@ -999,6 +993,7 @@ struct ravb_hw_info { unsigned tx_counters:1; /* E-MAC has TX counters */ unsigned multi_irqs:1; /* AVB-DMAC and E-MAC has
multiple
quoted
quoted
irqs */
quoted
unsigned no_ptp_cfg_active:1; /* AVB-DMAC does not support
gPTP
quoted
quoted
active in config mode */
quoted
+ unsigned ptp_cfg_active:1; /* AVB-DMAC has gPTP support
active in
quoted
quoted
quoted
quoted
config mode */
Huh?
quoted
};
struct ravb_private {
[...]
quoted
@@ -2216,7 +2213,7 @@ static int ravb_probe(struct platform_device
*pdev)
quoted
INIT_LIST_HEAD(&priv->ts_skb_list);
/* Initialise PTP Clock driver */
- if (info->chip_id != RCAR_GEN2)
+ if (info->ptp_cfg_active)
ravb_ptp_init(ndev, pdev);
What's that? Didn't you touch this lie in patch #3?
This seems lie a NAK bait... :-(
Please refer the original patch[1] which introduced gPTP support
active
in config mode.
quoted
I am sure this will clear all your doubts.
It hasn't. Why do we need 2 bit fields (1 "positive" and 1
"negative") for the same feature is beyond me.
The reason is mentioned in commit description, Do you agree 1, 2 and 3
mutually exclusive?
quoted
1) On R-Car Gen3, gPTP support is active in config mode.
2) On R-Car Gen2, gPTP support is not active in config mode.
3) RZ/G2L does not support the gPTP feature.
From: Sergei Shtylyov <hidden> Date: 2021-08-26 18:06:51
On 8/26/21 1:52 PM, Biju Das wrote:
[...]
quoted
quoted
quoted
quoted
quoted
quoted
There are some H/W differences for the gPTP feature between R-Car
Gen3, R-Car Gen2, and RZ/G2L as below.
1) On R-Car Gen3, gPTP support is active in config mode.
2) On R-Car Gen2, gPTP support is not active in config mode.
3) RZ/G2L does not support the gPTP feature.
Add a ptp_cfg_active hw feature bit to struct ravb_hw_info for
supporting gPTP active in config mode for R-Car Gen3.
Wait, we've just done this ion the previous patch!
quoted
This patch also removes enum ravb_chip_id, chip_id from both
struct ravb_hw_info and struct ravb_private, as it is unused.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar
[off-list ref]
---
drivers/net/ethernet/renesas/ravb.h | 8 +-------
drivers/net/ethernet/renesas/ravb_main.c | 12 +++++-------
2 files changed, 6 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h
b/drivers/net/ethernet/renesas/ravb.h
index 9ecf1a8c3ca8..209e030935aa 100644
@@ -999,6 +993,7 @@ struct ravb_hw_info { unsigned tx_counters:1; /* E-MAC has TX counters */ unsigned multi_irqs:1; /* AVB-DMAC and E-MAC has
multiple
quoted
quoted
irqs */
quoted
unsigned no_ptp_cfg_active:1; /* AVB-DMAC does not support
gPTP
quoted
quoted
active in config mode */
quoted
+ unsigned ptp_cfg_active:1; /* AVB-DMAC has gPTP support
active in
quoted
quoted
quoted
quoted
config mode */
Huh?
quoted
};
struct ravb_private {
[...]
quoted
@@ -2216,7 +2213,7 @@ static int ravb_probe(struct platform_device
*pdev)
quoted
INIT_LIST_HEAD(&priv->ts_skb_list);
/* Initialise PTP Clock driver */
- if (info->chip_id != RCAR_GEN2)
+ if (info->ptp_cfg_active)
ravb_ptp_init(ndev, pdev);
What's that? Didn't you touch this lie in patch #3?
This seems lie a NAK bait... :-(
Please refer the original patch[1] which introduced gPTP support
active
in config mode.
quoted
I am sure this will clear all your doubts.
It hasn't. Why do we need 2 bit fields (1 "positive" and 1
"negative") for the same feature is beyond me.
The reason is mentioned in commit description, Do you agree 1, 2 and 3
mutually exclusive?
quoted
1) On R-Car Gen3, gPTP support is active in config mode.
2) On R-Car Gen2, gPTP support is not active in config mode.
3) RZ/G2L does not support the gPTP feature.
No, (1) includes (2).
patch[1] is for supporting gPTP support active in config mode.
Yes.
Do you agree GAC register(gPTP active in Config) bit in AVB-DMAC mode register(CCC) present only in R-Car Gen3?
Yes.
But you feature naming is totally misguiding, nevertheless...
The ravb_ring_format function uses an extended descriptor in RX
for R-Car compared to the normal descriptor for RZ/G2L. Factorise
RX ring buffer buildup to extend the support for later SoC.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-08-26 19:09:49
On Thu, Aug 26, 2021 at 10:02:07PM +0300, Sergey Shtylyov wrote:
On 8/26/21 9:57 PM, Andrew Lunn wrote:
quoted
quoted
quoted
Do you agree GAC register(gPTP active in Config) bit in AVB-DMAC mode register(CCC) present only in R-Car Gen3?
Yes.
But you feature naming is totally misguiding, nevertheless...
It can still be changed.
Thank goodness, yea!
We have to live with the first version of this in the git history, but
we can add more patches fixing up whatever is broken in the unreviewed
code which got merged.
quoted
Just suggest a new name.
I'd prolly go with 'gptp' for the gPTP support and 'ccc_gac' for the gPTP working also in CONFIG mode
(CCC.GAC controls this feature).
Biju, please could you work on a couple of patches to change the names.
I also suggest you post further refactoring patches as RFC. We might
get a chance to review them then.
Andrew
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-26 19:34:05
Hi Sergei,
Subject: Re: [PATCH net-next 06/13] ravb: Factorise ravb_ring_format
function
On 8/25/21 10:01 AM, Biju Das wrote:
quoted
The ravb_ring_format function uses an extended descriptor in RX for
R-Car compared to the normal descriptor for RZ/G2L. Factorise RX ring
buffer buildup to extend the support for later SoC.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-26 19:37:40
Hi Andrew,
Thanks for the feedback.
Subject: Re: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct
ravb_hw_info
On Thu, Aug 26, 2021 at 10:02:07PM +0300, Sergey Shtylyov wrote:
quoted
On 8/26/21 9:57 PM, Andrew Lunn wrote:
quoted
quoted
quoted
Do you agree GAC register(gPTP active in Config) bit in AVB-DMAC
mode register(CCC) present only in R-Car Gen3?
quoted
quoted
quoted
Yes.
But you feature naming is totally misguiding, nevertheless...
It can still be changed.
Thank goodness, yea!
We have to live with the first version of this in the git history, but we
can add more patches fixing up whatever is broken in the unreviewed code
which got merged.
quoted
quoted
Just suggest a new name.
I'd prolly go with 'gptp' for the gPTP support and 'ccc_gac' for
the gPTP working also in CONFIG mode (CCC.GAC controls this feature).
Biju, please could you work on a couple of patches to change the names.
Yes. Will work on the patches to change the names as suggested.
I also suggest you post further refactoring patches as RFC. We might get a
chance to review them then.
Do you agree GAC register(gPTP active in Config) bit in AVB-DMAC
mode register(CCC) present only in R-Car Gen3?
quoted
quoted
quoted
Yes.
But you feature naming is totally misguiding, nevertheless...
It can still be changed.
Thank goodness, yea!
We have to live with the first version of this in the git history, but we
can add more patches fixing up whatever is broken in the unreviewed code
which got merged.
quoted
quoted
Just suggest a new name.
I'd prolly go with 'gptp' for the gPTP support and 'ccc_gac' for
the gPTP working also in CONFIG mode (CCC.GAC controls this feature).
Biju, please could you work on a couple of patches to change the names.
Yes. Will work on the patches to change the names as suggested.
TIA!
After some more thinking, 'no_gptp' seems to suit better for the 1st case
Might need to invert the checks tho...
[...]
The ravb_ring_init function uses an extended descriptor in RX for
R-Car and normal descriptor for RZ/G2L. Add a helper function
for RX ring buffer allocation to support later SoC.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Here as well...
[...]
Seems sane, so:
Reviewed-by: Sergey Shtylyov <redacted>
MBR, Sergey
R-Car uses an extended descriptor in RX whereas, RZ/G2L uses
normal descriptor in RX. Factorise the ravb_rx function to
support the later SoC.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-27 06:28:08
Hi Sergei,
Subject: Re: [PATCH net-next 08/13] ravb: Factorise ravb_rx function
On 8/25/21 10:01 AM, Biju Das wrote:
quoted
R-Car uses an extended descriptor in RX whereas, RZ/G2L uses normal
descriptor in RX. Factorise the ravb_rx function to support the later
SoC.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-27 06:36:18
Hi Sergei,
Thanks for the feedback.
Subject: Re: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct
ravb_hw_info
On 8/26/21 10:37 PM, Biju Das wrote:
[...]
quoted
quoted
quoted
quoted
quoted
quoted
Do you agree GAC register(gPTP active in Config) bit in AVB-DMAC
mode register(CCC) present only in R-Car Gen3?
quoted
quoted
quoted
Yes.
But you feature naming is totally misguiding, nevertheless...
It can still be changed.
Thank goodness, yea!
We have to live with the first version of this in the git history,
but we can add more patches fixing up whatever is broken in the
unreviewed code which got merged.
quoted
quoted
Just suggest a new name.
I'd prolly go with 'gptp' for the gPTP support and 'ccc_gac' for
the gPTP working also in CONFIG mode (CCC.GAC controls this feature).
Biju, please could you work on a couple of patches to change the names.
Yes. Will work on the patches to change the names as suggested.
TIA!
After some more thinking, 'no_gptp' seems to suit better for the 1st
case Might need to invert the checks tho...
OK, Will do with invert checks.
So just to conclude,
'no_gptp' and 'ccc_gac' are the suggested names changes for the previous patch
and current patch.
Cheers,
Biju
Do you agree GAC register(gPTP active in Config) bit in AVB-DMAC
mode register(CCC) present only in R-Car Gen3?
quoted
quoted
quoted
Yes.
But you feature naming is totally misguiding, nevertheless...
It can still be changed.
Thank goodness, yea!
We have to live with the first version of this in the git history,
but we can add more patches fixing up whatever is broken in the
unreviewed code which got merged.
quoted
quoted
Just suggest a new name.
I'd prolly go with 'gptp' for the gPTP support and 'ccc_gac' for
the gPTP working also in CONFIG mode (CCC.GAC controls this feature).
Biju, please could you work on a couple of patches to change the names.
Yes. Will work on the patches to change the names as suggested.
TIA!
After some more thinking, 'no_gptp' seems to suit better for the 1st
case Might need to invert the checks tho...
OK, Will do with invert checks.
So just to conclude,
'no_gptp' and 'ccc_gac' are the suggested names changes for the previous patch
and current patch.
Your patches have been merged already. Might try to encompass all gPTP
features with one patch (just a thought)...
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-27 15:56:04
Hi Sergei,
Thanks for the feedback.
Subject: Re: [PATCH net-next 04/13] ravb: Add ptp_cfg_active to struct
ravb_hw_info
On 27.08.2021 9:36, Biju Das wrote:
[...]
quoted
quoted
quoted
quoted
quoted
quoted
quoted
quoted
Do you agree GAC register(gPTP active in Config) bit in
AVB-DMAC
mode register(CCC) present only in R-Car Gen3?
quoted
quoted
quoted
Yes.
But you feature naming is totally misguiding, nevertheless...
It can still be changed.
Thank goodness, yea!
We have to live with the first version of this in the git history,
but we can add more patches fixing up whatever is broken in the
unreviewed code which got merged.
quoted
quoted
Just suggest a new name.
I'd prolly go with 'gptp' for the gPTP support and 'ccc_gac'
for the gPTP working also in CONFIG mode (CCC.GAC controls this
feature).
quoted
quoted
quoted
quoted
Biju, please could you work on a couple of patches to change the
names.
quoted
quoted
quoted
Yes. Will work on the patches to change the names as suggested.
TIA!
After some more thinking, 'no_gptp' seems to suit better for the
1st case Might need to invert the checks tho...
OK, Will do with invert checks.
So just to conclude,
'no_gptp' and 'ccc_gac' are the suggested names changes for the
previous patch and current patch.
Your patches have been merged already. Might try to encompass all
gPTP features with one patch (just a thought)...
OK, in that case it will be taken care in next RFC patch set.
Regards,
Biju
How about ravb_set_features_rcar() or s/th alike?
[...]
Other than that:
Reviewed-by: Sergey Shtylyov <redacted>
Let's see the TOC code now...
MBR, Sergey
The DMAC IP on the R-Car AVB module has different initialization
parameters for RCR, TGC, TCCR, RIC0, RIC2, and TIC compared to
DMAC IP on the RZ/G2L Gigabit Ethernet module. Factorise the
ravb_dmac_init function to support the later SoC.
Couldn't we resolve these differencies like the sh_eth driver does,
by adding the register values into the *struct* ravb_hw_info?
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
The E-MAC IP on the R-Car AVB module has different initialization
parameters for RX frame size, duplex settings, different offset
for transfer speed setting and has magic packet detection support
compared to E-MAC on RZ/G2L Gigabit Ethernet module. Factorise
the ravb_emac_init function to support the later SoC.
Again, couldn't we resolve these differencies like the sh_eth driver does,
by adding the register values into the *struct* ravb_hw_info?
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
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>
Is it possible to get into/out of reset in open()/close() methods?
Otherwise, looks good (I'm not much into reset h/w)
Reviewed-by: Sergey Shtylyov <redacted>
MBR, Sergey
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-28 09:21:07
-----Original Message-----
From: Sergey Shtylyov <redacted>
Sent: 27 August 2021 20:17
To: Biju Das <biju.das.jz@bp.renesas.com>; David S. Miller
[off-list ref]; Jakub Kicinski [off-list ref]
Cc: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>;
Andrew Lunn [off-list ref]; Sergei Shtylyov [off-list ref];
Geert Uytterhoeven [off-list ref]; Adam Ford
[off-list ref]; Yoshihiro Shimoda
[off-list ref]; netdev@vger.kernel.org; linux-renesas-
soc@vger.kernel.org; Chris Paterson [off-list ref]; Biju
Das [off-list ref]
Subject: Re: [PATCH net-next 10/13] ravb: Factorise ravb_set_features
On 8/25/21 10:01 AM, Biju Das wrote:
quoted
RZ/G2L supports HW checksum on RX and TX whereas R-Car supports on RX.
Factorise ravb_set_features to support this feature.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
drivers/net/ethernet/renesas/ravb.h | 1 +
drivers/net/ethernet/renesas/ravb_main.c | 15 +++++++++++++--
2 files changed, 14 insertions(+), 2 deletions(-)
spin_unlock_irqrestore(&priv->lock, flags); }
-static int ravb_set_features(struct net_device *ndev,
- netdev_features_t features)
+static int ravb_set_features_rx_csum(struct net_device *ndev,
+ netdev_features_t features)
How about ravb_set_features_rcar() or s/th alike?
What about
ravb_rcar_set_features_csum()?
and
ravb_rgeth_set_features_csum()?
If you are ok with this name change I will incorporate this changes in next - RFC patchset?
If you still want ravb_set_features_rcar() and ravb_set_features_rgeth(), I am ok with that as well.
Please let me know, which name you like.
Regards,
Biju
[...]
Other than that:
Reviewed-by: Sergey Shtylyov <redacted>
Let's see the TOC code now...
MBR, Sergey
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-28 09:28:55
Hi Sergei,
Thanks for the feedback.
Subject: Re: [PATCH net-next 11/13] ravb: Factorise ravb_dmac_init
function
On 8/25/21 10:01 AM, Biju Das wrote:
quoted
The DMAC IP on the R-Car AVB module has different initialization
parameters for RCR, TGC, TCCR, RIC0, RIC2, and TIC compared to DMAC IP
on the RZ/G2L Gigabit Ethernet module. Factorise the ravb_dmac_init
function to support the later SoC.
Couldn't we resolve these differencies like the sh_eth driver does, by
adding the register values into the *struct* ravb_hw_info?
I will evaluate your proposal in terms of code size and data size
And with the current code and share the details in next RFC patchset
for supporting RZ/G2L with dmac_init function.
Based on the RFC discussion, we can conclude it.
Currently by looking at your proposal, I am seeing duplication of
Data in R-Car Gen3 and R-Car Gen2.
If statement for adding RIC3 register for RZ/G2L, which involves
Exposing another hwinfo bit.
Regards,
Biju
quoted
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-28 09:34:30
Hi Sergei,
Thanks for the feedback.
Subject: Re: [PATCH net-next 12/13] ravb: Factorise ravb_emac_init
function
On 8/25/21 10:01 AM, Biju Das wrote:
quoted
The E-MAC IP on the R-Car AVB module has different initialization
parameters for RX frame size, duplex settings, different offset for
transfer speed setting and has magic packet detection support compared
to E-MAC on RZ/G2L Gigabit Ethernet module. Factorise the
ravb_emac_init function to support the later SoC.
Again, couldn't we resolve these differencies like the sh_eth driver
does, by adding the register values into the *struct* ravb_hw_info?
I will evaluate your proposal in terms of code size and data size
And with the current code and share the details in next RFC patchset
for supporting RZ/G2L with emac_init function.
Based on the RFC discussion, we can conclude it.
Currently by looking at your proposal, I am seeing duplication of
Data in R-Car Gen3 and R-Car Gen2.
Multiple if statement for handling duplex, initialising CSR0(Checksum operating mode register),
CXR31(In-band status register)
Regards,
Biju
quoted
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-28 09:41:36
Hi Sergei,
Thanks for the feedback.
Subject: Re: [PATCH net-next 13/13] ravb: Add reset support
On 8/25/21 10:01 AM, Biju Das wrote:
quoted
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>
Is it possible to get into/out of reset in open()/close() methods?
No, Reason, Normally reset will be called
ravb_mdio_release(priv);
pm_runtime_disable(&pdev->dev);
reset_control_assert(priv->rstc);
After reset assert, We should not access any RAVB registers, otherwise system will hang.
There is a high chance that other users(for eg:- mdio) may access ravb registers and system hangs.
Regards,
Biju
Otherwise, looks good (I'm not much into reset h/w)
Reviewed-by: Sergey Shtylyov <redacted>
MBR, Sergey
spin_unlock_irqrestore(&priv->lock, flags); }
-static int ravb_set_features(struct net_device *ndev,
- netdev_features_t features)
+static int ravb_set_features_rx_csum(struct net_device *ndev,
+ netdev_features_t features)
How about ravb_set_features_rcar() or s/th alike?
What about
ravb_rcar_set_features_csum()?
and
ravb_rgeth_set_features_csum()?
>
If you are ok with this name change I will incorporate this changes in next - RFC patchset?
If you still want ravb_set_features_rcar() and ravb_set_features_rgeth(), I am ok with that as well.
Please let me know, which name you like.
Looking back at sh_eth, my variant seems to fit better...