On 2/13/24 12:41 PM, Claudiu wrote: > From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> > > Commit c2da9408579d ("ravb: Add Rx checksum offload support for GbEth") > introduced support for setting GbEth features. With this the IP-specific > features update functions update the ndev->features individually. > > Next commits add runtime PM support for the ravb driver. The runtime PM > implementation will enable/disable the IP clocks on > the ravb_open()/ravb_close() functions. Accessing the IP registers with > clocks disabled blocks the system. > > The ravb_set_features() function could be executed when the Ethernet > interface is closed so we need to ensure we don't access IP registers while > the interface is down when runtime PM support will be in place. > > For these, move the update of ndev->features to ravb_set_features() and > make the IP-specific features set function return int. In this way we > update the ndev->features only when the IP-specific features set function > returns success and we can avoid code duplication when introducing > runtime PM registers protection. > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> [...] > diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c > index 7a7f743a1fef..b3b91783bb7a 100644 > --- a/drivers/net/ethernet/renesas/ravb_main.c > +++ b/drivers/net/ethernet/renesas/ravb_main.c > @@ -2475,7 +2475,7 @@ static int ravb_change_mtu(struct net_device *ndev, int new_mtu) > return 0; > } > > -static void ravb_set_rx_csum(struct net_device *ndev, bool enable) > +static int ravb_set_rx_csum(struct net_device *ndev, bool enable) > { > struct ravb_private *priv = netdev_priv(ndev); > unsigned long flags; > @@ -2492,6 +2492,8 @@ static void ravb_set_rx_csum(struct net_device *ndev, bool enable) > ravb_rcv_snd_enable(ndev); > > spin_unlock_irqrestore(&priv->lock, flags); > + > + return 0; > } > > static int ravb_endisable_csum_gbeth(struct net_device *ndev, enum ravb_reg reg, Wait! You're not updating the call site of ravb_set_rx_csum(), are you? It looks like the above 2 hunks aren't needed... [...] MBR, Sergey