Hi Sergei,
> 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@xxxxxxxxxxxxxx>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
>
> Reviewed-by: Sergey Shtylyov <s.shtylyov@xxxxxx>
>
> 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?
It has NC queue on both RX and TX.
If needed, I can send a follow up patch as RFC with the following changes.
unsigned nc_queue:1; /* AVB-DMAC has NC queue on both RX and TX */
or
unsigned nc_queues:1; /* AVB-DMAC has RX and TX NC queues */
please let me know.
Regards,
Biju
>
> [...]
> > 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
