On 19.12.2023 20:54, Sergey Shtylyov wrote: > On 12/17/23 3:54 PM, claudiu beznea wrote: > > [...] > >>>>> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> >>>>> >>>>> DBAT setup was done in the driver's probe API. As some IP variants switch >>>>> to reset mode (and thus registers' content is lost) when setting clocks >>>>> (due to module standby functionality) to be able to implement runtime PM >>>>> move the DBAT configuration in the driver's ndo_open API. >>>>> >>>>> This commit prepares the code for the addition of runtime PM. >>>>> >>>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> >>>> >>>> Reviewed-by: Sergey Shtylyov <s.shtylyov@xxxxxx> >>>> >>>> [...] >>>>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c >>>>> index 04eaa1967651..6b8ca08be35e 100644 >>>>> --- a/drivers/net/ethernet/renesas/ravb_main.c >>>>> +++ b/drivers/net/ethernet/renesas/ravb_main.c >>>>> @@ -1822,6 +1822,7 @@ static int ravb_open(struct net_device *ndev) >>>>> napi_enable(&priv->napi[RAVB_NC]); >>>>> >>>>> ravb_set_delay_mode(ndev); >>>>> + ravb_write(ndev, priv->desc_bat_dma, DBAT); >>> >>> Looking at it again, I suspect this belong in ravb_dmac_init()... >> >> ravb_dmac_init() is called from multiple places in this driver, e.g., > > It's purpose is to configure AVB-DMAC and DBAT is the AVB-DMAC register, > right? It is. But it is pointless to configure it more than one time after ravb_open() has been called as the register content is not changed until IP enters reset mode (though ravb_close() now). > >> ravb_set_ringparam(), ravb_tx_timeout_work(). > > I know. Its value is only calculated once, in ravb_probe(), right? right > >> I'm afraid we may broke the behavior of these if DBAT setup is moved I was wrong here. DBAT is not changed by IP while TX/RX is working. > > Do not be afraid! :-) > >> in ravb_dmac_init(). This is also >> valid for setting delay (see patch 10/12). > > I don't think there will be a problem either... but maybe we > should call it in ravb_emac_init() indeed. > > [...] > > MBR, Sergey