[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2e70a095-8079-84f1-f842-eb90059610ed@omp.ru>
Date: Tue, 19 Dec 2023 19:49:29 +0300
From: Sergey Shtylyov <s.shtylyov@....ru>
To: claudiu beznea <claudiu.beznea@...on.dev>, <davem@...emloft.net>,
<edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>,
<claudiu.beznea.uj@...renesas.com>, <yoshihiro.shimoda.uh@...esas.com>,
<wsa+renesas@...g-engineering.com>, <niklas.soderlund+renesas@...natech.se>,
<biju.das.jz@...renesas.com>, <prabhakar.mahadev-lad.rj@...renesas.com>,
<mitsuhiro.kimura.kc@...esas.com>, <geert+renesas@...der.be>
CC: <netdev@...r.kernel.org>, <linux-renesas-soc@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net 1/2] net: ravb: Wait for operation mode to be applied
On 12/15/23 1:04 PM, claudiu beznea wrote:
[...]
>>> From: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
>>>
>>> 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?
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)...
[...]
>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
>>> ---
>>> 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
Powered by blists - more mailing lists