netdev - Re: [PATCH 03/10] ravb: Add nc_queue to struct ravb_hw

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sat, 2 Oct 2021 21:35:52 +0300
From:   Sergey Shtylyov <s.shtylyov@....ru>
To:     Biju Das <biju.das.jz@...renesas.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>
CC:     Sergei Shtylyov <sergei.shtylyov@...il.com>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Sergey Shtylyov <s.shtylyov@...russia.ru>,
        "Adam Ford" <aford173@...il.com>, Andrew Lunn <andrew@...n.ch>,
        Yuusuke Ashizuka <ashiduka@...itsu.com>,
        Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
        <netdev@...r.kernel.org>, <linux-renesas-soc@...r.kernel.org>,
        Chris Paterson <Chris.Paterson2@...esas.com>,
        Biju Das <biju.das@...renesas.com>,
        "Prabhakar Mahadev Lad" <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: Re: [PATCH 03/10] ravb: Add nc_queue to struct ravb_hw_info

On 10/1/21 6:06 PM, Biju Das wrote:

> R-Car supports network control queue whereas RZ/G2L does not support
> it. Add nc_queue to struct ravb_hw_info, so that NC queue is handled
> only by R-Car.
> 
> This patch also renames ravb_rcar_dmac_init to ravb_dmac_init_rcar
> to be consistent with the naming convention used in sh_eth driver.
> 
> Signed-off-by: Biju Das <biju.das.jz@...renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>

Reviewed-by: Sergey Shtylyov <s.shtylyov@....ru>

   One little nit below:

> ---
> RFC->v1:
>  * Handled NC queue only for R-Car.
> ---
>  drivers/net/ethernet/renesas/ravb.h      |   3 +-
>  drivers/net/ethernet/renesas/ravb_main.c | 140 +++++++++++++++--------
>  2 files changed, 94 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
> index a33fbcb4aac3..c91e93e5590f 100644
> --- a/drivers/net/ethernet/renesas/ravb.h
> +++ b/drivers/net/ethernet/renesas/ravb.h
> @@ -986,7 +986,7 @@ struct ravb_hw_info {
>  	bool (*receive)(struct net_device *ndev, int *quota, int q);
>  	void (*set_rate)(struct net_device *ndev);
>  	int (*set_feature)(struct net_device *ndev, netdev_features_t features);
> -	void (*dmac_init)(struct net_device *ndev);
> +	int (*dmac_init)(struct net_device *ndev);
>  	void (*emac_init)(struct net_device *ndev);
>  	const char (*gstrings_stats)[ETH_GSTRING_LEN];
>  	size_t gstrings_size;
> @@ -1002,6 +1002,7 @@ struct ravb_hw_info {
>  	unsigned multi_irqs:1;		/* AVB-DMAC and E-MAC has multiple irqs */
>  	unsigned gptp:1;		/* AVB-DMAC has gPTP support */
>  	unsigned ccc_gac:1;		/* AVB-DMAC has gPTP support active in config mode */
> +	unsigned nc_queue:1;		/* AVB-DMAC has NC queue */

   Rather "queues" as there are RX and TX NC queues, no?

[...]
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index dc7654abfe55..8bf13586e90a 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
[...]
> @@ -1698,28 +1717,38 @@ static struct net_device_stats *ravb_get_stats(struct net_device *ndev)
>  
>  	nstats = &ndev->stats;
>  	stats0 = &priv->stats[RAVB_BE];
> -	stats1 = &priv->stats[RAVB_NC];
>  
>  	if (info->tx_counters) {
>  		nstats->tx_dropped += ravb_read(ndev, TROCR);
>  		ravb_write(ndev, 0, TROCR);	/* (write clear) */
>  	}
>  
> -	nstats->rx_packets = stats0->rx_packets + stats1->rx_packets;
> -	nstats->tx_packets = stats0->tx_packets + stats1->tx_packets;
> -	nstats->rx_bytes = stats0->rx_bytes + stats1->rx_bytes;
> -	nstats->tx_bytes = stats0->tx_bytes + stats1->tx_bytes;
> -	nstats->multicast = stats0->multicast + stats1->multicast;
> -	nstats->rx_errors = stats0->rx_errors + stats1->rx_errors;
> -	nstats->rx_crc_errors = stats0->rx_crc_errors + stats1->rx_crc_errors;
> -	nstats->rx_frame_errors =
> -		stats0->rx_frame_errors + stats1->rx_frame_errors;
> -	nstats->rx_length_errors =
> -		stats0->rx_length_errors + stats1->rx_length_errors;
> -	nstats->rx_missed_errors =
> -		stats0->rx_missed_errors + stats1->rx_missed_errors;
> -	nstats->rx_over_errors =
> -		stats0->rx_over_errors + stats1->rx_over_errors;
> +	nstats->rx_packets = stats0->rx_packets;
> +	nstats->tx_packets = stats0->tx_packets;
> +	nstats->rx_bytes = stats0->rx_bytes;
> +	nstats->tx_bytes = stats0->tx_bytes;
> +	nstats->multicast = stats0->multicast;
> +	nstats->rx_errors = stats0->rx_errors;
> +	nstats->rx_crc_errors = stats0->rx_crc_errors;
> +	nstats->rx_frame_errors = stats0->rx_frame_errors;
> +	nstats->rx_length_errors = stats0->rx_length_errors;
> +	nstats->rx_missed_errors = stats0->rx_missed_errors;
> +	nstats->rx_over_errors = stats0->rx_over_errors;
> +	if (info->nc_queue) {
> +		stats1 = &priv->stats[RAVB_NC];
> +
> +		nstats->rx_packets += stats1->rx_packets;
> +		nstats->tx_packets += stats1->tx_packets;
> +		nstats->rx_bytes += stats1->rx_bytes;
> +		nstats->tx_bytes += stats1->tx_bytes;
> +		nstats->multicast += stats1->multicast;
> +		nstats->rx_errors += stats1->rx_errors;
> +		nstats->rx_crc_errors += stats1->rx_crc_errors;
> +		nstats->rx_frame_errors += stats1->rx_frame_errors;
> +		nstats->rx_length_errors += stats1->rx_length_errors;
> +		nstats->rx_missed_errors += stats1->rx_missed_errors;
> +		nstats->rx_over_errors += stats1->rx_over_errors;
> +	}

   Good! :-)

[...]

MBR, Sergey