Received-SPF: pass (google.com: domain of linux-kernel+bounces-2606-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <15c867d9-f77e-4b92-8b90-08d27116ce84@tuxon.dev>
Date: Sun, 17 Dec 2023 15:22:18 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net-next v2 17/21] net: ravb: Keep clock request
 operations grouped together
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,
 yoshihiro.shimoda.uh@renesas.com, wsa+renesas@sang-engineering.com,
 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: <20231214114600.2451162-1-claudiu.beznea.uj@bp.renesas.com>
 <20231214114600.2451162-18-claudiu.beznea.uj@bp.renesas.com>
 <2cb29821-7135-4369-ebc7-c742226e6230@omp.ru>
From: claudiu beznea <claudiu.beznea@tuxon.dev>
In-Reply-To: <2cb29821-7135-4369-ebc7-c742226e6230@omp.ru>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit


On 16.12.2023 21:43, Sergey Shtylyov wrote:
> On 12/14/23 2:45 PM, Claudiu wrote:
> 
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> Keep clock request operations grouped togeter to have all clock-related
>> code in a single place. This makes the code simpler to follow.
>>
>> 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 | 28 ++++++++++++------------
>>  1 file changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
>> index 38999ef1ea85..a2a64c22ec41 100644
>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>> @@ -2768,6 +2768,20 @@ static int ravb_probe(struct platform_device *pdev)
>>  	if (error)
>>  		goto out_reset_assert;
>>  
>> +	priv->clk = devm_clk_get(&pdev->dev, NULL);
>> +	if (IS_ERR(priv->clk)) {
>> +		error = PTR_ERR(priv->clk);
>> +		goto out_reset_assert;
>> +	}
>> +
>> +	if (info->gptp_ref_clk) {
>> +		priv->gptp_clk = devm_clk_get(&pdev->dev, "gptp");
>> +		if (IS_ERR(priv->gptp_clk)) {
>> +			error = PTR_ERR(priv->gptp_clk);
>> +			goto out_reset_assert;
>> +		}
>> +	}
>> +
>>  	priv->refclk = devm_clk_get_optional(&pdev->dev, "refclk");
>>  	if (IS_ERR(priv->refclk)) {
>>  		error = PTR_ERR(priv->refclk);
> 
>    Hmm... I think we currently have all these calls in one place.
> Perhaps you just shouldn't have moved this code around?

refclk have been moved at this point due to runtime PM. As refclk was
changed to be part of driver's runtime PM APIs we need to have it requested
(and prepared) before pm_runtime_resume_and_get(). Calling
pm_runtime_resume_and_get() will call driver's runtime PM resume.

The idea with this patch was to have all clock requests (clk, gptp, refclk)
in a single place (it's easier to follow the code this way, in my opinion).
If you prefer I can squash this patch with patch 07/21 "net: ravb: Move
reference clock enable/disable on runtime PM APIs". Please, let me know
what do you think.

> 
> MBR, Sergey