From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-02 10:27:07
The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC are
similar to the R-Car Ethernet AVB IP.
The Gigabit Ethernet IP consists of Ethernet controller (E-MAC), Internal
TCP/IP Offload Engine (TOE) and Dedicated Direct memory access controller
(DMAC).
With a few changes in the driver we can support both IPs.
Currently a runtime decision based on the chip type is used to distinguish
the HW differences between the SoC families.
This patch series is in preparation for supporting the RZ/G2L SoC by
replacing driver data chip type with struct ravb_hw_info by moving chip
type to it and also adding gstrings_stats, gstrings_size, net_hw_features,
net_features, num_gstat_queue, num_tx_desc, stats_len, skb_sz variables to
it. This patch also adds the feature bit for {RX, TX} clock internal
delays and TX Drop counters HW features found on R-Car Gen3 to struct
ravb_hw_info.
This patch series is based on net-next.
v1->v2:
* Replaced driver data chip type with struct ravb_hw_info
* Added gstrings_stats, gstrings_size, net_hw_features, net_features,
num_gstat_queue, num_tx_desc, stats_len, skb_sz to struct ravb_hw_info
* Added internal_delay and tx_drop_cntrs hw feature bit to struct ravb_hw_info
RFC->V1
* Incorporated feedback from Andrew, Sergei, Geert and Prabhakar
* https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=515525
Biju Das (8):
ravb: Add struct ravb_hw_info to driver data
ravb: Add skb_sz to struct ravb_hw_info
ravb: Add num_gstat_queue to struct ravb_hw_info
ravb: Add stats_len to struct ravb_hw_info
ravb: Add gstrings_stats and gstrings_size to struct ravb_hw_info
ravb: Add net_features and net_hw_features to struct ravb_hw_info
ravb: Add internal delay hw feature to struct ravb_hw_info
ravb: Add tx_drop_cntrs to struct ravb_hw_info
drivers/net/ethernet/renesas/ravb.h | 18 +++++
drivers/net/ethernet/renesas/ravb_main.c | 91 ++++++++++++++++--------
2 files changed, 80 insertions(+), 29 deletions(-)
--
2.17.1
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-02 10:27:13
The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC are
similar to the R-Car Ethernet AVB IP. With a few changes in the driver we
can support both IPs.
Currently a runtime decision based on the chip type is used to distinguish
the HW differences between the SoC families.
The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2 and
RZ/G2L it is 2. For cases like this it is better to select the number of
TX descriptors by using a structure with a value, rather than a runtime
decision based on the chip type.
This patch adds the num_tx_desc variable to struct ravb_hw_info and also
replaces the driver data chip type with struct ravb_hw_info by moving chip
type to it.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it smaller patch
and provided detailed description.
---
drivers/net/ethernet/renesas/ravb.h | 7 +++++
drivers/net/ethernet/renesas/ravb_main.c | 38 +++++++++++++++---------
2 files changed, 31 insertions(+), 14 deletions(-)
@@ -2058,9 +2068,9 @@ static int ravb_probe(struct platform_device *pdev)pm_runtime_enable(&pdev->dev);pm_runtime_get_sync(&pdev->dev);-chip_id=(enumravb_chip_id)of_device_get_match_data(&pdev->dev);+info=of_device_get_match_data(&pdev->dev);-if(chip_id==RCAR_GEN3)+if(info->chip_id==RCAR_GEN3)irq=platform_get_irq_byname(pdev,"ch22");elseirq=platform_get_irq(pdev,0);
@@ -2073,6 +2083,7 @@ static int ravb_probe(struct platform_device *pdev)SET_NETDEV_DEV(ndev,&pdev->dev);priv=netdev_priv(ndev);+priv->info=info;priv->ndev=ndev;priv->pdev=pdev;priv->num_tx_ring[RAVB_BE]=BE_TX_RING_SIZE;
@@ -2099,7 +2110,7 @@ static int ravb_probe(struct platform_device *pdev)priv->avb_link_active_low=of_property_read_bool(np,"renesas,ether-link-active-low");-if(chip_id==RCAR_GEN3){+if(info->chip_id==RCAR_GEN3){irq=platform_get_irq_byname(pdev,"ch24");if(irq<0){error=irq;
@@ -2124,7 +2135,7 @@ static int ravb_probe(struct platform_device *pdev)}}-priv->chip_id=chip_id;+priv->chip_id=info->chip_id;priv->clk=devm_clk_get(&pdev->dev,NULL);if(IS_ERR(priv->clk)){
@@ -2142,8 +2153,7 @@ static int ravb_probe(struct platform_device *pdev)ndev->max_mtu=2048-(ETH_HLEN+VLAN_HLEN+ETH_FCS_LEN);ndev->min_mtu=ETH_MIN_MTU;-priv->num_tx_desc=chip_id==RCAR_GEN2?-NUM_TX_DESC_GEN2:NUM_TX_DESC_GEN3;+priv->num_tx_desc=info->num_tx_desc;/* Set function */ndev->netdev_ops=&ravb_netdev_ops;
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-02 10:27:19
The maximum descriptor size that can be specified on the reception side for
R-Car is 2048 bytes, whereas for RZ/G2L it is 8096.
Add the skb_size variable to struct ravb_hw_info for allocating different
skb buffer sizes for R-Car and RZ/G2L using the netdev_alloc_skb function.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it smaller patch
and provided detailed description.
---
drivers/net/ethernet/renesas/ravb.h | 1 +
drivers/net/ethernet/renesas/ravb_main.c | 10 ++++++----
2 files changed, 7 insertions(+), 4 deletions(-)
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-02 10:27:23
The number of queues used in retrieving device stats for R-Car is 2,
whereas for RZ/G2L it is 1.
Add the num_gstat_queue variable to struct ravb_hw_info, to add subsequent
SoCs without any code changes to the ravb_get_ethtool_stats function.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it smaller patch
and provided detailed description.
---
drivers/net/ethernet/renesas/ravb.h | 1 +
drivers/net/ethernet/renesas/ravb_main.c | 5 ++++-
2 files changed, 5 insertions(+), 1 deletion(-)
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-02 10:27:28
R-Car provides 30 device stats, whereas RZ/G2L provides only 15. In
addition, RZ/G2L has stats "rx_queue_0_csum_offload_errors" instead of
"rx_queue_0_missed_errors".
Replace RAVB_STATS_LEN macro with a structure variable stats_len to
struct ravb_hw_info, to support subsequent SoCs without any code changes
to the ravb_get_sset_count function.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it smaller patch
and provided detailed description.
---
drivers/net/ethernet/renesas/ravb.h | 1 +
drivers/net/ethernet/renesas/ravb_main.c | 9 ++++++---
2 files changed, 7 insertions(+), 3 deletions(-)
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-02 10:27:34
The device stats strings for R-Car and RZ/G2L are different.
R-Car provides 30 device stats, whereas RZ/G2L provides only 15. In
addition, RZ/G2L has stats "rx_queue_0_csum_offload_errors" instead of
"rx_queue_0_missed_errors".
Add structure variables gstrings_stats and gstrings_size to struct
ravb_hw_info, so that subsequent SoCs can be added without any code
changes in the ravb_get_strings function.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it smaller patch
and provided detailed description.
---
drivers/net/ethernet/renesas/ravb.h | 2 ++
drivers/net/ethernet/renesas/ravb_main.c | 9 ++++++++-
2 files changed, 10 insertions(+), 1 deletion(-)
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-02 10:27:43
On R-Car the checksum calculation on RX frames is done by the E-MAC
module, whereas on RZ/G2L it is done by the TOE.
TOE calculates the checksum of received frames from E-MAC and outputs it to
DMAC. TOE also calculates the checksum of transmission frames from DMAC and
outputs it E-MAC.
Add net_features and net_hw_features to struct ravb_hw_info, to support
subsequent SoCs without any code changes in the ravb_probe function.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it smaller patch
and provided detailed description.
---
drivers/net/ethernet/renesas/ravb.h | 2 ++
drivers/net/ethernet/renesas/ravb_main.c | 12 ++++++++----
2 files changed, 10 insertions(+), 4 deletions(-)
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-02 10:27:47
R-Car Gen3 supports TX and RX clock internal delay modes, whereas R-Car
Gen2 and RZ/G2L do not support it.
Add an internal_delay hw feature bit to struct ravb_hw_info to enable this
only for R-Car Gen3.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it smaller patch
and provided detailed description.
---
drivers/net/ethernet/renesas/ravb.h | 3 +++
drivers/net/ethernet/renesas/ravb_main.c | 6 ++++--
2 files changed, 7 insertions(+), 2 deletions(-)
@@ -2362,6 +2363,7 @@ static int __maybe_unused ravb_resume(struct device *dev){structnet_device*ndev=dev_get_drvdata(dev);structravb_private*priv=netdev_priv(ndev);+conststructravb_hw_info*info=priv->info;intret=0;/* If WoL is enabled set reset mode to rearm the WoL logic */
@@ -2384,7 +2386,7 @@ static int __maybe_unused ravb_resume(struct device *dev)/* Request GTI loading */ravb_modify(ndev,GCCR,GCCR_LTI,GCCR_LTI);-if(priv->chip_id!=RCAR_GEN2)+if(info->internal_delay)ravb_set_delay_mode(ndev);/* Restore descriptor base address table */
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-02 10:27:54
The register for retrieving TX drop counters is present only on R-Car Gen3
and RZ/G2L; it is not present on R-Car Gen2.
Add the tx_drop_cntrs hw feature bit to struct ravb_hw_info, to enable this
feature specifically for R-Car Gen3 now and later extend it to RZ/G2L.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it smaller patch
and provided detailed description.
---
drivers/net/ethernet/renesas/ravb.h | 1 +
drivers/net/ethernet/renesas/ravb_main.c | 4 +++-
2 files changed, 4 insertions(+), 1 deletion(-)
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-08-02 15:03:11
On Mon, Aug 02, 2021 at 11:26:47AM +0100, Biju Das wrote:
The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC are
similar to the R-Car Ethernet AVB IP. With a few changes in the driver we
can support both IPs.
Currently a runtime decision based on the chip type is used to distinguish
the HW differences between the SoC families.
The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2 and
RZ/G2L it is 2. For cases like this it is better to select the number of
TX descriptors by using a structure with a value, rather than a runtime
decision based on the chip type.
This patch adds the num_tx_desc variable to struct ravb_hw_info and also
replaces the driver data chip type with struct ravb_hw_info by moving chip
type to it.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Hi Biju
This is better. A lot clearer what is going on. I personally would of
done the num_tx_desc change as a separate patch, but this is O.K.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-08-02 15:08:50
On Mon, Aug 02, 2021 at 11:26:48AM +0100, Biju Das wrote:
The maximum descriptor size that can be specified on the reception side for
R-Car is 2048 bytes, whereas for RZ/G2L it is 8096.
Add the skb_size variable to struct ravb_hw_info for allocating different
skb buffer sizes for R-Car and RZ/G2L using the netdev_alloc_skb function.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-08-02 15:09:27
On Mon, Aug 02, 2021 at 11:26:49AM +0100, Biju Das wrote:
The number of queues used in retrieving device stats for R-Car is 2,
whereas for RZ/G2L it is 1.
Add the num_gstat_queue variable to struct ravb_hw_info, to add subsequent
SoCs without any code changes to the ravb_get_ethtool_stats function.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-08-02 15:11:34
On Mon, Aug 02, 2021 at 11:26:51AM +0100, Biju Das wrote:
The device stats strings for R-Car and RZ/G2L are different.
R-Car provides 30 device stats, whereas RZ/G2L provides only 15. In
addition, RZ/G2L has stats "rx_queue_0_csum_offload_errors" instead of
"rx_queue_0_missed_errors".
Add structure variables gstrings_stats and gstrings_size to struct
ravb_hw_info, so that subsequent SoCs can be added without any code
changes in the ravb_get_strings function.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-08-02 15:12:38
On Mon, Aug 02, 2021 at 11:26:52AM +0100, Biju Das wrote:
On R-Car the checksum calculation on RX frames is done by the E-MAC
module, whereas on RZ/G2L it is done by the TOE.
TOE calculates the checksum of received frames from E-MAC and outputs it to
DMAC. TOE also calculates the checksum of transmission frames from DMAC and
outputs it E-MAC.
Add net_features and net_hw_features to struct ravb_hw_info, to support
subsequent SoCs without any code changes in the ravb_probe function.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-08-02 15:13:38
On Mon, Aug 02, 2021 at 11:26:53AM +0100, Biju Das wrote:
R-Car Gen3 supports TX and RX clock internal delay modes, whereas R-Car
Gen2 and RZ/G2L do not support it.
Add an internal_delay hw feature bit to struct ravb_hw_info to enable this
only for R-Car Gen3.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-08-02 15:14:26
On Mon, Aug 02, 2021 at 11:26:54AM +0100, Biju Das wrote:
The register for retrieving TX drop counters is present only on R-Car Gen3
and RZ/G2L; it is not present on R-Car Gen2.
Add the tx_drop_cntrs hw feature bit to struct ravb_hw_info, to enable this
feature specifically for R-Car Gen3 now and later extend it to RZ/G2L.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
From: Sergei Shtylyov <hidden> Date: 2021-08-02 20:42:23
On 8/2/21 1:26 PM, Biju Das wrote:
quoted hunk
The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC are
similar to the R-Car Ethernet AVB IP. With a few changes in the driver we
can support both IPs.
Currently a runtime decision based on the chip type is used to distinguish
the HW differences between the SoC families.
The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2 and
RZ/G2L it is 2. For cases like this it is better to select the number of
TX descriptors by using a structure with a value, rather than a runtime
decision based on the chip type.
This patch adds the num_tx_desc variable to struct ravb_hw_info and also
replaces the driver data chip type with struct ravb_hw_info by moving chip
type to it.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it smaller patch
and provided detailed description.
---
drivers/net/ethernet/renesas/ravb.h | 7 +++++
drivers/net/ethernet/renesas/ravb_main.c | 38 +++++++++++++++---------
2 files changed, 31 insertions(+), 14 deletions(-)
From: Sergei Shtylyov <hidden> Date: 2021-08-02 20:55:03
On 8/2/21 1:26 PM, Biju Das wrote:
quoted hunk
The maximum descriptor size that can be specified on the reception side for
R-Car is 2048 bytes, whereas for RZ/G2L it is 8096.
Add the skb_size variable to struct ravb_hw_info for allocating different
skb buffer sizes for R-Car and RZ/G2L using the netdev_alloc_skb function.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it smaller patch
and provided detailed description.
---
drivers/net/ethernet/renesas/ravb.h | 1 +
drivers/net/ethernet/renesas/ravb_main.c | 10 ++++++----
2 files changed, 7 insertions(+), 4 deletions(-)
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-03 05:57:33
Hi Sergei,
Thanks for the feedback.
Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to
driver data
On 8/2/21 1:26 PM, Biju Das wrote:
quoted
The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC
are similar to the R-Car Ethernet AVB IP. With a few changes in the
driver we can support both IPs.
Currently a runtime decision based on the chip type is used to
distinguish the HW differences between the SoC families.
The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2
and RZ/G2L it is 2. For cases like this it is better to select the
number of TX descriptors by using a structure with a value, rather
than a runtime decision based on the chip type.
This patch adds the num_tx_desc variable to struct ravb_hw_info and
also replaces the driver data chip type with struct ravb_hw_info by
moving chip type to it.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it
I think this is rather the driver's choice, than the h/w feature...
Perhaps a rename would help with that? :-)
It is consistent with current naming convention used by the driver. NUM_TX_DESC macro is replaced by num_tx_desc and the below run time decision based on chip type for H/W configuration for Gen2/Gen3 is replaced by info->num_tx_desc.
priv->num_tx_desc = chip_id == RCAR_GEN2 ? NUM_TX_DESC_GEN2 : NUM_TX_DESC_GEN3;
Please let me know, if I am missing anything,
Previously there is a suggestion to change the generic struct ravb_driver_data(which holds driver differences and HW features) with struct ravb_hw_info.
Regards,
Biju
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-03 06:36:52
Hi Sergei,
Subject: RE: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to
driver data
Hi Sergei,
Thanks for the feedback.
quoted
Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to
driver data
On 8/2/21 1:26 PM, Biju Das wrote:
quoted
The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC
are similar to the R-Car Ethernet AVB IP. With a few changes in the
driver we can support both IPs.
Currently a runtime decision based on the chip type is used to
distinguish the HW differences between the SoC families.
The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car
Gen2 and RZ/G2L it is 2. For cases like this it is better to select
the number of TX descriptors by using a structure with a value,
rather than a runtime decision based on the chip type.
This patch adds the num_tx_desc variable to struct ravb_hw_info and
also replaces the driver data chip type with struct ravb_hw_info by
moving chip type to it.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it
I think this is rather the driver's choice, than the h/w feature...
Perhaps a rename would help with that? :-)
It is consistent with current naming convention used by the driver.
NUM_TX_DESC macro is replaced by num_tx_desc.
So the name should be ok.
Indeed we are agreed to add function pointers to struct ravb_hw_info to avoid another level of indirection.
If the concern is related to duplication of data(ie,priv->num_tx_desc vs info->num_tx_desc)
I have a plan to remove priv->num_tx_desc with info->num_tx_desc later.
Regards,
Biju
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-03 07:13:30
Hi Sergei,
Thanks for the feedback.
Subject: Re: [PATCH net-next v2 2/8] ravb: Add skb_sz to struct
ravb_hw_info
On 8/2/21 1:26 PM, Biju Das wrote:
quoted
The maximum descriptor size that can be specified on the reception
side for R-Car is 2048 bytes, whereas for RZ/G2L it is 8096.
Add the skb_size variable to struct ravb_hw_info for allocating
different skb buffer sizes for R-Car and RZ/G2L using the
netdev_alloc_skb function.
quoted
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it
Bad naming -- refers to software ISO hatdware, I suggest max_rx_len or
s/th of that sort.
From the api description
* netdev_alloc_skb - allocate an skbuff for rx on a specific device
* @length: length to allocate
Since it allocates skbuff, I thought skb_sz (size of skb buffer) is a good name.
Is there any restriction in Linux, not to use skb_sz because of
"software ISO hardware" as you mentioned?
I may have chosen bad name because of this restriction.
Please correct me, if that is the case.
Regards,
Biju
From: Sergei Shtylyov <hidden> Date: 2021-08-03 18:21:58
Hello!
On 8/2/21 1:26 PM, Biju Das wrote:
The number of queues used in retrieving device stats for R-Car is 2,
whereas for RZ/G2L it is 1.
Mhm, how many RX queues are on your platform, 1? Then we don't need so specific name, just num_rx_queue.
Add the num_gstat_queue variable to struct ravb_hw_info, to add subsequent
SoCs without any code changes to the ravb_get_ethtool_stats function.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
From: Sergei Shtylyov <hidden> Date: 2021-08-03 18:35:57
On 8/2/21 1:26 PM, Biju Das wrote:
R-Car provides 30 device stats, whereas RZ/G2L provides only 15. In
addition, RZ/G2L has stats "rx_queue_0_csum_offload_errors" instead of
"rx_queue_0_missed_errors".
Replace RAVB_STATS_LEN macro with a structure variable stats_len to
struct ravb_hw_info, to support subsequent SoCs without any code changes
to the ravb_get_sset_count function.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
[...]
Finally a patch that I can agree with. :-)
Reviewed-by: ergei Shtylyov <redacted>
MBR, Sergei
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-03 18:47:49
Hi Sergei,
Subject: Re: [PATCH net-next v2 4/8] ravb: Add stats_len to struct
ravb_hw_info
On 8/2/21 1:26 PM, Biju Das wrote:
quoted
R-Car provides 30 device stats, whereas RZ/G2L provides only 15. In
addition, RZ/G2L has stats "rx_queue_0_csum_offload_errors" instead of
"rx_queue_0_missed_errors".
Replace RAVB_STATS_LEN macro with a structure variable stats_len to
struct ravb_hw_info, to support subsequent SoCs without any code
changes to the ravb_get_sset_count function.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
[...]
Finally a patch that I can agree with. :-)
Reviewed-by: ergei Shtylyov <redacted>
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-03 19:13:45
Hi Sergei,
Thanks for the feedback.
Subject: Re: [PATCH net-next v2 3/8] ravb: Add num_gstat_queue to struct
ravb_hw_info
Hello!
On 8/2/21 1:26 PM, Biju Das wrote:
quoted
The number of queues used in retrieving device stats for R-Car is 2,
whereas for RZ/G2L it is 1.
Mhm, how many RX queues are on your platform, 1? Then we don't need so
specific name, just num_rx_queue.
There are 2 RX queues, but we provide only device stats information from first queue.
R-Car = 2x15 = 30 device stats
RZ/G2L = 1x15 = 15 device stats.
Cheers,
Biju
quoted
Add the num_gstat_queue variable to struct ravb_hw_info, to add
subsequent SoCs without any code changes to the ravb_get_ethtool_stats
function.
quoted
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
From: Sergei Shtylyov <hidden> Date: 2021-08-03 19:20:31
On 8/3/21 9:47 PM, Biju Das wrote:
[...]
quoted
quoted
R-Car provides 30 device stats, whereas RZ/G2L provides only 15. In
addition, RZ/G2L has stats "rx_queue_0_csum_offload_errors" instead of
"rx_queue_0_missed_errors".
Replace RAVB_STATS_LEN macro with a structure variable stats_len to
struct ravb_hw_info, to support subsequent SoCs without any code
changes to the ravb_get_sset_count function.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
[...]
Finally a patch that I can agree with. :-)
Reviewed-by: ergei Shtylyov <redacted>
^Typo here.
Sorry, here's a good one:
Reviewed-by: Sergei Shtylyov <redacted>
From: Sergei Shtylyov <hidden> Date: 2021-08-03 21:07:03
On 8/2/21 1:26 PM, Biju Das wrote:
R-Car Gen3 supports TX and RX clock internal delay modes, whereas R-Car
Gen2 and RZ/G2L do not support it.
Add an internal_delay hw feature bit to struct ravb_hw_info to enable this
only for R-Car Gen3.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
[...]
OK, this one also seems uncontroversial:
Reviewed-by: Sergei Shtylyov <redacted>
MBR, Sergei
From: Sergei Shtylyov <hidden> Date: 2021-08-03 21:13:05
On 8/2/21 1:26 PM, Biju Das wrote:
quoted hunk
R-Car Gen3 supports TX and RX clock internal delay modes, whereas R-Car
Gen2 and RZ/G2L do not support it.
Add an internal_delay hw feature bit to struct ravb_hw_info to enable this
only for R-Car Gen3.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it smaller patch
and provided detailed description.
---
drivers/net/ethernet/renesas/ravb.h | 3 +++
drivers/net/ethernet/renesas/ravb_main.c | 6 ++++--
2 files changed, 7 insertions(+), 2 deletions(-)
@@ -998,6 +998,9 @@ struct ravb_hw_info {intnum_tx_desc;intstats_len;size_tskb_sz;++/* hardware features */+unsignedinternal_delay:1;/* RAVB has internal delays */
Oops, missed it initially:
RAVB? That's not a device name, according to the manuals. It seems to be the driver's name.
I'd drop this comment...
[...]
MBR, Sergei
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-04 05:13:18
Hi Sergei,
Thanks for the feedback
Subject: Re: [PATCH net-next v2 7/8] ravb: Add internal delay hw feature
to struct ravb_hw_info
On 8/2/21 1:26 PM, Biju Das wrote:
quoted
R-Car Gen3 supports TX and RX clock internal delay modes, whereas
R-Car
Gen2 and RZ/G2L do not support it.
Add an internal_delay hw feature bit to struct ravb_hw_info to enable
this only for R-Car Gen3.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-04 06:20:03
Hi Sergei,
Thanks for the feedback.
Subject: Re: [PATCH net-next v2 7/8] ravb: Add internal delay hw feature
to struct ravb_hw_info
On 8/2/21 1:26 PM, Biju Das wrote:
quoted
R-Car Gen3 supports TX and RX clock internal delay modes, whereas
R-Car
Gen2 and RZ/G2L do not support it.
Add an internal_delay hw feature bit to struct ravb_hw_info to enable
this only for R-Car Gen3.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
[...]
OK, this one also seems uncontroversial:
So far the comments I received
1) I have replaced NUM_TX_DESC to num_tx_desc. But you are recommending to rename it,
is ravb_num_tx_desc good choice?
2) skb_sz to max_rx_len, I am ok for it, if there is no objection from others.
3) patches related to device stats.
I already provided the output of ethtool -S eth0 for both R-Car and RZ/G2L.
For RZ/G2L there is an "rx_queue_0_csum_offload_errors: 0", instead of
"rx_queue_0_missed_errors: 0", Both uses MSC bit 6 for collecting this info.
To provide correct output to the user using command "ethtool -S eth0",
RZ/G2L need to have a different string LUT.
Q1) Do you agree with this?
Cheers,
Biju
Reviewed-by: Sergei Shtylyov <redacted>
MBR, Sergei
Subject: Re: [PATCH net-next v2 7/8] ravb: Add internal delay hw feature
to struct ravb_hw_info
On 8/2/21 1:26 PM, Biju Das wrote:
quoted
R-Car Gen3 supports TX and RX clock internal delay modes, whereas
R-Car
Gen2 and RZ/G2L do not support it.
Add an internal_delay hw feature bit to struct ravb_hw_info to enable
this only for R-Car Gen3.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-04 10:08:57
Hi Sergei,
Thanks for feedback
Subject: Re: [PATCH net-next v2 7/8] ravb: Add internal delay hw feature
to struct ravb_hw_info
On 04.08.2021 8:13, Biju Das wrote:
quoted
Hi Sergei,
Thanks for the feedback
quoted
Subject: Re: [PATCH net-next v2 7/8] ravb: Add internal delay hw
feature to struct ravb_hw_info
On 8/2/21 1:26 PM, Biju Das wrote:
quoted
R-Car Gen3 supports TX and RX clock internal delay modes, whereas
R-Car
Gen2 and RZ/G2L do not support it.
Add an internal_delay hw feature bit to struct ravb_hw_info to
enable this only for R-Car Gen3.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it
@@ -998,6 +998,9 @@ struct ravb_hw_info {intnum_tx_desc;intstats_len;size_tskb_sz;++/* hardware features */+unsignedinternal_delay:1;/* RAVB has internal delays */
Oops, missed it initially:
RAVB? That's not a device name, according to the manuals. It
seems to be the driver's name.
OK. will change it to AVB-DMAC has internal delays.
Please don't -- E-MAC has them, not AVB-DMAC.
By looking at HW manual for R-Car AVB-DMAC (APSR register, offset:-0x08C) has TDM and RDM registers for Setting internal delay mode which can give TX clock delay up to 2.0ns and RX Clock delay 2.8ns.
Please correct me, if this is not the case.
Regards,
Biju
From: Sergei Shtylyov <hidden> Date: 2021-08-04 10:20:58
On 04.08.2021 8:13, Biju Das wrote:
[...]
quoted
quoted
R-Car Gen3 supports TX and RX clock internal delay modes, whereas
R-Car
Gen2 and RZ/G2L do not support it.
Add an internal_delay hw feature bit to struct ravb_hw_info to enable
this only for R-Car Gen3.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-04 10:32:29
Hi Sergei,
Subject: Re: [PATCH net-next v2 7/8] ravb: Add internal delay hw feature
to struct ravb_hw_info
On 04.08.2021 8:13, Biju Das wrote:
[...]
quoted
quoted
quoted
R-Car Gen3 supports TX and RX clock internal delay modes, whereas
R-Car
Gen2 and RZ/G2L do not support it.
Add an internal_delay hw feature bit to struct ravb_hw_info to
enable this only for R-Car Gen3.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it
@@ -998,6 +998,9 @@ struct ravb_hw_info {intnum_tx_desc;intstats_len;size_tskb_sz;++/* hardware features */+unsignedinternal_delay:1;/* RAVB has internal delays */
Oops, missed it initially:
RAVB? That's not a device name, according to the manuals. It
seems to be the driver's name.
OK. will change it to AVB-DMAC has internal delays.
Please don't -- E-MAC has them, not AVB-DMAC.
Since the register for setting internal delay mode is coming from product specific register(0x8c),
I am agreeing with your statement, "E-MAC has internal delays".
Cheers,
Biju
From: Sergei Shtylyov <hidden> Date: 2021-08-04 10:34:22
On 04.08.2021 13:08, Biju Das wrote:
Hi Sergei,
Thanks for feedback
quoted
Subject: Re: [PATCH net-next v2 7/8] ravb: Add internal delay hw feature
to struct ravb_hw_info
On 04.08.2021 8:13, Biju Das wrote:
quoted
Hi Sergei,
Thanks for the feedback
quoted
Subject: Re: [PATCH net-next v2 7/8] ravb: Add internal delay hw
feature to struct ravb_hw_info
On 8/2/21 1:26 PM, Biju Das wrote:
quoted
R-Car Gen3 supports TX and RX clock internal delay modes, whereas
R-Car
Gen2 and RZ/G2L do not support it.
Add an internal_delay hw feature bit to struct ravb_hw_info to
enable this only for R-Car Gen3.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it
@@ -998,6 +998,9 @@ struct ravb_hw_info {intnum_tx_desc;intstats_len;size_tskb_sz;++/* hardware features */+unsignedinternal_delay:1;/* RAVB has internal delays */
Oops, missed it initially:
RAVB? That's not a device name, according to the manuals. It
seems to be the driver's name.
OK. will change it to AVB-DMAC has internal delays.
Please don't -- E-MAC has them, not AVB-DMAC.
By looking at HW manual for R-Car AVB-DMAC (APSR register, offset:-0x08C) has TDM and RDM registers for Setting internal delay mode which can give TX clock delay up to 2.0ns and RX Clock delay 2.8ns.
Please correct me, if this is not the case.
You're correct indeed -- though being counter-intuitive, APSR belongs to
the AVB-DMAC block. Sorry about that. :-/
From: Sergei Shtylyov <hidden> Date: 2021-08-04 19:27:31
On 8/3/21 8:57 AM, Biju Das wrote:
quoted
Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to
driver data
On 8/2/21 1:26 PM, Biju Das wrote:
quoted
The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC
are similar to the R-Car Ethernet AVB IP. With a few changes in the
driver we can support both IPs.
Currently a runtime decision based on the chip type is used to
distinguish the HW differences between the SoC families.
The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2
and RZ/G2L it is 2. For cases like this it is better to select the
number of TX descriptors by using a structure with a value, rather
than a runtime decision based on the chip type.
This patch adds the num_tx_desc variable to struct ravb_hw_info and
also replaces the driver data chip type with struct ravb_hw_info by
moving chip type to it.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it
How about leaving that field in the *struct* ravb_private? And adding the following instead:
unsigned unaligned_tx: 1;
quoted
I think this is rather the driver's choice, than the h/w feature...
Perhaps a rename would help with that? :-)
It is consistent with current naming convention used by the driver. NUM_TX_DESC macro is replaced by num_tx_desc and the below run time decision based on chip type for H/W configuration for Gen2/Gen3 is replaced by info->num_tx_desc.
priv->num_tx_desc = chip_id == RCAR_GEN2 ? NUM_TX_DESC_GEN2 : NUM_TX_DESC_GEN3;
.. and then:
priv->num_tx_desc = info->unaligned_tx ? NUM_TX_DESC_GEN2 : NUM_TX_DESC_GEN3;
Please let me know, if I am missing anything,
Previously there is a suggestion to change the generic struct ravb_driver_data (which holds driver differences and HW features) with struct ravb_hw_info.
Well, my plan was to place all the hardware features supported into the *struct* ravb_hw_info and leave all
the driver's software data in the *struct* ravb_private.
From: Sergei Shtylyov <hidden> Date: 2021-08-04 20:27:18
On 8/4/21 10:27 PM, Sergei Shtylyov wrote:
quoted
quoted
Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to
driver data
On 8/2/21 1:26 PM, Biju Das wrote:
quoted
The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC
are similar to the R-Car Ethernet AVB IP. With a few changes in the
driver we can support both IPs.
Currently a runtime decision based on the chip type is used to
distinguish the HW differences between the SoC families.
The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2
and RZ/G2L it is 2. For cases like this it is better to select the
number of TX descriptors by using a structure with a value, rather
than a runtime decision based on the chip type.
This patch adds the num_tx_desc variable to struct ravb_hw_info and
also replaces the driver data chip type with struct ravb_hw_info by
moving chip type to it.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it
How about leaving that field in the *struct* ravb_private? And adding the following instead:
unsigned unaligned_tx: 1;
Or aligned_tx, so that gen2 has it set, and gen3 has it cleared.
quoted
quoted
I think this is rather the driver's choice, than the h/w feature...
Perhaps a rename would help with that? :-)
It is consistent with current naming convention used by the driver. NUM_TX_DESC macro is replaced by num_tx_desc and the below run time decision based on chip type for H/W configuration for Gen2/Gen3 is replaced by info->num_tx_desc.
priv->num_tx_desc = chip_id == RCAR_GEN2 ? NUM_TX_DESC_GEN2 : NUM_TX_DESC_GEN3;
.. and then:
priv->num_tx_desc = info->unaligned_tx ? NUM_TX_DESC_GEN2 : NUM_TX_DESC_GEN3;
Sorry, mixed the values, should have been:
priv->num_tx_desc = info->unaligned_tx ? NUM_TX_DESC_GEN3 : NUM_TX_DESC_GEN2;
quoted
Please let me know, if I am missing anything,
Previously there is a suggestion to change the generic struct ravb_driver_data (which holds driver differences and HW features) with struct ravb_hw_info.
Well, my plan was to place all the hardware features supported into the *struct* ravb_hw_info and leave all
the driver's software data in the *struct* ravb_private.
... just like *struct* sh_eth_cpu_data and sh_eth_private in the sh_eth driver.
From: Sergei Shtylyov <hidden> Date: 2021-08-04 20:36:44
On 8/2/21 1:26 PM, Biju Das wrote:
The device stats strings for R-Car and RZ/G2L are different.
R-Car provides 30 device stats, whereas RZ/G2L provides only 15. In
addition, RZ/G2L has stats "rx_queue_0_csum_offload_errors" instead of
"rx_queue_0_missed_errors".
Add structure variables gstrings_stats and gstrings_size to struct
ravb_hw_info, so that subsequent SoCs can be added without any code
changes in the ravb_get_strings function.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Sergei Shtylyov <redacted>
[...]
MBR, Sergei
From: Sergei Shtylyov <hidden> Date: 2021-08-04 20:50:47
On 8/2/21 1:26 PM, Biju Das wrote:
quoted hunk
The register for retrieving TX drop counters is present only on R-Car Gen3
and RZ/G2L; it is not present on R-Car Gen2.
Add the tx_drop_cntrs hw feature bit to struct ravb_hw_info, to enable this
feature specifically for R-Car Gen3 now and later extend it to RZ/G2L.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it smaller patch
and provided detailed description.
---
drivers/net/ethernet/renesas/ravb.h | 1 +
drivers/net/ethernet/renesas/ravb_main.c | 4 +++-
2 files changed, 4 insertions(+), 1 deletion(-)
@@ -1001,6 +1001,7 @@ struct ravb_hw_info {/* hardware features */unsignedinternal_delay:1;/* RAVB has internal delays */+unsignedtx_drop_cntrs:1;/* RAVB has TX error counters */
I suggest 'tx_counters' -- this name comes from the sh_eth driver for the same regs
(but negated meaning). And please don't call the hardware RAVB. :-)
[...]
MBR, Sergei
From: Sergei Shtylyov <hidden> Date: 2021-08-05 19:07:17
On 8/2/21 1:26 PM, Biju Das wrote:
On R-Car the checksum calculation on RX frames is done by the E-MAC
module, whereas on RZ/G2L it is done by the TOE.
TOE calculates the checksum of received frames from E-MAC and outputs it to
DMAC. TOE also calculates the checksum of transmission frames from DMAC and
outputs it E-MAC.
Add net_features and net_hw_features to struct ravb_hw_info, to support
subsequent SoCs without any code changes in the ravb_probe function.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-05 19:19:08
Hi Sergei,
Thanks for the feedback.
Subject: Re: [PATCH net-next v2 6/8] ravb: Add net_features and
net_hw_features to struct ravb_hw_info
On 8/2/21 1:26 PM, Biju Das wrote:
quoted
On R-Car the checksum calculation on RX frames is done by the E-MAC
module, whereas on RZ/G2L it is done by the TOE.
TOE calculates the checksum of received frames from E-MAC and outputs
it to DMAC. TOE also calculates the checksum of transmission frames
from DMAC and outputs it E-MAC.
Add net_features and net_hw_features to struct ravb_hw_info, to
support subsequent SoCs without any code changes in the ravb_probe
function.
quoted
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
[...]
quoted
diff --git a/drivers/net/ethernet/renesas/ravb.h
b/drivers/net/ethernet/renesas/ravb.h
index b765b2b7d9e9..3df813b2e253 100644
R-Car has only Rx Checksum on E-Mac, where as Geth supports Rx Check Sum on E-Mac or Rx/Tx CheckSum on TOE.
So there is a hw difference. Please let me know what is the best way to handle this?
It seems like the 'feartures'
mirrors the enabled features?
Can you please explain this little bit?
quoted
enum ravb_chip_id chip_id;
int num_gstat_queue;
int num_tx_desc;
From: Sergei Shtylyov <hidden> Date: 2021-08-06 20:20:42
Hello!
On 8/5/21 10:18 PM, Biju Das wrote:
[...]
quoted
quoted
On R-Car the checksum calculation on RX frames is done by the E-MAC
module, whereas on RZ/G2L it is done by the TOE.
TOE calculates the checksum of received frames from E-MAC and outputs
it to DMAC. TOE also calculates the checksum of transmission frames
from DMAC and outputs it E-MAC.
Add net_features and net_hw_features to struct ravb_hw_info, to
support subsequent SoCs without any code changes in the ravb_probe
function.
quoted
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
[...]
quoted
diff --git a/drivers/net/ethernet/renesas/ravb.h
b/drivers/net/ethernet/renesas/ravb.h
index b765b2b7d9e9..3df813b2e253 100644
R-Car has only Rx Checksum on E-Mac, where as Geth supports Rx Check Sum on E-Mac or Rx/Tx CheckSum on TOE.
So there is a hw difference. Please let me know what is the best way to handle this?
I meant that we could go with only one field of the net_features... Alternatively, we could use our own
feature bits...
quoted
It seems like the 'feartures'
mirrors the enabled features?
From: Sergei Shtylyov <hidden> Date: 2021-08-06 20:31:46
On 8/2/21 1:26 PM, Biju Das wrote:
On R-Car the checksum calculation on RX frames is done by the E-MAC
module, whereas on RZ/G2L it is done by the TOE.
TOE calculates the checksum of received frames from E-MAC and outputs it to
DMAC. TOE also calculates the checksum of transmission frames from DMAC and
outputs it E-MAC.
Add net_features and net_hw_features to struct ravb_hw_info, to support
subsequent SoCs without any code changes in the ravb_probe function.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Sergei Shtylyov <redacted>
[...]
MBR, Sergei
Hi Biju,
On Mon, Aug 2, 2021 at 12:27 PM Biju Das [off-list ref] wrote:
The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC are
similar to the R-Car Ethernet AVB IP. With a few changes in the driver we
can support both IPs.
Currently a runtime decision based on the chip type is used to distinguish
the HW differences between the SoC families.
The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2 and
RZ/G2L it is 2. For cases like this it is better to select the number of
TX descriptors by using a structure with a value, rather than a runtime
decision based on the chip type.
This patch adds the num_tx_desc variable to struct ravb_hw_info and also
replaces the driver data chip type with struct ravb_hw_info by moving chip
type to it.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
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
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-12 07:26:51
Hi Geert,
Thanks for the feedback.
-----Original Message-----
Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to
driver data
Hi Biju,
On Mon, Aug 2, 2021 at 12:27 PM Biju Das [off-list ref]
wrote:
quoted
The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC
are similar to the R-Car Ethernet AVB IP. With a few changes in the
driver we can support both IPs.
Currently a runtime decision based on the chip type is used to
distinguish the HW differences between the SoC families.
The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2
and RZ/G2L it is 2. For cases like this it is better to select the
number of TX descriptors by using a structure with a value, rather
than a runtime decision based on the chip type.
This patch adds the num_tx_desc variable to struct ravb_hw_info and
also replaces the driver data chip type with struct ravb_hw_info by
moving chip type to it.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Exactly, this the reason.
Do you want me to change this into unsigned int? Please let me know.
Regards,
Biju
quoted
+
+ const struct ravb_hw_info *info;
};
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
m68k.org
In personal conversations with technical people, I call myself a hacker.
But when I'm talking to journalists I just say "programmer" or something
like that.
-- Linus Torvalds
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-12 07:35:41
Hi Sergei,
Thanks for the feedback.
Subject: Re: [PATCH net-next v2 6/8] ravb: Add net_features and
net_hw_features to struct ravb_hw_info
Hello!
On 8/5/21 10:18 PM, Biju Das wrote:
[...]
quoted
quoted
quoted
On R-Car the checksum calculation on RX frames is done by the E-MAC
module, whereas on RZ/G2L it is done by the TOE.
TOE calculates the checksum of received frames from E-MAC and
outputs it to DMAC. TOE also calculates the checksum of transmission
frames from DMAC and outputs it E-MAC.
Add net_features and net_hw_features to struct ravb_hw_info, to
support subsequent SoCs without any code changes in the ravb_probe
function.
quoted
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
[...]
quoted
diff --git a/drivers/net/ethernet/renesas/ravb.h
b/drivers/net/ethernet/renesas/ravb.h
index b765b2b7d9e9..3df813b2e253 100644
Hi Biju,
On Thu, Aug 12, 2021 at 9:26 AM Biju Das [off-list ref] wrote:
quoted
-----Original Message-----
On Mon, Aug 2, 2021 at 12:27 PM Biju Das [off-list ref]
wrote:
quoted
The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L SoC
are similar to the R-Car Ethernet AVB IP. With a few changes in the
driver we can support both IPs.
Currently a runtime decision based on the chip type is used to
distinguish the HW differences between the SoC families.
The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car Gen2
and RZ/G2L it is 2. For cases like this it is better to select the
number of TX descriptors by using a structure with a value, rather
than a runtime decision based on the chip type.
This patch adds the num_tx_desc variable to struct ravb_hw_info and
also replaces the driver data chip type with struct ravb_hw_info by
moving chip type to it.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Exactly, this the reason.
Do you want me to change this into unsigned int? Please let me know.
Up to you (or the maintainer? ;-)
For new fields (in the other patches), I would use unsigned for all
unsigned values. Signed values have more pitfalls related to
undefined behavior.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-12 08:13:23
Hi Geert,
Thanks for the feedback.
Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to
driver data
Hi Biju,
On Thu, Aug 12, 2021 at 9:26 AM Biju Das [off-list ref]
wrote:
quoted
quoted
-----Original Message-----
On Mon, Aug 2, 2021 at 12:27 PM Biju Das
[off-list ref]
wrote:
quoted
The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L
SoC are similar to the R-Car Ethernet AVB IP. With a few changes
in the driver we can support both IPs.
Currently a runtime decision based on the chip type is used to
distinguish the HW differences between the SoC families.
The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car
Gen2 and RZ/G2L it is 2. For cases like this it is better to
select the number of TX descriptors by using a structure with a
value, rather than a runtime decision based on the chip type.
This patch adds the num_tx_desc variable to struct ravb_hw_info
and also replaces the driver data chip type with struct
ravb_hw_info by moving chip type to it.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar
[off-list ref]
Exactly, this the reason.
Do you want me to change this into unsigned int? Please let me know.
Up to you (or the maintainer? ;-)
For new fields (in the other patches), I would use unsigned for all
unsigned values. Signed values have more pitfalls related to undefined
behavior.
Sergei, What is your thoughts here? Please let me know.
Cheers,
Biju
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-17 11:24:58
Hi all,
Subject: RE: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to
driver data
Hi Geert,
Thanks for the feedback.
quoted
Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to
driver data
Hi Biju,
On Thu, Aug 12, 2021 at 9:26 AM Biju Das [off-list ref]
wrote:
quoted
quoted
-----Original Message-----
On Mon, Aug 2, 2021 at 12:27 PM Biju Das
[off-list ref]
wrote:
quoted
The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L
SoC are similar to the R-Car Ethernet AVB IP. With a few changes
in the driver we can support both IPs.
Currently a runtime decision based on the chip type is used to
distinguish the HW differences between the SoC families.
The number of TX descriptors for R-Car Gen3 is 1 whereas on
R-Car
Gen2 and RZ/G2L it is 2. For cases like this it is better to
select the number of TX descriptors by using a structure with a
value, rather than a runtime decision based on the chip type.
This patch adds the num_tx_desc variable to struct ravb_hw_info
and also replaces the driver data chip type with struct
ravb_hw_info by moving chip type to it.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar
[off-list ref]
Exactly, this the reason.
Do you want me to change this into unsigned int? Please let me know.
Up to you (or the maintainer? ;-)
For new fields (in the other patches), I would use unsigned for all
unsigned values. Signed values have more pitfalls related to
undefined behavior.
Sergei, What is your thoughts here? Please let me know.
Here is my plan.
I will split this patch into two as Andrew suggested and
Then on the second patch will add as info->unaligned_tx as Sergei suggested.
Now the only open point is related to the data type of "int num_tx_desc"
and to align with sh_eth driver I will keep int.
Regards,
Biju
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-17 15:47:18
Hi Sergei,
Thanks for the feedback.
Subject: Re: [PATCH net-next v2 8/8] ravb: Add tx_drop_cntrs to struct
ravb_hw_info
On 8/2/21 1:26 PM, Biju Das wrote:
quoted
The register for retrieving TX drop counters is present only on R-Car
Gen3 and RZ/G2L; it is not present on R-Car Gen2.
Add the tx_drop_cntrs hw feature bit to struct ravb_hw_info, to enable
this feature specifically for R-Car Gen3 now and later extend it to
RZ/G2L.
quoted
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it
@@ -1001,6 +1001,7 @@ struct ravb_hw_info {/* hardware features */unsignedinternal_delay:1;/* RAVB has internal delays */+unsignedtx_drop_cntrs:1;/* RAVB has TX error counters */
I suggest 'tx_counters' -- this name comes from the sh_eth driver for
the same regs (but negated meaning). And please don't call the hardware
RAVB. :-)
Agreed. Will change it to 'tx_counters' on next version and comment it as
/* AVB-DMAC has TX counters */
Cheers,
Biju
The register for retrieving TX drop counters is present only on R-Car
Gen3 and RZ/G2L; it is not present on R-Car Gen2.
Add the tx_drop_cntrs hw feature bit to struct ravb_hw_info, to enable
this feature specifically for R-Car Gen3 now and later extend it to
RZ/G2L.
quoted
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it
@@ -1001,6 +1001,7 @@ struct ravb_hw_info {/* hardware features */unsignedinternal_delay:1;/* RAVB has internal delays */+unsignedtx_drop_cntrs:1;/* RAVB has TX error counters */
I suggest 'tx_counters' -- this name comes from the sh_eth driver for
the same regs (but negated meaning). And please don't call the hardware
RAVB. :-)
Agreed. Will change it to 'tx_counters' on next version and comment it as
/* AVB-DMAC has TX counters */
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-17 16:33:39
Hi Sergei,
Subject: Re: [PATCH net-next v2 8/8] ravb: Add tx_drop_cntrs to struct
ravb_hw_info
Hello!
On 8/17/21 6:47 PM, Biju Das wrote:
[...]
quoted
quoted
quoted
The register for retrieving TX drop counters is present only on
R-Car
Gen3 and RZ/G2L; it is not present on R-Car Gen2.
Add the tx_drop_cntrs hw feature bit to struct ravb_hw_info, to
enable this feature specifically for R-Car Gen3 now and later extend
it to
RZ/G2L.
quoted
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2:
* Incorporated Andrew and Sergei's review comments for making it
@@ -1001,6 +1001,7 @@ struct ravb_hw_info {/* hardware features */unsignedinternal_delay:1;/* RAVB has internal delays */+unsignedtx_drop_cntrs:1;/* RAVB has TX error counters */
I suggest 'tx_counters' -- this name comes from the sh_eth driver
for the same regs (but negated meaning). And please don't call the
hardware RAVB. :-)
Agreed. Will change it to 'tx_counters' on next version and comment it
as
/* AVB-DMAC has TX counters */
The counters belong to E-MAC, not AVB-DMAC.
You are correct, it is at offset 0x700 on E-MAC block.
Cheers,
Biju
-----Original Message-----
On Mon, Aug 2, 2021 at 12:27 PM Biju Das
[off-list ref]
wrote:
quoted
The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L
SoC are similar to the R-Car Ethernet AVB IP. With a few changes
in the driver we can support both IPs.
Currently a runtime decision based on the chip type is used to
distinguish the HW differences between the SoC families.
The number of TX descriptors for R-Car Gen3 is 1 whereas on
R-Car
Gen2 and RZ/G2L it is 2. For cases like this it is better to
select the number of TX descriptors by using a structure with a
value, rather than a runtime decision based on the chip type.
This patch adds the num_tx_desc variable to struct ravb_hw_info
and also replaces the driver data chip type with struct
ravb_hw_info by moving chip type to it.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar
[off-list ref]
Exactly, this the reason.
Do you want me to change this into unsigned int? Please let me know.
Up to you (or the maintainer? ;-)
For new fields (in the other patches), I would use unsigned for all
unsigned values. Signed values have more pitfalls related to
undefined behavior.
Sergei, What is your thoughts here? Please let me know.
Here is my plan.
I will split this patch into two as Andrew suggested and
If you mran changing the ravb_private::num_tx_desc to *unsigned*, it'll be
a good cleanup. What's would be the 2nd part tho?
Then on the second patch will add as info->unaligned_tx as Sergei suggested.
OK.
Now the only open point is related to the data type of "int num_tx_desc"
and to align with sh_eth driver I will keep int.
The sh_eth driver simply doesn't have this -- it always use 1 descriptor.
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-18 06:29:59
Hi Sergei,
Thanks for the feedback.
Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to
driver data
On 8/17/21 2:24 PM, Biju Das wrote:
[...]
quoted
quoted
quoted
quoted
quoted
-----Original Message-----
On Mon, Aug 2, 2021 at 12:27 PM Biju Das
[off-list ref]
wrote:
quoted
The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L
SoC are similar to the R-Car Ethernet AVB IP. With a few changes
in the driver we can support both IPs.
Currently a runtime decision based on the chip type is used to
distinguish the HW differences between the SoC families.
The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car
Gen2 and RZ/G2L it is 2. For cases like this it is better to
select the number of TX descriptors by using a structure with a
value, rather than a runtime decision based on the chip type.
This patch adds the num_tx_desc variable to struct ravb_hw_info
and also replaces the driver data chip type with struct
ravb_hw_info by moving chip type to it.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar
[off-list ref]
Exactly, this the reason.
Do you want me to change this into unsigned int? Please let me know.
Up to you (or the maintainer? ;-)
For new fields (in the other patches), I would use unsigned for all
unsigned values. Signed values have more pitfalls related to
undefined behavior.
Sergei, What is your thoughts here? Please let me know.
Here is my plan.
I will split this patch into two as Andrew suggested and
If you mran changing the ravb_private::num_tx_desc to *unsigned*, it'll
be a good cleanup. What's would be the 2nd part tho?
OK in that case, I will split this patch into 3.
First patch for adding struct ravb_hw_info to driver data and replace
driver data chip type with struct ravb_hw_info
Second patch for changing ravb_private::num_tx_desc from int to unsigned int.
Third patch for adding aligned_tx to struct ravb_hw_info.
Regards,
Biju
-----Original Message-----
On Mon, Aug 2, 2021 at 12:27 PM Biju Das
[off-list ref]
wrote:
quoted
The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L
SoC are similar to the R-Car Ethernet AVB IP. With a few changes
in the driver we can support both IPs.
Currently a runtime decision based on the chip type is used to
distinguish the HW differences between the SoC families.
The number of TX descriptors for R-Car Gen3 is 1 whereas on R-Car
Gen2 and RZ/G2L it is 2. For cases like this it is better to
select the number of TX descriptors by using a structure with a
value, rather than a runtime decision based on the chip type.
This patch adds the num_tx_desc variable to struct ravb_hw_info
and also replaces the driver data chip type with struct
ravb_hw_info by moving chip type to it.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar
[off-list ref]
Exactly, this the reason.
Do you want me to change this into unsigned int? Please let me know.
Up to you (or the maintainer? ;-)
For new fields (in the other patches), I would use unsigned for all
unsigned values. Signed values have more pitfalls related to
undefined behavior.
Sergei, What is your thoughts here? Please let me know.
Here is my plan.
I will split this patch into two as Andrew suggested and
If you mran changing the ravb_private::num_tx_desc to *unsigned*, it'll
be a good cleanup. What's would be the 2nd part tho?
OK in that case, I will split this patch into 3.
First patch for adding struct ravb_hw_info to driver data and replace
driver data chip type with struct ravb_hw_info
Couldn't this be a 2nd patch?..
Second patch for changing ravb_private::num_tx_desc from int to unsigned int.
... and this one the 1st?
Third patch for adding aligned_tx to struct ravb_hw_info.
Regards,
Biju
From: Biju Das <biju.das.jz@bp.renesas.com> Date: 2021-08-18 10:23:44
Hi Sergei,
-----Original Message-----
Subject: Re: [PATCH net-next v2 1/8] ravb: Add struct ravb_hw_info to
driver data
Hello!
On 18.08.2021 9:29, Biju Das wrote:
[...]
quoted
quoted
quoted
quoted
quoted
quoted
quoted
-----Original Message-----
On Mon, Aug 2, 2021 at 12:27 PM Biju Das
[off-list ref]
wrote:
quoted
The DMAC and EMAC blocks of Gigabit Ethernet IP found on RZ/G2L
SoC are similar to the R-Car Ethernet AVB IP. With a few
changes in the driver we can support both IPs.
Currently a runtime decision based on the chip type is used to
distinguish the HW differences between the SoC families.
The number of TX descriptors for R-Car Gen3 is 1 whereas on
R-Car
Gen2 and RZ/G2L it is 2. For cases like this it is better to
select the number of TX descriptors by using a structure with a
value, rather than a runtime decision based on the chip type.
This patch adds the num_tx_desc variable to struct ravb_hw_info
and also replaces the driver data chip type with struct
ravb_hw_info by moving chip type to it.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar
[off-list ref]
Exactly, this the reason.
Do you want me to change this into unsigned int? Please let me
know.
quoted
quoted
quoted
quoted
quoted
Up to you (or the maintainer? ;-)
For new fields (in the other patches), I would use unsigned for
all unsigned values. Signed values have more pitfalls related to
undefined behavior.
Sergei, What is your thoughts here? Please let me know.
Here is my plan.
I will split this patch into two as Andrew suggested and
If you mran changing the ravb_private::num_tx_desc to *unsigned*,
it'll be a good cleanup. What's would be the 2nd part tho?
OK in that case, I will split this patch into 3.
First patch for adding struct ravb_hw_info to driver data and replace
driver data chip type with struct ravb_hw_info
Couldn't this be a 2nd patch?..
quoted
Second patch for changing ravb_private::num_tx_desc from int to unsigned
int.
... and this one the 1st?
quoted
Third patch for adding aligned_tx to struct ravb_hw_info.