linux-kernel - Re: [PATCH net-next v2 17/21] net: ravb: Keep clock request operations grouped together

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <15c867d9-f77e-4b92-8b90-08d27116ce84@tuxon.dev>
Date: Sun, 17 Dec 2023 15:22:18 +0200
From: claudiu beznea <claudiu.beznea@...on.dev>
To: Sergey Shtylyov <s.shtylyov@....ru>, davem@...emloft.net,
 edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
 richardcochran@...il.com, p.zabel@...gutronix.de,
 yoshihiro.shimoda.uh@...esas.com, wsa+renesas@...g-engineering.com,
 geert+renesas@...der.be
Cc: netdev@...r.kernel.org, linux-renesas-soc@...r.kernel.org,
 linux-kernel@...r.kernel.org,
 Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
Subject: Re: [PATCH net-next v2 17/21] net: ravb: Keep clock request
 operations grouped together



On 16.12.2023 21:43, Sergey Shtylyov wrote:
> On 12/14/23 2:45 PM, Claudiu wrote:
> 
>> From: Claudiu Beznea <claudiu.beznea.uj@...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@...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