Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to ->init()
On Wednesday 09 July 2014 11:27 AM, Viresh Kumar wrote: > On 9 July 2014 20:56, Santosh Shilimkar wrote: We still *don't* support platforms with separate clock/voltage lines for CPUs. This would be done in a separate patch. > > Probably s/separate/later would be enough to make you happy ? > For reviewer, its easy if this later is "in the current series" or really a "later" series. Anyway I understood it now, so its upto you. regards, Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to ->init()
On 9 July 2014 20:59, Santosh Shilimkar wrote: > On Wednesday 09 July 2014 11:27 AM, Viresh Kumar wrote: >> On 9 July 2014 20:56, Santosh Shilimkar wrote: > We still *don't* support platforms with separate clock/voltage lines for > CPUs. > This would be done in a separate patch. >> >> Probably s/separate/later would be enough to make you happy ? >> > For reviewer, its easy if this later is "in the current series" or > really a "later" series. As you say BOSS :) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to ->init()
On Wednesday 09 July 2014 11:17 AM, Viresh Kumar wrote: > On 9 July 2014 20:23, Santosh Shilimkar wrote: >> I scanned this patch and subsequent patches from the series. Since you are >> modifying the interfaces and bindings, I just think its better if we can >> address the cases where separate clock lines will be used by CPUs. >> >> Surely don't want to increase your work neither want hold the progress >> of the series but if you look at the changes to the interfaces, they >> give you a feeling of incompleteness. > > Lets talk in the other thread you raised, 2/14 .. > >>> Signed-off-by: Viresh Kumar >>> --- >> Would you able to give some idea about what will it take to address that one >> remainder case as well as part of this series. > > Which one? This: > >> We still *don't* support platforms with separate clock/voltage lines for >> CPUs. >> This would be done in a separate patch. > > Its already fixed as part of this series. > I suggest you word the commit in that case. Saying subsequent patch adds support for the remainder case. Let me scan the remainder patches again to see how its done. Regards, Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to ->init()
On 9 July 2014 20:56, Santosh Shilimkar wrote: >>> We still *don't* support platforms with separate clock/voltage lines for >>> CPUs. >>> This would be done in a separate patch. Probably s/separate/later would be enough to make you happy ? >> Its already fixed as part of this series. >> > I suggest you word the commit in that case. Saying subsequent patch > adds support for the remainder case. Let me scan the remainder > patches again to see how its done. > > Regards, > Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to ->init()
On 9 July 2014 20:23, Santosh Shilimkar wrote: > I scanned this patch and subsequent patches from the series. Since you are > modifying the interfaces and bindings, I just think its better if we can > address the cases where separate clock lines will be used by CPUs. > > Surely don't want to increase your work neither want hold the progress > of the series but if you look at the changes to the interfaces, they > give you a feeling of incompleteness. Lets talk in the other thread you raised, 2/14 .. >> Signed-off-by: Viresh Kumar >> --- > Would you able to give some idea about what will it take to address that one > remainder case as well as part of this series. Which one? This: > We still *don't* support platforms with separate clock/voltage lines for CPUs. > This would be done in a separate patch. Its already fixed as part of this series. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to ->init()
On Tuesday 01 July 2014 12:32 PM, Viresh Kumar wrote: > Currently this driver only support platforms on which all CPUs share clock & > voltage lines and there is requirement to support platforms which have > separate > clock & voltage lines for CPUs, like Qualcomm's Krait and ARM's big LITTLE. > > Each group of CPUs sharing clock/voltage lines are represented by 'struct > cpufreq_policy' in cpufreq framework. And core calls ->init() once for each > policy. > > Currently we do all initialization/allocation from probe() which wouldn't work > for above scenario. To make it work for these platforms, the first step is to > move all initialization/allocation to ->init() and add ->exit() to do the > reverse of it. > > Also, remove all global variables and allocate space for them at runtime. > > This patch creates 'struct private_data' for keeping all such information and > a pointer to that would be stored in policy->driver_data. > > The changed probe() routine now tries to see if regulator/clocks are available > or we need to defer probe. In case they are available, it registers cpufreq > driver. Otherwise, returns with -EPROBE_DEFER. > > We still *don't* support platforms with separate clock/voltage lines for CPUs. > This would be done in a separate patch. > I scanned this patch and subsequent patches from the series. Since you are modifying the interfaces and bindings, I just think its better if we can address the cases where separate clock lines will be used by CPUs. Surely don't want to increase your work neither want hold the progress of the series but if you look at the changes to the interfaces, they give you a feeling of incompleteness. > Signed-off-by: Viresh Kumar > --- Would you able to give some idea about what will it take to address that one remainder case as well as part of this series. Regards, Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to -init()
On Tuesday 01 July 2014 12:32 PM, Viresh Kumar wrote: Currently this driver only support platforms on which all CPUs share clock voltage lines and there is requirement to support platforms which have separate clock voltage lines for CPUs, like Qualcomm's Krait and ARM's big LITTLE. Each group of CPUs sharing clock/voltage lines are represented by 'struct cpufreq_policy' in cpufreq framework. And core calls -init() once for each policy. Currently we do all initialization/allocation from probe() which wouldn't work for above scenario. To make it work for these platforms, the first step is to move all initialization/allocation to -init() and add -exit() to do the reverse of it. Also, remove all global variables and allocate space for them at runtime. This patch creates 'struct private_data' for keeping all such information and a pointer to that would be stored in policy-driver_data. The changed probe() routine now tries to see if regulator/clocks are available or we need to defer probe. In case they are available, it registers cpufreq driver. Otherwise, returns with -EPROBE_DEFER. We still *don't* support platforms with separate clock/voltage lines for CPUs. This would be done in a separate patch. I scanned this patch and subsequent patches from the series. Since you are modifying the interfaces and bindings, I just think its better if we can address the cases where separate clock lines will be used by CPUs. Surely don't want to increase your work neither want hold the progress of the series but if you look at the changes to the interfaces, they give you a feeling of incompleteness. Signed-off-by: Viresh Kumar viresh.ku...@linaro.org --- Would you able to give some idea about what will it take to address that one remainder case as well as part of this series. Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to -init()
On 9 July 2014 20:23, Santosh Shilimkar santosh.shilim...@ti.com wrote: I scanned this patch and subsequent patches from the series. Since you are modifying the interfaces and bindings, I just think its better if we can address the cases where separate clock lines will be used by CPUs. Surely don't want to increase your work neither want hold the progress of the series but if you look at the changes to the interfaces, they give you a feeling of incompleteness. Lets talk in the other thread you raised, 2/14 .. Signed-off-by: Viresh Kumar viresh.ku...@linaro.org --- Would you able to give some idea about what will it take to address that one remainder case as well as part of this series. Which one? This: We still *don't* support platforms with separate clock/voltage lines for CPUs. This would be done in a separate patch. Its already fixed as part of this series. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to -init()
On 9 July 2014 20:56, Santosh Shilimkar santosh.shilim...@ti.com wrote: We still *don't* support platforms with separate clock/voltage lines for CPUs. This would be done in a separate patch. Probably s/separate/later would be enough to make you happy ? Its already fixed as part of this series. I suggest you word the commit in that case. Saying subsequent patch adds support for the remainder case. Let me scan the remainder patches again to see how its done. Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to -init()
On Wednesday 09 July 2014 11:17 AM, Viresh Kumar wrote: On 9 July 2014 20:23, Santosh Shilimkar santosh.shilim...@ti.com wrote: I scanned this patch and subsequent patches from the series. Since you are modifying the interfaces and bindings, I just think its better if we can address the cases where separate clock lines will be used by CPUs. Surely don't want to increase your work neither want hold the progress of the series but if you look at the changes to the interfaces, they give you a feeling of incompleteness. Lets talk in the other thread you raised, 2/14 .. Signed-off-by: Viresh Kumar viresh.ku...@linaro.org --- Would you able to give some idea about what will it take to address that one remainder case as well as part of this series. Which one? This: We still *don't* support platforms with separate clock/voltage lines for CPUs. This would be done in a separate patch. Its already fixed as part of this series. I suggest you word the commit in that case. Saying subsequent patch adds support for the remainder case. Let me scan the remainder patches again to see how its done. Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to -init()
On 9 July 2014 20:59, Santosh Shilimkar santosh.shilim...@ti.com wrote: On Wednesday 09 July 2014 11:27 AM, Viresh Kumar wrote: On 9 July 2014 20:56, Santosh Shilimkar santosh.shilim...@ti.com wrote: We still *don't* support platforms with separate clock/voltage lines for CPUs. This would be done in a separate patch. Probably s/separate/later would be enough to make you happy ? For reviewer, its easy if this later is in the current series or really a later series. As you say BOSS :) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to -init()
On Wednesday 09 July 2014 11:27 AM, Viresh Kumar wrote: On 9 July 2014 20:56, Santosh Shilimkar santosh.shilim...@ti.com wrote: We still *don't* support platforms with separate clock/voltage lines for CPUs. This would be done in a separate patch. Probably s/separate/later would be enough to make you happy ? For reviewer, its easy if this later is in the current series or really a later series. Anyway I understood it now, so its upto you. regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to ->init()
On 3 July 2014 06:13, Stephen Boyd wrote: > drivers/cpufreq/cpufreq-generic.c:313:13: warning: 'cdev' may be used > uninitialized in this function [-Wmaybe-uninitialized] > > So I guess we should initialize it to NULL? I somehow didn't got this, I checked again. I have fixed it this way: diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c index e6dc8ea..05a18bd 100644 --- a/drivers/cpufreq/cpufreq-cpu0.c +++ b/drivers/cpufreq/cpufreq-cpu0.c @@ -240,10 +240,11 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy) dev_err(cpu_dev, "running cpufreq without cooling device: %ld\n", PTR_ERR(cdev)); + else + priv->cdev = cdev; } of_node_put(np); - priv->cdev = cdev; priv->cpu_dev = cpu_dev; priv->cpu_reg = cpu_reg; policy->driver_data = priv; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to ->init()
On 07/01/14 09:32, Viresh Kumar wrote:
> +static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
> +{
> + struct cpufreq_frequency_table *freq_table;
> + struct thermal_cooling_device *cdev;
This..
> + struct device_node *np;
> + struct private_data *priv;
> +
[...]
> ing them.
> @@ -223,28 +242,92 @@ static int cpu0_cpufreq_probe(struct platform_device
> *pdev)
> "running cpufreq without cooling device: %ld\n",
> PTR_ERR(cdev));
> }
> -
> of_node_put(np);
> +
> + priv->cdev = cdev;
Causes a build warning:
drivers/cpufreq/cpufreq-generic.c:313:13: warning: 'cdev' may be used
uninitialized in this function [-Wmaybe-uninitialized]
So I guess we should initialize it to NULL?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to -init()
On 07/01/14 09:32, Viresh Kumar wrote:
+static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
+{
+ struct cpufreq_frequency_table *freq_table;
+ struct thermal_cooling_device *cdev;
This..
+ struct device_node *np;
+ struct private_data *priv;
+
[...]
ing them.
@@ -223,28 +242,92 @@ static int cpu0_cpufreq_probe(struct platform_device
*pdev)
running cpufreq without cooling device: %ld\n,
PTR_ERR(cdev));
}
-
of_node_put(np);
+
+ priv-cdev = cdev;
Causes a build warning:
drivers/cpufreq/cpufreq-generic.c:313:13: warning: 'cdev' may be used
uninitialized in this function [-Wmaybe-uninitialized]
So I guess we should initialize it to NULL?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/14] cpufreq: cpu0: Move per-cluster initialization code to -init()
On 3 July 2014 06:13, Stephen Boyd sb...@codeaurora.org wrote: drivers/cpufreq/cpufreq-generic.c:313:13: warning: 'cdev' may be used uninitialized in this function [-Wmaybe-uninitialized] So I guess we should initialize it to NULL? I somehow didn't got this, I checked again. I have fixed it this way: diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c index e6dc8ea..05a18bd 100644 --- a/drivers/cpufreq/cpufreq-cpu0.c +++ b/drivers/cpufreq/cpufreq-cpu0.c @@ -240,10 +240,11 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy) dev_err(cpu_dev, running cpufreq without cooling device: %ld\n, PTR_ERR(cdev)); + else + priv-cdev = cdev; } of_node_put(np); - priv-cdev = cdev; priv-cpu_dev = cpu_dev; priv-cpu_reg = cpu_reg; policy-driver_data = priv; -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
