Received-SPF: pass (google.com: domain of linux-kernel+bounces-6815-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <a617f464-e61e-44a7-8483-8890d4b2142f@tuxon.dev>
Date: Wed, 20 Dec 2023 13:27:43 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net 1/2] net: ravb: Wait for operation mode to be applied
Content-Language: en-US
To: Sergey Shtylyov <s.shtylyov@omp.ru>, davem@davemloft.net,
 edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
 claudiu.beznea.uj@bp.renesas.com, yoshihiro.shimoda.uh@renesas.com,
 wsa+renesas@sang-engineering.com, niklas.soderlund+renesas@ragnatech.se,
 biju.das.jz@bp.renesas.com, prabhakar.mahadev-lad.rj@bp.renesas.com,
 mitsuhiro.kimura.kc@renesas.com, geert+renesas@glider.be
Cc: netdev@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
 linux-kernel@vger.kernel.org
References: <20231214113137.2450292-1-claudiu.beznea.uj@bp.renesas.com>
 <20231214113137.2450292-2-claudiu.beznea.uj@bp.renesas.com>
 <d08dbbd4-2e63-c436-6935-df68c291bf75@omp.ru>
 <0b807496-f387-4aef-8650-a43a9249468f@tuxon.dev>
 <2e70a095-8079-84f1-f842-eb90059610ed@omp.ru>
From: claudiu beznea <claudiu.beznea@tuxon.dev>
In-Reply-To: <2e70a095-8079-84f1-f842-eb90059610ed@omp.ru>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit


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@bp.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?

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@bp.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