diff mbox series

[net-next,v2,8/8] ravb: Add tx_drop_cntrs to struct ravb_hw_info

Message ID 20210802102654.5996-9-biju.das.jz@bp.renesas.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series Add Gigabit Ethernet driver support | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 1 maintainers not CCed: yangyingliang@huawei.com
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 29 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link

Commit Message

Biju Das Aug. 2, 2021, 10:26 a.m. UTC
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(-)

Comments

Andrew Lunn Aug. 2, 2021, 3:14 p.m. UTC | #1
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>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
Sergei Shtylyov Aug. 4, 2021, 8:50 p.m. UTC | #2
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
Biju Das Aug. 17, 2021, 3:47 p.m. UTC | #3
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
Sergey Shtylyov Aug. 17, 2021, 4:30 p.m. UTC | #4
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
Biju Das Aug. 17, 2021, 4:33 p.m. UTC | #5
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 mbox series

Patch

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 = {