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.
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
