HI Sergei,

Thanks for the feedback.

-----Original Message-----
Subject: Re: [RFC/PATCH 12/18] ravb: Add timestamp to struct ravb_hw_info

On 9/23/21 5:08 PM, Biju Das wrote:

quoted

R-Car AVB-DMAC supports timestamp feature.
Add a timestamp hw feature bit to struct ravb_hw_info to add this
feature only for R-Car.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/net/ethernet/renesas/ravb.h      |  2 +
 drivers/net/ethernet/renesas/ravb_main.c | 68
+++++++++++++++---------
 2 files changed, 45 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h

b/drivers/net/ethernet/renesas/ravb.h
index ab4909244276..2505de5d4a28 100644

--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h

@@ -1034,6 +1034,7 @@ struct ravb_hw_info {
 	unsigned mii_rgmii_selection:1;	/* E-MAC supports mii/rgmii

selection */

quoted

 	unsigned half_duplex:1;		/* E-MAC supports half duplex mode */
 	unsigned rx_2k_buffers:1;	/* AVB-DMAC has Max 2K buf size on RX

*/

quoted

+	unsigned timestamp:1;		/* AVB-DMAC has timestamp */

   Isn't this a matter of the gPTP support as well, i.e. no separate flag
needed?

Agreed. Previously it is suggested to use timestamp. I will change it to as part of gPTP support cases.

[...]

quoted

@@ -1089,6 +1090,7 @@ struct ravb_private {
 	unsigned int num_tx_desc;	/* TX descriptors per packet */

 	int duplex;
+	struct ravb_rx_desc *rgeth_rx_ring[NUM_RX_QUEUE];

   Strange place to declare this...

Agreed. This has to be on later patch. Will move it.

quoted

 	const struct ravb_hw_info *info;
 	struct reset_control *rstc;

diff --git a/drivers/net/ethernet/renesas/ravb_main.c

b/drivers/net/ethernet/renesas/ravb_main.c
index 9c0d35f4b221..2c375002ebcb 100644

--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c

@@ -949,11 +949,14 @@ static bool ravb_queue_interrupt(struct

net_device *ndev, int q)

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

 	if (tis & TIS_TFUF) {
 		ravb_write(ndev, ~(TIS_TFUF | TIS_RESERVED), TIS);
-		ravb_get_tx_tstamp(ndev);
+		if (info->timestamp)
+			ravb_get_tx_tstamp(ndev);

   Shouldn't we just disable TIS.TFUF permanently instead for the non-gPTP
case?

Good catch. As ravb_dmac_init_rgeth(will be renamed to "ravb_dmac_init_gbeth") is not enabling this interrupt as it is not documented in RZ/G2L hardware manual.
So this function never gets called for non-gPTP case.

I will remove this check.

Regards,
Biju

[...]

MBR, Sergey