From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754315AbcBBCeW (ORCPT ); Mon, 1 Feb 2016 21:34:22 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:44362 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753812AbcBBCeT (ORCPT ); Mon, 1 Feb 2016 21:34:19 -0500 Date: Mon, 1 Feb 2016 18:34:18 -0800 From: Stephen Boyd To: Viresh Kumar Cc: Rafael Wysocki , linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org, nm@ti.com, open list Subject: Re: [PATCH V2 11/16] cpufreq: dt: Pass regulator name to the OPP core Message-ID: <20160202023418.GK4848@codeaurora.org> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 01/28, Viresh Kumar wrote: > @@ -119,6 +120,49 @@ static int set_target(struct cpufreq_policy *policy, unsigned int index) > return ret; > } > > +/* > + * An earlier version of opp-v1 bindings used to name the regulator > + * "cpu0-supply", we still need to handle that for backwards compatibility. > + */ > +static const char *find_supply_name(struct device *dev) > +{ > + struct regulator *cpu_reg; > + char *reg_cpu0 = "cpu0", *reg_cpu = "cpu", *reg; > + int cpu = dev->id, ret; > + > + /* Try "cpu0" for older DTs */ > + if (!cpu) > + reg = reg_cpu0; > + else > + reg = reg_cpu; > + > +try_again: > + cpu_reg = regulator_get_optional(dev, reg); > + ret = PTR_ERR_OR_ZERO(cpu_reg); > + if (!ret) { > + regulator_put(cpu_reg); What's the point of creating a regulator just to find the name? It seems like we should just look in the DT node of the CPU for cpu-supply vs cpu0-supply. Then we don't need to involve the regulator framework at all. > + return reg; > + } > + > + /* > + * If cpu's regulator supply node is present, but regulator is not yet > + * registered, we should try defering probe. > + */ > + if (ret == -EPROBE_DEFER) { > + dev_dbg(dev, "cpu%d regulator not ready, retry\n", cpu); > + return ERR_PTR(ret); > + } > + > + /* Try with "cpu-supply" */ > + if (reg == reg_cpu0) { > + reg = reg_cpu; > + goto try_again; > + } > + > + dev_dbg(dev, "no regulator for cpu%d: %d\n", cpu, ret); > + return NULL; > +} > + > static int allocate_resources(int cpu, struct device **cdev, > struct regulator **creg, struct clk **cclk) > { > @@ -383,6 +450,9 @@ static int cpufreq_exit(struct cpufreq_policy *policy) > cpufreq_cooling_unregister(priv->cdev); > dev_pm_opp_free_cpufreq_table(priv->cpu_dev, &policy->freq_table); > dev_pm_opp_of_cpumask_remove_table(policy->related_cpus); > + if (priv->reg_name) > + dev_pm_opp_put_regulator(priv->cpu_dev); Let's hope this goes away because it's always right next to dev_pm_opp_of_cpumask_remove_table() anyway. Same for reg_name. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project