17.05.2021 17:52, Krzysztof Kozlowski пишет:
>>>> +static int tegra_core_dev_init_opp_state(struct device *dev)
>>>> +{
>>>> + struct dev_pm_opp *opp;
>>>> + unsigned long rate;
>>>> + struct clk *clk;
>>>> + int err;
>>>> +
>>>> + clk = devm_clk_get(dev, NULL);
>>>> + if (IS_ERR(clk)) {
>>>> + dev_err(dev, "failed to get clk: %pe\n", clk);
>>>> + return PTR_ERR(clk);
>>>> + }
>>>> +
>>>> + rate = clk_get_rate(clk);
>>>> + if (!rate) {
>>>> + dev_err(dev, "failed to get clk rate\n");
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + opp = dev_pm_opp_find_freq_ceil(dev, &rate);
>>>> +
>>>> + if (opp == ERR_PTR(-ERANGE))
>>>> + opp = dev_pm_opp_find_freq_floor(dev, &rate);
>>>> +
>>>> + err = PTR_ERR_OR_ZERO(opp);
>>>> + if (err) {
>>>> + dev_err(dev, "failed to get OPP for %ld Hz: %d\n",
>>>> + rate, err);
>>>> + return err;
>>>> + }
>>>> +
>>>> + dev_pm_opp_put(opp);
>>>> +
>>>> + /* first dummy rate-setting initializes voltage vote */
>>>> + err = dev_pm_opp_set_rate(dev, rate);
>>>> + if (err) {
>>>> + dev_err(dev, "failed to initialize OPP clock: %d\n", err);
>>>> + return err;
>>>> + }
>>>
>>> The devm_pm_opp_set_clkname will call clk_get(), so here you should drop
>>> the clk reference at the end. Why having it twice?
>> The devm_pm_opp_set_clkname assigns clock to the OPP table.
>>
>> The devm_clk_get() is needed for the clk_get_rate(). OPP core doesn't
>> initialize voltage vote and we need this initialization for the Tegra
>> memory drivers.
> I did not get the answer to my question. Why you need to keep the clk
> reference past this point? Why you cannot drop it after getting rate?
>
>> The reference count of the clk will be dropped automatically once device
>> driver is released. The resource-managed helper avoids the need to care
>> about the error unwinding in the code, making it clean and easy to follow.
> I am not saying there is a leak.
>
The clk reference is not needed past this point. It doesn't hurt to have
additional reference since this allows to make code cleaner.
