On 19.12.2023 18:49, Sergey Shtylyov wrote: > On 12/15/23 1:04 PM, claudiu beznea wrote: > [...] >>>> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> >>>> >>>> CSR.OPS bits specify the current operating mode and (according to >>>> documentation) they are updated when the operating mode change request >>>> is processed. Thus, check CSR.OPS before proceeding. >>> >>> The manuals I have indeed say we need to check CSR.OPS... But we only >>> need to wait iff we transfer from the operation mode to the config mode... >> >> RZ/G3S manual say about CSR.OPS "These bits are updated when an operating > > I was unable to find the RZ/G3 manuals on ther Renesas' website... :-( > >> mode changes is processed". From this I get we need to check it for any mode. > > I don't argue with the (safety) checking of CSR.OPS, I was just pointing > out that the R-Car gen3 manual says that only transfer from operation to > the config mode happens after a considerable amount of time, other transfers > do happen immediately after updating CCC.OPC. > >> Also, on configuration procedure (of RZ/G3S) it say CSR.OPS need to be >> checked when switching from reset -> config. > > Just checked or waited on? Manual say to check that the read value of CSR.OPS=2 > The R-car does have a specific algorithm for transferring from the operation > to the reset mode (you need to set CC.DTSR first and then wait for CSR.DTS to > clear before updating CCC.OPC)... In the stop procedure chapter of RZ/G3S manual, as the setup of DMAC.CCC=0 is the last operation that needs to be done, it doesn't specify either that CSR needs to be checked. > > [...] > >>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> >>>> --- >>>> drivers/net/ethernet/renesas/ravb_main.c | 47 ++++++++++++++++++++---- >>>> 1 file changed, 39 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c >>>> index 9178f6d60e74..ce95eb5af354 100644 >>>> --- a/drivers/net/ethernet/renesas/ravb_main.c >>>> +++ b/drivers/net/ethernet/renesas/ravb_main.c > [...] >>>> @@ -1744,6 +1747,18 @@ static inline int ravb_hook_irq(unsigned int irq, irq_handler_t handler, >>>> return error; >>>> } >>>> >>>> +static int ravb_set_reset_mode(struct net_device *ndev) >>>> +{ >>>> + int error; >>>> + >>>> + ravb_write(ndev, CCC_OPC_RESET, CCC); >>>> + error = ravb_wait(ndev, CSR, CSR_OPS, CSR_OPS_RESET); >>>> + if (error) >>>> + netdev_err(ndev, "failed to switch device to reset mode\n"); >>>> + >>>> + return error; >>>> +} >>>> + >>> >>> Again, ravb_wait() call doesn't seem necessary here... >> >> Ok. I followed the guideline from the description of CSR.OPS. Let me know >> if you want to keep it or not. I think I haven't saw any issues w/o this. > > Yes, please remove the waiting. > > [...] > > MBR, Sergey