RE: [PATCH v5 19/21] clk: tegra: Add Super Gen5 Logic
> -Original Message- > From: ble...@google.com [mailto:ble...@google.com] On Behalf Of Benson > Leung > Sent: Friday, May 15, 2015 3:37 AM > To: Rhyland Klein > Cc: Peter De Schrijver; Mike Turquette; Stephen Warren; Stephen Boyd; Thierry > Reding; Alexandre Courbot; Bill Huang; Paul Walmsley; Jim Lin; linux- > c...@vger.kernel.org; linux-te...@vger.kernel.org; > linux-kernel@vger.kernel.org > Subject: Re: [PATCH v5 19/21] clk: tegra: Add Super Gen5 Logic > > On Tue, May 12, 2015 at 10:24 AM, Rhyland Klein wrote: > > From: Bill Huang > > > > Super clock divider control and clock source mux of Tegra210 has > > changed a little against prior SoCs, this patch adds Gen5 logic to > > address those differences. > > > > Signed-off-by: Bill Huang > > --- > > v2: > > - Fixed sclk divider address (0x370 -> 0x2c) > > > > drivers/clk/tegra/Makefile |1 + > > drivers/clk/tegra/clk-tegra-super-gen5.c | 150 > ++ > > drivers/clk/tegra/clk.h |3 + > > 3 files changed, 154 insertions(+) > > create mode 100644 drivers/clk/tegra/clk-tegra-super-gen5.c > > I've diffed clk-tegra-super-gen5.c and the existing clk-tegra-super-gen4.c, > and > there's a lot of code duplication here. > They're the same pair of functions, with several small changes. Since the idea > behind pulling out the super clock initialization into a common file was to > reuse > the same init, could we extend the super-gen4 file (rename if you have to) to > support both gens instead? > Thanks, we'll integrate gen5 into gen4 file in v6. > -- > Benson Leung > Software Engineer, Chrom* OS > ble...@chromium.org
[PATCH 1/1] clk: tegra: read correct iddq register in PLL_SS registration
This fixes bug in tegra_clk_register_pllss() which mistakenly assume the
iddq register is the PLL base address.
Signed-off-by: Bill Huang
---
drivers/clk/tegra/clk-pll.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
index 05c6d08..f225325 100644
--- a/drivers/clk/tegra/clk-pll.c
+++ b/drivers/clk/tegra/clk-pll.c
@@ -1826,7 +1826,7 @@ struct clk *tegra_clk_register_pllss(const char *name,
const char *parent_name,
struct clk *clk, *parent;
struct tegra_clk_pll_freq_table cfg;
unsigned long parent_rate;
- u32 val;
+ u32 val, val_iddq;
int i;
if (!pll_params->div_nmp)
@@ -1874,14 +1874,17 @@ struct clk *tegra_clk_register_pllss(const char *name,
const char *parent_name,
pll_writel(PLLSS_CTRL1_DEFAULT, pll_params->ext_misc_reg[2], pll);
val = pll_readl_base(pll);
+ val_iddq = readl_relaxed(clk_base + pll_params->iddq_reg);
if (val & PLL_BASE_ENABLE) {
- if (val & BIT(pll_params->iddq_bit_idx)) {
+ if (val_iddq & BIT(pll_params->iddq_bit_idx)) {
WARN(1, "%s is on but IDDQ set\n", name);
kfree(pll);
return ERR_PTR(-EINVAL);
}
- } else
- val |= BIT(pll_params->iddq_bit_idx);
+ } else {
+ val_iddq |= BIT(pll_params->iddq_bit_idx);
+ writel_relaxed(val_iddq, clk_base + pll_params->iddq_reg);
+ }
val &= ~PLLSS_LOCK_OVERRIDE;
pll_writel_base(val, pll);
--
1.9.1
--
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/
[PATCH 1/1] clk: tegra: fix WARN_ON in PLL_RE registration
This fixes two things.
- Read the correct IDDQ register
- Check the correct IDDQ bit position
Signed-off-by: Bill Huang
---
drivers/clk/tegra/clk-pll.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
index 05c6d08..734340e 100644
--- a/drivers/clk/tegra/clk-pll.c
+++ b/drivers/clk/tegra/clk-pll.c
@@ -1630,7 +1630,8 @@ struct clk *tegra_clk_register_pllre(const char *name,
const char *parent_name,
val = pll_readl_base(pll);
if (val & PLL_BASE_ENABLE)
- WARN_ON(val & pll_params->iddq_bit_idx);
+ WARN_ON(readl_relaxed(clk_base + pll_params->iddq_reg) &
+ BIT(pll_params->iddq_bit_idx));
else {
int m;
--
1.9.1
--
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: [RFC 1/1] driver core: re-order dpm_list after a succussful probe
On 12/23/2014 10:26 PM, Bibek Basu wrote: sorry for the delay in responding. On Thu, Dec 18, 2014 at 1:35 PM, Bill Huang mailto:bilhu...@nvidia.com>> wrote: On 12/17/2014 10:47 PM, Bibek Basu wrote: Hi Bill, Though I like your solution, I have a usecase where the driver probe sequence itself is not right. Both the driver are module_init but one depends on another during suspend sequence. In such a situation, my system hangs. What do you suggest to do in that case? Should I get my driver registration sequence right and how? Moving tegra-pcie driver above in the probe sequence by making the driver subsystem_initcall solved the issue I am facing with this patch. But I don't think that's allowed solution? To change the probe sequence, use defer probe is the right choice. What if there is no direct dependency to defer probe ? In your example, tegra-pcie driver doesn't have init dependency against pcieport but do have pm suspend dependency in between the two. Though I'm not familiar with pcie driver framework but it sounds like pcieport driver is framework driver so it might not be able to know/expect there will be "tegra-pcie" driver been probed later, that means defer probe cannot be used to resolve the pm sequence. If suspend function of tegra-pcie driver cannot be hooked to pcieport driver (so it won't hang while pcieport is suspending) and explicitly to be called to suspend after pcieport driver has done all its access to the controller then I personally think moving tegra-pcie to subsystem_initcall is an acceptable fix, but I'm not expert of the pm design so maybe there is some other proper fix... Example: Probe sequence: driver pcieport driver tegra-pcie Due to your patch, suspend_noirq for tegra_pcie will be called before Are you sure? My change will only affect pm devices in dpm_list, suspend_noirq should still be called after all devices in dpm_list were suspended. Yes, all lists are affected if you update dpm_list. Because using dpm_list, dm_prepared_list is prepared . During suspend, using dpm_prepared_list, dpm_suspended_list is prepared. And using dpc_suspended_list, in dpm_suspend_late, dpm_late_early_list is prepared and which is used to call suspend_noirq callbacks and also prepare dpm_noirq_list. So all are related!!! True they are all related and if both pcieport and tegra-pcie have suspend_noirq callback defined (I guess so since you said you have problem here:), then my fix will change the suspend sequence of the two. This is my main reason to send this RFC since it could break a lot of existing working pm sequence. pcieport. While pcieport tries to read through pci_bus_read_config_dword with clocks and power off to the pcie controller and eventually leads to a crash. regards Bibek On Fri, Dec 12, 2014 at 9:04 PM, Greg KH mailto:gre...@linuxfoundation.org> <mailto:gregkh@__linuxfoundation.org <mailto:gre...@linuxfoundation.org>>> wrote: On Fri, Dec 12, 2014 at 03:50:15AM -0800, Bill Huang wrote: > The dpm_list was added in the call "device_add" and when we do defer > probe we'll explicitly move the probed device to be the last in the > dpm_list, we should do the same for the normal probe since there are > cases that we won't have chance to do defer probe to change the PM order > as the below example. > > If we would like the device driver A to be suspended earlier than the > device driver B, we won't have chance to do defer probe to fix the > suspend dependency since at the time device driver A is probed, device B > was up and running. > > Device A was added > Device B was added > Driver for device B was binded > Driver for device A was binded > > Signed-off-by: Bill Huang mailto:bilhu...@nvidia.com> <mailto:bilhu...@nvidia.com <mailto:bilhu...@nvidia.com>>> > --- > > It seems to me this is a bug in the core driver, but I'm not sure how should > we fix it. > > - Do we have better fix? > - This proposed fix or any other fix might introduces side effect that breaks > existing working suspend dependencies which happen
Re: [RFC 1/1] driver core: re-order dpm_list after a succussful probe
On 12/17/2014 10:47 PM, Bibek Basu wrote:
Hi Bill,
Though I like your solution, I have a usecase where the driver probe
sequence itself is not right. Both the driver are module_init but one
depends on another during suspend sequence.
In such a situation, my system hangs. What do you suggest to do in that
case? Should I get my driver registration sequence right and how?
Moving tegra-pcie driver above in the probe sequence by making the
driver subsystem_initcall solved the issue I am facing with this patch.
But I don't think that's allowed solution?
To change the probe sequence, use defer probe is the right choice.
Example:
Probe sequence:
driver pcieport
driver tegra-pcie
Due to your patch, suspend_noirq for tegra_pcie will be called before
Are you sure? My change will only affect pm devices in dpm_list,
suspend_noirq should still be called after all devices in dpm_list were
suspended.
pcieport. While pcieport tries to read through pci_bus_read_config_dword
with clocks and power off to the pcie controller and eventually leads to
a crash.
regards
Bibek
On Fri, Dec 12, 2014 at 9:04 PM, Greg KH mailto:gre...@linuxfoundation.org>> wrote:
On Fri, Dec 12, 2014 at 03:50:15AM -0800, Bill Huang wrote:
> The dpm_list was added in the call "device_add" and when we do defer
> probe we'll explicitly move the probed device to be the last in the
> dpm_list, we should do the same for the normal probe since there are
> cases that we won't have chance to do defer probe to change the
PM order
> as the below example.
>
> If we would like the device driver A to be suspended earlier than the
> device driver B, we won't have chance to do defer probe to fix the
> suspend dependency since at the time device driver A is probed,
device B
> was up and running.
>
> Device A was added
> Device B was added
> Driver for device B was binded
> Driver for device A was binded
>
> Signed-off-by: Bill Huang mailto:bilhu...@nvidia.com>>
> ---
>
> It seems to me this is a bug in the core driver, but I'm not sure
how should
> we fix it.
>
> - Do we have better fix?
> - This proposed fix or any other fix might introduces side effect
that breaks
> existing working suspend dependencies which happen to work
based on the
> existing wrong suspend order.
>
> Any thoughts? Thanks.
>
> drivers/base/dd.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index cdc779c..54886d2 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -308,6 +308,10 @@ static int really_probe(struct device *dev,
struct device_driver *drv)
> goto probe_failed;
> }
>
> + device_pm_lock();
> + device_pm_move_last(dev);
> + device_pm_unlock();
> +
> driver_bound(dev);
> ret = 1;
> pr_debug("bus: '%s': %s: bound device %s to driver %s\n",
Adding Grant, as he did the deferred probe stuff...
And it's the middle of the merge window, I'll not have time to look at
this for a few weeks at the earliest, sorry.
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe
linux-kernel" in
the body of a message to majord...@vger.kernel.org
<mailto: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/
--
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/
[RFC 1/1] driver core: re-order dpm_list after a succussful probe
The dpm_list was added in the call "device_add" and when we do defer
probe we'll explicitly move the probed device to be the last in the
dpm_list, we should do the same for the normal probe since there are
cases that we won't have chance to do defer probe to change the PM order
as the below example.
If we would like the device driver A to be suspended earlier than the
device driver B, we won't have chance to do defer probe to fix the
suspend dependency since at the time device driver A is probed, device B
was up and running.
Device A was added
Device B was added
Driver for device B was binded
Driver for device A was binded
Signed-off-by: Bill Huang
---
It seems to me this is a bug in the core driver, but I'm not sure how should
we fix it.
- Do we have better fix?
- This proposed fix or any other fix might introduces side effect that breaks
existing working suspend dependencies which happen to work based on the
existing wrong suspend order.
Any thoughts? Thanks.
drivers/base/dd.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index cdc779c..54886d2 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -308,6 +308,10 @@ static int really_probe(struct device *dev, struct
device_driver *drv)
goto probe_failed;
}
+ device_pm_lock();
+ device_pm_move_last(dev);
+ device_pm_unlock();
+
driver_bound(dev);
ret = 1;
pr_debug("bus: '%s': %s: bound device %s to driver %s\n",
--
1.9.1
--
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/
[PATCH v5 1/1] cpufreq: tegra: Re-model Tegra20 cpufreq driver
Re-model Tegra20 cpufreq driver as below.
* Rename tegra-cpufreq.c to tegra20-cpufreq.c since this file supports
only Tegra20.
* Add probe function so defer probe can be used when we're going to
support DVFS.
* Create a fake cpufreq platform device with its name being
"${root_compatible}-cpufreq" so SoC cpufreq driver can bind to it
accordingly.
Signed-off-by: Bill Huang
---
This patch remodel Tegra cpufreq driver to make it more easy to add new
SoC support, in addition to that, adding probe function in the driver to
let probe defer can be used to control init sequence when we are going
to support DVFS.
Changes since v4:
- Remove module driver keyword since we'll not be built as a module driver
- Read SoC compatible Id from DT root instead of statically define it.
- Rebased on linux-next branch of
git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git
Changes since v3:
- Create separate driver for each SoCs instead of a central driver indirect
call to different SoC functions.
Changes since v2:
- Fix Kconfig.
- Rebase on top of branch 'cpufreq-next' on
git://git.linaro.org/people/vireshk/linux.git.
Changes since v1:
- Split up patches.
- Split configuration-time data out of structure "tegra_cpufreq_data".
- Bug fixes.
---
arch/arm/mach-tegra/tegra.c|2 +
drivers/cpufreq/Kconfig.arm| 12 +
drivers/cpufreq/Makefile |1 +
drivers/cpufreq/tegra-cpufreq.c| 283 +++-
.../cpufreq/{tegra-cpufreq.c => tegra20-cpufreq.c} | 96 +++
include/linux/tegra-cpufreq.h | 23 ++
6 files changed, 119 insertions(+), 298 deletions(-)
copy drivers/cpufreq/{tegra-cpufreq.c => tegra20-cpufreq.c} (66%)
create mode 100644 include/linux/tegra-cpufreq.h
diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
index 7336817..a9b23e9 100644
--- a/arch/arm/mach-tegra/tegra.c
+++ b/arch/arm/mach-tegra/tegra.c
@@ -34,6 +34,7 @@
#include
#include
#include
+#include
#include
#include
@@ -160,6 +161,7 @@ static void __init tegra_dt_init_late(void)
{
int i;
+ tegra_cpufreq_init();
tegra_init_suspend();
tegra_cpuidle_init();
tegra_powergate_debugfs_init();
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index ce52ed9..22dfc43 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -225,6 +225,18 @@ config ARM_TEGRA_CPUFREQ
help
This adds the CPUFreq driver support for TEGRA SOCs.
+config ARM_TEGRA20_CPUFREQ
+ bool "NVIDIA TEGRA20"
+ depends on ARM_TEGRA_CPUFREQ && ARCH_TEGRA_2x_SOC
+ default y
+ help
+ This enables Tegra20 cpufreq functionality, it adds
+ Tegra20 CPU frequency ladder and the call back functions
+ to set CPU rate. All the non-SoC dependant codes are
+ controlled by the config ARM_TEGRA_CPUFREQ.
+
+ If in doubt, say N.
+
config ARM_VEXPRESS_SPC_CPUFREQ
tristate "Versatile Express SPC based CPUfreq driver"
select ARM_BIG_LITTLE_CPUFREQ
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 7494565..331964b 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -74,6 +74,7 @@ obj-$(CONFIG_ARM_SA1100_CPUFREQ) += sa1100-cpufreq.o
obj-$(CONFIG_ARM_SA1110_CPUFREQ) += sa1110-cpufreq.o
obj-$(CONFIG_ARM_SPEAR_CPUFREQ)+= spear-cpufreq.o
obj-$(CONFIG_ARM_TEGRA_CPUFREQ)+= tegra-cpufreq.o
+obj-$(CONFIG_ARM_TEGRA20_CPUFREQ) += tegra20-cpufreq.o
obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ) += vexpress-spc-cpufreq.o
##
diff --git a/drivers/cpufreq/tegra-cpufreq.c b/drivers/cpufreq/tegra-cpufreq.c
index b7309c3..1ff8025 100644
--- a/drivers/cpufreq/tegra-cpufreq.c
+++ b/drivers/cpufreq/tegra-cpufreq.c
@@ -1,261 +1,42 @@
/*
- * Copyright (C) 2010 Google, Inc.
+ * Copyright (c) 2013, NVIDIA Corporation. All rights reserved.
*
- * Author:
- * Colin Cross
- * Based on arch/arm/plat-omap/cpu-omap.c, (C) 2005 Nokia Corporation
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
*
+ * This program
[PATCH v4 1/1] cpufreq: tegra: Re-model Tegra20 cpufreq driver
Re-model Tegra20 cpufreq driver as below.
* Rename tegra-cpufreq.c to tegra20-cpufreq.c since this file supports
only Tegra20.
* Add probe function so defer probe can be used when we're going to
support DVFS.
* Create a fake cpufreq platform device with its name being
"${root_compatible}-cpufreq" so SoC cpufreq driver can bind to it
accordingly.
Signed-off-by: Bill Huang
---
This patch remodel Tegra cpufreq driver to make it more easy to add new
SoC support, in addition to that, adding probe function in the driver to
let probe defer can be used to control init sequence when we are going
to support DVFS.
Changes since v3:
- Create separate driver for each SoCs instead of a central driver indirect
call to different SoC functions.
Changes since v2:
- Fix Kconfig.
- Rebase on top of branch 'cpufreq-next' on
git://git.linaro.org/people/vireshk/linux.git.
Changes since v1:
- Split up patches.
- Split configuration-time data out of structure "tegra_cpufreq_data".
- Bug fixes.
---
arch/arm/mach-tegra/tegra.c |2 +
drivers/cpufreq/Kconfig.arm | 12 +++
drivers/cpufreq/Makefile |1 +
drivers/cpufreq/tegra-cpufreq.c | 214 ++---
drivers/cpufreq/tegra20-cpufreq.c | 192 +
include/linux/tegra-cpufreq.h | 24 +
6 files changed, 265 insertions(+), 180 deletions(-)
create mode 100644 drivers/cpufreq/tegra20-cpufreq.c
create mode 100644 include/linux/tegra-cpufreq.h
diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
index 7336817..a9b23e9 100644
--- a/arch/arm/mach-tegra/tegra.c
+++ b/arch/arm/mach-tegra/tegra.c
@@ -34,6 +34,7 @@
#include
#include
#include
+#include
#include
#include
@@ -160,6 +161,7 @@ static void __init tegra_dt_init_late(void)
{
int i;
+ tegra_cpufreq_init();
tegra_init_suspend();
tegra_cpuidle_init();
tegra_powergate_debugfs_init();
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index ce52ed9..1cc9213 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -225,6 +225,18 @@ config ARM_TEGRA_CPUFREQ
help
This adds the CPUFreq driver support for TEGRA SOCs.
+config ARM_TEGRA20_CPUFREQ
+ bool "NVIDIA TEGRA20"
+ depends on ARM_TEGRA_CPUFREQ && ARCH_TEGRA_2x_SOC
+ default y
+ help
+ This enables Tegra20 cpufreq functionality, it adds
+ Tegra20 CPU frequency ladder and the call back functions
+ to set CPU rate. All the non-SoC dependant codes are
+ controlled by the config ARM_TEGRA20_CPUFREQ.
+
+ If in doubt, say N.
+
config ARM_VEXPRESS_SPC_CPUFREQ
tristate "Versatile Express SPC based CPUfreq driver"
select ARM_BIG_LITTLE_CPUFREQ
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 7494565..331964b 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -74,6 +74,7 @@ obj-$(CONFIG_ARM_SA1100_CPUFREQ) += sa1100-cpufreq.o
obj-$(CONFIG_ARM_SA1110_CPUFREQ) += sa1110-cpufreq.o
obj-$(CONFIG_ARM_SPEAR_CPUFREQ)+= spear-cpufreq.o
obj-$(CONFIG_ARM_TEGRA_CPUFREQ)+= tegra-cpufreq.o
+obj-$(CONFIG_ARM_TEGRA20_CPUFREQ) += tegra20-cpufreq.o
obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ) += vexpress-spc-cpufreq.o
##
diff --git a/drivers/cpufreq/tegra-cpufreq.c b/drivers/cpufreq/tegra-cpufreq.c
index 63f0059..a4be7c4 100644
--- a/drivers/cpufreq/tegra-cpufreq.c
+++ b/drivers/cpufreq/tegra-cpufreq.c
@@ -1,193 +1,47 @@
/*
- * Copyright (C) 2010 Google, Inc.
+ * Copyright (c) 2013, NVIDIA Corporation. All rights reserved.
*
- * Author:
- * Colin Cross
- * Based on arch/arm/plat-omap/cpu-omap.c, (C) 2005 Nokia Corporation
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
*
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
*/
#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
[PATCH v4 1/1] cpufreq: tegra: Re-model Tegra20 cpufreq driver
Re-model Tegra20 cpufreq driver as below.
* Rename tegra-cpufreq.c to tegra20-cpufreq.c since this file supports
only Tegra20.
* Add probe function so defer probe can be used when we're going to
support DVFS.
* Create a fake cpufreq platform device with its name being
"${root_compatible}-cpufreq" so SoC cpufreq driver can bind to it
accordingly.
Signed-off-by: Bill Huang
---
This patch remodel Tegra cpufreq driver to make it more easy to add new
SoC support, in addition to that, adding probe function in the driver to
let probe defer can be used to control init sequence when we are going
to support DVFS.
Changes since v3:
- Create separate driver for each SoCs instead of a central driver indirect
call to different SoC functions.
Changes since v2:
- Fix Kconfig.
- Rebase on top of branch 'cpufreq-next' on
git://git.linaro.org/people/vireshk/linux.git.
Changes since v1:
- Split up patches.
- Split configuration-time data out of structure "tegra_cpufreq_data".
- Bug fixes.
---
arch/arm/mach-tegra/tegra.c |2 +
drivers/cpufreq/Kconfig.arm | 12 +++
drivers/cpufreq/Makefile |1 +
drivers/cpufreq/tegra-cpufreq.c | 214 ++---
drivers/cpufreq/tegra20-cpufreq.c | 192 +
include/linux/tegra-cpufreq.h | 24 +
6 files changed, 265 insertions(+), 180 deletions(-)
create mode 100644 drivers/cpufreq/tegra20-cpufreq.c
create mode 100644 include/linux/tegra-cpufreq.h
diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
index 7336817..a9b23e9 100644
--- a/arch/arm/mach-tegra/tegra.c
+++ b/arch/arm/mach-tegra/tegra.c
@@ -34,6 +34,7 @@
#include
#include
#include
+#include
#include
#include
@@ -160,6 +161,7 @@ static void __init tegra_dt_init_late(void)
{
int i;
+ tegra_cpufreq_init();
tegra_init_suspend();
tegra_cpuidle_init();
tegra_powergate_debugfs_init();
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index ce52ed9..1cc9213 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -225,6 +225,18 @@ config ARM_TEGRA_CPUFREQ
help
This adds the CPUFreq driver support for TEGRA SOCs.
+config ARM_TEGRA20_CPUFREQ
+ bool "NVIDIA TEGRA20"
+ depends on ARM_TEGRA_CPUFREQ && ARCH_TEGRA_2x_SOC
+ default y
+ help
+ This enables Tegra20 cpufreq functionality, it adds
+ Tegra20 CPU frequency ladder and the call back functions
+ to set CPU rate. All the non-SoC dependant codes are
+ controlled by the config ARM_TEGRA20_CPUFREQ.
+
+ If in doubt, say N.
+
config ARM_VEXPRESS_SPC_CPUFREQ
tristate "Versatile Express SPC based CPUfreq driver"
select ARM_BIG_LITTLE_CPUFREQ
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 7494565..331964b 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -74,6 +74,7 @@ obj-$(CONFIG_ARM_SA1100_CPUFREQ) += sa1100-cpufreq.o
obj-$(CONFIG_ARM_SA1110_CPUFREQ) += sa1110-cpufreq.o
obj-$(CONFIG_ARM_SPEAR_CPUFREQ)+= spear-cpufreq.o
obj-$(CONFIG_ARM_TEGRA_CPUFREQ)+= tegra-cpufreq.o
+obj-$(CONFIG_ARM_TEGRA20_CPUFREQ) += tegra20-cpufreq.o
obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ) += vexpress-spc-cpufreq.o
##
diff --git a/drivers/cpufreq/tegra-cpufreq.c b/drivers/cpufreq/tegra-cpufreq.c
index 63f0059..a4be7c4 100644
--- a/drivers/cpufreq/tegra-cpufreq.c
+++ b/drivers/cpufreq/tegra-cpufreq.c
@@ -1,193 +1,47 @@
/*
- * Copyright (C) 2010 Google, Inc.
+ * Copyright (c) 2013, NVIDIA Corporation. All rights reserved.
*
- * Author:
- * Colin Cross
- * Based on arch/arm/plat-omap/cpu-omap.c, (C) 2005 Nokia Corporation
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
*
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
*/
#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
[PATCH v3 1/2] cpufreq: tegra: Call tegra_cpufreq_init() specifically in machine code
Move the call from module_init to Tegra machine codes so it won't be
called in a multi-platform kernel running on non-Tegra SoCs.
Signed-off-by: Bill Huang
---
arch/arm/mach-tegra/tegra.c |2 ++
drivers/cpufreq/tegra-cpufreq.c | 13 ++---
include/linux/tegra-soc.h | 11 ++-
3 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
index 7336817..14490ad 100644
--- a/arch/arm/mach-tegra/tegra.c
+++ b/arch/arm/mach-tegra/tegra.c
@@ -34,6 +34,7 @@
#include
#include
#include
+#include
#include
#include
@@ -160,6 +161,7 @@ static void __init tegra_dt_init_late(void)
{
int i;
+ tegra_cpufreq_init();
tegra_init_suspend();
tegra_cpuidle_init();
tegra_powergate_debugfs_init();
diff --git a/drivers/cpufreq/tegra-cpufreq.c b/drivers/cpufreq/tegra-cpufreq.c
index 63f0059..ae1c0f1 100644
--- a/drivers/cpufreq/tegra-cpufreq.c
+++ b/drivers/cpufreq/tegra-cpufreq.c
@@ -155,7 +155,7 @@ static struct cpufreq_driver tegra_cpufreq_driver = {
#endif
};
-static int __init tegra_cpufreq_init(void)
+int __init tegra_cpufreq_init(void)
{
cpu_clk = clk_get_sys(NULL, "cclk");
if (IS_ERR(cpu_clk))
@@ -177,17 +177,8 @@ static int __init tegra_cpufreq_init(void)
return cpufreq_register_driver(&tegra_cpufreq_driver);
}
-
-static void __exit tegra_cpufreq_exit(void)
-{
-cpufreq_unregister_driver(&tegra_cpufreq_driver);
- clk_put(emc_clk);
- clk_put(cpu_clk);
-}
-
+EXPORT_SYMBOL(tegra_cpufreq_init);
MODULE_AUTHOR("Colin Cross ");
MODULE_DESCRIPTION("cpufreq driver for Nvidia Tegra2");
MODULE_LICENSE("GPL");
-module_init(tegra_cpufreq_init);
-module_exit(tegra_cpufreq_exit);
diff --git a/include/linux/tegra-soc.h b/include/linux/tegra-soc.h
index 95f611d..a179aa5 100644
--- a/include/linux/tegra-soc.h
+++ b/include/linux/tegra-soc.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2012, NVIDIA CORPORATION. All rights reserved.
+ * Copyright (c) 2012,2013, NVIDIA CORPORATION. All rights reserved.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms and conditions of the GNU General Public License,
@@ -17,6 +17,15 @@
#ifndef __LINUX_TEGRA_SOC_H_
#define __LINUX_TEGRA_SOC_H_
+#ifdef CONFIG_ARM_TEGRA_CPUFREQ
+int tegra_cpufreq_init(void);
+#else
+static inline int tegra_cpufreq_init(void)
+{
+ return -EINVAL;
+}
+#endif
+
u32 tegra_read_chipid(void);
#endif /* __LINUX_TEGRA_SOC_H_ */
--
1.7.9.5
--
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/
[PATCH v3 0/2] Remodel Tegra cpufreq drivers to support Tegra series SoC
This patch series remodel Tegra cpufreq driver to make it more easy to add new SoC support, in addition to that, adding probe function in the driver to let probe defer can be used to control init sequence when we are going to support DVFS. Changes since v2: - Fix Kconfig. - Rebase on top of branch 'cpufreq-next' on git://git.linaro.org/people/vireshk/linux.git. Changes since v1: - Split up patches. - Split configuration-time data out of structure "tegra_cpufreq_data". - Bug fixes. Bill Huang (2): cpufreq: tegra: Call tegra_cpufreq_init() specifically in machine code cpufreq: tegra: Re-model Tegra cpufreq driver arch/arm/mach-tegra/tegra.c |2 + drivers/cpufreq/Kconfig.arm | 12 +++ drivers/cpufreq/Makefile |1 + drivers/cpufreq/tegra-cpufreq.c | 188 + drivers/cpufreq/tegra-cpufreq.h | 40 drivers/cpufreq/tegra20-cpufreq.c | 136 +++ include/linux/tegra-soc.h | 11 ++- 7 files changed, 287 insertions(+), 103 deletions(-) create mode 100644 drivers/cpufreq/tegra-cpufreq.h create mode 100644 drivers/cpufreq/tegra20-cpufreq.c -- 1.7.9.5 -- 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/
[PATCH v3 2/2] cpufreq: tegra: Re-model Tegra cpufreq driver
Re-model Tegra cpufreq driver to support all Tegra series of SoCs.
* Make tegra-cpufreq.c a generic Tegra cpufreq driver.
* Move Tegra20 specific codes into tegra20-cpufreq.c.
* Bind Tegra cpufreq dirver with a fake device so defer probe would work
when we're going to get regulator in the driver to support voltage
scaling (DVFS).
Signed-off-by: Bill Huang
---
drivers/cpufreq/Kconfig.arm | 12 +++
drivers/cpufreq/Makefile |1 +
drivers/cpufreq/tegra-cpufreq.c | 185 ++---
drivers/cpufreq/tegra-cpufreq.h | 40
drivers/cpufreq/tegra20-cpufreq.c | 136 +++
5 files changed, 278 insertions(+), 96 deletions(-)
create mode 100644 drivers/cpufreq/tegra-cpufreq.h
create mode 100644 drivers/cpufreq/tegra20-cpufreq.c
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index ce52ed9..1cc9213 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -225,6 +225,18 @@ config ARM_TEGRA_CPUFREQ
help
This adds the CPUFreq driver support for TEGRA SOCs.
+config ARM_TEGRA20_CPUFREQ
+ bool "NVIDIA TEGRA20"
+ depends on ARM_TEGRA_CPUFREQ && ARCH_TEGRA_2x_SOC
+ default y
+ help
+ This enables Tegra20 cpufreq functionality, it adds
+ Tegra20 CPU frequency ladder and the call back functions
+ to set CPU rate. All the non-SoC dependant codes are
+ controlled by the config ARM_TEGRA20_CPUFREQ.
+
+ If in doubt, say N.
+
config ARM_VEXPRESS_SPC_CPUFREQ
tristate "Versatile Express SPC based CPUfreq driver"
select ARM_BIG_LITTLE_CPUFREQ
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 7494565..331964b 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -74,6 +74,7 @@ obj-$(CONFIG_ARM_SA1100_CPUFREQ) += sa1100-cpufreq.o
obj-$(CONFIG_ARM_SA1110_CPUFREQ) += sa1110-cpufreq.o
obj-$(CONFIG_ARM_SPEAR_CPUFREQ)+= spear-cpufreq.o
obj-$(CONFIG_ARM_TEGRA_CPUFREQ)+= tegra-cpufreq.o
+obj-$(CONFIG_ARM_TEGRA20_CPUFREQ) += tegra20-cpufreq.o
obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ) += vexpress-spc-cpufreq.o
##
diff --git a/drivers/cpufreq/tegra-cpufreq.c b/drivers/cpufreq/tegra-cpufreq.c
index ae1c0f1..5f19c18 100644
--- a/drivers/cpufreq/tegra-cpufreq.c
+++ b/drivers/cpufreq/tegra-cpufreq.c
@@ -1,5 +1,6 @@
/*
* Copyright (C) 2010 Google, Inc.
+ * Copyright (c) 2013, NVIDIA Corporation.
*
* Author:
* Colin Cross
@@ -18,69 +19,17 @@
#include
#include
-#include
-#include
#include
-#include
-#include
#include
#include
-#include
-
-static struct cpufreq_frequency_table freq_table[] = {
- { .frequency = 216000 },
- { .frequency = 312000 },
- { .frequency = 456000 },
- { .frequency = 608000 },
- { .frequency = 76 },
- { .frequency = 816000 },
- { .frequency = 912000 },
- { .frequency = 100 },
- { .frequency = CPUFREQ_TABLE_END },
-};
-
-#define NUM_CPUS 2
+#include
+#include
+#include
-static struct clk *cpu_clk;
-static struct clk *pll_x_clk;
-static struct clk *pll_p_clk;
-static struct clk *emc_clk;
-
-static int tegra_cpu_clk_set_rate(unsigned long rate)
-{
- int ret;
+#include "tegra-cpufreq.h"
- /*
-* Take an extra reference to the main pll so it doesn't turn
-* off when we move the cpu off of it
-*/
- clk_prepare_enable(pll_x_clk);
-
- ret = clk_set_parent(cpu_clk, pll_p_clk);
- if (ret) {
- pr_err("Failed to switch cpu to clock pll_p\n");
- goto out;
- }
-
- if (rate == clk_get_rate(pll_p_clk))
- goto out;
-
- ret = clk_set_rate(pll_x_clk, rate);
- if (ret) {
- pr_err("Failed to change pll_x to %lu\n", rate);
- goto out;
- }
-
- ret = clk_set_parent(cpu_clk, pll_x_clk);
- if (ret) {
- pr_err("Failed to switch cpu to clock pll_x\n");
- goto out;
- }
-
-out:
- clk_disable_unprepare(pll_x_clk);
- return ret;
-}
+static struct tegra_cpufreq_data *tegra_data;
+static const struct tegra_cpufreq_config *soc_config;
static int tegra_update_cpu_speed(struct cpufreq_policy *policy,
unsigned long rate)
@@ -91,14 +40,10 @@ static int tegra_update_cpu_speed(struct cpufreq_policy
*policy,
* Vote on memory bus frequency based on cpu frequency
* This sets the minimum frequency, display or avp may request higher
*/
- if (rate >= 816000)
- clk_set_rate(emc_clk, 6); /* cpu 816 MHz, emc max */
- else if (rate >= 456000)
- clk_set_rate(emc_clk, 3); /* cpu
Re: [PATCH v2 3/4] cpufreq: tegra: Re-model Tegra cpufreq driver
On Sat, 2013-09-14 at 06:26 +0800, Stephen Warren wrote: > On 09/12/2013 05:24 AM, Bill Huang wrote: > > Re-model Tegra cpufreq driver to support all Tegra series of SoCs. > > > > * Make tegra-cpufreq.c a generic Tegra cpufreq driver. > > * Move Tegra20 specific codes into tegra20-cpufreq.c. > > * Bind Tegra cpufreq dirver with a fake device so defer probe would work > > when we're going to get regulator in the driver to support voltage > > scaling (DVFS). > > > diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm > > > @@ -232,6 +232,12 @@ config ARM_TEGRA_CPUFREQ > > bool "TEGRA CPUFreq support" > > depends on ARCH_TEGRA > > select CPU_FREQ_TABLE > > + > > +config ARM_TEGRA20_CPUFREQ > > + bool "NVIDIA TEGRA20" > > + depends on ARM_TEGRA_CPUFREQ && ARCH_TEGRA_2x_SOC > > default y > > help > > - This adds the CPUFreq driver support for TEGRA SOCs. > > + This adds the CPUFreq driver for NVIDIA TEGRA20 SoC. > > + > > + If in doubt, say N. > > This patch removes the following from "config ARM_TEGRA_CPUFREQ": > > default y > help > This adds the CPUFreq driver support for TEGRA SOCs. > > I think that option should be left unchanged, such that this patch > *just* adds the new option without changing the existing one. If you do > that, then you can completely drop patch 4/4. Thanks, I'll change and rebase in next version after Viresh's cleanup patch series is merged. > > Other than that, this series looks fine. -- 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/
[PATCH v2 1/4] cpufreq: tegra: Call tegra_cpufreq_init() specifically in machine code
Move the call from module_init to Tegra machine codes so it won't be
called in a multi-platform kernel running on non-Tegra SoCs.
Signed-off-by: Bill Huang
---
arch/arm/mach-tegra/common.c|2 ++
drivers/cpufreq/tegra-cpufreq.c | 13 ++---
include/linux/tegra-soc.h | 11 ++-
3 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/arch/arm/mach-tegra/common.c b/arch/arm/mach-tegra/common.c
index 94a119a..db4414e 100644
--- a/arch/arm/mach-tegra/common.c
+++ b/arch/arm/mach-tegra/common.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include
@@ -109,6 +110,7 @@ void __init tegra_init_early(void)
void __init tegra_init_late(void)
{
+ tegra_cpufreq_init();
tegra_init_suspend();
tegra_cpuidle_init();
tegra_powergate_debugfs_init();
diff --git a/drivers/cpufreq/tegra-cpufreq.c b/drivers/cpufreq/tegra-cpufreq.c
index a7b876f..09c5af0 100644
--- a/drivers/cpufreq/tegra-cpufreq.c
+++ b/drivers/cpufreq/tegra-cpufreq.c
@@ -253,7 +253,7 @@ static struct cpufreq_driver tegra_cpufreq_driver = {
.attr = tegra_cpufreq_attr,
};
-static int __init tegra_cpufreq_init(void)
+int __init tegra_cpufreq_init(void)
{
cpu_clk = clk_get_sys(NULL, "cclk");
if (IS_ERR(cpu_clk))
@@ -275,17 +275,8 @@ static int __init tegra_cpufreq_init(void)
return cpufreq_register_driver(&tegra_cpufreq_driver);
}
-
-static void __exit tegra_cpufreq_exit(void)
-{
-cpufreq_unregister_driver(&tegra_cpufreq_driver);
- clk_put(emc_clk);
- clk_put(cpu_clk);
-}
-
+EXPORT_SYMBOL(tegra_cpufreq_init);
MODULE_AUTHOR("Colin Cross ");
MODULE_DESCRIPTION("cpufreq driver for Nvidia Tegra2");
MODULE_LICENSE("GPL");
-module_init(tegra_cpufreq_init);
-module_exit(tegra_cpufreq_exit);
diff --git a/include/linux/tegra-soc.h b/include/linux/tegra-soc.h
index 95f611d..d86d081 100644
--- a/include/linux/tegra-soc.h
+++ b/include/linux/tegra-soc.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2012, NVIDIA CORPORATION. All rights reserved.
+ * Copyright (c) 2012,2013, NVIDIA CORPORATION. All rights reserved.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms and conditions of the GNU General Public License,
@@ -19,4 +19,13 @@
u32 tegra_read_chipid(void);
+#ifdef CONFIG_ARM_TEGRA_CPUFREQ
+int tegra_cpufreq_init(void);
+#else
+static inline int tegra_cpufreq_init(void)
+{
+ return -EINVAL;
+}
+#endif
+
#endif /* __LINUX_TEGRA_SOC_H_ */
--
1.7.9.5
--
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/
[PATCH v2 4/4] ARM: tegra: defconfig updates
Enable: * Tegra CPUFreq driver. Signed-off-by: Bill Huang --- arch/arm/configs/tegra_defconfig |1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig index ea042e8..64bc8bd 100644 --- a/arch/arm/configs/tegra_defconfig +++ b/arch/arm/configs/tegra_defconfig @@ -42,6 +42,7 @@ CONFIG_ZBOOT_ROM_BSS=0x0 CONFIG_KEXEC=y CONFIG_CPU_FREQ=y CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND=y +CONFIG_ARM_TEGRA_CPUFREQ=y CONFIG_CPU_IDLE=y CONFIG_VFP=y CONFIG_PM_RUNTIME=y -- 1.7.9.5 -- 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/
[PATCH v2 3/4] cpufreq: tegra: Re-model Tegra cpufreq driver
Re-model Tegra cpufreq driver to support all Tegra series of SoCs.
* Make tegra-cpufreq.c a generic Tegra cpufreq driver.
* Move Tegra20 specific codes into tegra20-cpufreq.c.
* Bind Tegra cpufreq dirver with a fake device so defer probe would work
when we're going to get regulator in the driver to support voltage
scaling (DVFS).
Signed-off-by: Bill Huang
---
drivers/cpufreq/Kconfig.arm |8 +-
drivers/cpufreq/Makefile |1 +
drivers/cpufreq/tegra-cpufreq.c | 176 +++--
drivers/cpufreq/tegra-cpufreq.h | 42 +
drivers/cpufreq/tegra20-cpufreq.c | 138 +
5 files changed, 278 insertions(+), 87 deletions(-)
create mode 100644 drivers/cpufreq/tegra-cpufreq.h
create mode 100644 drivers/cpufreq/tegra20-cpufreq.c
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 0fa204b..09a80a4 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -232,6 +232,12 @@ config ARM_TEGRA_CPUFREQ
bool "TEGRA CPUFreq support"
depends on ARCH_TEGRA
select CPU_FREQ_TABLE
+
+config ARM_TEGRA20_CPUFREQ
+ bool "NVIDIA TEGRA20"
+ depends on ARM_TEGRA_CPUFREQ && ARCH_TEGRA_2x_SOC
default y
help
- This adds the CPUFreq driver support for TEGRA SOCs.
+ This adds the CPUFreq driver for NVIDIA TEGRA20 SoC.
+
+ If in doubt, say N.
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index ad5866c..22a85f0 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -77,6 +77,7 @@ obj-$(CONFIG_ARM_SA1100_CPUFREQ) += sa1100-cpufreq.o
obj-$(CONFIG_ARM_SA1110_CPUFREQ) += sa1110-cpufreq.o
obj-$(CONFIG_ARM_SPEAR_CPUFREQ)+= spear-cpufreq.o
obj-$(CONFIG_ARM_TEGRA_CPUFREQ)+= tegra-cpufreq.o
+obj-$(CONFIG_ARM_TEGRA20_CPUFREQ) += tegra20-cpufreq.o
##
# PowerPC platform drivers
diff --git a/drivers/cpufreq/tegra-cpufreq.c b/drivers/cpufreq/tegra-cpufreq.c
index c2d2910..8616f6f 100644
--- a/drivers/cpufreq/tegra-cpufreq.c
+++ b/drivers/cpufreq/tegra-cpufreq.c
@@ -1,4 +1,5 @@
/*
+ * Copyright (c) 2013, NVIDIA Corporation.
* Copyright (C) 2010 Google, Inc.
*
* Author:
@@ -22,25 +23,16 @@
#include
#include
#include
+#include
+#include
+#include
-static struct cpufreq_frequency_table freq_table[] = {
- { .frequency = 216000 },
- { .frequency = 312000 },
- { .frequency = 456000 },
- { .frequency = 608000 },
- { .frequency = 76 },
- { .frequency = 816000 },
- { .frequency = 912000 },
- { .frequency = 100 },
- { .frequency = CPUFREQ_TABLE_END },
-};
+#include "tegra-cpufreq.h"
#define MAX_CPUS 4
-static struct clk *cpu_clk;
-static struct clk *pll_x_clk;
-static struct clk *pll_p_clk;
-static struct clk *emc_clk;
+static struct tegra_cpufreq_data *tegra_data;
+static const struct tegra_cpufreq_config *soc_config;
static unsigned long target_cpu_speed[MAX_CPUS];
static DEFINE_MUTEX(tegra_cpu_lock);
@@ -48,53 +40,17 @@ static bool is_suspended;
static int tegra_verify_speed(struct cpufreq_policy *policy)
{
- return cpufreq_frequency_table_verify(policy, freq_table);
+ return cpufreq_frequency_table_verify(policy, tegra_data->freq_table);
}
static unsigned int tegra_getspeed(unsigned int cpu)
{
unsigned long rate;
- rate = clk_get_rate(cpu_clk) / 1000;
+ rate = clk_get_rate(tegra_data->cpu_clk) / 1000;
return rate;
}
-static int tegra_cpu_clk_set_rate(unsigned long rate)
-{
- int ret;
-
- /*
-* Take an extra reference to the main pll so it doesn't turn
-* off when we move the cpu off of it
-*/
- clk_prepare_enable(pll_x_clk);
-
- ret = clk_set_parent(cpu_clk, pll_p_clk);
- if (ret) {
- pr_err("Failed to switch cpu to clock pll_p\n");
- goto out;
- }
-
- if (rate == clk_get_rate(pll_p_clk))
- goto out;
-
- ret = clk_set_rate(pll_x_clk, rate);
- if (ret) {
- pr_err("Failed to change pll_x to %lu\n", rate);
- goto out;
- }
-
- ret = clk_set_parent(cpu_clk, pll_x_clk);
- if (ret) {
- pr_err("Failed to switch cpu to clock pll_x\n");
- goto out;
- }
-
-out:
- clk_disable_unprepare(pll_x_clk);
- return ret;
-}
-
static int tegra_update_cpu_speed(struct cpufreq_policy *policy,
unsigned long rate)
{
@@ -111,12 +67,8 @@ static int tegra_update_cpu_speed(struct cpufreq_policy
*policy,
* Vote on memory bus frequency based on cpu frequency
* This sets the minimum frequency, display or avp may request higher
*/
-
[PATCH v2 2/4] cpufreq: tegra: Remove not used header files and clean up codes
Remove the inclustion of the not needed header files and remove the
logic to check the CPU ID to not exceeding the maximum supported CPUs.
Signed-off-by: Bill Huang
---
drivers/cpufreq/tegra-cpufreq.c | 15 ++-
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/drivers/cpufreq/tegra-cpufreq.c b/drivers/cpufreq/tegra-cpufreq.c
index 09c5af0..c2d2910 100644
--- a/drivers/cpufreq/tegra-cpufreq.c
+++ b/drivers/cpufreq/tegra-cpufreq.c
@@ -18,14 +18,9 @@
#include
#include
-#include
-#include
#include
-#include
-#include
#include
#include
-#include
#include
static struct cpufreq_frequency_table freq_table[] = {
@@ -40,14 +35,14 @@ static struct cpufreq_frequency_table freq_table[] = {
{ .frequency = CPUFREQ_TABLE_END },
};
-#define NUM_CPUS 2
+#define MAX_CPUS 4
static struct clk *cpu_clk;
static struct clk *pll_x_clk;
static struct clk *pll_p_clk;
static struct clk *emc_clk;
-static unsigned long target_cpu_speed[NUM_CPUS];
+static unsigned long target_cpu_speed[MAX_CPUS];
static DEFINE_MUTEX(tegra_cpu_lock);
static bool is_suspended;
@@ -60,9 +55,6 @@ static unsigned int tegra_getspeed(unsigned int cpu)
{
unsigned long rate;
- if (cpu >= NUM_CPUS)
- return 0;
-
rate = clk_get_rate(cpu_clk) / 1000;
return rate;
}
@@ -209,9 +201,6 @@ static struct notifier_block tegra_cpu_pm_notifier = {
static int tegra_cpu_init(struct cpufreq_policy *policy)
{
- if (policy->cpu >= NUM_CPUS)
- return -EINVAL;
-
clk_prepare_enable(emc_clk);
clk_prepare_enable(cpu_clk);
--
1.7.9.5
--
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/
[PATCH v2 0/4] Remodel Tegra cpufreq driver to support Tegra series SoCs
This patch series remodel Tegra cpufreq driver to make it more easy to add new SoC support, in addition to that, adding probe function in the driver to let probe defer can be used to control init sequence when we going to support DVFS. Changes since v1: - Split up patches. - Split configuration-time data out of structure "tegra_cpufreq_data". - Bug fixes. Bill Huang (4): cpufreq: tegra: Call tegra_cpufreq_init() specifically in machine code cpufreq: tegra: Remove not used header files and clean up codes cpufreq: tegra: Re-model Tegra cpufreq driver ARM: tegra: defconfig updates arch/arm/configs/tegra_defconfig |1 + arch/arm/mach-tegra/common.c |2 + drivers/cpufreq/Kconfig.arm |8 +- drivers/cpufreq/Makefile |1 + drivers/cpufreq/tegra-cpufreq.c | 194 + drivers/cpufreq/tegra-cpufreq.h | 42 drivers/cpufreq/tegra20-cpufreq.c | 138 ++ include/linux/tegra-soc.h | 11 ++- 8 files changed, 290 insertions(+), 107 deletions(-) create mode 100644 drivers/cpufreq/tegra-cpufreq.h create mode 100644 drivers/cpufreq/tegra20-cpufreq.c -- 1.7.9.5 -- 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 2/2] cpufreq: tegra: Re-model Tegra cpufreq driver
On Thu, 2013-09-12 at 18:14 +0800, Peter De Schrijver wrote:
> On Wed, Sep 11, 2013 at 01:19:14PM +0200, Bill Huang wrote:
>
> > +static inline int tegra_cpufreq_init(void)
> > +{
> > + retrun EOPNOTSUPP;
>
> This should be -EOPNOTSUPP at least. I think -EINVAL might be better.
Yeah thanks, I'll also fix the TYPO of return.
>
> Cheers,
>
> Peter.
--
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 2/2] cpufreq: tegra: Re-model Tegra cpufreq driver
On Thu, 2013-09-12 at 03:33 +0800, Stephen Warren wrote:
> On 09/11/2013 05:19 AM, Bill Huang wrote:
> > diff --git a/drivers/cpufreq/tegra20-cpufreq.c
> > b/drivers/cpufreq/tegra20-cpufreq.c
>
> > +static struct cpufreq_frequency_table freq_table[] = {
> > + { .frequency = 216000 },
> > + { .frequency = 312000 },
> > + { .frequency = 456000 },
> > + { .frequency = 608000 },
> > + { .frequency = 76 },
> > + { .frequency = 816000 },
> > + { .frequency = 912000 },
> > + { .frequency = 100 },
> > + { .frequency = CPUFREQ_TABLE_END },
> > +};
>
> Should/can we query that list from the clock driver?
>
It's not easy to query that list from clock driver so I'll leave it as
it was. I'll take all the other comments, thanks.
--
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/
[PATCH 1/2] cpufreq: tegra: Remove not used header and clean up codes
Remove inclustion of the not needed header files and remove the logic
to check the CPU ID to not exceeding the maximum supported CPUs.
Signed-off-by: Bill Huang
---
drivers/cpufreq/tegra-cpufreq.c | 14 ++
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/cpufreq/tegra-cpufreq.c b/drivers/cpufreq/tegra-cpufreq.c
index a7b876f..6eb3dd8 100644
--- a/drivers/cpufreq/tegra-cpufreq.c
+++ b/drivers/cpufreq/tegra-cpufreq.c
@@ -1,5 +1,6 @@
/*
- * Copyright (C) 2010 Google, Inc.
+ * Copyright (c) 2013, NVIDIA Corporation.
+ * Copyright (C) 2010-2013 Google, Inc.
*
* Author:
* Colin Cross
@@ -18,14 +19,9 @@
#include
#include
-#include
-#include
#include
-#include
-#include
#include
#include
-#include
#include
static struct cpufreq_frequency_table freq_table[] = {
@@ -60,9 +56,6 @@ static unsigned int tegra_getspeed(unsigned int cpu)
{
unsigned long rate;
- if (cpu >= NUM_CPUS)
- return 0;
-
rate = clk_get_rate(cpu_clk) / 1000;
return rate;
}
@@ -209,9 +202,6 @@ static struct notifier_block tegra_cpu_pm_notifier = {
static int tegra_cpu_init(struct cpufreq_policy *policy)
{
- if (policy->cpu >= NUM_CPUS)
- return -EINVAL;
-
clk_prepare_enable(emc_clk);
clk_prepare_enable(cpu_clk);
--
1.7.9.5
--
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/
[PATCH 2/2] cpufreq: tegra: Re-model Tegra cpufreq driver
Re-model Tegra cpufreq driver to support all Tegra series of SoCs.
* Make tegra-cpufreq.c a generic Tegra cpufreq driver.
* Move Tegra20 specific codes into tegra20-cpufreq.c.
* Bind Tegra cpufreq dirver with a fake device so defer probe would work
when we're going to get regulator in the driver to support voltage
scaling (DVFS).
* Call tegra_cpufreq_init() in Tegra machine code specifically so it
won't be called in multi-platform kernel which is not running on Tegra
SoCs.
Signed-off-by: Bill Huang
---
arch/arm/mach-tegra/common.c |2 +
drivers/cpufreq/Kconfig.arm | 16 +++-
drivers/cpufreq/Makefile |1 +
drivers/cpufreq/tegra-cpufreq.c | 178 ++---
drivers/cpufreq/tegra-cpufreq.h | 37
drivers/cpufreq/tegra20-cpufreq.c | 133 +++
include/linux/tegra-soc.h | 11 ++-
7 files changed, 281 insertions(+), 97 deletions(-)
create mode 100644 drivers/cpufreq/tegra-cpufreq.h
create mode 100644 drivers/cpufreq/tegra20-cpufreq.c
diff --git a/arch/arm/mach-tegra/common.c b/arch/arm/mach-tegra/common.c
index 94a119a..db4414e 100644
--- a/arch/arm/mach-tegra/common.c
+++ b/arch/arm/mach-tegra/common.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include
@@ -109,6 +110,7 @@ void __init tegra_init_early(void)
void __init tegra_init_late(void)
{
+ tegra_cpufreq_init();
tegra_init_suspend();
tegra_cpuidle_init();
tegra_powergate_debugfs_init();
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 0fa204b..d62902a 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -229,9 +229,19 @@ config ARM_SPEAR_CPUFREQ
This adds the CPUFreq driver support for SPEAr SOCs.
config ARM_TEGRA_CPUFREQ
- bool "TEGRA CPUFreq support"
- depends on ARCH_TEGRA
+ bool
+ select CPU_FREQ_TABLE
+
+config ARM_EXYNOS_CPUFREQ
+ bool
select CPU_FREQ_TABLE
+
+config ARM_TEGRA20_CPUFREQ
+ bool "NVIDIA TEGRA20"
+ depends on ARCH_TEGRA
default y
+ select ARM_TEGRA_CPUFREQ
help
- This adds the CPUFreq driver support for TEGRA SOCs.
+ This adds the CPUFreq driver for NVIDIA TEGRA20 SoC.
+
+ If in doubt, say N.
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index ad5866c..22a85f0 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -77,6 +77,7 @@ obj-$(CONFIG_ARM_SA1100_CPUFREQ) += sa1100-cpufreq.o
obj-$(CONFIG_ARM_SA1110_CPUFREQ) += sa1110-cpufreq.o
obj-$(CONFIG_ARM_SPEAR_CPUFREQ)+= spear-cpufreq.o
obj-$(CONFIG_ARM_TEGRA_CPUFREQ)+= tegra-cpufreq.o
+obj-$(CONFIG_ARM_TEGRA20_CPUFREQ) += tegra20-cpufreq.o
##
# PowerPC platform drivers
diff --git a/drivers/cpufreq/tegra-cpufreq.c b/drivers/cpufreq/tegra-cpufreq.c
index 6eb3dd8..d40428a 100644
--- a/drivers/cpufreq/tegra-cpufreq.c
+++ b/drivers/cpufreq/tegra-cpufreq.c
@@ -23,25 +23,15 @@
#include
#include
#include
+#include
+#include
+#include
-static struct cpufreq_frequency_table freq_table[] = {
- { .frequency = 216000 },
- { .frequency = 312000 },
- { .frequency = 456000 },
- { .frequency = 608000 },
- { .frequency = 76 },
- { .frequency = 816000 },
- { .frequency = 912000 },
- { .frequency = 100 },
- { .frequency = CPUFREQ_TABLE_END },
-};
+#include "tegra-cpufreq.h"
-#define NUM_CPUS 2
+#define NUM_CPUS 4
-static struct clk *cpu_clk;
-static struct clk *pll_x_clk;
-static struct clk *pll_p_clk;
-static struct clk *emc_clk;
+static struct tegra_cpufreq_data *tegra_data;
static unsigned long target_cpu_speed[NUM_CPUS];
static DEFINE_MUTEX(tegra_cpu_lock);
@@ -49,53 +39,17 @@ static bool is_suspended;
static int tegra_verify_speed(struct cpufreq_policy *policy)
{
- return cpufreq_frequency_table_verify(policy, freq_table);
+ return cpufreq_frequency_table_verify(policy, tegra_data->freq_table);
}
static unsigned int tegra_getspeed(unsigned int cpu)
{
unsigned long rate;
- rate = clk_get_rate(cpu_clk) / 1000;
+ rate = clk_get_rate(tegra_data->cpu_clk) / 1000;
return rate;
}
-static int tegra_cpu_clk_set_rate(unsigned long rate)
-{
- int ret;
-
- /*
-* Take an extra reference to the main pll so it doesn't turn
-* off when we move the cpu off of it
-*/
- clk_prepare_enable(pll_x_clk);
-
- ret = clk_set_parent(cpu_clk, pll_p_clk);
- if (ret) {
- pr_err("Failed to switch cpu to clock pll_p\n");
- goto out;
- }
-
- if (rate == clk_get_rate(pll_p_clk))
- goto out;
-
- ret = clk_set_rate(pll_x_
[PATCH 0/2] Remodel Tegra cpufreq driver to support Tegra series SoCs
This patch series remodel Tegra cpufreq driver to make it more easy to add new SoC support, in addition to that, adding probe function in the driver to let probe defer can be used to control init sequence when we going to support DVFS. Bill Huang (2): cpufreq: tegra: Remove not used header and clean up codes cpufreq: tegra: Re-model Tegra cpufreq driver arch/arm/mach-tegra/common.c |2 + drivers/cpufreq/Kconfig.arm | 16 +++- drivers/cpufreq/Makefile |1 + drivers/cpufreq/tegra-cpufreq.c | 192 + drivers/cpufreq/tegra-cpufreq.h | 37 +++ drivers/cpufreq/tegra20-cpufreq.c | 133 + include/linux/tegra-soc.h | 11 ++- 7 files changed, 283 insertions(+), 109 deletions(-) create mode 100644 drivers/cpufreq/tegra-cpufreq.h create mode 100644 drivers/cpufreq/tegra20-cpufreq.c -- 1.7.9.5 -- 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/
[PATCH 1/1] ARM: tegra: configure power off for Dalmore
Add DT property to tell the regulator to register pm_power_off to make
"shutdown" work.
Signed-off-by: Bill Huang
---
arch/arm/boot/dts/tegra114-dalmore.dts |2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/boot/dts/tegra114-dalmore.dts
b/arch/arm/boot/dts/tegra114-dalmore.dts
index 45056a2..6023028 100644
--- a/arch/arm/boot/dts/tegra114-dalmore.dts
+++ b/arch/arm/boot/dts/tegra114-dalmore.dts
@@ -845,6 +845,8 @@
#interrupt-cells = <2>;
interrupt-controller;
+ ti,system-power-controller;
+
palmas_gpio: gpio {
compatible = "ti,palmas-gpio";
gpio-controller;
--
1.7.9.5
--
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 RESEND v2 1/1] mfd: palmas: Add power off control
On Thu, 2013-08-08 at 20:32 +0800, Lee Jones wrote: > On Thu, 08 Aug 2013, Bill Huang wrote: > > > Hook up "pm_power_off" to palmas power off routine if there is DT > > property "ti,system-power-controller" defined, so platform which is > > powered by this regulator can be powered off properly. > > > > Signed-off-by: Mallikarjun Kasoju > > Signed-off-by: Bill Huang > > --- > > .../devicetree/bindings/regulator/palmas-pmic.txt |5 +++ > > drivers/mfd/palmas.c | 33 > > ++-- > > include/linux/mfd/palmas.h |1 + > > 3 files changed, 37 insertions(+), 2 deletions(-) > > Was a clear conclusion reached between Nishanth and yourself? > > As he has strong opinions on this I'd ideally like his Ack before > applying the patch. > Hi Nishanth, Feel free to NACK if you are feeling this can't fit what you would like to add, then I'll pass the ball to you for implementing Palmas power off since you have already wrote part of the driver (with USB IRQ unmask?). -- 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/
[PATCH RESEND v2 1/1] mfd: palmas: Add power off control
Hook up "pm_power_off" to palmas power off routine if there is DT
property "ti,system-power-controller" defined, so platform which is
powered by this regulator can be powered off properly.
Signed-off-by: Mallikarjun Kasoju
Signed-off-by: Bill Huang
---
.../devicetree/bindings/regulator/palmas-pmic.txt |5 +++
drivers/mfd/palmas.c | 33 ++--
include/linux/mfd/palmas.h |1 +
3 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
index 30b0581..4adca2a 100644
--- a/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
+++ b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
@@ -36,6 +36,9 @@ Optional nodes:
ti,smps-range - OTP has the wrong range set for the hardware so
override
0 - low range, 1 - high range.
+- ti,system-power-controller: Telling whether or not this pmic is controlling
+ the system power.
+
Example:
#include
@@ -48,6 +51,8 @@ pmic {
ti,ldo6-vibrator;
+ ti,system-power-controller;
+
regulators {
smps12_reg : smps12 {
regulator-name = "smps12";
diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
index e4d1c70..220a34d 100644
--- a/drivers/mfd/palmas.c
+++ b/drivers/mfd/palmas.c
@@ -229,6 +229,32 @@ static void palmas_dt_to_pdata(struct i2c_client *i2c,
PALMAS_POWER_CTRL_ENABLE2_MASK;
if (i2c->irq)
palmas_set_pdata_irq_flag(i2c, pdata);
+
+ pdata->pm_off = of_property_read_bool(node,
+ "ti,system-power-controller");
+}
+
+static struct palmas *palmas_dev;
+static void palmas_power_off(void)
+{
+ unsigned int addr;
+ int ret, slave;
+
+ if (!palmas_dev)
+ return;
+
+ slave = PALMAS_BASE_TO_SLAVE(PALMAS_PMU_CONTROL_BASE);
+ addr = PALMAS_BASE_TO_REG(PALMAS_PMU_CONTROL_BASE, PALMAS_DEV_CTRL);
+
+ ret = regmap_update_bits(
+ palmas_dev->regmap[slave],
+ addr,
+ PALMAS_DEV_CTRL_DEV_ON,
+ 0);
+
+ if (ret)
+ pr_err("%s: Unable to write to DEV_CTRL_DEV_ON: %d\n",
+ __func__, ret);
}
static unsigned int palmas_features = PALMAS_PMIC_FEATURE_SMPS10_BOOST;
@@ -423,10 +449,13 @@ no_irq:
*/
if (node) {
ret = of_platform_populate(node, NULL, NULL, &i2c->dev);
- if (ret < 0)
+ if (ret < 0) {
goto err_irq;
- else
+ } else if (pdata->pm_off && !pm_power_off) {
+ palmas_dev = palmas;
+ pm_power_off = palmas_power_off;
return ret;
+ }
}
return ret;
diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h
index 1a8dd7a..061cce0 100644
--- a/include/linux/mfd/palmas.h
+++ b/include/linux/mfd/palmas.h
@@ -258,6 +258,7 @@ struct palmas_platform_data {
*/
int mux_from_pdata;
u8 pad1, pad2;
+ bool pm_off;
struct palmas_pmic_platform_data *pmic_pdata;
struct palmas_gpadc_platform_data *gpadc_pdata;
--
1.7.9.5
--
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 v2 1/1] mfd: palmas: Add power off control
On Fri, 2013-08-02 at 22:39 +0800, Nishanth Menon wrote: > On 08/02/2013 12:45 AM, Bill Huang wrote: > > On Thu, 2013-08-01 at 21:08 +0800, Nishanth Menon wrote: > >> On 04:08-20130801, Bill Huang wrote: > >>> On Wed, 2013-07-31 at 19:57 +0800, Nishanth Menon wrote: > >>>> > >>>> If you notice the reference code I send, atleast on TWL6035/37 variants > >>>> of Palmas, USB IRQ unmask is mandatory for power on with USB cable - > >>>> example usage scenario: extremely low battery, device powered off, plug > >>>> in usb cable to restart charging - you'd like to initiate charging logic > >>>> in bootloader, but that wont work if the device does not do OFF-ON > >>>> transition with usb cable plugged in for vbus. > >>>> > >>> Why do we need to add Palmas USB_IRQ unmask logic in shutdown? Does that > >>> mean for all platform using Palmas has to unmask USB IRQ (including > >>> those do not power vbus through Palmas)? Can't we just have a simple > >>> shutdown function but have the VBus programming been done in USB driver > >>> or maybe platform driver since it is platform specific control? > >> we dont have a irq cleanup, irq handling is done in palmas-mfd. Further, > >> > >> Why would USB driver care about vbus supply needs in complete power off > >> - it is the job of palmas driver? Further, palmas-mfd shutdown > >> handler(currently missing) if probably cleansup things: > >> > >> mfd_remove_devices(palmas->dev); > >>palmas_irq_exit(palmas); > >> > >> shutdown sequence becomes complicated further esp if things are > >> cleanedup in shutdown (Dummy patch[1]). > >> > >> > >> All I am saying is this: shutdown should allow powerup functionality to > >> work as well, how we do that is upto us - I personally found it a little > >> easier to keep the IRQ unmask in shutdown easier to deal with, but other > >> options might be possible as well. > > > > I'm not sure if I understand your comments completely (maybe due to I'm > > not familiar with the mechanism of unmasking USB IRQ in Palmas driver) > > This is IMHO an weird configuration I saw on TWL6035/6037 Palmas device > - so I suspect should be the case in probably other palmas devices as > well. I hit power off, and I can start up the device again by supplying > vbus -(usecase was, at that point in time, battery charging) > > anyways, I was working with busybox and no usb driver even enabled, but > I am told by the twl6035/7 support folks that due to design USB IRQ > needs to be unmasked at shutdown/poweroff to allow OFF->ON transition > sequence to start inside Palmas when vbus is supplied. > > > > but doing cleanup in each driver shutdown handler makes sense to me, if > > those clean up can be done in shutdown then we can make power off > > function as simple as possible and being part of Palmas mfd driver? > > not really, mfd shutdown does not guarantee rest of the drivers' > shutdown functions are safely called, pm_power_off unfortunately is the > only "official point" where we can safely and cleanly shutdown the system. > It looks to me whether or not adding those extra control in a generic Palmas driver is still in doubt, will you send your patch for this in the near future? If not, any objection on making it as a basic and simple power off control (that said, any further processing which has to be in power off routine can be added on top of that, or can be moved to be under drivers/reset if needed) using the existing mechanism? -- 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 v2 1/1] mfd: palmas: Add power off control
On Thu, 2013-08-01 at 21:08 +0800, Nishanth Menon wrote:
> On 04:08-20130801, Bill Huang wrote:
> > On Wed, 2013-07-31 at 19:57 +0800, Nishanth Menon wrote:
> > >
> > > If you notice the reference code I send, atleast on TWL6035/37 variants
> > > of Palmas, USB IRQ unmask is mandatory for power on with USB cable -
> > > example usage scenario: extremely low battery, device powered off, plug
> > > in usb cable to restart charging - you'd like to initiate charging logic
> > > in bootloader, but that wont work if the device does not do OFF-ON
> > > transition with usb cable plugged in for vbus.
> > >
> > Why do we need to add Palmas USB_IRQ unmask logic in shutdown? Does that
> > mean for all platform using Palmas has to unmask USB IRQ (including
> > those do not power vbus through Palmas)? Can't we just have a simple
> > shutdown function but have the VBus programming been done in USB driver
> > or maybe platform driver since it is platform specific control?
> we dont have a irq cleanup, irq handling is done in palmas-mfd. Further,
>
> Why would USB driver care about vbus supply needs in complete power off
> - it is the job of palmas driver? Further, palmas-mfd shutdown
> handler(currently missing) if probably cleansup things:
>
> mfd_remove_devices(palmas->dev);
> palmas_irq_exit(palmas);
>
> shutdown sequence becomes complicated further esp if things are
> cleanedup in shutdown (Dummy patch[1]).
>
>
> All I am saying is this: shutdown should allow powerup functionality to
> work as well, how we do that is upto us - I personally found it a little
> easier to keep the IRQ unmask in shutdown easier to deal with, but other
> options might be possible as well.
I'm not sure if I understand your comments completely (maybe due to I'm
not familiar with the mechanism of unmasking USB IRQ in Palmas driver)
but doing cleanup in each driver shutdown handler makes sense to me, if
those clean up can be done in shutdown then we can make power off
function as simple as possible and being part of Palmas mfd driver?
>
> [1]
> diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
> index e4d1c70..6998863 100644
> --- a/drivers/mfd/palmas.c
> +++ b/drivers/mfd/palmas.c
> @@ -447,6 +447,11 @@ static int palmas_i2c_remove(struct i2c_client *i2c)
> return 0;
> }
>
> +static void palmas_i2c_shutdown(struct i2c_client *i2c)
> +{
> + palmas_i2c_remove(i2c);
> +}
> +
> static const struct i2c_device_id palmas_i2c_id[] = {
> { "palmas", },
> { "twl6035", },
> @@ -464,6 +469,7 @@ static struct i2c_driver palmas_i2c_driver = {
> },
> .probe = palmas_i2c_probe,
> .remove = palmas_i2c_remove,
> + .shutdown = palmas_i2c_shutdown,
> .id_table = palmas_i2c_id,
> };
>
--
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 v2 1/1] mfd: palmas: Add power off control
On Wed, 2013-07-31 at 19:57 +0800, Nishanth Menon wrote: > > If you notice the reference code I send, atleast on TWL6035/37 variants > of Palmas, USB IRQ unmask is mandatory for power on with USB cable - > example usage scenario: extremely low battery, device powered off, plug > in usb cable to restart charging - you'd like to initiate charging logic > in bootloader, but that wont work if the device does not do OFF-ON > transition with usb cable plugged in for vbus. > Why do we need to add Palmas USB_IRQ unmask logic in shutdown? Does that mean for all platform using Palmas has to unmask USB IRQ (including those do not power vbus through Palmas)? Can't we just have a simple shutdown function but have the VBus programming been done in USB driver or maybe platform driver since it is platform specific control? -- 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 1/1] mfd: palmas: Add power off control
On Wed, 2013-07-31 at 17:56 +0800, Lee Jones wrote: > On Tue, 30 Jul 2013, Bill Huang wrote: > > > Hook up "pm_power_off" to palmas power off routine if there is DT > > property "ti,system-power-controller" defined, so platform which is > > powered by this regulator can be powered off properly. > > > > Based on work by: > > Mallikarjun Kasoju > > > > Signed-off-by: Bill Huang > > cc: Mallikarjun Kasoju > > Please put the 'Cc:' (not 'cc:') above the SoBs, then drop the "Based > on work by:" and replace with: > > Signed-off-by: Mallikarjun Kasoju > Signed-off-by: Bill Huang > > This insinuates that the original patch was crated by Mallikarjun. > Thanks, will fix in next version. -- 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/
[PATCH v2 1/1] mfd: palmas: Add power off control
Hook up "pm_power_off" to palmas power off routine if there is DT
property "ti,system-power-controller" defined, so platform which is
powered by this regulator can be powered off properly.
Based on work by:
Mallikarjun Kasoju
Signed-off-by: Bill Huang
cc: Mallikarjun Kasoju
---
.../devicetree/bindings/regulator/palmas-pmic.txt |5 +++
drivers/mfd/palmas.c | 33 ++--
include/linux/mfd/palmas.h |1 +
3 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
index 30b0581..4adca2a 100644
--- a/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
+++ b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
@@ -36,6 +36,9 @@ Optional nodes:
ti,smps-range - OTP has the wrong range set for the hardware so
override
0 - low range, 1 - high range.
+- ti,system-power-controller: Telling whether or not this pmic is controlling
+ the system power.
+
Example:
#include
@@ -48,6 +51,8 @@ pmic {
ti,ldo6-vibrator;
+ ti,system-power-controller;
+
regulators {
smps12_reg : smps12 {
regulator-name = "smps12";
diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
index e4d1c70..220a34d 100644
--- a/drivers/mfd/palmas.c
+++ b/drivers/mfd/palmas.c
@@ -229,6 +229,32 @@ static void palmas_dt_to_pdata(struct i2c_client *i2c,
PALMAS_POWER_CTRL_ENABLE2_MASK;
if (i2c->irq)
palmas_set_pdata_irq_flag(i2c, pdata);
+
+ pdata->pm_off = of_property_read_bool(node,
+ "ti,system-power-controller");
+}
+
+static struct palmas *palmas_dev;
+static void palmas_power_off(void)
+{
+ unsigned int addr;
+ int ret, slave;
+
+ if (!palmas_dev)
+ return;
+
+ slave = PALMAS_BASE_TO_SLAVE(PALMAS_PMU_CONTROL_BASE);
+ addr = PALMAS_BASE_TO_REG(PALMAS_PMU_CONTROL_BASE, PALMAS_DEV_CTRL);
+
+ ret = regmap_update_bits(
+ palmas_dev->regmap[slave],
+ addr,
+ PALMAS_DEV_CTRL_DEV_ON,
+ 0);
+
+ if (ret)
+ pr_err("%s: Unable to write to DEV_CTRL_DEV_ON: %d\n",
+ __func__, ret);
}
static unsigned int palmas_features = PALMAS_PMIC_FEATURE_SMPS10_BOOST;
@@ -423,10 +449,13 @@ no_irq:
*/
if (node) {
ret = of_platform_populate(node, NULL, NULL, &i2c->dev);
- if (ret < 0)
+ if (ret < 0) {
goto err_irq;
- else
+ } else if (pdata->pm_off && !pm_power_off) {
+ palmas_dev = palmas;
+ pm_power_off = palmas_power_off;
return ret;
+ }
}
return ret;
diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h
index 1a8dd7a..061cce0 100644
--- a/include/linux/mfd/palmas.h
+++ b/include/linux/mfd/palmas.h
@@ -258,6 +258,7 @@ struct palmas_platform_data {
*/
int mux_from_pdata;
u8 pad1, pad2;
+ bool pm_off;
struct palmas_pmic_platform_data *pmic_pdata;
struct palmas_gpadc_platform_data *gpadc_pdata;
--
1.7.9.5
--
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 1/1] mfd: palmas: Add power off control
On Tue, 2013-07-30 at 21:17 +0800, Nishanth Menon wrote:
> On 07/30/2013 07:05 AM, Bill Huang wrote:
> > Hook up "pm_power_off" to palmas power off routine if there is DT
> > property "ti,system-power-controller" defined, so platform which is
> > powered by this regulator can be powered off properly.
> >
> > Based on work by:
> > Mallikarjun Kasoju
> >
> > Signed-off-by: Bill Huang
> > cc: Mallikarjun Kasoju
> > ---
> > .../devicetree/bindings/regulator/palmas-pmic.txt |5 +
> > drivers/mfd/palmas.c | 23
> > ++--
> > include/linux/mfd/palmas.h |1 +
> any reason why it wont fit in drivers/power/reset/ is'nt it the right
> place to add this?
> > 3 files changed, 27 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
> > b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
> > index 30b0581..4adca2a 100644
> > --- a/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
> > +++ b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
> > @@ -36,6 +36,9 @@ Optional nodes:
> >ti,smps-range - OTP has the wrong range set for the hardware so
> > override
> >0 - low range, 1 - high range.
> >
> > +- ti,system-power-controller: Telling whether or not this pmic is
> > controlling
> controller or master?
We name it controller since it controls the system power of the
platform, I'm not sure will it make more sense to be master? Or does it
makes sense to other pmic which also control the system power (the same
property are already used in pmic TPS6586x and TPS65910).
> > + the system power.
> > +
> > Example:
> >
> > #include
> > @@ -48,6 +51,8 @@ pmic {
> >
> > ti,ldo6-vibrator;
> >
> > + ti,system-power-controller;
> > +
> > regulators {
> > smps12_reg : smps12 {
> > regulator-name = "smps12";
> > diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
> > index e4d1c70..0662b21 100644
> > --- a/drivers/mfd/palmas.c
> > +++ b/drivers/mfd/palmas.c
> > @@ -229,6 +229,22 @@ static void palmas_dt_to_pdata(struct i2c_client *i2c,
> > PALMAS_POWER_CTRL_ENABLE2_MASK;
> > if (i2c->irq)
> > palmas_set_pdata_irq_flag(i2c, pdata);
> > +
> > + pdata->pm_off = of_property_read_bool(node,
> > + "ti,system-power-controller");
> > +}
> > +
> > +static struct palmas *palmas_dev;
> > +static void palmas_power_off(void)
> > +{
> > + unsigned int addr;
> > +
> > + if (!palmas_dev)
> > + return;
> > +
> > + addr = PALMAS_BASE_TO_REG(PALMAS_PMU_CONTROL_BASE, PALMAS_DEV_CTRL);
> > +
> > + regmap_update_bits(palmas_dev->regmap[0], addr, 1, 0);
> slave = PALMAS_BASE_TO_SLAVE(base);
> addr = PALMAS_BASE_TO_REG(base, reg);
>
> r = regmap_update_bits(palmas->regmap[slave], addr, mask, val);
> instead?
>
> just for reference, an old implementation I had done is available at [1]
>
Thanks, I'll send out v2.
>
> [1]
> http://git.omapzoom.org/?p=kernel/omap.git;a=blob;f=drivers/mfd/palmas-poweroff.c;hb=p-linux-omap-3.4
>
--
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/
[PATCH 1/1] mfd: palmas: Add power off control
Hook up "pm_power_off" to palmas power off routine if there is DT
property "ti,system-power-controller" defined, so platform which is
powered by this regulator can be powered off properly.
Based on work by:
Mallikarjun Kasoju
Signed-off-by: Bill Huang
cc: Mallikarjun Kasoju
---
.../devicetree/bindings/regulator/palmas-pmic.txt |5 +
drivers/mfd/palmas.c | 23 ++--
include/linux/mfd/palmas.h |1 +
3 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
index 30b0581..4adca2a 100644
--- a/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
+++ b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
@@ -36,6 +36,9 @@ Optional nodes:
ti,smps-range - OTP has the wrong range set for the hardware so
override
0 - low range, 1 - high range.
+- ti,system-power-controller: Telling whether or not this pmic is controlling
+ the system power.
+
Example:
#include
@@ -48,6 +51,8 @@ pmic {
ti,ldo6-vibrator;
+ ti,system-power-controller;
+
regulators {
smps12_reg : smps12 {
regulator-name = "smps12";
diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
index e4d1c70..0662b21 100644
--- a/drivers/mfd/palmas.c
+++ b/drivers/mfd/palmas.c
@@ -229,6 +229,22 @@ static void palmas_dt_to_pdata(struct i2c_client *i2c,
PALMAS_POWER_CTRL_ENABLE2_MASK;
if (i2c->irq)
palmas_set_pdata_irq_flag(i2c, pdata);
+
+ pdata->pm_off = of_property_read_bool(node,
+ "ti,system-power-controller");
+}
+
+static struct palmas *palmas_dev;
+static void palmas_power_off(void)
+{
+ unsigned int addr;
+
+ if (!palmas_dev)
+ return;
+
+ addr = PALMAS_BASE_TO_REG(PALMAS_PMU_CONTROL_BASE, PALMAS_DEV_CTRL);
+
+ regmap_update_bits(palmas_dev->regmap[0], addr, 1, 0);
}
static unsigned int palmas_features = PALMAS_PMIC_FEATURE_SMPS10_BOOST;
@@ -423,10 +439,13 @@ no_irq:
*/
if (node) {
ret = of_platform_populate(node, NULL, NULL, &i2c->dev);
- if (ret < 0)
+ if (ret < 0) {
goto err_irq;
- else
+ } else if (pdata->pm_off && !pm_power_off) {
+ palmas_dev = palmas;
+ pm_power_off = palmas_power_off;
return ret;
+ }
}
return ret;
diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h
index 1a8dd7a..061cce0 100644
--- a/include/linux/mfd/palmas.h
+++ b/include/linux/mfd/palmas.h
@@ -258,6 +258,7 @@ struct palmas_platform_data {
*/
int mux_from_pdata;
u8 pad1, pad2;
+ bool pm_off;
struct palmas_pmic_platform_data *pmic_pdata;
struct palmas_gpadc_platform_data *gpadc_pdata;
--
1.7.9.5
--
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 1/5] clk: allow reentrant calls into the clk framework
On Thu, 2013-02-28 at 12:49 +0800, Mike Turquette wrote: > Reentrancy into the clock framework from the clk.h api is highly > desirable. This feature is necessary for clocks that are prepared and > unprepared via i2c_transfer (which includes many PMICs and discrete > audio chips) and it is also necessary for performing dynamic voltage & > frequency scaling via clock rate-change notifiers. > > This patch implements reentrancy by adding a global atomic_t which > tracks the context of the current caller. Context in this case is the > return value from get_current(). The clk.h api implementations are > modified to first see if the relevant global lock is already held and if > so compare the global context (set by whoever is holding the lock) > against their own context (via a call to get_current()). If the two > match then this function is a nested call from the one already holding > the lock and we procede. If the context does not match then procede to > call mutex_lock and busy-wait for the existing task to complete. > > Thus this patch set does not increase concurrency for unrelated calls > into the clock framework. Instead it simply allows reentrancy by the > single task which is currently holding the global clock framework lock. > > Thanks to Rajagoapl Venkat for the original idea to use get_current() > and to David Brown for the suggestion to replace my previous rwlock > scheme with atomic operations during code review at ELC 2013. > > Signed-off-by: Mike Turquette > Cc: Rajagopal Venkat > Cc: David Brown > --- Hi Mike, Will this single patch be accepted? I guess you might not merge the whole series but I think this one is useful, is it possible that you can send out this single patch (or just merge this one) as an improvement of CCF? Or you think otherwise? Thanks, Bill -- 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 1/1] clk: Add notifier support in clk_prepare/clk_unprepare
On Wed, 2013-03-20 at 22:47 +0800, Mike Turquette wrote: > Quoting Bill Huang (2013-03-19 21:39:44) > > On Wed, 2013-03-20 at 11:31 +0800, Mike Turquette wrote: > > > Quoting Bill Huang (2013-03-19 19:55:49) > > > > On Wed, 2013-03-20 at 01:01 +0800, Mike Turquette wrote: > > > > > I'm still not sure about this approach. Based on feedback I got from > > > > > Linaro Connect I am not convinced that scaling voltage through clk > > > > > rate-change notifiers is the right way to go. As I understand it this > > > > > patch only exists for that single purpose, so if the voltage-notifier > > > > > idea gets dropped then I will not take this patch in. > > > > > > > > > Thanks Mike, actually we won't use your "clk: notifier handler for > > > > dynamic voltage scaling" patch instead we are trying to port our DVFS > > > > into Non-CPU DVFS framework "devfreq" which will need to hook those > > > > notifiers, without the clock notifiers been extended the framework is > > > > useless for us since we cannot do polling due to the fact that polling > > > > is not in real time. If it ended up extending the notifiers cannot > > > > happen then the only choice for us I think would be giving up "devfreq" > > > > and implement them in Tegra's "clk_hw". > > > > > > I'm familiar with the devfreq framework. Can you explain further how > > > you plan to use devfreq with the clock notifiers? What does the call > > > graph look like? > > > > > The call graph will look like this, when any DVFS interested clock rate > > changes (including enable and disable) happen -> Tegra devfreq clock > > notifier is called -> call into update_devfreq if needed -> in > > update_devfreq it will call into .get_target_freq in Tegra > > "devfreq_governor" -> and then call into .target of tegra > > devfreq_dev_profile to set voltage and done. More details are as below. > > > > We'll create devfreq driver for Tegra VDD_CORE rail, and the safe > > voltage level of the rail is determined by tens of clocks (2d, 3d, > > mpe,...), all the frequency ladders of those clocks are defined in DT > > also the operating points for VDD_CORE is declared in DT where its > > frequency will be more of a virtual clock or index. > > > > operating-points = < > > /* virtual-kHz uV */ > > 0 95 > > 1 100 > > 2 105 > > 3 110 > > 4 115 > > 5 120 > > 6 125 > > 7 130 > > 8 135 > > > > Register a Tegra governor where the callback .get_target_freq is the > > function to determine the overall frequency it can go to, and > > the .target callback in "devfreq_dev_profile" will be the function > > really do the voltage scaling. > > > > Tegra devfreq driver will register clock notifiers on all its interested > > clock and hence when any of those clock rate changes, disabled, enabled, > > we'll specifically call update_devfreq in the notifier. > > Thank you for the explanation. Do you plan to use actual devfreq > governors (like simple-ondemand, or something custom) for changing OPPs, > or do you just plan to use the clock framework as a trigger for DVFS? > I think the existing governors are all polling basis that do not work for us, we have only single voltage rails and have tens of clocks which are used by different device drivers, the voltage has to be boost before the frequency is increasing, and it can only be lower after all the interested clocks rate are allowing it to or we might crash. Clock notifier seem to be the only choice to build association with all those interested device driver (in real time) who might make changes on clock rate either by using runtime PM or the similar mechanism. We do use OPP in our devfreq driver, we just need to do some extra work on checking all the interested clocks for making sure we will be choosing the correct operating point for the power rail. -- 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 1/1] clk: Add notifier support in clk_prepare/clk_unprepare
On Wed, 2013-03-20 at 11:31 +0800, Mike Turquette wrote: > Quoting Bill Huang (2013-03-19 19:55:49) > > On Wed, 2013-03-20 at 01:01 +0800, Mike Turquette wrote: > > > Quoting Bill Huang (2013-03-19 06:28:32) > > > > Add notifier calls in clk_prepare and clk_unprepare so drivers which are > > > > interested in knowing that clk_prepare/unprepare call can act > > > > accordingly. > > > > > > > > The existing "clk_set_rate" notifier is not enough for normal DVFS > > > > inplementation since clock might be enabled/disabled at runtime. Adding > > > > these notifiers is useful on DVFS core which take clk_prepare as a hint > > > > on that the notified clock might be enabled later so it can raise > > > > voltage > > > > to a safe level before enabling the clock, and take clk_unprepare as a > > > > hint that the clock has been disabled and is safe to lower the voltage. > > > > > > > > The added notifier events are: > > > > > > > > PRE_CLK_PREPARE > > > > POST_CLK_PREPARE > > > > ABORT_CLK_PREPARE > > > > PRE_CLK_UNPREPARE > > > > POST_CLK_UNPREPARE > > > > > > > > Signed-off-by: Bill Huang > > > > > > I'm still not sure about this approach. Based on feedback I got from > > > Linaro Connect I am not convinced that scaling voltage through clk > > > rate-change notifiers is the right way to go. As I understand it this > > > patch only exists for that single purpose, so if the voltage-notifier > > > idea gets dropped then I will not take this patch in. > > > > > Thanks Mike, actually we won't use your "clk: notifier handler for > > dynamic voltage scaling" patch instead we are trying to port our DVFS > > into Non-CPU DVFS framework "devfreq" which will need to hook those > > notifiers, without the clock notifiers been extended the framework is > > useless for us since we cannot do polling due to the fact that polling > > is not in real time. If it ended up extending the notifiers cannot > > happen then the only choice for us I think would be giving up "devfreq" > > and implement them in Tegra's "clk_hw". > > I'm familiar with the devfreq framework. Can you explain further how > you plan to use devfreq with the clock notifiers? What does the call > graph look like? > The call graph will look like this, when any DVFS interested clock rate changes (including enable and disable) happen -> Tegra devfreq clock notifier is called -> call into update_devfreq if needed -> in update_devfreq it will call into .get_target_freq in Tegra "devfreq_governor" -> and then call into .target of tegra devfreq_dev_profile to set voltage and done. More details are as below. We'll create devfreq driver for Tegra VDD_CORE rail, and the safe voltage level of the rail is determined by tens of clocks (2d, 3d, mpe,...), all the frequency ladders of those clocks are defined in DT also the operating points for VDD_CORE is declared in DT where its frequency will be more of a virtual clock or index. operating-points = < /* virtual-kHz uV */ 0 95 1 100 2 105 3 110 4 115 5 120 6 125 7 130 8 135 Register a Tegra governor where the callback .get_target_freq is the function to determine the overall frequency it can go to, and the .target callback in "devfreq_dev_profile" will be the function really do the voltage scaling. Tegra devfreq driver will register clock notifiers on all its interested clock and hence when any of those clock rate changes, disabled, enabled, we'll specifically call update_devfreq in the notifier. -- 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 1/1] clk: Add notifier support in clk_prepare/clk_unprepare
On Wed, 2013-03-20 at 01:01 +0800, Mike Turquette wrote: > Quoting Bill Huang (2013-03-19 06:28:32) > > Add notifier calls in clk_prepare and clk_unprepare so drivers which are > > interested in knowing that clk_prepare/unprepare call can act accordingly. > > > > The existing "clk_set_rate" notifier is not enough for normal DVFS > > inplementation since clock might be enabled/disabled at runtime. Adding > > these notifiers is useful on DVFS core which take clk_prepare as a hint > > on that the notified clock might be enabled later so it can raise voltage > > to a safe level before enabling the clock, and take clk_unprepare as a > > hint that the clock has been disabled and is safe to lower the voltage. > > > > The added notifier events are: > > > > PRE_CLK_PREPARE > > POST_CLK_PREPARE > > ABORT_CLK_PREPARE > > PRE_CLK_UNPREPARE > > POST_CLK_UNPREPARE > > > > Signed-off-by: Bill Huang > > I'm still not sure about this approach. Based on feedback I got from > Linaro Connect I am not convinced that scaling voltage through clk > rate-change notifiers is the right way to go. As I understand it this > patch only exists for that single purpose, so if the voltage-notifier > idea gets dropped then I will not take this patch in. > Thanks Mike, actually we won't use your "clk: notifier handler for dynamic voltage scaling" patch instead we are trying to port our DVFS into Non-CPU DVFS framework "devfreq" which will need to hook those notifiers, without the clock notifiers been extended the framework is useless for us since we cannot do polling due to the fact that polling is not in real time. If it ended up extending the notifiers cannot happen then the only choice for us I think would be giving up "devfreq" and implement them in Tegra's "clk_hw". -- 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/
[PATCH 1/1] clk: Add notifier support in clk_prepare/clk_unprepare
Add notifier calls in clk_prepare and clk_unprepare so drivers which are
interested in knowing that clk_prepare/unprepare call can act accordingly.
The existing "clk_set_rate" notifier is not enough for normal DVFS
inplementation since clock might be enabled/disabled at runtime. Adding
these notifiers is useful on DVFS core which take clk_prepare as a hint
on that the notified clock might be enabled later so it can raise voltage
to a safe level before enabling the clock, and take clk_unprepare as a
hint that the clock has been disabled and is safe to lower the voltage.
The added notifier events are:
PRE_CLK_PREPARE
POST_CLK_PREPARE
ABORT_CLK_PREPARE
PRE_CLK_UNPREPARE
POST_CLK_UNPREPARE
Signed-off-by: Bill Huang
---
drivers/clk/clk.c | 88 ++-
include/linux/clk.h |5 +++
2 files changed, 57 insertions(+), 36 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index ed87b24..ac07c6e 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -516,6 +516,42 @@ struct clk *__clk_lookup(const char *name)
/***clk api***/
+/**
+ * __clk_notify - call clk notifier chain
+ * @clk: struct clk * that is changing rate
+ * @msg: clk notifier type (see include/linux/clk.h)
+ * @old_rate: old clk rate
+ * @new_rate: new clk rate
+ *
+ * Triggers a notifier call chain on the clk rate-change notification
+ * for 'clk'. Passes a pointer to the struct clk and the previous
+ * and current rates to the notifier callback. Intended to be called by
+ * internal clock code only. Returns NOTIFY_DONE from the last driver
+ * called if all went well, or NOTIFY_STOP or NOTIFY_BAD immediately if
+ * a driver returns that.
+ */
+static int __clk_notify(struct clk *clk, unsigned long msg,
+ unsigned long old_rate, unsigned long new_rate)
+{
+ struct clk_notifier *cn;
+ struct clk_notifier_data cnd;
+ int ret = NOTIFY_DONE;
+
+ cnd.clk = clk;
+ cnd.old_rate = old_rate;
+ cnd.new_rate = new_rate;
+
+ list_for_each_entry(cn, &clk_notifier_list, node) {
+ if (cn->clk == clk) {
+ ret = srcu_notifier_call_chain(&cn->notifier_head, msg,
+ &cnd);
+ break;
+ }
+ }
+
+ return ret;
+}
+
void __clk_unprepare(struct clk *clk)
{
if (!clk)
@@ -549,7 +585,14 @@ void __clk_unprepare(struct clk *clk)
void clk_unprepare(struct clk *clk)
{
mutex_lock(&prepare_lock);
+
+ if (clk->notifier_count)
+ __clk_notify(clk, PRE_CLK_UNPREPARE, clk->rate, clk->rate);
+
__clk_unprepare(clk);
+ if (clk->notifier_count)
+ __clk_notify(clk, POST_CLK_UNPREPARE, clk->rate, clk->rate);
+
mutex_unlock(&prepare_lock);
}
EXPORT_SYMBOL_GPL(clk_unprepare);
@@ -597,7 +640,16 @@ int clk_prepare(struct clk *clk)
int ret;
mutex_lock(&prepare_lock);
+
+ if (clk->notifier_count)
+ __clk_notify(clk, PRE_CLK_PREPARE, clk->rate, clk->rate);
+
ret = __clk_prepare(clk);
+ if (!ret && clk->notifier_count)
+ __clk_notify(clk, POST_CLK_PREPARE, clk->rate, clk->rate);
+ else if (clk->notifier_count)
+ __clk_notify(clk, ABORT_CLK_PREPARE, clk->rate, clk->rate);
+
mutex_unlock(&prepare_lock);
return ret;
@@ -749,42 +801,6 @@ long clk_round_rate(struct clk *clk, unsigned long rate)
EXPORT_SYMBOL_GPL(clk_round_rate);
/**
- * __clk_notify - call clk notifier chain
- * @clk: struct clk * that is changing rate
- * @msg: clk notifier type (see include/linux/clk.h)
- * @old_rate: old clk rate
- * @new_rate: new clk rate
- *
- * Triggers a notifier call chain on the clk rate-change notification
- * for 'clk'. Passes a pointer to the struct clk and the previous
- * and current rates to the notifier callback. Intended to be called by
- * internal clock code only. Returns NOTIFY_DONE from the last driver
- * called if all went well, or NOTIFY_STOP or NOTIFY_BAD immediately if
- * a driver returns that.
- */
-static int __clk_notify(struct clk *clk, unsigned long msg,
- unsigned long old_rate, unsigned long new_rate)
-{
- struct clk_notifier *cn;
- struct clk_notifier_data cnd;
- int ret = NOTIFY_DONE;
-
- cnd.clk = clk;
- cnd.old_rate = old_rate;
- cnd.new_rate = new_rate;
-
- list_for_each_entry(cn, &clk_notifier_list, node) {
- if (cn->clk == clk) {
- ret = srcu_notifier_call_chain(&cn->notifier_head, msg,
- &cnd);
- break;
- }
- }
-
- return ret;
-}
-
-/**
* __clk_recalc_rates
* @clk: first clk in the subtree
* @msg: notification type (
Re: [RFC 1/1] clk: Add notifier support in clk_prepare_enable/clk_disable_unprepare
On Sat, 2013-03-16 at 01:09 +0800, Russell King - ARM Linux wrote: > On Tue, Mar 12, 2013 at 10:40:04PM -0700, Bill Huang wrote: > > That will be too bad, it looks like we deadlock in the mechanism, we > > cannot change existing drivers behavior (that means some call > > clk_disable/enable directly, some are not), and we cannot hook notifier > > in clk_disable/enable either, that means there seems no any chance to > > get what we want, any idea? > > Look, the whole point is: > > - Drivers can call clk_enable/clk_disable from their atomic regions to > control the clock. Drivers which do this also call clk_prepare/ > clk_unprepare from a schedulable context to perform any operations > necessary to allow the clock to be used. > > - Drivers which only ever control the clock from a schedulable context > *can* use clk_prepare_enable()/clk_disable_unprepare() to control > their clock, which simplifies the coding in the driver. > > The whole point here is to cater for what is found on different SoCs and > not need to keep rewriting the drivers between different SoCs. > > So, the idea is that: > > - clk_prepare() does whatever is needed to prepare a clock for use which > may require waiting for the clock to be in a state which it can be > enabled. In other words, if there is a PLL, the PLL is setup and > we wait for it to report that it has locked. > > - clk_enable() is about turning the clock output on so that the device > receives the clock. > > Now, in the case of a PLL directly feeding a device, it's entirely possible > that clk_prepare() ends up providing the clock signal to the device, and > clk_enable() does absolutely nothing. > > Or, if the clock has a gate on it, it's entirely possible that clk_prepare() > does nothing, and clk_enable() unmasks the gate to allow the clock to be > provided to the device - which can happen from atomic contexts. > > The whole point about the separation of these two functions is that device > driver writers _can_ code their drivers for both situations and not care > about how the SoC implements the clocking at all. > > Why did we end up with this split in the first place? Because we ran into > the problem that some SoCs required a sleeping clk_enable() and others > didn't, and the whole thing was turning into an incompatible mess. > > So, please. Realise that clk_prepare() and clk_enable() are the _official_ > APIs, and that clk_prepare_enable() is merely a helper function for drivers > to allow them to automate the calling of those two functions in succession > with _no_ _further_ _processing_ at all. > > So, if your hooks need to be callable from schedulable contexts, then you > need to put them inside clk_prepare(). If your hooks are callable from > atomic contexts, then they can go into clk_enable(). But what you can > not do is put them into clk_prepare_enable(). Thanks a lot for good point. -- 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: [RFC 1/1] clk: Add notifier support in clk_prepare_enable/clk_disable_unprepare
On Fri, 2013-03-15 at 20:33 +0800, Ulf Hansson wrote: > I guess you did not fully got what I meant with "dvfs clock type". It > will not affect the clock API. But instead the dvfs is handled by > implementing a specific clk hw type. So the same thing is accomplished > as with clk notifiers, no changes should be needed to device drivers. > > The difference is only that no notifiers will be needed, and all the > dvfs stuff will be handled in the clk hw instead. It will mean that we > will bundle dvfs stuff into the clock drivers, instead of separating > the code outside the clock drivers. But, on the other hand no > notifiers will be needed. > Oh yes I misunderstand your origin point, but my thought is using existing devfreq framework as frequency/voltage policy driver instead of creating another one in clock driver and that's why I think we need the notifier work. By the way, some centralized DVFS implementation like Tegra's VDD_CORE rail has association with tens of clocks which will need to be taken care specially if we're doing those in clock driver I think. > Kind regards > Ulf Hansson -- 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 1/1] mfd: palmas: Add power off control
On Sat, 2013-03-16 at 01:22 +0800, Stephen Warren wrote: > On 03/14/2013 11:51 PM, Bill Huang wrote: > > On Fri, 2013-03-15 at 13:19 +0800, Stephen Warren wrote: > >> On 03/14/2013 04:58 AM, Bill Huang wrote: > >>> Hook up "pm_power_off" to palmas power off routine if there is DT > >>> property "ti,system-power-controller" defined, so platform which is > >>> powered by this regulator can be powered off properly. > >> > >>> diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c > >> > >>> + pdata->pm_off = of_property_read_bool(node, > >>> + "ti,system-power-controller"); > >> > >> You would need to add that property to the DT binding documentation for > >> this device. > > > > Does it work that some time later Laxmain helps to add it when he > > submits his pmic bindings for Palmas? > > I'll note that Ian Lartey and Graeme Gregory actually seem to be the > people defining the PMIC bindings for Palmas. See: > > http://comments.gmane.org/gmane.linux.documentation/9948 > > Or is Laxman planning to send some updates to that? > > But irrespective of that no, I don't think that influences this at all; > if you introduce driver support for new DT content, then either that DT > content should already be documented in the binding, or this patch > should add it to the documentation. Thanks, here I just need to add a property into the DT bindings but currently there is no any Palmas binding document exist so that means I have to hold off the patch. -- 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: [RFC v2 1/1] clk: Add notifier support in clk_prepare/clk_unprepare
On Sat, 2013-03-16 at 03:51 +0800, Stephen Warren wrote:
> On 03/15/2013 11:45 AM, Russell King - ARM Linux wrote:
> > On Thu, Mar 14, 2013 at 02:31:04AM -0700, Bill Huang wrote:
> >> Add the below two notifier events so drivers which are interested in
> >> knowing the clock status can act accordingly. This is extremely useful
> >> in some of the DVFS (Dynamic Voltage Frequency Scaling) design.
> >>
> >> CLK_PREPARED
> >> CLK_UNPREPARED
> >>
> >> Signed-off-by: Bill Huang
> >> ---
> >> drivers/clk/clk.c |3 +++
> >> include/linux/clk.h |2 ++
> >> 2 files changed, 5 insertions(+)
> >>
> >> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> >> index ed87b24..3292cec 100644
> >> --- a/drivers/clk/clk.c
> >> +++ b/drivers/clk/clk.c
> >> @@ -550,6 +550,7 @@ void clk_unprepare(struct clk *clk)
> >> {
> >>mutex_lock(&prepare_lock);
> >>__clk_unprepare(clk);
> >> + __clk_notify(clk, CLK_UNPREPARED, clk->rate, clk->rate);
> >>mutex_unlock(&prepare_lock);
> >> }
> >> EXPORT_SYMBOL_GPL(clk_unprepare);
> >> @@ -598,6 +599,8 @@ int clk_prepare(struct clk *clk)
> >>
> >>mutex_lock(&prepare_lock);
> >>ret = __clk_prepare(clk);
> >> + if (!ret)
> >> + __clk_notify(clk, CLK_PREPARED, clk->rate, clk->rate);
> >
> > So, on prepare, we notify after we've prepared the clock. On unprepare,
> > we notify after the clock has been shut down. Are you sure that's the
> > correct ordering? Would it not be better to view it in a stack-like
> > fashion, iow:
>
> > get
> > prepare
> > notify-prepare
> > enable
> > disable
> > notify-unprepare
> > unprepare
> > put
>
> Yes, these should be stacked/nested better for consistency.
>
> But for DVFS, the voltage needs to be raised before the prepare body is
> run, so that if clk_prepare actually enables the clock, the voltage is
> already at the safe level required by that clock. Similarly, for
> unprepare, you can only lower the voltage after having turned off the
> clock, which is guaranteed after the unprepare body. So, I think you
> want to move the notifier for prepare in the code above (and rename it
> to pre/before_prepare?), rather than the notifier for unprepare.
Oh yes I should raised notify before prepare body is run.
>
> In order to cover more cases, you might have both
> {pre,post}_{un,}prepare notifiers, although I'm not sure when you'd use
> the other two options.
Right, maybe {pre,post}_{un,}prepare will be useful.
>
> >> diff --git a/include/linux/clk.h b/include/linux/clk.h
>
> >> +#define CLK_PREPARED BIT(3)
> >> +#define CLK_UNPREPAREDBIT(4)
>
--
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: [RFC 1/1] clk: Add notifier support in clk_prepare_enable/clk_disable_unprepare
On Sat, 2013-03-16 at 03:38 +0800, Stephen Warren wrote: > On 03/15/2013 06:33 AM, Ulf Hansson wrote: > > On 15 March 2013 13:06, Bill Huang wrote: > >> On Fri, 2013-03-15 at 18:08 +0800, Ulf Hansson wrote: > ... > >>> Some prerequisites; I think am in favor of using the clk API to > >>> trigger DVFS changes and then I agree on that clk_prepare|unprepare > >>> needs to be possible to track from a DVFS perspective. clk_set_rate is > >>> not enough. > >>> > >>> So if we decide to do the above (using the clk API to trigger DVFS > >>> changes), I believe we should discuss two possible solutions; > >>> - clk notifiers or.. > >>> - dvfs clock type. > >>> > >>> I am trying to make up my mind of what I think is the best solution. > >>> Have you considered "dvfs clock type"? > >>> I put some comments about this for "[PATCH 2/5] clk: notifier handler > >>> for dynamic voltage scaling" recently as well. > >>> > >>> What could the advantages/disadvantages be between the two options? > >> > >> I personally prefer clk notifiers since that's easy and all the existing > >> device drivers don't need to be modified, a new clock or API might be > >> more thoroughly considered (and hence maybe more graceful) but that > >> means we need more time to cook and many drivers need to plug into that > >> API when it comes out, a lot of test/verification or maybe chaos > >> follows, I'm not sure will that be a little overkill. > > > > I guess you did not fully got what I meant with "dvfs clock type". It > > will not affect the clock API. But instead the dvfs is handled by > > implementing a specific clk hw type. So the same thing is accomplished > > as with clk notifiers, no changes should be needed to device drivers. > > > > The difference is only that no notifiers will be needed, and all the > > dvfs stuff will be handled in the clk hw instead. It will mean that we > > will bundle dvfs stuff into the clock drivers, instead of separating > > the code outside the clock drivers. But, on the other hand no > > notifiers will be needed. > > The advantage here is that I assume that a notifier would continually > have to check whether the clock being modified was one that the DVFS > notifier cared about. By integrating the CVFS logic into the clk_hw Actually, we can register notifier only on clocks that DVFS care about. -- 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: [RFC 1/1] clk: Add notifier support in clk_prepare_enable/clk_disable_unprepare
On Fri, 2013-03-15 at 18:08 +0800, Ulf Hansson wrote: > On 15 March 2013 10:39, Peter De Schrijver wrote: > > On Fri, Mar 15, 2013 at 06:22:47AM +0100, Stephen Warren wrote: > >> On 03/14/2013 07:20 PM, Bill Huang wrote: > >> > On Fri, 2013-03-15 at 01:54 +0800, Stephen Warren wrote: > >> >> On 03/14/2013 03:28 AM, Bill Huang wrote: > >> >>> On Thu, 2013-03-14 at 17:21 +0800, Peter De Schrijver wrote: > >> >>>> On Thu, Mar 14, 2013 at 03:15:11AM +0100, Bill Huang wrote: > >> >>>> > >> >>>>> I don't think deferring will work either, considering the usage of > >> >>>>> DVFS, > >> >>>>> device voltage is tightly coupled with frequency, when clock rate is > >> >>>>> about to increase, we have to boost voltage first and we can lower > >> >>>>> the > >> >>>>> voltage after the clock rate has decreased. All the above sequence > >> >>>>> have > >> >>>>> to be guaranteed or you might crash, so deferring not only make thing > >> >>>>> complicated in controlling the order but also hurt performance. > >> >>>> > >> >>>> But we could use notifiers in clk_prepare/clk_unprepare to set the > >> >>>> voltage no? > >> >>>> As clk_prepare/clk_unprepare have to be called before clk_enable or > >> >>>> after > >> >>>> clk_disable, the voltage can be raised to a safe level, before the > >> >>>> clock > >> >>>> becomes active. > >> >>> > >> >>> Thanks Peter, actually I'm just about to propose my v2 RFC which add > >> >>> notifier in clk_prepare/clk_unprepare. > >> >> > >> >> Can't clk_set_rate() be called while the clock is prepared, or even > >> >> enabled? I don't see how your proposal would work. > >> > > >> > I think it works with just a little sacrifice on saving more power but > >> > that's related minor. Taking clk_prepare as an indicator on that clock > >> > will be enabled later, so we can raise the voltage to a safe level > >> > (according to the current rate or maybe default rate when clk_prepare is > >> > called, some time late when clk_set_rate() is called we can adjust again > >> > according to the requested rate change) > >> > >> Is clk_set_rate() only legal to call in non-atomic contexts then? The > >> header file doesn't say, although I guess since many other functions > >> explicitly say they can't, then by omission it can... > > > > Yes. Only clk_enable() and clk_disable() can be called in an atomic context. > > > > Cheers, > > > > Peter. > > > > ___ > > linaro-dev mailing list > > linaro-...@lists.linaro.org > > http://lists.linaro.org/mailman/listinfo/linaro-dev > > Just wanted to add my reflection on this topic; > > Some prerequisites; I think am in favor of using the clk API to > trigger DVFS changes and then I agree on that clk_prepare|unprepare > needs to be possible to track from a DVFS perspective. clk_set_rate is > not enough. > > So if we decide to do the above (using the clk API to trigger DVFS > changes), I believe we should discuss two possible solutions; > - clk notifiers or.. > - dvfs clock type. > > I am trying to make up my mind of what I think is the best solution. > Have you considered "dvfs clock type"? > I put some comments about this for "[PATCH 2/5] clk: notifier handler > for dynamic voltage scaling" recently as well. > > What could the advantages/disadvantages be between the two options? I personally prefer clk notifiers since that's easy and all the existing device drivers don't need to be modified, a new clock or API might be more thoroughly considered (and hence maybe more graceful) but that means we need more time to cook and many drivers need to plug into that API when it comes out, a lot of test/verification or maybe chaos follows, I'm not sure will that be a little overkill. I'm not against designing a more robust API, I'm just suggesting before we have that defined we should at least let clk notifiers work. > > Kind regards > Ulf Hansson -- 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 1/1] mfd: palmas: Add power off control
On Fri, 2013-03-15 at 13:19 +0800, Stephen Warren wrote: > On 03/14/2013 04:58 AM, Bill Huang wrote: > > Hook up "pm_power_off" to palmas power off routine if there is DT > > property "ti,system-power-controller" defined, so platform which is > > powered by this regulator can be powered off properly. > > > diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c > > > + pdata->pm_off = of_property_read_bool(node, > > + "ti,system-power-controller"); > > You would need to add that property to the DT binding documentation for > this device. Does it work that some time later Laxmain helps to add it when he submits his pmic bindings for Palmas? -- 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: [RFC 1/1] clk: Add notifier support in clk_prepare_enable/clk_disable_unprepare
On Fri, 2013-03-15 at 13:22 +0800, Stephen Warren wrote: > On 03/14/2013 07:20 PM, Bill Huang wrote: > > On Fri, 2013-03-15 at 01:54 +0800, Stephen Warren wrote: > >> On 03/14/2013 03:28 AM, Bill Huang wrote: > >>> On Thu, 2013-03-14 at 17:21 +0800, Peter De Schrijver wrote: > >>>> On Thu, Mar 14, 2013 at 03:15:11AM +0100, Bill Huang wrote: > >>>> > >>>>> I don't think deferring will work either, considering the usage of DVFS, > >>>>> device voltage is tightly coupled with frequency, when clock rate is > >>>>> about to increase, we have to boost voltage first and we can lower the > >>>>> voltage after the clock rate has decreased. All the above sequence have > >>>>> to be guaranteed or you might crash, so deferring not only make thing > >>>>> complicated in controlling the order but also hurt performance. > >>>> > >>>> But we could use notifiers in clk_prepare/clk_unprepare to set the > >>>> voltage no? > >>>> As clk_prepare/clk_unprepare have to be called before clk_enable or after > >>>> clk_disable, the voltage can be raised to a safe level, before the clock > >>>> becomes active. > >>> > >>> Thanks Peter, actually I'm just about to propose my v2 RFC which add > >>> notifier in clk_prepare/clk_unprepare. > >> > >> Can't clk_set_rate() be called while the clock is prepared, or even > >> enabled? I don't see how your proposal would work. > > > > I think it works with just a little sacrifice on saving more power but > > that's related minor. Taking clk_prepare as an indicator on that clock > > will be enabled later, so we can raise the voltage to a safe level > > (according to the current rate or maybe default rate when clk_prepare is > > called, some time late when clk_set_rate() is called we can adjust again > > according to the requested rate change) > > Is clk_set_rate() only legal to call in non-atomic contexts then? The > header file doesn't say, although I guess since many other functions > explicitly say they can't, then by omission it can... I think clk_set_rate can only be called in non-atomic contexts since there are existing non-atomic clock notifier hooked in it. -- 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: [RFC 1/1] clk: Add notifier support in clk_prepare_enable/clk_disable_unprepare
On Fri, 2013-03-15 at 01:54 +0800, Stephen Warren wrote: > On 03/14/2013 03:28 AM, Bill Huang wrote: > > On Thu, 2013-03-14 at 17:21 +0800, Peter De Schrijver wrote: > >> On Thu, Mar 14, 2013 at 03:15:11AM +0100, Bill Huang wrote: > >> > >>> I don't think deferring will work either, considering the usage of DVFS, > >>> device voltage is tightly coupled with frequency, when clock rate is > >>> about to increase, we have to boost voltage first and we can lower the > >>> voltage after the clock rate has decreased. All the above sequence have > >>> to be guaranteed or you might crash, so deferring not only make thing > >>> complicated in controlling the order but also hurt performance. > >> > >> But we could use notifiers in clk_prepare/clk_unprepare to set the voltage > >> no? > >> As clk_prepare/clk_unprepare have to be called before clk_enable or after > >> clk_disable, the voltage can be raised to a safe level, before the clock > >> becomes active. > > > > Thanks Peter, actually I'm just about to propose my v2 RFC which add > > notifier in clk_prepare/clk_unprepare. > > Can't clk_set_rate() be called while the clock is prepared, or even > enabled? I don't see how your proposal would work. > I think it works with just a little sacrifice on saving more power but that's related minor. Taking clk_prepare as an indicator on that clock will be enabled later, so we can raise the voltage to a safe level (according to the current rate or maybe default rate when clk_prepare is called, some time late when clk_set_rate() is called we can adjust again according to the requested rate change) but there should be case that clock will not be enabled after clk_prepare (I'm not sure is normal though), that the case power might be wasted. Similarly, taking clk_unprepare as an indicator on clock has been disabled but there might be case clock is disabled but do not call clk_unprepare any time soon, this is another case power is wasted but this should not be normal case. So the point is, we can get notified on runtime clock change events (no matter it is clock enable/disable or clock rate change) and act (get best balance on power saving and system stability) accordingly. -- 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/
[PATCH 1/1] mfd: palmas: Add power off control
Hook up "pm_power_off" to palmas power off routine if there is DT
property "ti,system-power-controller" defined, so platform which is
powered by this regulator can be powered off properly.
Based on work by:
Mallikarjun Kasoju
Signed-off-by: Bill Huang
---
drivers/mfd/palmas.c | 25 +++--
include/linux/mfd/palmas.h |1 +
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
index bbdbc50..ee8180d 100644
--- a/drivers/mfd/palmas.c
+++ b/drivers/mfd/palmas.c
@@ -283,6 +283,22 @@ static void palmas_dt_to_pdata(struct device_node *node,
pdata->power_ctrl = PALMAS_POWER_CTRL_NSLEEP_MASK |
PALMAS_POWER_CTRL_ENABLE1_MASK |
PALMAS_POWER_CTRL_ENABLE2_MASK;
+
+ pdata->pm_off = of_property_read_bool(node,
+ "ti,system-power-controller");
+}
+
+static struct palmas *palmas_dev;
+static void palmas_power_off(void)
+{
+ unsigned int addr;
+
+ if (!palmas_dev)
+ return;
+
+ addr = PALMAS_BASE_TO_REG(PALMAS_PMU_CONTROL_BASE, PALMAS_DEV_CTRL);
+
+ regmap_update_bits(palmas_dev->regmap[0], addr, 1, 0);
}
static int palmas_i2c_probe(struct i2c_client *i2c,
@@ -435,10 +451,15 @@ static int palmas_i2c_probe(struct i2c_client *i2c,
*/
if (node) {
ret = of_platform_populate(node, NULL, NULL, &i2c->dev);
- if (ret < 0)
+ if (ret < 0) {
goto err_irq;
- else
+ } else {
+ if (pdata->pm_off && !pm_power_off) {
+ palmas_dev = palmas;
+ pm_power_off = palmas_power_off;
+ }
return ret;
+ }
}
children = kmemdup(palmas_children, sizeof(palmas_children),
diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h
index a4d13d7..a447e54 100644
--- a/include/linux/mfd/palmas.h
+++ b/include/linux/mfd/palmas.h
@@ -232,6 +232,7 @@ struct palmas_platform_data {
*/
int mux_from_pdata;
u8 pad1, pad2;
+ bool pm_off;
struct palmas_pmic_platform_data *pmic_pdata;
struct palmas_gpadc_platform_data *gpadc_pdata;
--
1.7.9.5
--
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/
[RFC v3 1/1] clk: Add notifier support in clk_prepare/clk_unprepare
Add the below two notifier events so drivers which are interested in
knowing the clock status can act accordingly. This is extremely useful
in some of the DVFS (Dynamic Voltage Frequency Scaling) design.
CLK_PREPARED
CLK_UNPREPARED
Signed-off-by: Bill Huang
---
drivers/clk/clk.c | 75 ++-
include/linux/clk.h |2 ++
2 files changed, 41 insertions(+), 36 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index ed87b24..d78ed16 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -516,6 +516,42 @@ struct clk *__clk_lookup(const char *name)
/***clk api***/
+/**
+ * __clk_notify - call clk notifier chain
+ * @clk: struct clk * that is changing rate
+ * @msg: clk notifier type (see include/linux/clk.h)
+ * @old_rate: old clk rate
+ * @new_rate: new clk rate
+ *
+ * Triggers a notifier call chain on the clk rate-change notification
+ * for 'clk'. Passes a pointer to the struct clk and the previous
+ * and current rates to the notifier callback. Intended to be called by
+ * internal clock code only. Returns NOTIFY_DONE from the last driver
+ * called if all went well, or NOTIFY_STOP or NOTIFY_BAD immediately if
+ * a driver returns that.
+ */
+static int __clk_notify(struct clk *clk, unsigned long msg,
+ unsigned long old_rate, unsigned long new_rate)
+{
+ struct clk_notifier *cn;
+ struct clk_notifier_data cnd;
+ int ret = NOTIFY_DONE;
+
+ cnd.clk = clk;
+ cnd.old_rate = old_rate;
+ cnd.new_rate = new_rate;
+
+ list_for_each_entry(cn, &clk_notifier_list, node) {
+ if (cn->clk == clk) {
+ ret = srcu_notifier_call_chain(&cn->notifier_head, msg,
+ &cnd);
+ break;
+ }
+ }
+
+ return ret;
+}
+
void __clk_unprepare(struct clk *clk)
{
if (!clk)
@@ -550,6 +586,7 @@ void clk_unprepare(struct clk *clk)
{
mutex_lock(&prepare_lock);
__clk_unprepare(clk);
+ __clk_notify(clk, CLK_UNPREPARED, clk->rate, clk->rate);
mutex_unlock(&prepare_lock);
}
EXPORT_SYMBOL_GPL(clk_unprepare);
@@ -598,6 +635,8 @@ int clk_prepare(struct clk *clk)
mutex_lock(&prepare_lock);
ret = __clk_prepare(clk);
+ if (!ret)
+ __clk_notify(clk, CLK_PREPARED, clk->rate, clk->rate);
mutex_unlock(&prepare_lock);
return ret;
@@ -749,42 +788,6 @@ long clk_round_rate(struct clk *clk, unsigned long rate)
EXPORT_SYMBOL_GPL(clk_round_rate);
/**
- * __clk_notify - call clk notifier chain
- * @clk: struct clk * that is changing rate
- * @msg: clk notifier type (see include/linux/clk.h)
- * @old_rate: old clk rate
- * @new_rate: new clk rate
- *
- * Triggers a notifier call chain on the clk rate-change notification
- * for 'clk'. Passes a pointer to the struct clk and the previous
- * and current rates to the notifier callback. Intended to be called by
- * internal clock code only. Returns NOTIFY_DONE from the last driver
- * called if all went well, or NOTIFY_STOP or NOTIFY_BAD immediately if
- * a driver returns that.
- */
-static int __clk_notify(struct clk *clk, unsigned long msg,
- unsigned long old_rate, unsigned long new_rate)
-{
- struct clk_notifier *cn;
- struct clk_notifier_data cnd;
- int ret = NOTIFY_DONE;
-
- cnd.clk = clk;
- cnd.old_rate = old_rate;
- cnd.new_rate = new_rate;
-
- list_for_each_entry(cn, &clk_notifier_list, node) {
- if (cn->clk == clk) {
- ret = srcu_notifier_call_chain(&cn->notifier_head, msg,
- &cnd);
- break;
- }
- }
-
- return ret;
-}
-
-/**
* __clk_recalc_rates
* @clk: first clk in the subtree
* @msg: notification type (see include/linux/clk.h)
diff --git a/include/linux/clk.h b/include/linux/clk.h
index b3ac22d..16c1d92 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -43,6 +43,8 @@ struct clk;
#define PRE_RATE_CHANGEBIT(0)
#define POST_RATE_CHANGE BIT(1)
#define ABORT_RATE_CHANGE BIT(2)
+#define CLK_PREPARED BIT(3)
+#define CLK_UNPREPARED BIT(4)
/**
* struct clk_notifier - associate a clk with a notifier
--
1.7.9.5
--
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: [RFC v2 1/1] clk: Add notifier support in clk_prepare/clk_unprepare
On Thu, 2013-03-14 at 17:31 +0800, Bill Huang wrote:
> Add the below two notifier events so drivers which are interested in
> knowing the clock status can act accordingly. This is extremely useful
> in some of the DVFS (Dynamic Voltage Frequency Scaling) design.
>
> CLK_PREPARED
> CLK_UNPREPARED
>
> Signed-off-by: Bill Huang
> ---
> drivers/clk/clk.c |3 +++
> include/linux/clk.h |2 ++
> 2 files changed, 5 insertions(+)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index ed87b24..3292cec 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -550,6 +550,7 @@ void clk_unprepare(struct clk *clk)
> {
> mutex_lock(&prepare_lock);
> __clk_unprepare(clk);
> + __clk_notify(clk, CLK_UNPREPARED, clk->rate, clk->rate);
> mutex_unlock(&prepare_lock);
> }
> EXPORT_SYMBOL_GPL(clk_unprepare);
> @@ -598,6 +599,8 @@ int clk_prepare(struct clk *clk)
>
> mutex_lock(&prepare_lock);
> ret = __clk_prepare(clk);
> + if (!ret)
> + __clk_notify(clk, CLK_PREPARED, clk->rate, clk->rate);
> mutex_unlock(&prepare_lock);
>
> return ret;
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index b3ac22d..16c1d92 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -43,6 +43,8 @@ struct clk;
> #define PRE_RATE_CHANGE BIT(0)
> #define POST_RATE_CHANGE BIT(1)
> #define ABORT_RATE_CHANGEBIT(2)
> +#define CLK_PREPARED BIT(3)
> +#define CLK_UNPREPARED BIT(4)
>
> /**
> * struct clk_notifier - associate a clk with a notifier
Sorry, I'll send v3 since this bread build.
--
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/
[RFC v2 1/1] clk: Add notifier support in clk_prepare/clk_unprepare
Add the below two notifier events so drivers which are interested in
knowing the clock status can act accordingly. This is extremely useful
in some of the DVFS (Dynamic Voltage Frequency Scaling) design.
CLK_PREPARED
CLK_UNPREPARED
Signed-off-by: Bill Huang
---
drivers/clk/clk.c |3 +++
include/linux/clk.h |2 ++
2 files changed, 5 insertions(+)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index ed87b24..3292cec 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -550,6 +550,7 @@ void clk_unprepare(struct clk *clk)
{
mutex_lock(&prepare_lock);
__clk_unprepare(clk);
+ __clk_notify(clk, CLK_UNPREPARED, clk->rate, clk->rate);
mutex_unlock(&prepare_lock);
}
EXPORT_SYMBOL_GPL(clk_unprepare);
@@ -598,6 +599,8 @@ int clk_prepare(struct clk *clk)
mutex_lock(&prepare_lock);
ret = __clk_prepare(clk);
+ if (!ret)
+ __clk_notify(clk, CLK_PREPARED, clk->rate, clk->rate);
mutex_unlock(&prepare_lock);
return ret;
diff --git a/include/linux/clk.h b/include/linux/clk.h
index b3ac22d..16c1d92 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -43,6 +43,8 @@ struct clk;
#define PRE_RATE_CHANGEBIT(0)
#define POST_RATE_CHANGE BIT(1)
#define ABORT_RATE_CHANGE BIT(2)
+#define CLK_PREPARED BIT(3)
+#define CLK_UNPREPARED BIT(4)
/**
* struct clk_notifier - associate a clk with a notifier
--
1.7.9.5
--
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: [RFC 1/1] clk: Add notifier support in clk_prepare_enable/clk_disable_unprepare
On Thu, 2013-03-14 at 17:21 +0800, Peter De Schrijver wrote: > On Thu, Mar 14, 2013 at 03:15:11AM +0100, Bill Huang wrote: > > > I don't think deferring will work either, considering the usage of DVFS, > > device voltage is tightly coupled with frequency, when clock rate is > > about to increase, we have to boost voltage first and we can lower the > > voltage after the clock rate has decreased. All the above sequence have > > to be guaranteed or you might crash, so deferring not only make thing > > complicated in controlling the order but also hurt performance. > > But we could use notifiers in clk_prepare/clk_unprepare to set the voltage no? > As clk_prepare/clk_unprepare have to be called before clk_enable or after > clk_disable, the voltage can be raised to a safe level, before the clock > becomes active. Thanks Peter, actually I'm just about to propose my v2 RFC which add notifier in clk_prepare/clk_unprepare. > > Cheers, > > Peter. -- 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: [RFC 1/1] clk: Add notifier support in clk_prepare_enable/clk_disable_unprepare
On Thu, 2013-03-14 at 02:10 +0800, Stephen Warren wrote: > On 03/12/2013 11:40 PM, Bill Huang wrote: > > On Wed, 2013-03-13 at 13:24 +0800, Stephen Warren wrote: > >> On 03/12/2013 11:08 PM, Bill Huang wrote: > >>> On Wed, 2013-03-13 at 12:42 +0800, Stephen Warren wrote: > >>>> On 03/12/2013 07:47 PM, Bill Huang wrote: > >>>>> On Tue, 2013-03-12 at 21:40 +0800, Russell King - ARM Linux wrote: > >>>>>> On Tue, Mar 12, 2013 at 05:37:41AM -0700, Bill Huang wrote: > >>>>>>> Add the below four notifier events so drivers which are interested in > >>>>>>> knowing the clock status can act accordingly. This is extremely useful > >>>>>>> in some of the DVFS (Dynamic Voltage Frequency Scaling) design. > >>>>>>> > >>>>>>> PRE_CLK_ENABLE > >>>>>>> POST_CLK_ENABLE > >>>>>>> PRE_CLK_DISABLE > >>>>>>> POST_CLK_DISABLE > ... > >>> Thanks, I know the point, but unfortunately there is no good choice for > >>> hooking this since those low level functions clk_enable/clk_disable will > >>> be called in interrupt context so it is not possible to send notify. We > >>> might need to come out a better approach if we can think of any. > >>> Currently I still think this is acceptable (Having all the drivers which > >>> are using our interested clocks call these function to enable/disable > >>> clock in their runtime_pm calls) though it's not perfect. > >> > >> No, that definitely won't work. Not all drivers use those APIs, nor > >> should they. > >> > > That will be too bad, it looks like we deadlock in the mechanism, we > > cannot change existing drivers behavior (that means some call > > clk_disable/enable directly, some are not), and we cannot hook notifier > > in clk_disable/enable either, that means there seems no any chance to > > get what we want, any idea? > > I don't know the correct answer. > > But I have a question: Why can't we run notifications from the real > clk_enable? Does the notification mechanism itself inherently block, or > are we worried that implementations/receivers of these notifications > might block. Perhaps we can simply define that these notification types I think both. > may be triggered in atomic context and hence implementations have to > support executing in atomic context. Is possible to make that a > requirement? Is it possible to implement the code that receives these > notifications in atomic context? Making it executing in atomic context won't work since for DVFS we generally having to set voltage (while receiving the notify) through I2C interface which can sleep. > > Is it possible to defer triggering of notifications to some non-atomic > context, even if they happen in an atomic context? I don't think deferring will work either, considering the usage of DVFS, device voltage is tightly coupled with frequency, when clock rate is about to increase, we have to boost voltage first and we can lower the voltage after the clock rate has decreased. All the above sequence have to be guaranteed or you might crash, so deferring not only make thing complicated in controlling the order but also hurt performance. > > I also wonder if this is the right conceptual level to be hooking into > the system. Perhaps DVFS is not something that should be triggered by > noticing that clocks have changed rates, but rather it should be some > form of higher layer that controls (calls into) both the regulator and > clock APIs? I wonder will there be a feasible API can do that since most of SoC is characterizing DVFS on frequency vs. voltage, it makes sense that reviewing voltage when the clock rate change happen, and the only mechanism seem to be hooking notifier in not only clock rate change but also clock enable/disable. -- 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: [RFC 1/1] clk: Add notifier support in clk_prepare_enable/clk_disable_unprepare
On Wed, 2013-03-13 at 13:24 +0800, Stephen Warren wrote: > On 03/12/2013 11:08 PM, Bill Huang wrote: > > On Wed, 2013-03-13 at 12:42 +0800, Stephen Warren wrote: > >> On 03/12/2013 07:47 PM, Bill Huang wrote: > >>> On Tue, 2013-03-12 at 21:40 +0800, Russell King - ARM Linux wrote: > >>>> On Tue, Mar 12, 2013 at 05:37:41AM -0700, Bill Huang wrote: > >>>>> Add the below four notifier events so drivers which are interested in > >>>>> knowing the clock status can act accordingly. This is extremely useful > >>>>> in some of the DVFS (Dynamic Voltage Frequency Scaling) design. > >>>>> > >>>>> PRE_CLK_ENABLE > >>>>> POST_CLK_ENABLE > >>>>> PRE_CLK_DISABLE > >>>>> POST_CLK_DISABLE > >>>>> > >>>>> Signed-off-by: Bill Huang > >>>> > >>>> NAK. *Sigh* NO, this is the wrong level to be doing stuff like this. > >>>> > >>>> The *ONLY* thing that clk_prepare_enable() and clk_prepare_disable() > >>>> should > >>>> *EVER* be doing is calling clk_prepare(), clk_enable(), clk_disable() and > >>>> clk_unprepare(). Those two functions are *merely* helpers for drivers > >>>> who don't wish to make the individual calls. > >>>> > >>>> Drivers are still completely free to call the individual functions, at > >>>> which point your proposal breaks horribly - and they _do_ call the > >>>> individual functions. > >>> > >>> I'm proposing to give device driver a choice when it knows that some > >>> driver might be interested in knowing its clock's enabled/disabled state > >>> change at runtime, this is very important for centralized DVFS core > >>> driver. It is not meant to be covering all cases especially for drivers > >>> which is not part of the DVFS, so we don't care if it is calling > >>> clk_enable/disable directly or not. > >> > >> I believe the point Russell is making is not that the idea behind this > >> patch is wrong, but simply that the function where you put the hooks is > >> wrong. The hooks should at least be in clk_enable/clk_disable and not > >> clk_prepare_enable/clk_disable_unprepare, since any driver is free to > >> call clk_prepare separately from clk_enable. The hooks should be > >> implemented in the lowest-level common function that all > >> driver-accessible paths call through. > > > > Thanks, I know the point, but unfortunately there is no good choice for > > hooking this since those low level functions clk_enable/clk_disable will > > be called in interrupt context so it is not possible to send notify. We > > might need to come out a better approach if we can think of any. > > Currently I still think this is acceptable (Having all the drivers which > > are using our interested clocks call these function to enable/disable > > clock in their runtime_pm calls) though it's not perfect. > > No, that definitely won't work. Not all drivers use those APIs, nor > should they. > That will be too bad, it looks like we deadlock in the mechanism, we cannot change existing drivers behavior (that means some call clk_disable/enable directly, some are not), and we cannot hook notifier in clk_disable/enable either, that means there seems no any chance to get what we want, any idea? -- 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: [RFC 1/1] clk: Add notifier support in clk_prepare_enable/clk_disable_unprepare
On Wed, 2013-03-13 at 12:42 +0800, Stephen Warren wrote: > On 03/12/2013 07:47 PM, Bill Huang wrote: > > On Tue, 2013-03-12 at 21:40 +0800, Russell King - ARM Linux wrote: > >> On Tue, Mar 12, 2013 at 05:37:41AM -0700, Bill Huang wrote: > >>> Add the below four notifier events so drivers which are interested in > >>> knowing the clock status can act accordingly. This is extremely useful > >>> in some of the DVFS (Dynamic Voltage Frequency Scaling) design. > >>> > >>> PRE_CLK_ENABLE > >>> POST_CLK_ENABLE > >>> PRE_CLK_DISABLE > >>> POST_CLK_DISABLE > >>> > >>> Signed-off-by: Bill Huang > >> > >> NAK. *Sigh* NO, this is the wrong level to be doing stuff like this. > >> > >> The *ONLY* thing that clk_prepare_enable() and clk_prepare_disable() should > >> *EVER* be doing is calling clk_prepare(), clk_enable(), clk_disable() and > >> clk_unprepare(). Those two functions are *merely* helpers for drivers > >> who don't wish to make the individual calls. > >> > >> Drivers are still completely free to call the individual functions, at > >> which point your proposal breaks horribly - and they _do_ call the > >> individual functions. > > > > I'm proposing to give device driver a choice when it knows that some > > driver might be interested in knowing its clock's enabled/disabled state > > change at runtime, this is very important for centralized DVFS core > > driver. It is not meant to be covering all cases especially for drivers > > which is not part of the DVFS, so we don't care if it is calling > > clk_enable/disable directly or not. > > I believe the point Russell is making is not that the idea behind this > patch is wrong, but simply that the function where you put the hooks is > wrong. The hooks should at least be in clk_enable/clk_disable and not > clk_prepare_enable/clk_disable_unprepare, since any driver is free to > call clk_prepare separately from clk_enable. The hooks should be > implemented in the lowest-level common function that all > driver-accessible paths call through. Thanks, I know the point, but unfortunately there is no good choice for hooking this since those low level functions clk_enable/clk_disable will be called in interrupt context so it is not possible to send notify. We might need to come out a better approach if we can think of any. Currently I still think this is acceptable (Having all the drivers which are using our interested clocks call these function to enable/disable clock in their runtime_pm calls) though it's not perfect. -- 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: [RFC 1/1] clk: Add notifier support in clk_prepare_enable/clk_disable_unprepare
On Tue, 2013-03-12 at 21:40 +0800, Russell King - ARM Linux wrote: > On Tue, Mar 12, 2013 at 05:37:41AM -0700, Bill Huang wrote: > > Add the below four notifier events so drivers which are interested in > > knowing the clock status can act accordingly. This is extremely useful > > in some of the DVFS (Dynamic Voltage Frequency Scaling) design. > > > > PRE_CLK_ENABLE > > POST_CLK_ENABLE > > PRE_CLK_DISABLE > > POST_CLK_DISABLE > > > > Signed-off-by: Bill Huang > > NAK. *Sigh* NO, this is the wrong level to be doing stuff like this. > > The *ONLY* thing that clk_prepare_enable() and clk_prepare_disable() should > *EVER* be doing is calling clk_prepare(), clk_enable(), clk_disable() and > clk_unprepare(). Those two functions are *merely* helpers for drivers > who don't wish to make the individual calls. > > Drivers are still completely free to call the individual functions, at > which point your proposal breaks horribly - and they _do_ call the > individual functions. I'm proposing to give device driver a choice when it knows that some driver might be interested in knowing its clock's enabled/disabled state change at runtime, this is very important for centralized DVFS core driver. It is not meant to be covering all cases especially for drivers which is not part of the DVFS, so we don't care if it is calling clk_enable/disable directly or not. One major side effect being that it affects those existing drivers who were calling clk_prepare_enable and clk_prepare_disable but it should not hurt if they don't care the notifier, or maybe we should define a new set of functions for the purpose? Mike, how do you think? Thanks. -- 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/
[RFC 1/1] clk: Add notifier support in clk_prepare_enable/clk_disable_unprepare
Add the below four notifier events so drivers which are interested in
knowing the clock status can act accordingly. This is extremely useful
in some of the DVFS (Dynamic Voltage Frequency Scaling) design.
PRE_CLK_ENABLE
POST_CLK_ENABLE
PRE_CLK_DISABLE
POST_CLK_DISABLE
Signed-off-by: Bill Huang
---
drivers/clk/clk.c | 58 +++
include/linux/clk.h | 40 ---
2 files changed, 81 insertions(+), 17 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index ed87b24..720a16a 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1104,6 +1104,64 @@ struct clk *clk_get_parent(struct clk *clk)
}
EXPORT_SYMBOL_GPL(clk_get_parent);
+/**
+ * clk_prepare_enable - call clk_enable in non-atomic context.
+ * @clk: the clk to enable
+ *
+ * Send PRE_CLK_ENABLE/POST_CLK_ENABLE before/after calling
+ * clk_enalbe respectively
+ *
+ * Return success (0) or errno.
+ */
+int clk_prepare_enable(struct clk *clk)
+{
+ int ret;
+
+ mutex_lock(&prepare_lock);
+ ret = __clk_prepare(clk);
+ if (ret)
+ return ret;
+
+ if (clk->notifier_count)
+ __clk_notify(clk, PRE_CLK_ENABLE, clk->rate, clk->rate);
+
+ ret = clk_enable(clk);
+ if (ret)
+ __clk_unprepare(clk);
+
+ if (clk->notifier_count)
+ __clk_notify(clk, POST_CLK_ENABLE, clk->rate, clk->rate);
+
+ mutex_unlock(&prepare_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(clk_prepare_enable);
+
+/**
+ * clk_disable_unprepare - call clk_disable in non-atomic context.
+ * @clk: the clk to enable
+ *
+ * Send PRE_CLK_DISABLE/POST_CLK_DISABLE before/after calling
+ * clk_disalbe respectively
+ */
+void clk_disable_unprepare(struct clk *clk)
+{
+ mutex_lock(&prepare_lock);
+
+ if (clk->notifier_count)
+ __clk_notify(clk, PRE_CLK_DISABLE, clk->rate, clk->rate);
+
+ clk_disable(clk);
+ __clk_unprepare(clk);
+
+ if (clk->notifier_count)
+ __clk_notify(clk, POST_CLK_DISABLE, clk->rate, clk->rate);
+
+ mutex_unlock(&prepare_lock);
+}
+EXPORT_SYMBOL_GPL(clk_disable_unprepare);
+
/*
* .get_parent is mandatory for clocks with multiple possible parents. It is
* optional for single-parent clocks. Always call .get_parent if it is
diff --git a/include/linux/clk.h b/include/linux/clk.h
index b3ac22d..4bbe1903 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -43,6 +43,10 @@ struct clk;
#define PRE_RATE_CHANGEBIT(0)
#define POST_RATE_CHANGE BIT(1)
#define ABORT_RATE_CHANGE BIT(2)
+#define PRE_CLK_ENABLE BIT(3)
+#define POST_CLK_ENABLEBIT(4)
+#define PRE_CLK_DISABLEBIT(5)
+#define POST_CLK_DISABLE BIT(6)
/**
* struct clk_notifier - associate a clk with a notifier
@@ -276,6 +280,20 @@ struct clk *clk_get_parent(struct clk *clk);
*/
struct clk *clk_get_sys(const char *dev_id, const char *con_id);
+/**
+ * clk_prepare_enable - helps cases using clk_enable in non-atomic context.
+ * @clk: clock source
+ *
+ * Return success (0) or nagative errno.
+ */
+int clk_prepare_enable(struct clk *clk);
+
+/**
+ * clk_disable_unprepare - helps cases using clk_disable in non-atomic context.
+ * @clk: clock source
+ */
+void clk_disable_unprepare(struct clk *clk);
+
#else /* !CONFIG_HAVE_CLK */
static inline struct clk *clk_get(struct device *dev, const char *id)
@@ -324,30 +342,18 @@ static inline struct clk *clk_get_parent(struct clk *clk)
return NULL;
}
-#endif
-
-/* clk_prepare_enable helps cases using clk_enable in non-atomic context. */
static inline int clk_prepare_enable(struct clk *clk)
{
- int ret;
-
- ret = clk_prepare(clk);
- if (ret)
- return ret;
- ret = clk_enable(clk);
- if (ret)
- clk_unprepare(clk);
-
- return ret;
+ return 0;
}
-/* clk_disable_unprepare helps cases using clk_disable in non-atomic context.
*/
-static inline void clk_disable_unprepare(struct clk *clk)
+static inline int clk_disable_unprepare(struct clk *clk)
{
- clk_disable(clk);
- clk_unprepare(clk);
+ return 0;
}
+#endif
+
/**
* clk_add_alias - add a new clock alias
* @alias: name for clock alias
--
1.7.9.5
--
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 2/5] clk: notifier handler for dynamic voltage scaling
On Sat, 2013-03-02 at 04:49 +0800, Stephen Warren wrote: > On 03/01/2013 02:41 AM, Bill Huang wrote: > > On Thu, 2013-02-28 at 12:49 +0800, Mike Turquette wrote: > >> There are three prerequisites to using this feature: > >> > >> 1) the affected clocks must be using the common clk framework > >> 2) voltage must be scaled using the regulator framework > >> 3) clock frequency and regulator voltage values must be paired via the > >> OPP library > > > > Just a note, Tegra Core won't meet prerequisite #3 since each regulator > > voltage values is associated with clocks driving those many sub-HW > > blocks in it. > > Perhaps that "just" means extending the dvfs.c code here to iterate over > each clock consumer (rather than each clock provider), and having each > set a minimum voltage (rather than a specific voltage), and having the > regulator core apply the maximum of those minimum constraints? > > Or something like that anyway. Thanks, I'll think about this or maybe study a bit, it sounds like we can leverage existing api in regulator framework (which I don't know) to do what you've proposed, please clarify if I misunderstand. > -- > 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/ -- 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 2/5] clk: notifier handler for dynamic voltage scaling
On Sat, 2013-03-02 at 04:48 +0800, Mike Turquette wrote:
> Quoting Mike Turquette (2013-03-01 10:22:34)
> > Quoting Bill Huang (2013-03-01 01:41:31)
> > > On Thu, 2013-02-28 at 12:49 +0800, Mike Turquette wrote:
> > > > Dynamic voltage and frequency scaling (dvfs) is a common power saving
> > > > technique in many of today's modern processors. This patch introduces a
> > > > common clk rate-change notifier handler which scales voltage
> > > > appropriately whenever clk_set_rate is called on an affected clock.
> > >
> > > I really think clk_enable and clk_disable should also be triggering
> > > notifier call and DVFS should act accordingly since there are cases
> > > drivers won't set clock rate but instead disable its clock directly, do
> > > you agree?
> > > >
> >
> > Hi Bill,
> >
> > I'll think about this. Perhaps a better solution would be to adapt
> > these drivers to runtime PM. Then a call to runtime_pm_put() would
> > result in a call to clk_disable(...) and regulator_set_voltage(...).
> >
> > There is no performance-based equivalent to runtime PM, which is one
> > reason why clk_set_rate is a likely entry point into dvfs. But for
> > operations that have nice api's like runtime PM it would be better to
> > use those interfaces and not overload the clk.h api unnecessarily.
> >
>
> Bill,
>
> I wasn't thinking at all when I wrote this. Trying to rush to the
> airport I guess...
>
> clk_enable() and clk_disable() must not sleep and all operations are
> done under a spinlock. So this rules out most use of notifiers. It is
> expected for some drivers to very aggressively enable/disable clocks in
> interrupt handlers so scaling voltage as a function of clk_{en|dis}able
> calls is also likely out of the question.
Yeah for those existing drivers to call enable/disable clocks in
interrupt have ruled out this, I didn't think through when I was asking.
>
> Some platforms have chosen to implement voltage scaling in their
> .prepare callbacks. I personally do not like this and still prefer
> drivers be adapted to runtime pm and let those callbacks handle voltage
> scaling along with clock handling.
I think different SoC have different mechanisms or constraints on doing
their DVFS, such as Tegra VDD_CORE rail, it supplies power to many
devices and as a consequence each device do not have their own power
rail to control, instead a central driver to handle/control this power
rail is needed (to set voltage at the maximum of the requested voltage
from all its sub-devices), so I'm wondering even if every drivers are
doing DVFS through runtime pm, we're still having hole on knowing
whether or not clocks of the interested devices are enabled/disabled at
runtime, I'm not familiar with runtime pm and hence do not know if there
is a mechanism to handle this, I'll study a bit. Thanks.
>
> Regards,
> Mike
>
> > > > There are three prerequisites to using this feature:
> > > >
> > > > 1) the affected clocks must be using the common clk framework
> > > > 2) voltage must be scaled using the regulator framework
> > > > 3) clock frequency and regulator voltage values must be paired via the
> > > > OPP library
> > >
> > > Just a note, Tegra Core won't meet prerequisite #3 since each regulator
> > > voltage values is associated with clocks driving those many sub-HW
> > > blocks in it.
> >
> > This patch isn't the one and only way to perform dvfs. It is just a
> > helper function for registering notifier handlers for systems that meet
> > the above three requirements. Even if you do not use the OPP library
> > there is no reason why you could not register your own rate-change
> > notifier handler to implement dvfs using whatever lookup-table you use
> > today.
> >
> > And patches are welcome to extend the usefulness of this helper. I'd
> > like as many people to benefit from this mechanism as possible.
The extension is not so easy for us though since OPP library is assuming
each device has a 1-1 mapping on its operating frequency and voltage.
> >
> > At some point we should think hard about DT bindings for these operating
> > points...
> >
> > Regards,
> > Mike
--
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 2/5] clk: notifier handler for dynamic voltage scaling
On Thu, 2013-02-28 at 12:49 +0800, Mike Turquette wrote: > Dynamic voltage and frequency scaling (dvfs) is a common power saving > technique in many of today's modern processors. This patch introduces a > common clk rate-change notifier handler which scales voltage > appropriately whenever clk_set_rate is called on an affected clock. I really think clk_enable and clk_disable should also be triggering notifier call and DVFS should act accordingly since there are cases drivers won't set clock rate but instead disable its clock directly, do you agree? > > There are three prerequisites to using this feature: > > 1) the affected clocks must be using the common clk framework > 2) voltage must be scaled using the regulator framework > 3) clock frequency and regulator voltage values must be paired via the > OPP library Just a note, Tegra Core won't meet prerequisite #3 since each regulator voltage values is associated with clocks driving those many sub-HW blocks in it. -- 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 v2 0/2] mfd: dt: add power off support for Tegra20/Tegra30
Hi all, Could somebody review this? Thanks, Bill > > This patch series add new property into regulator DT for telling whether or > not to hook pmic's power off routine to system call > "pm_power_off". > > Patch 1 add power off support for Tegra20 boards using TPS6586x Patch 2 add > power off support for Tegra30 boards using TPS65910 > > Verified on Seaboard (Tegra20) and Cardhu (Tegra30) > > V2: > * Take multiple pmic instances into consideration while assigning global > variables > as per suggestion from Thierry Reding > > V1: > * Based on master branch of sameo/mfd-2.6.git > > Bill Huang (2): > mfd: dt: tps6586x: Add power off control > mfd: dt: tps65910: add power off control > > Documentation/devicetree/bindings/mfd/tps65910.txt |4 +++ > .../devicetree/bindings/regulator/tps6586x.txt |6 + > drivers/mfd/tps6586x.c | 19 + > drivers/mfd/tps65910.c | 22 > > include/linux/mfd/tps6586x.h |1 + > include/linux/mfd/tps65910.h |3 ++ > 6 files changed, 55 insertions(+), 0 deletions(-) > > -- > 1.7.4.1 nvpublic -- 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 v2 1/2] mfd: dt: tps6586x: Add power off control
> nvpublic > > On 08/24/2012 06:36 PM, Bill Huang wrote: > > >>> On Sun, Aug 19, 2012 at 06:07:55PM -0700, Bill Huang wrote: > > >>>> Add DT property "ti,system-power-controller" telling whether or > > >>>> not this pmic is in charge of controlling the system power, so > > >>>> the power off routine can be hooked up to system call "pm_power_off". > > ... > > >>> I've seen the following while trying this patch applied on top of > > >>> next-20120817: > > >>> > > >>> [ 40.581151] Power down. > > >>> [ 41.583160] [ cut here ] > > >>> [ 41.587784] WARNING: at /home/thierry.reding/src/kernel/linux- > ipmp.git/drivers/i2c/busses/i2c- > > >>> tegra.c:525 tegra_i2c_xfer+0x21c/0x29c() > > ... > > >> Thanks Thierry, I can repro this on Tegra20 inconsistently and > > >> found, if current cpu is not cpu0 when doing "machine_shutdown" (it > > >> will call "smp_send_stop"), i2c controller will failed to do any > > >> transaction (looks like gic interrupt > > will be disabled), I'll debug further to find out the root cause. > > >> > > >> By the way, Tegra30 is good since it will always be cpu0 when doing > > >> "machine_shutdown", I still don't know why it makes the difference > > >> against Tegra20 since I'm not familiar with those cpu stuffs and > > >> what make it behave differently, > > I'll study a bit, thanks. > > > > > > I've sent the shutdown issue for discussion in ARM list: Shutdown > > > problem in SMP system happened on > > Tegra20. > > > The cause of the i2c timeout is pretty clear now and it is not > > > directly related to this patch, so is this patch series acceptable? Any > > > thoughts or comment? > Thanks. > > > > I tend to agree; power off never worked without this patch, and > > sometimes does with the patch, due to nothing wrong with this patch. > > > > Bill, please do follow up on getting the underlying Tegra issue solved > > somehow though. IIRC, Joseph Lo or Prashant has a patch which enabled > > the config option that Russell mentioned, so the fix may just be to wait > > for that patch to get > finalized, but please double-check that solves it. > > Thanks! > > As per the shutdown issue discussion, enabling CONFIG_PM_SLEEP_SMP is the > only solution and I've > confirmed that fix the issue, thanks. Hi sameo, Is there any concern on this patch series? Thanks, Bill -- 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: Shutdown problem in SMP system happened on Tegra20
> On Sat, Aug 25, 2012 at 08:00:56AM +0800, Bill Huang wrote: > > nvpublic > > > So what you're asking for is a feature to do what > > > CONFIG_PM_SLEEP_SMP does, but without CONFIG_PM_SLEEP_SMP enabled? > > > > Yeah pretty much, I'm actually asking should we take care of this > > since maybe not all platforms will have this config enabled? > > > > > > Why not just ensure that CONFIG_PM_SLEEP_SMP is enabled if your > > > platform requires that the lowest CPU number be the CPU dealing with > > > reboot? > > > > Someday we will have it enabled, but before that we'll hit the issue, > > so you don't think this should be taken care of? Thanks. > > Well, just enable CONFIG_PM_SLEEP_SMP then - it'll be far better than trying > to implement something in > the ARM code, especially as sysrq can be used for poweroff/reboot etc (which > will run from atomic > contexts where you can't switch to another CPU.) > > What you're asking for will make these callbacks fragile for other platforms. OK, thanks. -- 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 v2 1/2] mfd: dt: tps6586x: Add power off control
nvpublic > On 08/24/2012 06:36 PM, Bill Huang wrote: > >>> On Sun, Aug 19, 2012 at 06:07:55PM -0700, Bill Huang wrote: > >>>> Add DT property "ti,system-power-controller" telling whether or not > >>>> this pmic is in charge of controlling the system power, so the > >>>> power off routine can be hooked up to system call "pm_power_off". > ... > >>> I've seen the following while trying this patch applied on top of > >>> next-20120817: > >>> > >>> [ 40.581151] Power down. > >>> [ 41.583160] [ cut here ] > >>> [ 41.587784] WARNING: at > >>> /home/thierry.reding/src/kernel/linux-ipmp.git/drivers/i2c/busses/i2c- > >>> tegra.c:525 tegra_i2c_xfer+0x21c/0x29c() > ... > >> Thanks Thierry, I can repro this on Tegra20 inconsistently and found, > >> if current cpu is not cpu0 when doing "machine_shutdown" (it will > >> call "smp_send_stop"), i2c controller will failed to do any transaction > >> (looks like gic interrupt > will be disabled), I'll debug further to find out the root cause. > >> > >> By the way, Tegra30 is good since it will always be cpu0 when doing > >> "machine_shutdown", I still don't know why it makes the difference > >> against Tegra20 since I'm not familiar with those cpu stuffs and what make > >> it behave differently, > I'll study a bit, thanks. > > > > I've sent the shutdown issue for discussion in ARM list: Shutdown problem > > in SMP system happened on > Tegra20. > > The cause of the i2c timeout is pretty clear now and it is not > > directly related to this patch, so is this patch series acceptable? Any > > thoughts or comment? Thanks. > > I tend to agree; power off never worked without this patch, and sometimes > does with the patch, due to > nothing wrong with this patch. > > Bill, please do follow up on getting the underlying Tegra issue solved > somehow though. IIRC, Joseph Lo > or Prashant has a patch which enabled the config option that Russell > mentioned, so the fix may just be > to wait for that patch to get finalized, but please double-check that solves > it. > Thanks! As per the shutdown issue discussion, enabling CONFIG_PM_SLEEP_SMP is the only solution and I've confirmed that fix the issue, thanks. -- 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 v2 1/2] mfd: dt: tps6586x: Add power off control
nvpublic > > On Sun, Aug 19, 2012 at 06:07:55PM -0700, Bill Huang wrote: > > > Add DT property "ti,system-power-controller" telling whether or not > > > this pmic is in charge of controlling the system power, so the power > > > off routine can be hooked up to system call "pm_power_off". > > > > > > Based on the work by: > > > Dan Willemsen > > > > > > Signed-off-by: Bill Huang > > > Tested-by: Stephen Warren > > > --- > > > .../devicetree/bindings/regulator/tps6586x.txt |6 ++ > > > drivers/mfd/tps6586x.c | 19 > > > +++ > > > include/linux/mfd/tps6586x.h |1 + > > > 3 files changed, 26 insertions(+), 0 deletions(-) > > > > Hi, > > > > I've seen the following while trying this patch applied on top of > > next-20120817: > > > > [ 40.581151] Power down. > > [ 41.583160] [ cut here ] > > [ 41.587784] WARNING: at > > /home/thierry.reding/src/kernel/linux-ipmp.git/drivers/i2c/busses/i2c- > > tegra.c:525 tegra_i2c_xfer+0x21c/0x29c() > > [ 41.599850] Modules linked in: > > [ 41.602927] [] (unwind_backtrace+0x0/0xf8) from [] > > (warn_slowpath_common+0x4c/0x64) > > [ 41.612304] [] (warn_slowpath_common+0x4c/0x64) from > > [] > > (warn_slowpath_null+0x1c/0x24) > > [ 41.621947] [] (warn_slowpath_null+0x1c/0x24) from [] > > (tegra_i2c_xfer+0x21c/0x29c) > > [ 41.631244] [] (tegra_i2c_xfer+0x21c/0x29c) from [] > > (__i2c_transfer+0x44/0x80) > > [ 41.640192] [] (__i2c_transfer+0x44/0x80) from [] > > (i2c_transfer+0x7c/0xb8) > > [ 41.648796] [] (i2c_transfer+0x7c/0xb8) from [] > > (regmap_i2c_read+0x48/0x64) > > [ 41.657485] [] (regmap_i2c_read+0x48/0x64) from [] > (_regmap_raw_read+0x90/0x98) > > [ 41.666518] [] (_regmap_raw_read+0x90/0x98) from [] > > (_regmap_read+0x50/0xa8) > > [ 41.675290] [] (_regmap_read+0x50/0xa8) from [] > (_regmap_update_bits+0x24/0x64) > > [ 41.684322] [] (_regmap_update_bits+0x24/0x64) from > > [] > > (regmap_update_bits+0x3c/0x58) > > [ 41.693885] [] (regmap_update_bits+0x3c/0x58) from [] > > (tps6586x_power_off+0x18/0x38) > > [ 41.703362] [] (tps6586x_power_off+0x18/0x38) from [] > > (machine_power_off+0x1c/0x24) > > [ 41.712749] [] (machine_power_off+0x1c/0x24) from [] > > (sys_reboot+0x138/0x1b0) > > [ 41.721612] [] (sys_reboot+0x138/0x1b0) from [] > > (ret_fast_syscall+0x0/0x30) > > [ 41.730293] ---[ end trace 9af366974fefa459 ]--- > > [ 41.734906] tegra-i2c tegra-i2c.3: i2c transfer timed out > > [ 41.740689] Kernel panic - not syncing: Attempted to kill init! > > exitcode=0x > > [ 41.740689] > > [ 41.749823] [] (unwind_backtrace+0x0/0xf8) from [] > > (panic+0x8c/0x1d8) > > [ 41.757993] [] (panic+0x8c/0x1d8) from [] > > (do_exit+0x694/0x750) > > [ 41.765636] [] (do_exit+0x694/0x750) from [] > > (do_group_exit+0x3c/0xb0) > > [ 41.773884] [] (do_group_exit+0x3c/0xb0) from [] > > (__wake_up_parent+0x0/0x18) > > Thanks Thierry, I can repro this on Tegra20 inconsistently and found, if > current cpu is not cpu0 when > doing "machine_shutdown" (it will call "smp_send_stop"), i2c controller will > failed to do any > transaction (looks like gic interrupt will be disabled), I'll debug further > to find out the root cause. > > By the way, Tegra30 is good since it will always be cpu0 when doing > "machine_shutdown", I still don't > know why it makes the difference against Tegra20 since I'm not familiar with > those cpu stuffs and what > make it behave differently, I'll study a bit, thanks. > I've sent the shutdown issue for discussion in ARM list: Shutdown problem in SMP system happened on Tegra20. The cause of the i2c timeout is pretty clear now and it is not directly related to this patch, so is this patch series acceptable? Any thoughts or comment? Thanks. > > > > Thierry > > > > * Unknown Key > > * 0x7F3EB3A1 -- 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: Shutdown problem in SMP system happened on Tegra20
nvpublic
> On Fri, Aug 24, 2012 at 04:23:39PM +0800, Bill Huang wrote:
> > When doing shutdown on Tegra20/Tegra30, we need to read/write PMIC
> > registers through I2C to perform the power off sequence.
> > Unfortunately, sometimes we'll fail to shutdown due to I2C timeout on
> > Tegra20. And the cause of the timeout is due to the CPU which I2C
> > controller IRQ affined to will have chance to be offlined without
> > migrating all irqs affined to it, so the following I2C transactions
> > will fail (no any CPU will handle that interrupt since then).
>
> > Some snippet of the shutdown codes:
> >
> > void kernel_power_off(void)
> > {
> > kernel_shutdown_prepare(SYSTEM_POWER_OFF);
> > :
> > disable_nonboot_cpus();
> > :
> > machine_power_off();
> > }
> >
> > void machine_power_off(void)
> > {
> > machine_shutdown();
> > if (pm_power_off)
> > pm_power_off(); /* this is where we send I2C write to shutdown
> > */ }
> >
> > void machine_shutdown(void)
> > {
> > #ifdef CONFIG_SMP
> > smp_send_stop();
> > #endif
> > }
> >
> > In "smp_send_stop()", it will send "IPI_CPU_STOPS" to offline other
> > cpus except current cpu (smp_processor_id()), however, current cpu
> > will not always be cpu0 at least at Tegra20, that said for example
> > cpu1 might be the current cpu and cpu0 will be offlined and this is the
> > case why the I2C transaction
> will timeout.
> >
> > For normal case, "disable_nonboot_cpus()" call will disable all other
> > Cpus except cpu0, that means we won't hit the problem mentioned here
> > since cpu0 will always be the current cpu in the call "smp_send_stop", but
> > the call to
> "disable_nonboot_cpus"
> > will happen only when "CONFIG_PM_SLEEP_SMP" is enabled which is not
> > the case for Tegra20/Tegra30, we don't support suspend yet so this can't be
> > enabled.
>
> So what you're asking for is a feature to do what CONFIG_PM_SLEEP_SMP does,
> but without
> CONFIG_PM_SLEEP_SMP enabled?
Yeah pretty much, I'm actually asking should we take care of this since maybe
not all platforms
will have this config enabled?
>
> Why not just ensure that CONFIG_PM_SLEEP_SMP is enabled if your platform
> requires that the lowest CPU
> number be the CPU dealing with reboot?
Someday we will have it enabled, but before that we'll hit the issue, so you
don't think
this should be taken care of? Thanks.
--
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/
Shutdown problem in SMP system happened on Tegra20
Hi,
When doing shutdown on Tegra20/Tegra30, we need to read/write PMIC registers
through I2C
to perform the power off sequence. Unfortunately, sometimes we'll fail to
shutdown
due to I2C timeout on Tegra20. And the cause of the timeout is due to the CPU
which I2C
controller IRQ affined to will have chance to be offlined without migrating all
irqs affined
to it, so the following I2C transactions will fail (no any CPU will handle that
interrupt
since then).
Some snippet of the shutdown codes:
void kernel_power_off(void)
{
kernel_shutdown_prepare(SYSTEM_POWER_OFF);
:
disable_nonboot_cpus();
:
machine_power_off();
}
void machine_power_off(void)
{
machine_shutdown();
if (pm_power_off)
pm_power_off(); /* this is where we send I2C write to shutdown
*/
}
void machine_shutdown(void)
{
#ifdef CONFIG_SMP
smp_send_stop();
#endif
}
In "smp_send_stop()", it will send "IPI_CPU_STOPS" to offline other cpus except
current cpu (smp_processor_id()), however, current cpu will not always be cpu0
at
least at Tegra20, that said for example cpu1 might be the current cpu and cpu0
will
be offlined and this is the case why the I2C transaction will timeout.
For normal case, "disable_nonboot_cpus()" call will disable all other Cpus
except
cpu0, that means we won't hit the problem mentioned here since cpu0 will always
be
the current cpu in the call "smp_send_stop", but the call to
"disable_nonboot_cpus"
will happen only when "CONFIG_PM_SLEEP_SMP" is enabled which is not the case for
Tegra20/Tegra30, we don't support suspend yet so this can't be enabled.
There are two known fix for this, the first one is enable suspend
(ARCH_SUSPEND_POSSIBLE)
so the cpu0 will be the only online cpu while doing "machine_shutdown". The
second
fix is adding call to "migrate_irqs()" in "ipi_cpu_stop" so all irqs can be
migrated to
the active cpu.
Could someone familiar with the ARM SMP design help to answer my two questions?
1. Does it make sense that "smp_processor_id()" could be non-cpu0 in the call
"smp_send_stop()"? In Tegra30 it will always be cpu0 but Tegra20 will be
50-50,
I just can't find the magic.
2. If current cpu is not necessarily be cpu0 in the call "smp_send_stop()", then
does it make sense to add "migrate_irqs()" in "ipi_cpu_stop()"? Or is there
any
other fix which makes more sense?
Thanks,
Bill
nvpublic
--
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 v2 1/2] mfd: dt: tps6586x: Add power off control
nvpublic > On Sun, Aug 19, 2012 at 06:07:55PM -0700, Bill Huang wrote: > > Add DT property "ti,system-power-controller" telling whether or not > > this pmic is in charge of controlling the system power, so the power > > off routine can be hooked up to system call "pm_power_off". > > > > Based on the work by: > > Dan Willemsen > > > > Signed-off-by: Bill Huang > > Tested-by: Stephen Warren > > --- > > .../devicetree/bindings/regulator/tps6586x.txt |6 ++ > > drivers/mfd/tps6586x.c | 19 > > +++ > > include/linux/mfd/tps6586x.h |1 + > > 3 files changed, 26 insertions(+), 0 deletions(-) > > Hi, > > I've seen the following while trying this patch applied on top of > next-20120817: > > [ 40.581151] Power down. > [ 41.583160] [ cut here ] > [ 41.587784] WARNING: at > /home/thierry.reding/src/kernel/linux-ipmp.git/drivers/i2c/busses/i2c- > tegra.c:525 tegra_i2c_xfer+0x21c/0x29c() > [ 41.599850] Modules linked in: > [ 41.602927] [] (unwind_backtrace+0x0/0xf8) from [] > (warn_slowpath_common+0x4c/0x64) > [ 41.612304] [] (warn_slowpath_common+0x4c/0x64) from [] > (warn_slowpath_null+0x1c/0x24) > [ 41.621947] [] (warn_slowpath_null+0x1c/0x24) from [] > (tegra_i2c_xfer+0x21c/0x29c) > [ 41.631244] [] (tegra_i2c_xfer+0x21c/0x29c) from [] > (__i2c_transfer+0x44/0x80) > [ 41.640192] [] (__i2c_transfer+0x44/0x80) from [] > (i2c_transfer+0x7c/0xb8) > [ 41.648796] [] (i2c_transfer+0x7c/0xb8) from [] > (regmap_i2c_read+0x48/0x64) > [ 41.657485] [] (regmap_i2c_read+0x48/0x64) from [] > (_regmap_raw_read+0x90/0x98) > [ 41.666518] [] (_regmap_raw_read+0x90/0x98) from [] > (_regmap_read+0x50/0xa8) > [ 41.675290] [] (_regmap_read+0x50/0xa8) from [] > (_regmap_update_bits+0x24/0x64) > [ 41.684322] [] (_regmap_update_bits+0x24/0x64) from [] > (regmap_update_bits+0x3c/0x58) > [ 41.693885] [] (regmap_update_bits+0x3c/0x58) from [] > (tps6586x_power_off+0x18/0x38) > [ 41.703362] [] (tps6586x_power_off+0x18/0x38) from [] > (machine_power_off+0x1c/0x24) > [ 41.712749] [] (machine_power_off+0x1c/0x24) from [] > (sys_reboot+0x138/0x1b0) > [ 41.721612] [] (sys_reboot+0x138/0x1b0) from [] > (ret_fast_syscall+0x0/0x30) > [ 41.730293] ---[ end trace 9af366974fefa459 ]--- > [ 41.734906] tegra-i2c tegra-i2c.3: i2c transfer timed out > [ 41.740689] Kernel panic - not syncing: Attempted to kill init! > exitcode=0x > [ 41.740689] > [ 41.749823] [] (unwind_backtrace+0x0/0xf8) from [] > (panic+0x8c/0x1d8) > [ 41.757993] [] (panic+0x8c/0x1d8) from [] > (do_exit+0x694/0x750) > [ 41.765636] [] (do_exit+0x694/0x750) from [] > (do_group_exit+0x3c/0xb0) > [ 41.773884] [] (do_group_exit+0x3c/0xb0) from [] > (__wake_up_parent+0x0/0x18) Thanks Thierry, I can repro this on Tegra20 inconsistently and found, if current cpu is not cpu0 when doing "machine_shutdown" (it will call "smp_send_stop"), i2c controller will failed to do any transaction (looks like gic interrupt will be disabled), I'll debug further to find out the root cause. By the way, Tegra30 is good since it will always be cpu0 when doing "machine_shutdown", I still don't know why it makes the difference against Tegra20 since I'm not familiar with those cpu stuffs and what make it behave differently, I'll study a bit, thanks. > > Thierry > > * Unknown Key > * 0x7F3EB3A1 -- 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/
[PATCH v2 2/2] mfd: dt: tps65910: add power off control
Add DT property "ti,system-power-controller" telling whether or not this
pmic is in charge of controlling the system power, so the power off
routine can be hooked up to system call "pm_power_off".
Based on the work by:
Dan Willemsen
Signed-off-by: Bill Huang
Tested-by: Stephen Warren
---
Documentation/devicetree/bindings/mfd/tps65910.txt |4 +++
drivers/mfd/tps65910.c | 22
include/linux/mfd/tps65910.h |3 ++
3 files changed, 29 insertions(+), 0 deletions(-)
diff --git a/Documentation/devicetree/bindings/mfd/tps65910.txt
b/Documentation/devicetree/bindings/mfd/tps65910.txt
index db03599..2e33048 100644
--- a/Documentation/devicetree/bindings/mfd/tps65910.txt
+++ b/Documentation/devicetree/bindings/mfd/tps65910.txt
@@ -59,6 +59,8 @@ Optional properties:
in TPS6591X datasheet)
- ti,en-gpio-sleep: enable sleep control for gpios
There should be 9 entries here, one for each gpio.
+- ti,system-power-controller: Telling whether or not this pmic is controlling
+ the system power.
Regulator Optional properties:
- ti,regulator-ext-sleep-control: enable external sleep
@@ -79,6 +81,8 @@ Example:
#interrupt-cells = <2>;
interrupt-controller;
+ ti,system-power-controller;
+
ti,vmbch-threshold = 0;
ti,vmbch2-threshold = 0;
ti,en-ck32k-xtal;
diff --git a/drivers/mfd/tps65910.c b/drivers/mfd/tps65910.c
index 1c56379..574ae2b 100644
--- a/drivers/mfd/tps65910.c
+++ b/drivers/mfd/tps65910.c
@@ -198,6 +198,8 @@ static struct tps65910_board *tps65910_parse_dt(struct
i2c_client *client,
board_info->irq = client->irq;
board_info->irq_base = -1;
+ board_info->pm_off = of_property_read_bool(np,
+ "ti,system-power-controller");
return board_info;
}
@@ -210,6 +212,21 @@ struct tps65910_board *tps65910_parse_dt(struct i2c_client
*client,
}
#endif
+static struct i2c_client *tps65910_i2c_client;
+static void tps65910_power_off(void)
+{
+ struct tps65910 *tps65910;
+
+ tps65910 = dev_get_drvdata(&tps65910_i2c_client->dev);
+
+ if (tps65910_reg_set_bits(tps65910, TPS65910_DEVCTRL,
+ DEVCTRL_PWR_OFF_MASK) < 0)
+ return;
+
+ tps65910_reg_clear_bits(tps65910, TPS65910_DEVCTRL,
+ DEVCTRL_DEV_ON_MASK);
+}
+
static __devinit int tps65910_i2c_probe(struct i2c_client *i2c,
const struct i2c_device_id *id)
{
@@ -267,6 +284,11 @@ static __devinit int tps65910_i2c_probe(struct i2c_client
*i2c,
tps65910_ck32k_init(tps65910, pmic_plat_data);
tps65910_sleepinit(tps65910, pmic_plat_data);
+ if (pmic_plat_data->pm_off && !pm_power_off) {
+ tps65910_i2c_client = i2c;
+ pm_power_off = tps65910_power_off;
+ }
+
return ret;
}
diff --git a/include/linux/mfd/tps65910.h b/include/linux/mfd/tps65910.h
index 9bf8767..ac772b3 100644
--- a/include/linux/mfd/tps65910.h
+++ b/include/linux/mfd/tps65910.h
@@ -366,6 +366,8 @@
/*Register DEVCTRL (0x80) register.RegisterDescription */
+#define DEVCTRL_PWR_OFF_MASK 0x80
+#define DEVCTRL_PWR_OFF_SHIFT 7
#define DEVCTRL_RTC_PWDN_MASK 0x40
#define DEVCTRL_RTC_PWDN_SHIFT 6
#define DEVCTRL_CK32K_CTRL_MASK0x20
@@ -809,6 +811,7 @@ struct tps65910_board {
int vmbch2_threshold;
bool en_ck32k_xtal;
bool en_dev_slp;
+ bool pm_off;
struct tps65910_sleep_keepon_data *slp_keepon;
bool en_gpio_sleep[TPS6591X_MAX_NUM_GPIO];
unsigned long regulator_ext_sleep_control[TPS65910_NUM_REGS];
--
1.7.4.1
--
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/
[PATCH v2 1/2] mfd: dt: tps6586x: Add power off control
Add DT property "ti,system-power-controller" telling whether or not this
pmic is in charge of controlling the system power, so the power off
routine can be hooked up to system call "pm_power_off".
Based on the work by:
Dan Willemsen
Signed-off-by: Bill Huang
Tested-by: Stephen Warren
---
.../devicetree/bindings/regulator/tps6586x.txt |6 ++
drivers/mfd/tps6586x.c | 19 +++
include/linux/mfd/tps6586x.h |1 +
3 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/Documentation/devicetree/bindings/regulator/tps6586x.txt
b/Documentation/devicetree/bindings/regulator/tps6586x.txt
index d156e1b..03dfa4e 100644
--- a/Documentation/devicetree/bindings/regulator/tps6586x.txt
+++ b/Documentation/devicetree/bindings/regulator/tps6586x.txt
@@ -18,6 +18,10 @@ Required properties:
- vinldo678-supply: The input supply for the LDO6, LDO7 and LDO8
- vinldo9-supply: The input supply for the LDO9
+Optional properties:
+- ti,system-power-controller: Telling whether or not this pmic is controlling
+ the system power.
+
Each regulator is defined using the standard binding for regulators.
Example:
@@ -30,6 +34,8 @@ Example:
#gpio-cells = <2>;
gpio-controller;
+ ti,system-power-controller;
+
sm0-supply = <&some_reg>;
sm1-supply = <&some_reg>;
sm2-supply = <&some_reg>;
diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c
index 353c348..d08f59c 100644
--- a/drivers/mfd/tps6586x.c
+++ b/drivers/mfd/tps6586x.c
@@ -29,6 +29,10 @@
#include
#include
+#define TPS6586X_SUPPLYENE 0x14
+#define EXITSLREQ_BIT BIT(1)
+#define SLEEP_MODE_BIT BIT(3)
+
/* interrupt control registers */
#define TPS6586X_INT_ACK1 0xb5
#define TPS6586X_INT_ACK2 0xb6
@@ -409,6 +413,7 @@ static struct tps6586x_platform_data
*tps6586x_parse_dt(struct i2c_client *clien
pdata->subdevs = devs;
pdata->gpio_base = -1;
pdata->irq_base = -1;
+ pdata->pm_off = of_property_read_bool(np, "ti,system-power-controller");
return pdata;
}
@@ -441,6 +446,15 @@ static const struct regmap_config tps6586x_regmap_config =
{
.cache_type = REGCACHE_RBTREE,
};
+static struct device *tps6586x_dev;
+static void tps6586x_power_off(void)
+{
+ if (tps6586x_clr_bits(tps6586x_dev, TPS6586X_SUPPLYENE, EXITSLREQ_BIT))
+ return;
+
+ tps6586x_set_bits(tps6586x_dev, TPS6586X_SUPPLYENE, SLEEP_MODE_BIT);
+}
+
static int __devinit tps6586x_i2c_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -505,6 +519,11 @@ static int __devinit tps6586x_i2c_probe(struct i2c_client
*client,
goto err_add_devs;
}
+ if (pdata->pm_off && !pm_power_off) {
+ tps6586x_dev = &client->dev;
+ pm_power_off = tps6586x_power_off;
+ }
+
return 0;
err_add_devs:
diff --git a/include/linux/mfd/tps6586x.h b/include/linux/mfd/tps6586x.h
index f350fd0..08f109f 100644
--- a/include/linux/mfd/tps6586x.h
+++ b/include/linux/mfd/tps6586x.h
@@ -77,6 +77,7 @@ struct tps6586x_platform_data {
int gpio_base;
int irq_base;
+ bool pm_off;
};
/*
--
1.7.4.1
--
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/
[PATCH v2 0/2] mfd: dt: add power off support for Tegra20/Tegra30
This patch series add new property into regulator DT for telling whether or not to hook pmic's power off routine to system call "pm_power_off". Patch 1 add power off support for Tegra20 boards using TPS6586x Patch 2 add power off support for Tegra30 boards using TPS65910 Verified on Seaboard (Tegra20) and Cardhu (Tegra30) V2: * Take multiple pmic instances into consideration while assigning global variables as per suggestion from Thierry Reding V1: * Based on master branch of sameo/mfd-2.6.git Bill Huang (2): mfd: dt: tps6586x: Add power off control mfd: dt: tps65910: add power off control Documentation/devicetree/bindings/mfd/tps65910.txt |4 +++ .../devicetree/bindings/regulator/tps6586x.txt |6 + drivers/mfd/tps6586x.c | 19 + drivers/mfd/tps65910.c | 22 include/linux/mfd/tps6586x.h |1 + include/linux/mfd/tps65910.h |3 ++ 6 files changed, 55 insertions(+), 0 deletions(-) -- 1.7.4.1 -- 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 1/2] mfd: dt: tps6586x: Add power off control
nvpublic > On Fri, Aug 17, 2012 at 02:16:28AM -0700, Bill Huang wrote: > [...] > > diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c > [...] > > @@ -505,6 +519,11 @@ static int __devinit tps6586x_i2c_probe(struct > > i2c_client *client, > > goto err_add_devs; > > } > > > > + tps6586x_dev = &client->dev; > > + > > + if (pdata->pm_off && !pm_power_off) > > + pm_power_off = tps6586x_power_off; > > + > > I think the assignment of tps6586x_dev needs to go inside the if block as > well. Otherwise it might be > overwritten by another instance for systems that actually have more than one > tps6586x. Since currently > the driver can't be built as a module it probably makes little sense to clean > this up in .remove(), > but it might still be worth adding so it isn't forgotten if and when somebody > tries to convert the > driver to a module. > Thanks, good point. > I should note that I don't like very much how the pm_power_off works. > Maybe this should really be changed to allow passing a context for the > function to work from. > Something like: > > pm_power_off = tps6586x_power_off; > pm_power_off_data = &client->dev; > > Where pm_power_off() would receive pm_power_off_data as an argument. > > Even that's not very pretty. On the other hand this doesn't really buy us > much because only the > storage location of the variable would change and nothing else. But it would > still make the > association of the data clearer. > > Thierry > > * Unknown Key > * 0x7F3EB3A1 -- 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/
[PATCH 1/2] mfd: dt: tps6586x: Add power off control
Add DT property "ti,system-power-controller" telling whether or not this
pmic is in charge of controlling the system power, so the power off
routine can be hooked up to system call "pm_power_off".
Based on the work by:
Dan Willemsen
Signed-off-by: Bill Huang
---
.../devicetree/bindings/regulator/tps6586x.txt |6 ++
drivers/mfd/tps6586x.c | 19 +++
include/linux/mfd/tps6586x.h |1 +
3 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/Documentation/devicetree/bindings/regulator/tps6586x.txt
b/Documentation/devicetree/bindings/regulator/tps6586x.txt
index d156e1b..03dfa4e 100644
--- a/Documentation/devicetree/bindings/regulator/tps6586x.txt
+++ b/Documentation/devicetree/bindings/regulator/tps6586x.txt
@@ -18,6 +18,10 @@ Required properties:
- vinldo678-supply: The input supply for the LDO6, LDO7 and LDO8
- vinldo9-supply: The input supply for the LDO9
+Optional properties:
+- ti,system-power-controller: Telling whether or not this pmic is controlling
+ the system power.
+
Each regulator is defined using the standard binding for regulators.
Example:
@@ -30,6 +34,8 @@ Example:
#gpio-cells = <2>;
gpio-controller;
+ ti,system-power-controller;
+
sm0-supply = <&some_reg>;
sm1-supply = <&some_reg>;
sm2-supply = <&some_reg>;
diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c
index 353c348..93d57df 100644
--- a/drivers/mfd/tps6586x.c
+++ b/drivers/mfd/tps6586x.c
@@ -29,6 +29,10 @@
#include
#include
+#define TPS6586X_SUPPLYENE 0x14
+#define EXITSLREQ_BIT BIT(1)
+#define SLEEP_MODE_BIT BIT(3)
+
/* interrupt control registers */
#define TPS6586X_INT_ACK1 0xb5
#define TPS6586X_INT_ACK2 0xb6
@@ -409,6 +413,7 @@ static struct tps6586x_platform_data
*tps6586x_parse_dt(struct i2c_client *clien
pdata->subdevs = devs;
pdata->gpio_base = -1;
pdata->irq_base = -1;
+ pdata->pm_off = of_property_read_bool(np, "ti,system-power-controller");
return pdata;
}
@@ -441,6 +446,15 @@ static const struct regmap_config tps6586x_regmap_config =
{
.cache_type = REGCACHE_RBTREE,
};
+static struct device *tps6586x_dev;
+static void tps6586x_power_off(void)
+{
+ if (tps6586x_clr_bits(tps6586x_dev, TPS6586X_SUPPLYENE, EXITSLREQ_BIT))
+ return;
+
+ tps6586x_set_bits(tps6586x_dev, TPS6586X_SUPPLYENE, SLEEP_MODE_BIT);
+}
+
static int __devinit tps6586x_i2c_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -505,6 +519,11 @@ static int __devinit tps6586x_i2c_probe(struct i2c_client
*client,
goto err_add_devs;
}
+ tps6586x_dev = &client->dev;
+
+ if (pdata->pm_off && !pm_power_off)
+ pm_power_off = tps6586x_power_off;
+
return 0;
err_add_devs:
diff --git a/include/linux/mfd/tps6586x.h b/include/linux/mfd/tps6586x.h
index f350fd0..08f109f 100644
--- a/include/linux/mfd/tps6586x.h
+++ b/include/linux/mfd/tps6586x.h
@@ -77,6 +77,7 @@ struct tps6586x_platform_data {
int gpio_base;
int irq_base;
+ bool pm_off;
};
/*
--
1.7.4.1
--
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/
[PATCH 2/2] mfd: dt: tps65910: add power off control
Add DT property "ti,system-power-controller" telling whether or not this
pmic is in charge of controlling the system power, so the power off
routine can be hooked up to system call "pm_power_off".
Based on the work by:
Dan Willemsen
Signed-off-by: Bill Huang
---
Documentation/devicetree/bindings/mfd/tps65910.txt |4 +++
drivers/mfd/tps65910.c | 22
include/linux/mfd/tps65910.h |3 ++
3 files changed, 29 insertions(+), 0 deletions(-)
diff --git a/Documentation/devicetree/bindings/mfd/tps65910.txt
b/Documentation/devicetree/bindings/mfd/tps65910.txt
index db03599..2e33048 100644
--- a/Documentation/devicetree/bindings/mfd/tps65910.txt
+++ b/Documentation/devicetree/bindings/mfd/tps65910.txt
@@ -59,6 +59,8 @@ Optional properties:
in TPS6591X datasheet)
- ti,en-gpio-sleep: enable sleep control for gpios
There should be 9 entries here, one for each gpio.
+- ti,system-power-controller: Telling whether or not this pmic is controlling
+ the system power.
Regulator Optional properties:
- ti,regulator-ext-sleep-control: enable external sleep
@@ -79,6 +81,8 @@ Example:
#interrupt-cells = <2>;
interrupt-controller;
+ ti,system-power-controller;
+
ti,vmbch-threshold = 0;
ti,vmbch2-threshold = 0;
ti,en-ck32k-xtal;
diff --git a/drivers/mfd/tps65910.c b/drivers/mfd/tps65910.c
index 1c56379..a7925a9 100644
--- a/drivers/mfd/tps65910.c
+++ b/drivers/mfd/tps65910.c
@@ -198,6 +198,8 @@ static struct tps65910_board *tps65910_parse_dt(struct
i2c_client *client,
board_info->irq = client->irq;
board_info->irq_base = -1;
+ board_info->pm_off = of_property_read_bool(np,
+ "ti,system-power-controller");
return board_info;
}
@@ -210,6 +212,21 @@ struct tps65910_board *tps65910_parse_dt(struct i2c_client
*client,
}
#endif
+static struct i2c_client *tps65910_i2c_client;
+static void tps65910_power_off(void)
+{
+ struct tps65910 *tps65910;
+
+ tps65910 = dev_get_drvdata(&tps65910_i2c_client->dev);
+
+ if (tps65910_reg_set_bits(tps65910, TPS65910_DEVCTRL,
+ DEVCTRL_PWR_OFF_MASK) < 0)
+ return;
+
+ tps65910_reg_clear_bits(tps65910, TPS65910_DEVCTRL,
+ DEVCTRL_DEV_ON_MASK);
+}
+
static __devinit int tps65910_i2c_probe(struct i2c_client *i2c,
const struct i2c_device_id *id)
{
@@ -267,6 +284,11 @@ static __devinit int tps65910_i2c_probe(struct i2c_client
*i2c,
tps65910_ck32k_init(tps65910, pmic_plat_data);
tps65910_sleepinit(tps65910, pmic_plat_data);
+ tps65910_i2c_client = i2c;
+
+ if (pmic_plat_data->pm_off && !pm_power_off)
+ pm_power_off = tps65910_power_off;
+
return ret;
}
diff --git a/include/linux/mfd/tps65910.h b/include/linux/mfd/tps65910.h
index 9bf8767..ac772b3 100644
--- a/include/linux/mfd/tps65910.h
+++ b/include/linux/mfd/tps65910.h
@@ -366,6 +366,8 @@
/*Register DEVCTRL (0x80) register.RegisterDescription */
+#define DEVCTRL_PWR_OFF_MASK 0x80
+#define DEVCTRL_PWR_OFF_SHIFT 7
#define DEVCTRL_RTC_PWDN_MASK 0x40
#define DEVCTRL_RTC_PWDN_SHIFT 6
#define DEVCTRL_CK32K_CTRL_MASK0x20
@@ -809,6 +811,7 @@ struct tps65910_board {
int vmbch2_threshold;
bool en_ck32k_xtal;
bool en_dev_slp;
+ bool pm_off;
struct tps65910_sleep_keepon_data *slp_keepon;
bool en_gpio_sleep[TPS6591X_MAX_NUM_GPIO];
unsigned long regulator_ext_sleep_control[TPS65910_NUM_REGS];
--
1.7.4.1
--
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/
[PATCH 0/2] mfd: dt: add power off support for Tegra20/Tegra30
This patch series add new property into regulator DT for telling whether or not to hook pmic's power off routine to system call "pm_power_off". Patch 1 add power off support for Tegra20 boards using TPS6586x Patch 2 add power off support for Tegra30 boards using TPS65910 Verified on Seaboard (Tegra20) and Cardhu (Tegra30) Bill Huang (2): mfd: dt: tps6586x: Add power off control mfd: dt: tps65910: add power off control Documentation/devicetree/bindings/mfd/tps65910.txt |4 +++ .../devicetree/bindings/regulator/tps6586x.txt |6 + drivers/mfd/tps6586x.c | 19 + drivers/mfd/tps65910.c | 22 include/linux/mfd/tps6586x.h |1 + include/linux/mfd/tps65910.h |3 ++ 6 files changed, 55 insertions(+), 0 deletions(-) -- 1.7.4.1 -- 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/
