Re: [RFC PATCH v2 0/6] ARM: OMAP3+: Introduce ABB driver
On 04/16/2013 10:18 PM, Kevin Hilman wrote:
Andrii Tseglytskyi andrii.tseglyts...@ti.com writes:
Hi Kevin,
On 04/16/2013 12:53 AM, Kevin Hilman wrote:
Andrii Tseglytskyi andrii.tseglyts...@ti.com writes:
From: Andrii.Tseglytskyi andrii.tseglyts...@ti.com
Following patch series introduces the Adaptive Body-Bias
LDO driver, which handles LDOs voltage during OPP change routine.
Current implementation is based on patch series from
Mike Turquette:
http://marc.info/?l=linux-omapm=134931341818379w=2
ABB transition is a part of OPP changing sequence.
ABB can operate in the following modes:
- Bypass mode: Activated when ABB is not required
- FBB mode: Fast Body Bias mode, used on fast OPPs
Fast? I thought the 'F' was for Forward?
You are right. Should be 'Forward' here.
- RBB mode: Reverse Body Bias mode, used on slow OPPs
In current implementation ABB is converted to regulator.
Standalone OPP table is used to store ABB mode, it is defined
in device tree for each ABB regulator. It has the following format:
operating-points =
/* uV ABB (0 - Bypass, 1 - FBB, 2 - RBB) */
88 0
106 1
125 1
126 1
;
ABB regulator is linked to regulator chain
In addition to Mike's comments (which I completely agree with), it would
be very helfpul to see how this is actually used. e.g, how the
regulators are chained together, how the proper ordering is managed,
etc. etc.
We would like to handle voltage scaling in the following way:
What I meant is that a detailed description of the use case should be
included in the changelog.
cpufreq_cpu0
clk_set_rate(cpu0)
|
|--set_voltage(ABB regulator) /* all ABB related stuff will be
handled here */
|
|--set_voltage(smps123 regulator) /* actual voltage
scaling */
-EASCII_ART_WRAP
This simple model will be extended to handle AVS as a part of the chain.
smps123 regulator may be changed to VP/VC regulator.
Following example is from integration branch, which already has
smps123 regulator.
I don't know what integration branch you're referring to, and I don't
know what the smps123 regulator is.
It demonstrates an example of linkage to chain. ABB regulator is
linked with smps123 and cpu0 inside device tree.
cpu0 calls set_voltage() function for ABB, and then ABB calls
set_voltage() function for smps123 to do actual voltage scaling.
diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index bb5ee70..c8cbbee 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -36,7 +36,7 @@
cpus {
cpu@0 {
compatible = arm,cortex-a15;
- cpu0-supply = smps123_reg;
+ cpu0-supply = abb_mpu;
operating-points =
/* kHzuV */
/* Only for Nominal Samples */
@@ -94,6 +94,7 @@
reg = 0x4ae07cdc 0x8,
0x4ae06014 0x4;
ti,tranxdone_status_mask = 0x80;
+ avs-supply = smps123_reg;
operating-points =
/* uV ABB */
88 0
This RFC patch series is verified together with:
https://patchwork.kernel.org/patch/2445091/
Kevin, what do you think about this model in general? Does it fit to
regulator framework?
I don't know yet, because I don't think the use case has been described
well enough for me to fully understand it the motiviation behind the
series.
In addition, there are alternative approaches that seem to have been
ruled out without describing why. For example, the regulator framework
already allows you to override methods with custom hooks (as we do for
VC/VP controlled regulators already.) Without thinking about it too
deeply, it seems this approach could be used to manage the chain of
events you need as well. I can imagine there are limitations to this
approach for what you're trying to do, but I don't feel they have been
described in the changelog as part of the motivation for this series.
So for now, the guidance I have is this:
First, write changelogs (and cover letters) assuming your audience has
not been staring at the code as long as you have. Even if they have
been staring at the same code, assume they've been staring at mainline,
and not a random integration branch somewhere. My general advice is to
write changelogs in a way that you will understand what you wrote a year
from now after having forgotten all the details currently in your brains
cache. Even better, write them so that I will understand them in a year
since I forget much better than I remember.
Second, before inventing something new, start with the existing
framework. When the existing framework doesn't work, make an argument
for your new approach or extentions to the framework
Re: [RFC PATCH v2 0/6] ARM: OMAP3+: Introduce ABB driver
On 04/16/2013 10:07 PM, Mike Turquette wrote: Quoting Andrii Tseglytskyi (2013-04-16 05:40:44) On 04/16/2013 12:53 AM, Kevin Hilman wrote: In addition to Mike's comments (which I completely agree with), it would be very helfpul to see how this is actually used. e.g, how the regulators are chained together, how the proper ordering is managed, etc. etc. We would like to handle voltage scaling in the following way: I expanded the example below to include the SR AVS regulator. cpufreq_cpu0 clk_set_rate(cpu0) | |--set_voltage(ABB regulator) | |--set_voltage(AVS) | |--set_voltage(smps123 regulator) Hi Andrii, Why was regulator chaining chosen over a simple sequence of calls to regulator_set_voltage? Instead of nested calls into the regulator framework, why don't you just make the calls serially? E.g: regulator_set_voltage(abb_reg, foo_volt); regulator_set_voltage(avs_reg, bar_volt); regulator_set_voltage(smps123_reg, baz_volt); It is still to be determined where these calls originate from; maybe from clock notifiers, maybe directly from the cpufreq driver's .target() callback, or maybe somewhere else. Regardless, I do not see why regulator chaining is truly necessary here. You are just calling regulator_set_voltage in sequence on a few regulators, right? I think it would help me a lot to understand why regulator chaining is a requirement for this to work properly. Thanks, Mike Hi Mike, Kevin I'd like to provide some explanation regarding proposed TI DVFS design which we would like to follow (regulator chaining is part of it). The following two patch series was intended to prove the possibility of its implementation: - this one 'ARM: OMAP3+: Introduce ABB driver - and http://www.spinics.net/lists/linux-omap/msg90022.html - [RFC v1 0/1] introduce regulator chain locking scheme but, seems, we started to make it public from tail instead of 'head - sorry for that. While trying to move forward with TI DVFS we've taken into account following major points: - only DT should be used to configure DVFS; - no xxx_initcall, cpu_is_xx() function should be used; - DVFS should be scalable to fit wide SoC/platform’s and multiplatform kernel requirements; - minimize creation of TI specific API; Now, there are two entry points for DVFS in kernel: - CPUFreq - currently it's been decided to use cpufreq-cpu0 for all OMAPs in Main line; - CCF callbacks - have RFC DVFS implementation introduced here http://lwn.net/Articles/540422/. In both cases, the only one regulator need to be provided for CCF or CPUFreq for voltage changing proposes, so DVFS can done in the following way: || || --| RegulatorY |--| DVFS | || || \ \ \ \_ \\ |---| |---| |-| --| RegulatorX (PMIC) |--| Regulator AVS |--| ABB LDO |-- |---| |---| |-| /|\ | |__| Voltage adjustment 1) The following use-cases have been taken into account: - SoC/Platform don't need ABB/AVS (supports MPU OPPLOW/OPPNOM for example) - any regulator (VC-VP/I2C/SPI/GPIO ..) may be connected to DVFS - SoC/Platform need ABB - build chain in DT device-CCF-abb_regulator-any regulator(VC-VP/I2C/SPI..) - SoC/Platform need ABB+AVS - build chain in DT device-CCF-abb_regulator-avsX-any regulator(VC-VP/I2C..) 2) Implementation of each part of Voltage scale chain as Regulator will allow: - add each item one by one; - don't expose too much of custom TI API; - handle multi-voltage scaling requests to one rail (ganged rails) automatically (handled by regulator FW already). 3) The regulator chaining will allow: - easily configure DVFS form DT depending on current SoC/platform needs (using xxx-supply standard binding in DT); - continue use cpufreq-cpu0 for all OMAP to scale MPU domain; - use RFC DVFS implementation from http://lwn.net/Articles/540422/ for other domains (with some modifications - the most difficult thing will be multi-freq requests handling to one clock). In case, if regulator chaining approach is not accepted: - yes, it's possible to create some Super TI regulator which will handle ABB+AVS+VC/VP for most of current TI SoCs. - No, for the newest TI SoCs (like DRA7xxx without VC/VP) any regulator can be used as power supply (I2C/SPI/GPIO) and ABB is needed. a) As result, it will be impossible to use cpufreq-cpu0 driver for it, at least, and will need to drop cpufreq-cpu0 support for OMAPs and roll-back to omap-cpufreq. b) for other domains it's possible to create omap_dvfs.c in the similar way as it done in dvfs.c and hack it to handle ABB+AVS+I2C regulator. - will need to add custom TI
Re: [RFC PATCH v2 0/6] ARM: OMAP3+: Introduce ABB driver
Hi Kevin,
On 04/16/2013 12:53 AM, Kevin Hilman wrote:
Andrii Tseglytskyi andrii.tseglyts...@ti.com writes:
From: Andrii.Tseglytskyi andrii.tseglyts...@ti.com
Following patch series introduces the Adaptive Body-Bias
LDO driver, which handles LDOs voltage during OPP change routine.
Current implementation is based on patch series from
Mike Turquette:
http://marc.info/?l=linux-omapm=134931341818379w=2
ABB transition is a part of OPP changing sequence.
ABB can operate in the following modes:
- Bypass mode: Activated when ABB is not required
- FBB mode: Fast Body Bias mode, used on fast OPPs
Fast? I thought the 'F' was for Forward?
You are right. Should be 'Forward' here.
- RBB mode: Reverse Body Bias mode, used on slow OPPs
In current implementation ABB is converted to regulator.
Standalone OPP table is used to store ABB mode, it is defined
in device tree for each ABB regulator. It has the following format:
operating-points =
/* uV ABB (0 - Bypass, 1 - FBB, 2 - RBB) */
88 0
106 1
125 1
126 1
;
ABB regulator is linked to regulator chain
In addition to Mike's comments (which I completely agree with), it would
be very helfpul to see how this is actually used. e.g, how the
regulators are chained together, how the proper ordering is managed,
etc. etc.
We would like to handle voltage scaling in the following way:
cpufreq_cpu0
clk_set_rate(cpu0)
|
|--set_voltage(ABB regulator) /* all ABB related stuff will be
handled here */
|
|--set_voltage(smps123 regulator) /* actual voltage
scaling */
This simple model will be extended to handle AVS as a part of the chain.
smps123 regulator may be changed to VP/VC regulator.
Following example is from integration branch, which already has smps123
regulator.
It demonstrates an example of linkage to chain. ABB regulator is linked
with smps123 and cpu0 inside device tree.
cpu0 calls set_voltage() function for ABB, and then ABB calls
set_voltage() function for smps123 to do actual voltage scaling.
diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index bb5ee70..c8cbbee 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -36,7 +36,7 @@
cpus {
cpu@0 {
compatible = arm,cortex-a15;
- cpu0-supply = smps123_reg;
+ cpu0-supply = abb_mpu;
operating-points =
/* kHzuV */
/* Only for Nominal Samples */
@@ -94,6 +94,7 @@
reg = 0x4ae07cdc 0x8,
0x4ae06014 0x4;
ti,tranxdone_status_mask = 0x80;
+ avs-supply = smps123_reg;
operating-points =
/* uV ABB */
88 0
This RFC patch series is verified together with:
https://patchwork.kernel.org/patch/2445091/
Kevin, what do you think about this model in general? Does it fit to
regulator framework?
Thank you.
Regards,
Andrii
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 0/6] ARM: OMAP3+: Introduce ABB driver
Quoting Andrii Tseglytskyi (2013-04-16 05:40:44)
On 04/16/2013 12:53 AM, Kevin Hilman wrote:
In addition to Mike's comments (which I completely agree with), it would
be very helfpul to see how this is actually used. e.g, how the
regulators are chained together, how the proper ordering is managed,
etc. etc.
We would like to handle voltage scaling in the following way:
I expanded the example below to include the SR AVS regulator.
cpufreq_cpu0
clk_set_rate(cpu0)
|
|--set_voltage(ABB regulator)
|
|--set_voltage(AVS)
|
|--set_voltage(smps123 regulator)
Hi Andrii,
Why was regulator chaining chosen over a simple sequence of calls to
regulator_set_voltage? Instead of nested calls into the regulator
framework, why don't you just make the calls serially? E.g:
regulator_set_voltage(abb_reg, foo_volt);
regulator_set_voltage(avs_reg, bar_volt);
regulator_set_voltage(smps123_reg, baz_volt);
It is still to be determined where these calls originate from; maybe
from clock notifiers, maybe directly from the cpufreq driver's .target()
callback, or maybe somewhere else. Regardless, I do not see why
regulator chaining is truly necessary here. You are just calling
regulator_set_voltage in sequence on a few regulators, right?
I think it would help me a lot to understand why regulator chaining is a
requirement for this to work properly.
Thanks,
Mike
This simple model will be extended to handle AVS as a part of the chain.
smps123 regulator may be changed to VP/VC regulator.
Following example is from integration branch, which already has smps123
regulator.
It demonstrates an example of linkage to chain. ABB regulator is linked
with smps123 and cpu0 inside device tree.
cpu0 calls set_voltage() function for ABB, and then ABB calls
set_voltage() function for smps123 to do actual voltage scaling.
diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index bb5ee70..c8cbbee 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -36,7 +36,7 @@
cpus {
cpu@0 {
compatible = arm,cortex-a15;
- cpu0-supply = smps123_reg;
+ cpu0-supply = abb_mpu;
operating-points =
/* kHzuV */
/* Only for Nominal Samples */
@@ -94,6 +94,7 @@
reg = 0x4ae07cdc 0x8,
0x4ae06014 0x4;
ti,tranxdone_status_mask = 0x80;
+ avs-supply = smps123_reg;
operating-points =
/* uV ABB */
88 0
This RFC patch series is verified together with:
https://patchwork.kernel.org/patch/2445091/
Kevin, what do you think about this model in general? Does it fit to
regulator framework?
Thank you.
Regards,
Andrii
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 0/6] ARM: OMAP3+: Introduce ABB driver
Andrii Tseglytskyi andrii.tseglyts...@ti.com writes:
Hi Kevin,
On 04/16/2013 12:53 AM, Kevin Hilman wrote:
Andrii Tseglytskyi andrii.tseglyts...@ti.com writes:
From: Andrii.Tseglytskyi andrii.tseglyts...@ti.com
Following patch series introduces the Adaptive Body-Bias
LDO driver, which handles LDOs voltage during OPP change routine.
Current implementation is based on patch series from
Mike Turquette:
http://marc.info/?l=linux-omapm=134931341818379w=2
ABB transition is a part of OPP changing sequence.
ABB can operate in the following modes:
- Bypass mode: Activated when ABB is not required
- FBB mode: Fast Body Bias mode, used on fast OPPs
Fast? I thought the 'F' was for Forward?
You are right. Should be 'Forward' here.
- RBB mode: Reverse Body Bias mode, used on slow OPPs
In current implementation ABB is converted to regulator.
Standalone OPP table is used to store ABB mode, it is defined
in device tree for each ABB regulator. It has the following format:
operating-points =
/* uV ABB (0 - Bypass, 1 - FBB, 2 - RBB) */
88 0
106 1
125 1
126 1
;
ABB regulator is linked to regulator chain
In addition to Mike's comments (which I completely agree with), it would
be very helfpul to see how this is actually used. e.g, how the
regulators are chained together, how the proper ordering is managed,
etc. etc.
We would like to handle voltage scaling in the following way:
What I meant is that a detailed description of the use case should be
included in the changelog.
cpufreq_cpu0
clk_set_rate(cpu0)
|
|--set_voltage(ABB regulator) /* all ABB related stuff will be
handled here */
|
|--set_voltage(smps123 regulator) /* actual voltage
scaling */
-EASCII_ART_WRAP
This simple model will be extended to handle AVS as a part of the chain.
smps123 regulator may be changed to VP/VC regulator.
Following example is from integration branch, which already has
smps123 regulator.
I don't know what integration branch you're referring to, and I don't
know what the smps123 regulator is.
It demonstrates an example of linkage to chain. ABB regulator is
linked with smps123 and cpu0 inside device tree.
cpu0 calls set_voltage() function for ABB, and then ABB calls
set_voltage() function for smps123 to do actual voltage scaling.
diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index bb5ee70..c8cbbee 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -36,7 +36,7 @@
cpus {
cpu@0 {
compatible = arm,cortex-a15;
- cpu0-supply = smps123_reg;
+ cpu0-supply = abb_mpu;
operating-points =
/* kHzuV */
/* Only for Nominal Samples */
@@ -94,6 +94,7 @@
reg = 0x4ae07cdc 0x8,
0x4ae06014 0x4;
ti,tranxdone_status_mask = 0x80;
+ avs-supply = smps123_reg;
operating-points =
/* uV ABB */
88 0
This RFC patch series is verified together with:
https://patchwork.kernel.org/patch/2445091/
Kevin, what do you think about this model in general? Does it fit to
regulator framework?
I don't know yet, because I don't think the use case has been described
well enough for me to fully understand it the motiviation behind the
series.
In addition, there are alternative approaches that seem to have been
ruled out without describing why. For example, the regulator framework
already allows you to override methods with custom hooks (as we do for
VC/VP controlled regulators already.) Without thinking about it too
deeply, it seems this approach could be used to manage the chain of
events you need as well. I can imagine there are limitations to this
approach for what you're trying to do, but I don't feel they have been
described in the changelog as part of the motivation for this series.
So for now, the guidance I have is this:
First, write changelogs (and cover letters) assuming your audience has
not been staring at the code as long as you have. Even if they have
been staring at the same code, assume they've been staring at mainline,
and not a random integration branch somewhere. My general advice is to
write changelogs in a way that you will understand what you wrote a year
from now after having forgotten all the details currently in your brains
cache. Even better, write them so that I will understand them in a year
since I forget much better than I remember.
Second, before inventing something new, start with the existing
framework. When the existing framework doesn't work, make an argument
for your new approach or extentions to the framework based on why the
[RFC PATCH v2 0/6] ARM: OMAP3+: Introduce ABB driver
From: Andrii.Tseglytskyi andrii.tseglyts...@ti.com Following patch series introduces the Adaptive Body-Bias LDO driver, which handles LDOs voltage during OPP change routine. Current implementation is based on patch series from Mike Turquette: http://marc.info/?l=linux-omapm=134931341818379w=2 ABB transition is a part of OPP changing sequence. ABB can operate in the following modes: - Bypass mode: Activated when ABB is not required - FBB mode: Fast Body Bias mode, used on fast OPPs - RBB mode: Reverse Body Bias mode, used on slow OPPs In current implementation ABB is converted to regulator. Standalone OPP table is used to store ABB mode, it is defined in device tree for each ABB regulator. It has the following format: operating-points = /* uV ABB (0 - Bypass, 1 - FBB, 2 - RBB) */ 88 0 106 1 125 1 126 1 ; ABB regulator is linked to regulator chain. Related discussions: regulator: query on regulator re-entrance http://marc.info/?l=linux-omapm=136513861315970w=2 regulator: core: introduce regulator chain locking scheme https://patchwork.kernel.org/patch/2445091/ clk: notifier handler for dynamic voltage scaling https://lkml.org/lkml/2013/2/27/414 Andrii.Tseglytskyi (6): ARM: dts: OMAP36xx: add device tree for ABB ARM: dts: OMAP4: add device tree for ABB ARM: dts: OMAP5: add device tree for ABB ARM: OMAP3+: ABB: add aliases for sysclk used in ABB driver ARM: OMAP3+: ABB: introduce ABB driver ARM: OMAP3+: ABB: introduce debugfs entry Documentation/devicetree/bindings/power/abb.txt | 38 ++ arch/arm/boot/dts/omap36xx.dtsi | 20 + arch/arm/boot/dts/omap4.dtsi| 22 + arch/arm/boot/dts/omap443x.dtsi | 23 + arch/arm/boot/dts/omap446x.dtsi | 39 ++ arch/arm/boot/dts/omap5.dtsi| 39 ++ arch/arm/mach-omap2/cclock3xxx_data.c |1 + arch/arm/mach-omap2/cclock44xx_data.c |2 + drivers/regulator/Kconfig |6 + drivers/regulator/Makefile |1 + drivers/regulator/abb-regulator.c | 710 +++ 11 files changed, 901 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/abb.txt create mode 100644 arch/arm/boot/dts/omap446x.dtsi create mode 100644 drivers/regulator/abb-regulator.c -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 0/6] ARM: OMAP3+: Introduce ABB driver
Andrii Tseglytskyi andrii.tseglyts...@ti.com writes: From: Andrii.Tseglytskyi andrii.tseglyts...@ti.com Following patch series introduces the Adaptive Body-Bias LDO driver, which handles LDOs voltage during OPP change routine. Current implementation is based on patch series from Mike Turquette: http://marc.info/?l=linux-omapm=134931341818379w=2 ABB transition is a part of OPP changing sequence. ABB can operate in the following modes: - Bypass mode: Activated when ABB is not required - FBB mode: Fast Body Bias mode, used on fast OPPs Fast? I thought the 'F' was for Forward? - RBB mode: Reverse Body Bias mode, used on slow OPPs In current implementation ABB is converted to regulator. Standalone OPP table is used to store ABB mode, it is defined in device tree for each ABB regulator. It has the following format: operating-points = /* uV ABB (0 - Bypass, 1 - FBB, 2 - RBB) */ 88 0 106 1 125 1 126 1 ; ABB regulator is linked to regulator chain In addition to Mike's comments (which I completely agree with), it would be very helfpul to see how this is actually used. e.g, how the regulators are chained together, how the proper ordering is managed, etc. etc. IOW, This series gives a bunch of low-level details without demonstrating the actual use case and showing the regulator API usage that would make this work. Kevin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
