lkml.org 
[lkml]   [2023]   [Nov]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 06/13] net: ravb: Let IP specific receive function to interrogate descriptors
    From
    Date
    On 11/20/23 11:45 AM, Claudiu wrote:

    > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
    >
    > ravb_poll() initial code used to interrogate the first descriptor of the
    > RX queue in case gptp is false to know if ravb_rx() should be called.
    > This is done for non GPTP IPs. For GPTP IPs the driver PTP specific

    It's called gPTP, AFAIK.

    > information was used to know if receive function should be called. As
    > every IP has it's own receive function that interrogates the RX descriptor

    Its own.

    > list in the same way the ravb_poll() was doing there is no need to double
    > check this in ravb_poll(). Removing the code form ravb_poll() and

    From ravb_poll().

    > adjusting ravb_rx_gbeth() leads to a cleaner code.
    >
    > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
    > ---
    > drivers/net/ethernet/renesas/ravb_main.c | 18 ++++++------------
    > 1 file changed, 6 insertions(+), 12 deletions(-)
    >
    > diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
    > index 588e3be692d3..0fc9810c5e78 100644
    > --- a/drivers/net/ethernet/renesas/ravb_main.c
    > +++ b/drivers/net/ethernet/renesas/ravb_main.c
    > @@ -771,12 +771,15 @@ static bool ravb_rx_gbeth(struct net_device *ndev, int *quota, int q)
    > int limit;
    >
    > entry = priv->cur_rx[q] % priv->num_rx_ring[q];
    > + desc = &priv->gbeth_rx_ring[entry];
    > + if (desc->die_dt == DT_FEMPTY)
    > + return false;
    > +

    I don't understand what this buys us, the following *while* loop will
    be skipped anyway, and the *for* loop as well, I think... And ravb_rx_rcar()
    doesn't have that anyway...

    > @@ -1279,25 +1282,16 @@ static int ravb_poll(struct napi_struct *napi, int budget)
    > struct net_device *ndev = napi->dev;
    > struct ravb_private *priv = netdev_priv(ndev);
    > const struct ravb_hw_info *info = priv->info;
    > - bool gptp = info->gptp || info->ccc_gac;
    > - struct ravb_rx_desc *desc;
    > unsigned long flags;
    > int q = napi - priv->napi;
    > int mask = BIT(q);
    > int quota = budget;
    > - unsigned int entry;
    >
    > - if (!gptp) {
    > - entry = priv->cur_rx[q] % priv->num_rx_ring[q];
    > - desc = &priv->gbeth_rx_ring[entry];
    > - }
    > /* Processing RX Descriptor Ring */
    > /* Clear RX interrupt */
    > ravb_write(ndev, ~(mask | RIS0_RESERVED), RIS0);
    > - if (gptp || desc->die_dt != DT_FEMPTY) {
    > - if (ravb_rx(ndev, &quota, q))
    > - goto out;
    > - }

    I don't really understand this code (despite I've AKCed it)...
    Biju, could you explain this (well, you tried already but I don't
    buy it anymore)?

    > + if (ravb_rx(ndev, &quota, q))
    > + goto out;

    This restores the behavior before:

    https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3d4e37df882b0f4f28b7223a42492650b71252c5

    So does look correct. :-)

    MBR, Sergey

    \
     
     \ /
      Last update: 2023-11-23 17:38    [W:6.632 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site