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

[Sergey Shtylyov <s.shtylyov@omp.ru>]

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

>> On Thu, Nov 23, 2023 at 5:35 AM Claudiu <claudiu.beznea@tuxon.dev> wrote:
>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
[...]
>>> 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@bp.renesas.com>

[...]
>>>  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