From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

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

Changes in v2:
- none; this patch is new

 drivers/net/ethernet/renesas/ravb_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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);
 
 	/* Device init */
 	error = ravb_dmac_init(ndev);
@@ -2841,7 +2842,6 @@ static int ravb_probe(struct platform_device *pdev)
 	}
 	for (q = RAVB_BE; q < DBAT_ENTRY_NUM; q++)
 		priv->desc_bat[q].die_dt = DT_EOS;
-	ravb_write(ndev, priv->desc_bat_dma, DBAT);
 
 	/* Initialise HW timestamp list */
 	INIT_LIST_HEAD(&priv->ts_skb_list);
-- 
2.39.2

On 12/14/23 2:45 PM, Claudiu wrote:

> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> 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@bp.renesas.com>

Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>

[...]
> 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);
>  
>  	/* Device init */
>  	error = ravb_dmac_init(ndev);
> @@ -2841,7 +2842,6 @@ static int ravb_probe(struct platform_device *pdev)
>  	}
>  	for (q = RAVB_BE; q < DBAT_ENTRY_NUM; q++)
>  		priv->desc_bat[q].die_dt = DT_EOS;
> -	ravb_write(ndev, priv->desc_bat_dma, DBAT);
>  
>  	/* Initialise HW timestamp list */
>  	INIT_LIST_HEAD(&priv->ts_skb_list);
> 

  How about also removing the DBAT write from ravb_resume()?

MBR, Sergey

On 12/15/23 12:03 AM, Sergey Shtylyov wrote:
[...]

>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> 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@bp.renesas.com>
> 
> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> 
> [...]
>> 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()...

>>  
>>  	/* Device init */
>>  	error = ravb_dmac_init(ndev);
[...]

MBR, Sergey

On 15.12.2023 22:01, Sergey Shtylyov wrote:
> On 12/15/23 12:03 AM, Sergey Shtylyov wrote:
> [...]
> 
>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>
>>> 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@bp.renesas.com>
>>
>> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
>>
>> [...]
>>> 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.,
ravb_set_ringparam(), ravb_tx_timeout_work(). I'm afraid we may broke the
behavior of these if DBAT setup is moved in ravb_dmac_init(). This is also
valid for setting delay (see patch 10/12).

> 
>>>  
>>>  	/* Device init */
>>>  	error = ravb_dmac_init(ndev);
> [...]
> 
> MBR, Sergey

On 12/17/23 3:54 PM, claudiu beznea wrote:

[...]

>>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>>
>>>> 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@bp.renesas.com>
>>>
>>> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
>>>
>>> [...]
>>>> 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?

> ravb_set_ringparam(), ravb_tx_timeout_work().

   I know. Its value is only calculated once, in ravb_probe(), right?

> I'm afraid we may broke the behavior of these if DBAT setup is moved

   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

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@bp.renesas.com>
>>>>>
>>>>> 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@bp.renesas.com>
>>>>
>>>> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
>>>>
>>>> [...]
>>>>> 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

On 12/20/23 2:41 PM, claudiu beznea wrote:

[...]

>>>>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>>>>
>>>>>> 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@bp.renesas.com>
>>>>>
>>>>> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
>>>>>
>>>>> [...]
>>>>>> 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).

   The same is true for the most registers set by ravb_dmac_init()!

[...]

MBR, Sergey