> -----Original Message-----
> From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> Sent: 06 October 2021 08:44
> To: Sergey Shtylyov <s.shtylyov@xxxxxx>; David S. Miller
> <davem@xxxxxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>
> Cc: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>; Sergey Shtylyov
> <s.shtylyov@xxxxxxxxxxxx>; Adam Ford <aford173@xxxxxxxxx>; Andrew Lunn
> <andrew@xxxxxxx>; Yuusuke Ashizuka <ashiduka@xxxxxxxxxxx>; Yoshihiro
> Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; linux-
> renesas-soc@xxxxxxxxxxxxxxx; Chris Paterson <Chris.Paterson2@xxxxxxxxxxx>;
> Biju Das <biju.das@xxxxxxxxxxxxxx>; Prabhakar Mahadev Lad
> <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> Subject: RE: [RFC 03/12] ravb: Fillup ravb_set_features_gbeth() stub
>
> Hi Sergei,
>
> Thanks for the feedback.
>
> > Subject: Re: [RFC 03/12] ravb: Fillup ravb_set_features_gbeth() stub
> >
> > On 10/5/21 2:06 PM, Biju Das wrote:
> >
> > > Fillup ravb_set_features_gbeth() function to support RZ/G2L.
> > > Also set the net_hw_features bits supported by GbEthernet
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > [...]
> >
> > > diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> > > b/drivers/net/ethernet/renesas/ravb_main.c
> > > index ed0328a90200..37f50c041114 100644
> > > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> > [...]
> > > @@ -2086,7 +2087,37 @@ static void ravb_set_rx_csum(struct
> > > net_device *ndev, bool enable) static int
> > > ravb_set_features_gbeth(struct
> > net_device *ndev,
> > > netdev_features_t features)
> > > {
> > > - /* Place holder */
> > > + netdev_features_t changed = features ^ ndev->features;
> > > + int error;
> > > + u32 csr0;
> > > +
> > > + csr0 = ravb_read(ndev, CSR0);
> > > + ravb_write(ndev, csr0 & ~(CSR0_RPE | CSR0_TPE), CSR0);
> > > + error = ravb_wait(ndev, CSR0, CSR0_RPE | CSR0_TPE, 0);
> > > + if (error) {
> > > + ravb_write(ndev, csr0, CSR0);
> > > + return error;
> > > + }
> > > +
> > > + if (changed & NETIF_F_RXCSUM) {
> > > + if (features & NETIF_F_RXCSUM)
> > > + ravb_write(ndev, CSR2_ALL, CSR2);
> > > + else
> > > + ravb_write(ndev, 0, CSR2);
> > > + }
> > > +
> > > + if (changed & NETIF_F_HW_CSUM) {
> > > + if (features & NETIF_F_HW_CSUM) {
> > > + ravb_write(ndev, CSR1_ALL, CSR1);
> > > + ndev->features |= NETIF_F_CSUM_MASK;
> >
> > Hm, the >linux/netdev_features.h> says those are contradictory to
> > have both NETIF_F_HW_CSUM and NETIF_F_CSUM_MASK set...
>
> It is a mistake from my side, I am taking out this setting. Any way below
> code overrides it.
> This will answer all your comments below.
I am deferring this patch and will take out RX checksum offload functionality from patch#7
Will post this 2 patches as RFC, as looks like it needs more discussions related to HW checksum.
Regards,
Biju
>
> Regards,
> Biju
>
> >
> > > + } else {
> > > + ravb_write(ndev, 0, CSR1);
> >
> > No need to mask off the 'features' field?
> >
> > > + }
> > > + }
> > > + ravb_write(ndev, csr0, CSR0);
> > > +
> > > + ndev->features = features;
> >
> > Mhm, doesn't that clear NETIF_F_CSUM_MASK?
> >
> > [...]
> >
> > MBR, Sergey
