On 09.01.2024 21:53, Sergey Shtylyov wrote: > On 1/5/24 11:23 AM, Claudiu wrote: > >> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> >> >> Add runtime PM support for the ravb driver. As the driver is used by >> different IP variants, with different behaviors, to be able to have the >> runtime PM support available for all devices, the preparatory commits >> moved all the resources parsing and allocations in the driver's probe >> function and kept the settings for ravb_open(). This is due to the fact >> that on some IP variants-platforms tuples disabling/enabling the clocks >> will switch the IP to the reset operation mode where registers' content is >> lost and reconfiguration needs to be done. For this the rabv_open() >> function enables the clocks, switches the IP to configuration mode, applies >> all the registers settings and switches the IP to the operational mode. At >> the end of ravb_open() IP is ready to send/receive data. >> >> In ravb_close() necessary reverts are done (compared with ravb_open()), the >> IP is switched to reset mode and clocks are disabled. >> >> The ethtool APIs or IOCTLs that might execute while the interface is down >> are either cached (and applied in ravb_open()) or rejected (as at that time >> the IP is in reset mode). Keeping the IP in the reset mode also increases >> the power saved (according to the hardware manual). >> >> Reviewed-by: Sergey Shtylyov <s.shtylyov@xxxxxx> >> 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 e909960fbc30..e99351fe8d7f 100644 >> --- a/drivers/net/ethernet/renesas/ravb_main.c >> +++ b/drivers/net/ethernet/renesas/ravb_main.c > [...] >> @@ -2233,7 +2243,14 @@ static int ravb_close(struct net_device *ndev) >> ravb_get_stats(ndev); >> >> /* Set reset mode. */ >> - return ravb_set_opmode(ndev, CCC_OPC_RESET); >> + error = ravb_set_opmode(ndev, CCC_OPC_RESET); >> + if (error) >> + return error; >> + >> + pm_runtime_mark_last_busy(dev); >> + pm_runtime_put_autosuspend(dev); > > BTW, doesn't make sense to check/return the result? Most users of this are not checking return code, I followed this pattern, too. But, yes, would be better to check it, AFAICT. > >> + >> + return 0; >> } >> >> static int ravb_hwtstamp_get(struct net_device *ndev, struct ifreq *req) > [...] > > MBR, Sergey
