Re: [PATCH v2 net-next 1/5] net: fec: use a member variable for maximum buffer size

[Shenwei Wang]

> -----Original Message-----
> From: Andrew Lunn <andrew@xxxxxxx>
> Sent: Thursday, August 21, 2025 1:56 PM
> To: Shenwei Wang <shenwei.wang@xxxxxxx>
> Cc: Wei Fang <wei.fang@xxxxxxx>; Andrew Lunn <andrew+netdev@xxxxxxx>;
> David S. Miller <davem@xxxxxxxxxxxxx>; Eric Dumazet
> <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni
> <pabeni@xxxxxxxxxx>; Alexei Starovoitov <ast@xxxxxxxxxx>; Daniel Borkmann
> <daniel@xxxxxxxxxxxxx>; Jesper Dangaard Brouer <hawk@xxxxxxxxxx>; John
> Fastabend <john.fastabend@xxxxxxxxx>; Clark Wang
> <xiaoning.wang@xxxxxxx>; Stanislav Fomichev <sdf@xxxxxxxxxxx>;
> imx@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; dl-
> linux-imx <linux-imx@xxxxxxx>
> Subject: [EXT] Re: [PATCH v2 net-next 1/5] net: fec: use a member variable for
> maximum buffer size
> 
> > @@ -1145,9 +1145,12 @@ static void
> >  fec_restart(struct net_device *ndev)
> >  {
> >       struct fec_enet_private *fep = netdev_priv(ndev);
> > -     u32 rcntl = OPT_FRAME_SIZE | FEC_RCR_MII;
> > +     u32 rcntl = FEC_RCR_MII;
> >       u32 ecntl = FEC_ECR_ETHEREN;
> >
> > +     if (fep->max_buf_size == OPT_FRAME_SIZE)
> > +             rcntl |= (fep->max_buf_size << 16);
> 
> I was expecting something like s/OPT_FRAME_SIZE/fep->max_buf_size/g
> 

We can't simply replace the value here because it needs to be left-shifted by 16 bits. I intentionally 
preserved the original logic for consistency. In patch #3, the max_buf_size may become a different
one for Jumbo frame mode and the condition is updated to:
+	if (OPT_FRAME_SIZE != 0)

Thanks,
Shenwei

> This is introducing extra logic. I think the if (...) belongs in another patch. The
> assignment is however what i expected.
> 
>     Andrew
> 
> ---
> pw-bot: cr