Message-ID: <MN2PR19MB369398DE5E919BBFB38D7EAEB1239@MN2PR19MB3693.namprd19.prod.outlook.com>
Date:   Tue, 11 Oct 2022 07:33:56 +0000
From:   Rahul Tanwar <rtanwar@...linear.com>
To:     Stephen Boyd <sboyd@...nel.org>,
        "linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>,
        "mturquette@...libre.com" <mturquette@...libre.com>
CC:     linux-lgm-soc <linux-lgm-soc@...linear.com>,
        Yi xin Zhu <yzhu@...linear.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RESEND v2 3/5] clk: mxl: Avoid disabling gate clocks from
 clk driver

Hi Stephen,

On 6/10/2022 4:20 am, Stephen Boyd wrote:
> This email was sent from outside of MaxLinear.
> 
> 
> Quoting Rahul Tanwar (2022-10-05 02:36:00)
>>>>
>>>
>>> Why is the clk driver probing on the new SoCs? Is it providing
>>> something? Can we detect that the power management IP exists and not
>>> register these clks?
>>>
>>
>> We discussed in the team about not registering gate clks at all as you
>> mentioned. But if we do that, all peripheral drivers that use these clks
>> would need modifications so their probe does not fail due to failure
>> returns of clk related standard calls for e.g devm_clk_get(),
>> clk_prepare_enable(). These are standard calls in probe for all the
>> drivers and a lot of them use gate clks. So this would need a lot of
>> changes with possibility of breaking working functionalities.
> 
> Why? We can have clk_get() return NULL when these clks can't be used.
> This is how we implement "dummy" clks on systems where the clk provider
> can provide the clk but there's nothing to do for the clk operations.
> The clk API treats NULL as a valid clk but bails out early when calling
> any consumer APIs.
> 


I agree that what you are suggesting is the ideal way to handle such
clks. If i understand you correctly, you are suggesting below (please
correct me if i am mistaken):

1. Disable/remove such clocks from corresponding driver's devicetree
nodes. This will make devm_clk_get() or clk_get() return failure.

2. Modify all drivers which use such clks

from:

     clk = devm_clk_get(...);
     if (IS_ERR(clk))
         return -ERROR;
     clk_prepare_enable(clk);
     clk_get/set_rate();
     ...

to:
     clk = devm_clk_get(...);
     if (!(IS_ERR(clk)) {
         clk_prepare_enable(clk);
         clk_get/set_rate();
         ...
     } else {
        // print error info - do nothing, no clk_ops calls
     }

But the problem that we have is that 80% of the clks that we use fall
under this category (around 65 clks). And if we follow this approach,
then we will have to make above changes in all of the drivers which use
these clks & retest them again. That seems like a overhaul. We already
keep a internal driver flag in each clk entry data structure and we
already use it in the driver for other types of clks for e.g MUX_CLKs.
So using the internal flag to handle this becomes a preferable &
existing driver design aligned approach for us.

Thanks,
Rahul


>>
>> Also, i incorrectly mentioned about the reason being backward
>> compatibility with older SoCs. Main reason is to support different power
>> profiles use cases as per end product requirements some of which might
>> control it from clk framework i.e. this driver. We keep a internal
>> driver flag just for this purpose to provide this flexibility depending
>> on the use case which is what we have used here.
>>
>> I am sending v3 with more clear & correct description about it to
>> justify the need for these changes.
>>
> 
> Ok
> 
> 

Powered by blists - more mailing lists