Re: [PATCH 08/13] net: ravb: Rely on PM domain to enable refclk

From: Sergey Shtylyov
Date: Thu Nov 23 2023 - 14:26:53 EST


On 11/23/23 8:10 PM, claudiu beznea wrote:
[...]

>> On Thu, Nov 23, 2023 at 5:35 AM Claudiu <claudiu.beznea@xxxxxxxxx> wrote:
>>> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
[...]
>>> w/o the need to add clock enable/disable specific calls in runtime PM
>>> ops of ravb driver and interfere with other IP specific implementations,
>>> add a new variable to struct_hw_info and enable the reference clock
>>> based on the value of this variable (the variable states if reference
>>> clock is part of the Ethernet's power domain).
>>>
>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>

[...]
>>> static const struct of_device_id ravb_match_table[] = {
>>> @@ -2749,12 +2750,14 @@ static int ravb_probe(struct platform_device *pdev)
>>> goto out_release;
>>> }
>>>
>>> - priv->refclk = devm_clk_get_optional(&pdev->dev, "refclk");
>>> - if (IS_ERR(priv->refclk)) {
>>> - error = PTR_ERR(priv->refclk);
>>> - goto out_release;
>>> + if (!info->refclk_in_pd) {
>>> + priv->refclk = devm_clk_get_optional(&pdev->dev, "refclk");
>>> + if (IS_ERR(priv->refclk)) {
>>> + error = PTR_ERR(priv->refclk);
>>> + goto out_release;
>>> + }
>>> + clk_prepare_enable(priv->refclk);
>>> }
>>> - clk_prepare_enable(priv->refclk);
>>
>> Is this patch really needed? It doesn't hurt to manually enable a
>> clock that is also under Runtime PM control. Clock prepare/enable
>> refcounting will take care of that.
>
> I agree with that. I chose this path to not interfere w/ the comments
> ravb_runtime_nop() which I didn't understand. Also I fail to understand why
> the ravb_runtime_nop() is there...

Looks like it was blindly copied from the sh_eth driver and doesn't (yet?)
apply to ravb...

[...]

MBR, Sergey