Received-SPF: pass (google.com: domain of linux-kernel+bounces-42841-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <1c680be4-67b7-435b-9609-b5f7a45ca675@tuxon.dev>
Date: Mon, 29 Jan 2024 15:55:54 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net-next v4 10/15] net: ravb: Move delay mode set in the
 driver's ndo_open API
Content-Language: en-US
To: Sergey Shtylyov <s.shtylyov@omp.ru>, davem@davemloft.net,
 edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
 richardcochran@gmail.com, p.zabel@pengutronix.de, geert+renesas@glider.be
Cc: netdev@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
 linux-kernel@vger.kernel.org,
 Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
References: <20240123125829.3970325-1-claudiu.beznea.uj@bp.renesas.com>
 <20240123125829.3970325-11-claudiu.beznea.uj@bp.renesas.com>
 <b0a4f320-6218-2631-a5a0-26cfa9a25ee1@omp.ru>
From: claudiu beznea <claudiu.beznea@tuxon.dev>
In-Reply-To: <b0a4f320-6218-2631-a5a0-26cfa9a25ee1@omp.ru>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit


On 24.01.2024 22:18, Sergey Shtylyov wrote:
> On 1/23/24 3:58 PM, Claudiu wrote:
> 
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> Delay parsing and setting were done in the driver's probe API. As some IP
>> variants switch to reset mode (and thus registers content is lost) when
> 
>    s/registers content/the register contents/, perhaps?
> 
>> setting clocks (due to module standby functionality) to be able to
>> implement runtime PM keep the delay parsing in the driver's probe function
>> and move the delay applying function to the driver's ndo_open API.
>>
>> Along with it, both delay specific functions were kept together.
>>
>> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> [...]
> 
>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
>> index 8956c33c60e2..69d4854d6738 100644
>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>> @@ -1800,6 +1800,59 @@ static int ravb_compute_gti(struct net_device *ndev)
>>  	return 0;
>>  }
>>  
>> +/* Set tx and rx clock internal delay modes */
>> +static void ravb_parse_delay_mode(struct device_node *np, struct net_device *ndev)
> 
>    Wait, why are you moving this? Its call site didn't move -- it's still
> down in the probe() method...

As explained in the patch description ("Along with it, both delay specific
functions were kept together.") functions were moved closed to each other
to have the code dealing with delay in the same place.

> 
> [...]
>> @@ -2785,10 +2793,7 @@ static int ravb_probe(struct platform_device *pdev)
>>  	if (error)
>>  		goto out_rpm_put;
>>  
>> -	if (info->internal_delay) {
>> -		ravb_parse_delay_mode(np, ndev);
>> -		ravb_set_delay_mode(ndev);
>> -	}
>> +	ravb_parse_delay_mode(np, ndev);
>>  
>>  	/* Allocate descriptor base address table */
>>  	priv->desc_bat_size = sizeof(struct ravb_desc) * DBAT_ENTRY_NUM;
>>
> 
> MBR, Sergey