| Message ID | 20210802102654.5996-9-biju.das.jz@bp.renesas.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add Gigabit Ethernet driver support | expand |
On 8/2/21 1:26 PM, 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> > --- > 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(-) > > diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h > index 0d640dbe1eed..35fbb9f60ba8 100644 > --- a/drivers/net/ethernet/renesas/ravb.h > +++ b/drivers/net/ethernet/renesas/ravb.h > @@ -1001,6 +1001,7 @@ struct ravb_hw_info { > > /* hardware features */ > unsigned internal_delay:1; /* RAVB has internal delays */ > + unsigned tx_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
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: > > > 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(-) > > > > diff --git a/drivers/net/ethernet/renesas/ravb.h > > b/drivers/net/ethernet/renesas/ravb.h > > index 0d640dbe1eed..35fbb9f60ba8 100644 > > --- a/drivers/net/ethernet/renesas/ravb.h > > +++ b/drivers/net/ethernet/renesas/ravb.h > > @@ -1001,6 +1001,7 @@ struct ravb_hw_info { > > > > /* hardware features */ > > unsigned internal_delay:1; /* RAVB has internal delays */ > > + unsigned tx_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
Hello! On 8/17/21 6:47 PM, 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> >>> --- >>> 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(-) >>> >>> diff --git a/drivers/net/ethernet/renesas/ravb.h >>> b/drivers/net/ethernet/renesas/ravb.h >>> index 0d640dbe1eed..35fbb9f60ba8 100644 >>> --- a/drivers/net/ethernet/renesas/ravb.h >>> +++ b/drivers/net/ethernet/renesas/ravb.h >>> @@ -1001,6 +1001,7 @@ struct ravb_hw_info { >>> >>> /* hardware features */ >>> unsigned internal_delay:1; /* RAVB has internal delays */ >>> + unsigned tx_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. > Cheers, > Biju MBR, Sergey
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: > > [...] > >>> 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(-) > >>> > >>> diff --git a/drivers/net/ethernet/renesas/ravb.h > >>> b/drivers/net/ethernet/renesas/ravb.h > >>> index 0d640dbe1eed..35fbb9f60ba8 100644 > >>> --- a/drivers/net/ethernet/renesas/ravb.h > >>> +++ b/drivers/net/ethernet/renesas/ravb.h > >>> @@ -1001,6 +1001,7 @@ struct ravb_hw_info { > >>> > >>> /* hardware features */ > >>> unsigned internal_delay:1; /* RAVB has internal delays */ > >>> + unsigned tx_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
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h index 0d640dbe1eed..35fbb9f60ba8 100644 --- a/drivers/net/ethernet/renesas/ravb.h +++ b/drivers/net/ethernet/renesas/ravb.h @@ -1001,6 +1001,7 @@ struct ravb_hw_info { /* hardware features */ unsigned internal_delay:1; /* RAVB has internal delays */ + unsigned tx_drop_cntrs:1; /* RAVB has TX error counters */ }; struct ravb_private { diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c index 02acae4d51c1..6af3f978c84c 100644 --- a/drivers/net/ethernet/renesas/ravb_main.c +++ b/drivers/net/ethernet/renesas/ravb_main.c @@ -1633,13 +1633,14 @@ static u16 ravb_select_queue(struct net_device *ndev, struct sk_buff *skb, static struct net_device_stats *ravb_get_stats(struct net_device *ndev) { struct ravb_private *priv = netdev_priv(ndev); + const struct ravb_hw_info *info = priv->info; struct net_device_stats *nstats, *stats0, *stats1; nstats = &ndev->stats; stats0 = &priv->stats[RAVB_BE]; stats1 = &priv->stats[RAVB_NC]; - if (priv->chip_id == RCAR_GEN3) { + if (info->tx_drop_cntrs) { nstats->tx_dropped += ravb_read(ndev, TROCR); ravb_write(ndev, 0, TROCR); /* (write clear) */ } @@ -1940,6 +1941,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info = { .stats_len = ARRAY_SIZE(ravb_gstrings_stats), .skb_sz = RX_BUF_SZ + RAVB_ALIGN - 1, .internal_delay = 1, + .tx_drop_cntrs = 1, }; static const struct ravb_hw_info ravb_gen2_hw_info = {