Re: [PATCH v3 13/21] ARM: omap: convert wakeupgen to stacked domains
On 14:28-20150115, Marc Zyngier wrote: Assuming the workaround I posted earlier works, the OMAP/DRA7 part of this series is going to require some rework too (I need to know where these legacy interrupts are attached: crossbar, WUGEN, or GIC?). crossbar will never work with legacy static interrupts anyways - since there was never a static interrupt possible - I believe we had removed all the legacy hardcoded interrupt definitions for DRA7. ideally, they should all be dt only now. -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 08/21] irqchip: crossbar: convert dra7 crossbar to stacked domains
On 17:42-20150107, Marc Zyngier wrote:
[..]
diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
index 10b725c..048cfeb 100644
--- a/arch/arm/boot/dts/dra7-evm.dts
+++ b/arch/arm/boot/dts/dra7-evm.dts
@@ -423,7 +423,7 @@
status = okay;
pinctrl-names = default;
pinctrl-0 = uart1_pins;
- interrupts-extended = gic GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH,
+ interrupts-extended = crossbar_mpu GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH,
dra7_pmx_core 0x3e0;
^^ interrrupt-extended for uart1 here
[..]
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
[...]
@@ -344,7 +344,7 @@
uart1: serial@4806a000 {
compatible = ti,omap4-uart;
reg = 0x4806a000 0x100;
- interrupts-extended = gic GIC_SPI 67
IRQ_TYPE_LEVEL_HIGH;
+ interrupts = GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH;
^^ implies we will have both interrupts and interrupts-extended
properties for uart1 in dra7-evm.dtb
Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
does not make it clear as to what the priority will be when both
properties are present.
[...]
Also, for 3.19-rc3, Missing the following causing x15 to fail boot.
diff --git a/arch/arm/boot/dts/am57xx-beagle-x15.dts
b/arch/arm/boot/dts/am57xx-beagle-x15.dts
index 49edbda68cd5..c2241c2e5d9d 100644
--- a/arch/arm/boot/dts/am57xx-beagle-x15.dts
+++ b/arch/arm/boot/dts/am57xx-beagle-x15.dts
@@ -335,7 +335,6 @@
mcp_rtc: rtc@6f {
compatible = microchip,mcp7941x;
reg = 0x6f;
- interrupt-parent = gic;
interrupts = GIC_SPI 2 IRQ_TYPE_LEVEL_LOW; /* IRQ_SYS_1N */
pinctrl-names = default;
@@ -358,7 +357,7 @@
uart3 {
status = okay;
- interrupts-extended = gic GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH,
+ interrupts-extended = crossbar_mpu GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH,
dra7_pmx_core 0x248;
pinctrl-names = default;
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 02/21] irqchip: tegra: add DT-based support for legacy interrupt controller
On 17:42-20150107, Marc Zyngier wrote: Tegra's LIC (Legacy Interrupt Controller) has been so far only supported as a weird extension of the GIC, which is not exactly pretty. The stacked irq domain framework fits this pretty well, and allows the LIC code to be turned into a standalone irqchip. In the process, make the driver DT aware, something that was sorely missing from the mach-tegra implementation. Signed-off-by: Marc Zyngier marc.zyng...@arm.com --- Saw a few checkpatch warnings as below: all of them seem minors. @@ -0,0 +1,35 @@ +WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? +#36: +new file mode 100644 +WARNING: line over 80 characters +#169: FILE: drivers/irqchip/irq-tegra.c:129: ++ tegra_ictlr_info-cpu_ier[i] = readl_relaxed(ictlr + ICTLR_CPU_IER); +WARNING: line over 80 characters +#170: FILE: drivers/irqchip/irq-tegra.c:130: ++ tegra_ictlr_info-cpu_iep[i] = readl_relaxed(ictlr + ICTLR_CPU_IEP_CLASS); +WARNING: line over 80 characters +#171: FILE: drivers/irqchip/irq-tegra.c:131: ++ tegra_ictlr_info-cop_ier[i] = readl_relaxed(ictlr + ICTLR_COP_IER); +WARNING: line over 80 characters +#172: FILE: drivers/irqchip/irq-tegra.c:132: ++ tegra_ictlr_info-cop_iep[i] = readl_relaxed(ictlr + ICTLR_COP_IEP_CLASS); +WARNING: line over 80 characters +#181: FILE: drivers/irqchip/irq-tegra.c:141: ++ writel_relaxed(tegra_ictlr_info-ictlr_wake_mask[i], ictlr + ICTLR_CPU_IER_SET); +WARNING: Missing a blank line after declarations +#196: FILE: drivers/irqchip/irq-tegra.c:156: ++ void __iomem *ictlr = tegra_ictlr_info-ictlr_reg_base[i]; ++ writel_relaxed(tegra_ictlr_info-cpu_iep[i], +WARNING: line over 80 characters +#284: FILE: drivers/irqchip/irq-tegra.c:244: ++ return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, parent_args); +CHECK: Please don't use multiple blank lines +#287: FILE: drivers/irqchip/irq-tegra.c:247: ++ ++ +WARNING: Missing a blank line after declarations +#296: FILE: drivers/irqchip/irq-tegra.c:256: ++ struct irq_data *d = irq_domain_get_irq_data(domain, virq + i); ++ irq_domain_reset_irq_data(d); ` -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 13/21] DT: omap4/5: add binding for the wake-up generator
On 17:42-20150107, Marc Zyngier wrote:
Signed-off-by: Marc Zyngier marc.zyng...@arm.com
---
.../interrupt-controller/ti,omap4-wugen-mpu| 32
++
1 file changed, 32 insertions(+)
create mode 100644
Documentation/devicetree/bindings/interrupt-controller/ti,omap4-wugen-mpu
diff --git
a/Documentation/devicetree/bindings/interrupt-controller/ti,omap4-wugen-mpu
b/Documentation/devicetree/bindings/interrupt-controller/ti,omap4-wugen-mpu
new file mode 100644
index 000..16149d9
--- /dev/null
+++
b/Documentation/devicetree/bindings/interrupt-controller/ti,omap4-wugen-mpu
@@ -0,0 +1,32 @@
+TI OMAP4 Wake-up Generator
+
+All TI OMAP4/5 (and their derivatives) an interrupt controllerthat
controller that
+routes interrupts to the GIC, and also serves as a wakeup source. It
+is also refered to as WUGEN-MPU, hence the name of the binding.
+
+Reguired properties:
+
+- compatible : should contain at least ti,omap4-wugen-mpu
Could we also document ti,omap5-wugen-mpu. In addition, if you could
make this patch prior to patch #12, it helps the checkpatch at the very
least ;)
also saw a few checkpatch warnings:
+WARNING: 'refered' may be misspelled - perhaps 'referred'?
+#22: FILE:
Documentation/devicetree/bindings/interrupt-controller/ti,omap4-wugen-mpu:5:
++is also refered to as WUGEN-MPU, hence the name of the binding.
+WARNING: 'explicitely' may be misspelled - perhaps 'explicitly'?
+#39: FILE:
Documentation/devicetree/bindings/interrupt-controller/ti,omap4-wugen-mpu:22:
++ are explicitely forbiden.
+- reg : Specifies base physical address and size of the registers.
+- interrupt-controller : Identifies the node as an interrupt controller.
+- #interrupt-cells : Specifies the number of cells needed to encode an
+ interrupt source. The value must be 3.
+- interrupt-parent : a phandle to the GIC these interrupts are routed
+ to.
+
+Notes:
+
+- Because this HW ultimately routes interrupts to the GIC, the
+ interrupt specifier must be that of the GIC.
+- Only SPIs can use the ictlr as an interrupt parent. SGIs and PPIs
I think you mean interrupt controller and not nvidia ictlr here.. :)
+ are explicitely forbiden.
+
+Example:
+
+ wakeupgen: interrupt-controller@48281000 {
+ compatible = ti,omap5-wugen-mpu, ti,omap4-wugen-mpu;
+ interrupt-controller;
+ #interrupt-cells = 3;
+ reg = 0x48281000 0x1000;
+ interrupt-parent = gic;
+ };
--
2.1.4
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 14/21] ARM: imx6: convert GPC to stacked domains
On 17:42-20150107, Marc Zyngier wrote: IMX6 has been (ab)using the gic_arch_extn to provide wakeup from suspend, and it makes a lot of sense to convert this code to use stacked domains instead. This patch does just this, updating the DT files to actually reflect what the HW provides. minor checkpatch warnings below: @@ -0,0 +1,14 @@ +CHECK: Alignment should match open parenthesis +#249: FILE: arch/arm/mach-imx/gpc.c:158: ++static int imx_gpc_domain_alloc(struct irq_domain *domain, ++unsigned int virq, +WARNING: line over 80 characters +#272: FILE: arch/arm/mach-imx/gpc.c:181: ++ return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, parent_args); +ERROR: code indent should use tabs where possible +#305: FILE: arch/arm/mach-imx/gpc.c:209: ++^Ireturn -ENOMEM;$ +NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or + scripts/cleanfile -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 05/21] DT: tegra: add binding for the legacy interrupt controller
On 17:42-20150107, Marc Zyngier wrote:
Signed-off-by: Marc Zyngier marc.zyng...@arm.com
---
.../interrupt-controller/nvidia,tegra-ictlr.txt| 39
++
1 file changed, 39 insertions(+)
create mode 100644
Documentation/devicetree/bindings/interrupt-controller/nvidia,tegra-ictlr.txt
diff --git
a/Documentation/devicetree/bindings/interrupt-controller/nvidia,tegra-ictlr.txt
b/Documentation/devicetree/bindings/interrupt-controller/nvidia,tegra-ictlr.txt
new file mode 100644
index 000..44fd873
--- /dev/null
+++
b/Documentation/devicetree/bindings/interrupt-controller/nvidia,tegra-ictlr.txt
@@ -0,0 +1,39 @@
+NVIDIA Legacy Interrupt Controller
+
+All Tegra SoCs contain a legacy interrupt controller that routes
+interrupts to the GIC, and also serves as a wakeup source. It is also
+refered to as ictlr, hence the name of the binding.
+
+The HW block exposes a number of frames, each implementing a set of 32
+interrupts.
+
+Reguired properties:
+
+- compatible : should contain at least nvidia,tegra-ictlr.
perhaps list the specific versions here..?
+WARNING: DT compatible string nvidia,tegra114-ictlr appears un-documented --
check ./Documentation/devicetree/bindings/
+#39: FILE: arch/arm/boot/dts/tegra114.dtsi:141:
++ compatible = nvidia,tegra114-ictlr, nvidia,tegra-ictlr;
+WARNING: DT compatible string nvidia,tegra-ictlr appears un-documented --
check ./Documentation/devicetree/bindings/
+#39: FILE: arch/arm/boot/dts/tegra114.dtsi:141:
++ compatible = nvidia,tegra114-ictlr, nvidia,tegra-ictlr;
+WARNING: DT compatible string nvidia,tegra124-ictlr appears un-documented --
check ./Documentation/devicetree/bindings/
+#84: FILE: arch/arm/boot/dts/tegra124.dtsi:195:
++ compatible = nvidia,tegra124-ictlr, nvidia,tegra-ictlr;
+WARNING: DT compatible string nvidia,tegra-ictlr appears un-documented --
check ./Documentation/devicetree/bindings/
+#84: FILE: arch/arm/boot/dts/tegra124.dtsi:195:
++ compatible = nvidia,tegra124-ictlr, nvidia,tegra-ictlr;
+WARNING: DT compatible string nvidia,tegra20-ictlr appears un-documented --
check ./Documentation/devicetree/bindings/
+#139: FILE: arch/arm/boot/dts/tegra20.dtsi:171:
++ compatible = nvidia,tegra20-ictlr, nvidia,tegra-ictlr;
+WARNING: DT compatible string nvidia,tegra-ictlr appears un-documented --
check ./Documentation/devicetree/bindings/
+#139: FILE: arch/arm/boot/dts/tegra20.dtsi:171:
++ compatible = nvidia,tegra20-ictlr, nvidia,tegra-ictlr;
+WARNING: DT compatible string nvidia,tegra30-ictlr appears un-documented --
check ./Documentation/devicetree/bindings/
+#186: FILE: arch/arm/boot/dts/tegra30.dtsi:256:
++ compatible = nvidia,tegra30-ictlr, nvidia,tegra-ictlr;
+WARNING: DT compatible string nvidia,tegra-ictlr appears un-documented --
check ./Documentation/devicetree/bindings/
+#186: FILE: arch/arm/boot/dts/tegra30.dtsi:256:
++ compatible = nvidia,tegra30-ictlr, nvidia,tegra-ictlr;
+- reg : Specifies base physical address and size of the registers.
+ Each frame must be described separately.
+- interrupt-controller : Identifies the node as an interrupt controller.
+- #interrupt-cells : Specifies the number of cells needed to encode an
+ interrupt source. The value must be 3.
+- interrupt-parent : a phandle to the GIC these interrupts are routed
+ to.
+
+Notes:
+
+- Because this HW ultimately routes interrupts to the GIC, the
+ interrupt specifier must be that of the GIC.
+- Only SPIs can use the ictlr as an interrupt parent. SGIs and PPIs
+ are explicitely forbiden.
+
+Example:
+
+ ictlr: interrupt-controller@60004000 {
+ compatible = nvidia,tegra20-ictlr, nvidia,tegra-ictlr;
+ reg = 0x60004000 64,
+ 0x60004100 64,
+ 0x60004200 64,
+ 0x60004300 64;
+ interrupt-controller;
+ #interrupt-cells = 3;
+ interrupt-parent = intc;
+ };
--
2.1.4
Might be good to have this patch before patch #3, since the binding
defined here is implemented in #3 and used in #4. also:
+WARNING: 'refered' may be misspelled - perhaps 'referred'?
+#23: FILE:
Documentation/devicetree/bindings/interrupt-controller/nvidia,tegra-ictlr.txt:5:
++refered to as ictlr, hence the name of the binding.
+WARNING: 'explicitely' may be misspelled - perhaps 'explicitly'?
+#44: FILE:
Documentation/devicetree/bindings/interrupt-controller/nvidia,tegra-ictlr.txt:26:
++ are explicitely forbiden.
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 07/21] genirq: Add irqchip_set_wake_parent
On 17:42-20150107, Marc Zyngier wrote: This proves to be usefull with stacked domains, when the current ^^ useful ? minor: +WARNING: 'usefull' may be misspelled - perhaps 'useful'? +#6: +This proves to be usefull with stacked domains, when the current +CHECK: extern prototypes should be avoided in .h files +#23: FILE: include/linux/irq.h:463: ++extern int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on); -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 12/21] ARM: omap: convert wakeupgen to stacked domains
.dts
@@ -335,7 +335,6 @@
mcp_rtc: rtc@6f {
compatible = microchip,mcp7941x;
reg = 0x6f;
- interrupt-parent = gic;
interrupts = GIC_SPI 2 IRQ_TYPE_LEVEL_LOW; /* IRQ_SYS_1N */
pinctrl-names = default;
@@ -358,7 +357,7 @@
uart3 {
status = okay;
- interrupts-extended = gic GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH,
+ interrupts-extended = crossbar_mpu GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH,
dra7_pmx_core 0x248;
pinctrl-names = default;
diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index f9c75c782c48..b056156e2a7a 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -893,14 +893,12 @@
usbhsohci: ohci@4a064800 {
compatible = ti,ohci-omap3;
reg = 0x4a064800 0x400;
- interrupt-parent = gic;
interrupts = GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH;
};
usbhsehci: ehci@4a064c00 {
compatible = ti,ehci-omap;
reg = 0x4a064c00 0x400;
- interrupt-parent = gic;
interrupts = GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH;
};
};
Also saw the following checkpatch warnings at this point - but, I
suppose they were pre-existing.
@@ -0,0 +1,26 @@
+WARNING: DT compatible string ti,omap4-wugen-mpu appears un-documented --
check ./Documentation/devicetree/bindings/
+#95: FILE: arch/arm/boot/dts/am4372.dtsi:55:
++ compatible = ti,omap4-wugen-mpu;
+WARNING: DT compatible string ti,omap5-wugen-mpu appears un-documented --
check ./Documentation/devicetree/bindings/
+#112: FILE: arch/arm/boot/dts/dra7.dtsi:68:
++ compatible = ti,omap5-wugen-mpu, ti,omap4-wugen-mpu;
+WARNING: DT compatible string ti,omap4-wugen-mpu appears un-documented --
check ./Documentation/devicetree/bindings/
+#112: FILE: arch/arm/boot/dts/dra7.dtsi:68:
++ compatible = ti,omap5-wugen-mpu, ti,omap4-wugen-mpu;
+WARNING: line over 80 characters
+#127: FILE: arch/arm/boot/dts/dra7.dtsi:103:
++wakeupgen GIC_SPI 10
IRQ_TYPE_LEVEL_HIGH;
+WARNING: DT compatible string ti,omap4-wugen-mpu appears un-documented --
check ./Documentation/devicetree/bindings/
+#320: FILE: arch/arm/boot/dts/omap4.dtsi:78:
++ compatible = ti,omap4-wugen-mpu;
+WARNING: DT compatible string ti,omap5-wugen-mpu appears un-documented --
check ./Documentation/devicetree/bindings/
+#416: FILE: arch/arm/boot/dts/omap5.dtsi:103:
++ compatible = ti,omap5-wugen-mpu, ti,omap4-wugen-mpu;
+WARNING: DT compatible string ti,omap4-wugen-mpu appears un-documented --
check ./Documentation/devicetree/bindings/
+#416: FILE: arch/arm/boot/dts/omap5.dtsi:103:
++ compatible = ti,omap5-wugen-mpu, ti,omap4-wugen-mpu;
+WARNING: line over 80 characters
+#600: FILE: arch/arm/mach-omap2/omap-wakeupgen.c:441:
++ return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
parent_args);
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 10/21] irqchip: GIC: get rid of routable domain
On 17:42-20150107, Marc Zyngier wrote: The only user of the so called routable domain functionnality minor: s/functionnality/functionality? -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/21] irqchip: gic: killing gic_arch_extn and co, slowly
TOTAL CPUFREQ enabled boards = 13. CPUFREQ functional boards = 13, CPUFREQ failed boards = 0 TOTAL CPUIdle enabled boards = 7. CPUIdle functional boards = 3, CPUIdle failed boards = 4 -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v12 0/9] Enable L2 cache support on Exynos4210/4x12 SoCs
On 01/07/2015 05:30 AM, Marek Szyprowski wrote:
This is an updated patchset, which intends to add support for L2 cache
on Exynos4 SoCs on boards running under secure firmware, which requires
certain initialization steps to be done with help of firmware, as
selected registers are writable only from secure mode.
First patch updates Omap2+ platforms by moving l2cache initialization to
common code. This will resolve too early call to l2cache init, what might
cause kmalloc failure in code added in next patches.
Next patch fixes access method to latency and filter settings in l2cache
driver.
Next four patches extend existing support for secure write in L2C driver
to account for design of secure firmware running on Exynos. Namely:
1) direct read access to certain registers is needed on Exynos, because
secure firmware calls set several registers at once,
2) not all boards are running secure firmware, so .write_sec callback
needs to be installed in Exynos firmware ops initialization code,
3) write access to {DATA,TAG}_LATENCY_CTRL registers fron non-secure world
is not allowed and so must use l2c_write_sec as well,
4) on certain boards, default value of prefetch register is incorrect
and must be overridden at L2C initialization.
For boards running with firmware that provides access to individual
L2C registers this series should introduce no functional changes. However
since the driver is widely used on other platforms I'd like to kindly ask
any interested people for testing.
Further three patches add implementation of .write_sec and .configure
callbacks for Exynos secure firmware and necessary DT nodes to enable
L2 cache.
Changes in this version tested on Exynos4412-based TRATS2 and OdroidU3+
boards (both with secure firmware). There should be no functional change
for Exynos boards running without secure firmware. OMAP based platforms
were tested by Nishanth Menon and Tony Lindgren.
Depends on:
- v3.19-rc2
Changelog:
Changes since v11:
(https://lkml.org/lkml/2015/1/5/254)
- Added changes suggested by Nishanth to 'ARM: l2c: use l2c_write_sec()
for restoring latency and filter regs' patch
- Fixed 'checkpatch --strict' issues
- Added Nishanth's and Tony's acked/tested tags
Changes since v10:
(https://lkml.org/lkml/2014/12/23/151)
- Added patch, which fixes access method to latency and filter settings
in l2cache
Changes since v9:
(https://lkml.org/lkml/2014/11/17/217)
- Rebased onto vanilla v3.19-rc1
- Added patch for Omap2+ (move l2cache initialization to common code), what
fixes too early initialization (kmalloc failure)
Changes since v8:
(http://lkml.org/lkml/2014/11/13/263)
- Rebased onto vanilla v3.18-rc3 and added required includes, which were
previously added by other patches
- Added Acked-by tags for Exynos part
Changes since v7:
(https://lkml.org/lkml/2014/10/29/158)
- rebased onto arm-soc/for-next kernel tree (depends on patches merged to
v3.18-rc3 and arm-soc/samsung/pm2 branch)
- removed 'ARM: l2c: unify L2C-310 OF initialization error messages' patch
(no longer needed)
Changes since v6:
(https://lkml.org/lkml/2014/10/27/233)
- changed PL310 to L2C-310 prefix in error messages
- added patch shortening the error message about incorrect associativity
Changes since v5:
(https://lkml.org/lkml/2014/9/24/364)
- rebased onto v3.18-rc2
- added error message about missing properties values
Changes since v4:
(https://lkml.org/lkml/2014/8/26/461)
- rewrote the code accessing l2x0_saved_regs from assembly code
- added comment and reworked unconditional call to SMC_CMD_L2X0INVALL
Patch summary:
Marek Szyprowski (2):
ARM: OMAP2+: use common l2cache initialization code
ARM: l2c: use l2c_write_sec() for restoring latency and filter regs
Tomasz Figa (7):
ARM: l2c: Refactor the driver to use commit-like interface
ARM: l2c: Add interface to ask hypervisor to configure L2C
ARM: l2c: Get outer cache .write_sec callback from mach_desc only if
not NULL
ARM: l2c: Add support for overriding prefetch settings
ARM: EXYNOS: Add .write_sec outer cache callback for L2C-310
ARM: EXYNOS: Add support for non-secure L2X0 resume
ARM: dts: exynos4: Add nodes for L2 cache controller
Documentation/devicetree/bindings/arm/l2cc.txt | 10 +
arch/arm/boot/dts/exynos4210.dtsi | 9 +
arch/arm/boot/dts/exynos4x12.dtsi | 14 ++
arch/arm/include/asm/outercache.h | 3 +
arch/arm/kernel/irq.c | 3 +-
arch/arm/mach-exynos/firmware.c| 50 +
arch/arm/mach-exynos/sleep.S | 46 +
arch/arm/mach-omap2/board-generic.c| 6 +
arch/arm/mach-omap2/common.h | 8 +
arch/arm/mach-omap2/omap4-common.c | 16 +-
arch/arm/mm/cache-l2x0.c | 272
-
11 files changed, 325
Re: [PATCH v11 2/9] ARM: l2c: use l2c_write_sec() for restoring latency and filter regs
On 13:19-20150105, Marek Szyprowski wrote:
All four register for latency and filter settings cannot be written in
non-secure mode and they should go through l2c_write_sec(). More on this
can be found in CoreLink Level 2 Cache Controller L2C-310 Technical
Reference Manual, 3.2. Register summary, table 3.1. This have been checked
the TRM for r3p3, but it should be uniform for all revisions.
Reported-by: Nishanth Menon n...@ti.com
Suggested-by: Tomasz Figa tomasz.f...@gmail.com
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
arch/arm/mm/cache-l2x0.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 5e65ca8dea62..0aeeaa95c42d 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -623,14 +623,14 @@ static void l2c310_resume(void)
unsigned revision;
/* restore pl310 setup */
- writel_relaxed(l2x0_saved_regs.tag_latency,
-base + L310_TAG_LATENCY_CTRL);
- writel_relaxed(l2x0_saved_regs.data_latency,
-base + L310_DATA_LATENCY_CTRL);
- writel_relaxed(l2x0_saved_regs.filter_end,
-base + L310_ADDR_FILTER_END);
- writel_relaxed(l2x0_saved_regs.filter_start,
-base + L310_ADDR_FILTER_START);
+ l2c_write_sec(l2x0_saved_regs.tag_latency, base,
+ L310_TAG_LATENCY_CTRL);
+ l2c_write_sec(l2x0_saved_regs.data_latency, base,
+ L310_DATA_LATENCY_CTRL);
+ l2c_write_sec(l2x0_saved_regs.filter_end, base,
+ L310_ADDR_FILTER_END);
+ l2c_write_sec(l2x0_saved_regs.filter_start, base,
+ L310_ADDR_FILTER_START);
revision = readl_relaxed(base + L2X0_CACHE_ID)
L2X0_CACHE_ID_RTL_MASK;
Do you need the following as well at this point in the patch series?
Agreed that the writes will disappear later in the series.
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 0aeeaa9..7afab37 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -1135,28 +1135,28 @@ static void __init l2c310_of_parse(const struct
device_node *np,
of_property_read_u32_array(np, arm,tag-latency, tag, ARRAY_SIZE(tag));
if (tag[0] tag[1] tag[2])
- writel_relaxed(
+ l2c_write_sec(
L310_LATENCY_CTRL_RD(tag[0] - 1) |
L310_LATENCY_CTRL_WR(tag[1] - 1) |
L310_LATENCY_CTRL_SETUP(tag[2] - 1),
- l2x0_base + L310_TAG_LATENCY_CTRL);
+ l2x0_base, L310_TAG_LATENCY_CTRL);
of_property_read_u32_array(np, arm,data-latency,
data, ARRAY_SIZE(data));
if (data[0] data[1] data[2])
- writel_relaxed(
+ l2c_write_sec(
L310_LATENCY_CTRL_RD(data[0] - 1) |
L310_LATENCY_CTRL_WR(data[1] - 1) |
L310_LATENCY_CTRL_SETUP(data[2] - 1),
- l2x0_base + L310_DATA_LATENCY_CTRL);
+ l2x0_base, L310_DATA_LATENCY_CTRL);
of_property_read_u32_array(np, arm,filter-ranges,
filter, ARRAY_SIZE(filter));
if (filter[1]) {
- writel_relaxed(ALIGN(filter[0] + filter[1], SZ_1M),
- l2x0_base + L310_ADDR_FILTER_END);
- writel_relaxed((filter[0] ~(SZ_1M - 1)) | L310_ADDR_FILTER_EN,
- l2x0_base + L310_ADDR_FILTER_START);
+ l2c_write_sec(ALIGN(filter[0] + filter[1], SZ_1M),
+ l2x0_base, L310_ADDR_FILTER_END);
+ l2c_write_sec((filter[0] ~(SZ_1M - 1)) | L310_ADDR_FILTER_EN,
+ l2x0_base, L310_ADDR_FILTER_START);
}
ret = l2x0_cache_size_of_parse(np, aux_val, aux_mask, assoc, SZ_512K);
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v11 0/9] Enable L2 cache support on Exynos4210/4x12 SoCs
: am437x-sk: BOOT: PASS: http://slexy.org/raw/s203dOrLId 4: am43xx-epos: BOOT: PASS: http://slexy.org/raw/s2uWmV1XkM 5: am43xx-gpevm: BOOT: PASS: http://slexy.org/raw/s20jVsjU00 6: BeagleBoard-X15(am57xx-evm): BOOT: PASS: http://slexy.org/raw/s21ezwoOop 7: BeagleBoard-XM: BOOT: PASS: http://slexy.org/raw/s21M0IYcdI 8: beagleboard-vanilla: BOOT: PASS: http://slexy.org/raw/s2gCJ5VRr9 9: beaglebone-black: BOOT: PASS: http://slexy.org/raw/s2vvB4u86O 10: beaglebone: BOOT: PASS: http://slexy.org/raw/s208k0tU3V 11: dra72x-evm: BOOT: PASS: http://slexy.org/raw/s212oTeZ2w 12: dra7xx-evm: BOOT: PASS: http://slexy.org/raw/s24deU2xtY 13: omap5-evm: BOOT: PASS: http://slexy.org/raw/s2DkUAGU0i 14: pandaboard-es: BOOT: PASS: http://slexy.org/raw/s2UKuDaAFf 15: pandaboard-vanilla: BOOT: PASS: http://slexy.org/raw/s21VAyb14r 16: sdp4430: BOOT: PASS: http://slexy.org/raw/s2J6nAHynd for TI SoCs impacted OMAP4 and AM437x (other than the exynos patches): Tested-by: Nishanth Menon n...@ti.com Acked-by: Nishanth Menon n...@ti.com -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v11 3/9] ARM: l2c: Refactor the driver to use commit-like interface
*base)
l2x0_saved_regs.aux_ctrl = readl_relaxed(base + L2X0_AUX_CTRL);
}
-static void aurora_resume(void)
-{
- void __iomem *base = l2x0_base;
-
- if (!(readl(base + L2X0_CTRL) L2X0_CTRL_EN)) {
- writel_relaxed(l2x0_saved_regs.aux_ctrl, base + L2X0_AUX_CTRL);
- writel_relaxed(l2x0_saved_regs.ctrl, base + L2X0_CTRL);
- }
-}
-
/*
* For Aurora cache in no outer mode, enable via the CP15 coprocessor
* broadcasting of cache commands to L2.
@@ -1401,7 +1423,7 @@ static const struct l2c_init_data
of_aurora_with_outer_data __initconst = {
.flush_all = l2x0_flush_all,
.disable = l2x0_disable,
.sync= l2x0_cache_sync,
- .resume = aurora_resume,
+ .resume = l2c_resume,
},
};
@@ -1414,7 +1436,7 @@ static const struct l2c_init_data
of_aurora_no_outer_data __initconst = {
.fixup = aurora_fixup,
.save = aurora_save,
.outer_cache = {
- .resume = aurora_resume,
+ .resume = l2c_resume,
},
};
@@ -1562,6 +1584,7 @@ static const struct l2c_init_data of_bcm_l2x0_data
__initconst = {
.of_parse = l2c310_of_parse,
.enable = l2c310_enable,
.save = l2c310_save,
+ .configure = l2c310_configure,
.outer_cache = {
.inv_range = bcm_inv_range,
.clean_range = bcm_clean_range,
@@ -1583,18 +1606,12 @@ static void __init tauros3_save(void __iomem *base)
readl_relaxed(base + L310_PREFETCH_CTRL);
}
-static void tauros3_resume(void)
+static void tauros3_configure(void __iomem *base)
{
- void __iomem *base = l2x0_base;
-
- if (!(readl_relaxed(base + L2X0_CTRL) L2X0_CTRL_EN)) {
- writel_relaxed(l2x0_saved_regs.aux2_ctrl,
-base + TAUROS3_AUX2_CTRL);
- writel_relaxed(l2x0_saved_regs.prefetch_ctrl,
-base + L310_PREFETCH_CTRL);
-
- l2c_enable(base, l2x0_saved_regs.aux_ctrl, 8);
- }
+ writel_relaxed(l2x0_saved_regs.aux2_ctrl,
+base + TAUROS3_AUX2_CTRL);
+ writel_relaxed(l2x0_saved_regs.prefetch_ctrl,
+base + L310_PREFETCH_CTRL);
}
static const struct l2c_init_data of_tauros3_data __initconst = {
@@ -1603,9 +1620,10 @@ static const struct l2c_init_data of_tauros3_data
__initconst = {
.num_lock = 8,
.enable = l2c_enable,
.save = tauros3_save,
+ .configure = tauros3_configure,
/* Tauros3 broadcasts L1 cache operations to L2 */
.outer_cache = {
- .resume = tauros3_resume,
+ .resume = l2c_resume,
},
};
@@ -1661,6 +1679,10 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
if (!of_property_read_bool(np, cache-unified))
pr_err(L2C: device tree omits to specify unified cache\n);
+ /* Read back current (default) hardware configuration */
+ if (data-save)
+ data-save(l2x0_base);
+
/* L2 configuration can only be changed if the cache is disabled */
if (!(readl_relaxed(l2x0_base + L2X0_CTRL) L2X0_CTRL_EN))
if (data-of_parse)
@@ -1671,8 +1693,6 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
else
cache_id = readl_relaxed(l2x0_base + L2X0_CACHE_ID);
- __l2c_init(data, aux_val, aux_mask, cache_id);
-
- return 0;
+ return __l2c_init(data, aux_val, aux_mask, cache_id);
}
#endif
--
1.9.2
Based on two painful debugs.. It would really be nice to have this patch
split up into tinier pieces.. I know I have no issues with this patch
anymore, but for others who might discover similar behavior, debug is a
little easier with a split up series.
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 2/8] ARM: l2c: Refactor the driver to use commit-like interface
On 01/02/2015 02:55 AM, Tomasz Figa wrote:
On 30.12.2014 03:23, Nishanth Menon wrote:
On 12/23/2014 04:48 AM, Marek Szyprowski wrote:
-static void l2c310_resume(void)
+static void l2c310_configure(void __iomem *base)
{
- void __iomem *base = l2x0_base;
+ unsigned revision;
- if (!(readl_relaxed(base + L2X0_CTRL) L2X0_CTRL_EN)) {
- unsigned revision;
-
- /* restore pl310 setup */
- writel_relaxed(l2x0_saved_regs.tag_latency,
- base + L310_TAG_LATENCY_CTRL);
- writel_relaxed(l2x0_saved_regs.data_latency,
- base + L310_DATA_LATENCY_CTRL);
- writel_relaxed(l2x0_saved_regs.filter_end,
- base + L310_ADDR_FILTER_END);
- writel_relaxed(l2x0_saved_regs.filter_start,
- base + L310_ADDR_FILTER_START);
-
- revision = readl_relaxed(base + L2X0_CACHE_ID)
- L2X0_CACHE_ID_RTL_MASK;
-
- if (revision = L310_CACHE_ID_RTL_R2P0)
- l2c_write_sec(l2x0_saved_regs.prefetch_ctrl, base,
- L310_PREFETCH_CTRL);
- if (revision = L310_CACHE_ID_RTL_R3P0)
- l2c_write_sec(l2x0_saved_regs.pwr_ctrl, base,
- L310_POWER_CTRL);
-
- l2c_enable(base, l2x0_saved_regs.aux_ctrl, 8);
-
- /* Re-enable full-line-of-zeros for Cortex-A9 */
- if (l2x0_saved_regs.aux_ctrl L310_AUX_CTRL_FULL_LINE_ZERO)
- set_auxcr(get_auxcr() | BIT(3) | BIT(2) | BIT(1));
- }
+ /* restore pl310 setup */
+ writel_relaxed(l2x0_saved_regs.tag_latency,
+ base + L310_TAG_LATENCY_CTRL);
+ writel_relaxed(l2x0_saved_regs.data_latency,
+ base + L310_DATA_LATENCY_CTRL);
+ writel_relaxed(l2x0_saved_regs.filter_end,
+ base + L310_ADDR_FILTER_END);
+ writel_relaxed(l2x0_saved_regs.filter_start,
+ base + L310_ADDR_FILTER_START);
+
^^ The above change broke AM437xx. Looks like the change causes the
following behavior difference on AM437x. For some reason, touching any
of the above 4 registers(even with the values read from the same
registers) causes AM437x to go beserk. Comment the 4 writes and we
reach shell. looks like l2c310_resume is not invoked prior to this
series. :(.. now that we reuse that logic to actually do programming,
we start to see the problem.
OK, I probably have answer for this. Apparently all four register above
cannot be written in non-secure mode and they should go through
l2c_write_sec(). More on this can be found in CoreLink Level 2 Cache
Controller L2C-310 Technical Reference Manual, 3.2. Register summary,
table 3.1. I have checked the TRM for r3p3, but I guess this should be
uniform for all revisions.
Yep, you seemed to have caught the issue correctly.
Why this worked before? The registers were not written unless respective
properties in DT were present and OMAP do not have them in DT. Current
code always writes them, which should not really matter if the code is
correct. (But it isn't - writel_relaxed() can't be used directly for
those registers.)
Could you check if replacing those four writel_relaxed() with
l2c_write_sec() does the thing?
Considering that
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/mach-omap2/omap4-common.c#n169
has no implementation of DATA,TAG_LATENCY, FILTER_*
I did a few patches for those - separate from this series and posted
(v2 of the series):
https://patchwork.kernel.org/patch/5560231/
https://patchwork.kernel.org/patch/5560211/
Anyways, l2c_write_sec will refuse to write if the read value is same
as requested value - that is exactly what we want here. So, for
testing, I hacked it and remove the check to force the writes
(http://slexy.org/view/s2p8c3gl32)
The replaced raw_writels with secure writes(fix needed for this
patch): http://slexy.org/view/s21PbM73tt
(with secure writes, my patches and force write hack applied):
1: am437x-sk: BOOT: PASS: http://slexy.org/raw/s2BxqX5NOz
2: am43xx-epos: BOOT: PASS: http://slexy.org/raw/s2VkKYuYed
3: am43xx-gpevm: BOOT: PASS: http://slexy.org/raw/s2kO95WSuY
4: pandaboard-es: BOOT: PASS: http://slexy.org/raw/s21sx5gmUl
5: pandaboard-vanilla: BOOT: PASS: http://slexy.org/raw/s21lpUJK6r
6: sdp4430: BOOT: PASS: http://slexy.org/raw/s21UVKHle5
(with just secure writes and none of my patches or hacks applied):
1: am437x-sk: BOOT: PASS: http://slexy.org/raw/s21UxoDdo5
2: am43xx-epos: BOOT: PASS: http://slexy.org/raw/s2ECrLZ1FH
3: am43xx-gpevm: BOOT: PASS: http://slexy.org/raw/s2CowTVA9P
4: pandaboard-es: BOOT: PASS: http://slexy.org/raw/s205MVIWf0
5: pandaboard-vanilla: BOOT: PASS: http://slexy.org/raw/s29oJ66Rqs
6: sdp4430: BOOT: PASS: http://slexy.org/raw/s20jLzW2cX
Awesome :). Thanks for catching this.
--
Regards
Re: [PATCH v10 2/8] ARM: l2c: Refactor the driver to use commit-like interface
On 01/02/2015 03:28 AM, Tomasz Figa wrote: On 02.01.2015 18:13, Tomasz Figa wrote: On 30.12.2014 23:51, Nishanth Menon wrote: Looks like the following also need addressing: data-save is called twice (once more after l2cof_init) l2c310_init_fns also needs l2c310_configure will be nice to use l2x0_data only after we kmemdup data in __l2c_init I'll check this. Thanks. Apparently the second save in __l2c_init() is not needed and it should have been removed. However it might be a good idea to actually do second save in l2c_enable() after l2c_configure() so that the values actually permitted by hardware and/or secure firmware are stored. l2c310_init_fns needs to be updated indeed. Hmm, apparently current patch already adds this (and I missed it reading it at first), so I'm not sure what's your concern about it. Uggh.. looks like I missed the same as well :( Sorry about that.. -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 2/8] ARM: l2c: Refactor the driver to use commit-like interface
On 01/02/2015 03:13 AM, Tomasz Figa wrote: However I'm not sure about your concern about using l2x0_data before kmemdup(). I don't see any code potentially doing this. It is not terribly important, but anyways [1] is what I had in mind.. [1] https://github.com/nmenon/linux-2.6-playground/commit/40f65e4707856f7b35872cf2225bf8beaf43552c#diff-56925866b8b499d75763bf5a1d7f1666L883 -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 2/8] ARM: l2c: Refactor the driver to use commit-like interface
On 12/30/2014 03:05 AM, Tomasz Figa wrote:
Thanks a lot for investigating this, even before I could look into
splitting this.
2014-12-30 3:23 GMT+09:00 Nishanth Menon n...@ti.com:
On 12/23/2014 04:48 AM, Marek Szyprowski wrote:
-static void l2c310_resume(void)
+static void l2c310_configure(void __iomem *base)
{
- void __iomem *base = l2x0_base;
+ unsigned revision;
- if (!(readl_relaxed(base + L2X0_CTRL) L2X0_CTRL_EN)) {
- unsigned revision;
-
- /* restore pl310 setup */
- writel_relaxed(l2x0_saved_regs.tag_latency,
-base + L310_TAG_LATENCY_CTRL);
- writel_relaxed(l2x0_saved_regs.data_latency,
-base + L310_DATA_LATENCY_CTRL);
- writel_relaxed(l2x0_saved_regs.filter_end,
-base + L310_ADDR_FILTER_END);
- writel_relaxed(l2x0_saved_regs.filter_start,
-base + L310_ADDR_FILTER_START);
-
- revision = readl_relaxed(base + L2X0_CACHE_ID)
- L2X0_CACHE_ID_RTL_MASK;
-
- if (revision = L310_CACHE_ID_RTL_R2P0)
- l2c_write_sec(l2x0_saved_regs.prefetch_ctrl, base,
- L310_PREFETCH_CTRL);
- if (revision = L310_CACHE_ID_RTL_R3P0)
- l2c_write_sec(l2x0_saved_regs.pwr_ctrl, base,
- L310_POWER_CTRL);
-
- l2c_enable(base, l2x0_saved_regs.aux_ctrl, 8);
-
- /* Re-enable full-line-of-zeros for Cortex-A9 */
- if (l2x0_saved_regs.aux_ctrl L310_AUX_CTRL_FULL_LINE_ZERO)
- set_auxcr(get_auxcr() | BIT(3) | BIT(2) | BIT(1));
- }
+ /* restore pl310 setup */
+ writel_relaxed(l2x0_saved_regs.tag_latency,
+base + L310_TAG_LATENCY_CTRL);
+ writel_relaxed(l2x0_saved_regs.data_latency,
+base + L310_DATA_LATENCY_CTRL);
+ writel_relaxed(l2x0_saved_regs.filter_end,
+base + L310_ADDR_FILTER_END);
+ writel_relaxed(l2x0_saved_regs.filter_start,
+base + L310_ADDR_FILTER_START);
+
^^ The above change broke AM437xx. Looks like the change causes the
following behavior difference on AM437x. For some reason, touching any
of the above 4 registers(even with the values read from the same
registers) causes AM437x to go beserk. Comment the 4 writes and we
reach shell. looks like l2c310_resume is not invoked prior to this
series. :(.. now that we reuse that logic to actually do programming,
we start to see the problem.
Hmm, but the thing is that .configure() should not be called if the
controller is already configured, i.e. L2X0_CTRL_EN in L2X0_CTRL is
set. Maybe I missed some check somewhere. Let me reread my code I
wrote quite a long time ago and make sure.
you have'nt missed a check here. it does indeed get called
l2c310_enable-l2c_enable - (if cache
disabled)-l2c_configure-l2c310_configure
It looks like a quirk of AM437x which remained hidden till this patch
exposed it. The original pl310 resume would have been invoked if
outer_resume was invoked (if my reading of code is correct), which in
the case of AM437x was never invoked during boot. By reusing the
restore code for resume (which in my opinion is a good change), we
seemed to have exposed quirky behavior on am437x. I started a thread
yesterday with hardware folks trying to understand the integration
aspects and explanation for this quirky behavior - unfortunately, with
holidays, I doubt I might get a quick answer fast. but the workaround
seems obvious - do not write to the the mentioned 4 registers on
am437x. This might make sense it arm,tag-latency , arm,data-latency,
arm,filter-ranges properties are not defined in OF.
one option might be to write only those registers that differ from
saved_registers (example: unmodified values dont need reprogramming).
Looks like the following also need addressing:
data-save is called twice (once more after l2cof_init)
l2c310_init_fns also needs l2c310_configure
will be nice to use l2x0_data only after we kmemdup data in __l2c_init
I'll check this.
Thanks.
if you'd like to split this up in pieces, [1] might be nice - will go
good to change the pl310, aurora etc in each chunks to enable better
review.
Thanks a lot, the split up version will be definitely useful. Just to
make sure, the parts look quite bisectable, but have you verified that
applying the changes one by one leave the L2 cache working on OMAP?
The split is not complete or properly done as I ignored aurora and was
trying to do it focussed on pl310 impacted code in a hurry, but yes,
the split is indeed bisectable for OMAP platforms. The intent was to
indicate the direction we should probably take and introduce the
refactor in stages. At the very least, it tends to be easier to debug
down to.
[1
Re: [PATCH v10 2/8] ARM: l2c: Refactor the driver to use commit-like interface
On 20:34-20141228, Tomasz Figa wrote: May I ask you (or anyone else working on OMAP) to try to figure out what the issue is? It is stopping L2 cache support for Exynos4 being http://slexy.org/view/s2BnzxVglj Took a register dump and compared - Got the same dump http://slexy.org/view/s21YRHpzeW . Basically, this implies: possible sequence change broke AM437x or some secure function call (which i have'nt dumped). Is it possible to break up this patch into easier patches? -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 2/8] ARM: l2c: Refactor the driver to use commit-like interface
On 12/23/2014 04:48 AM, Marek Szyprowski wrote:
-static void l2c310_resume(void)
+static void l2c310_configure(void __iomem *base)
{
- void __iomem *base = l2x0_base;
+ unsigned revision;
- if (!(readl_relaxed(base + L2X0_CTRL) L2X0_CTRL_EN)) {
- unsigned revision;
-
- /* restore pl310 setup */
- writel_relaxed(l2x0_saved_regs.tag_latency,
-base + L310_TAG_LATENCY_CTRL);
- writel_relaxed(l2x0_saved_regs.data_latency,
-base + L310_DATA_LATENCY_CTRL);
- writel_relaxed(l2x0_saved_regs.filter_end,
-base + L310_ADDR_FILTER_END);
- writel_relaxed(l2x0_saved_regs.filter_start,
-base + L310_ADDR_FILTER_START);
-
- revision = readl_relaxed(base + L2X0_CACHE_ID)
- L2X0_CACHE_ID_RTL_MASK;
-
- if (revision = L310_CACHE_ID_RTL_R2P0)
- l2c_write_sec(l2x0_saved_regs.prefetch_ctrl, base,
- L310_PREFETCH_CTRL);
- if (revision = L310_CACHE_ID_RTL_R3P0)
- l2c_write_sec(l2x0_saved_regs.pwr_ctrl, base,
- L310_POWER_CTRL);
-
- l2c_enable(base, l2x0_saved_regs.aux_ctrl, 8);
-
- /* Re-enable full-line-of-zeros for Cortex-A9 */
- if (l2x0_saved_regs.aux_ctrl L310_AUX_CTRL_FULL_LINE_ZERO)
- set_auxcr(get_auxcr() | BIT(3) | BIT(2) | BIT(1));
- }
+ /* restore pl310 setup */
+ writel_relaxed(l2x0_saved_regs.tag_latency,
+base + L310_TAG_LATENCY_CTRL);
+ writel_relaxed(l2x0_saved_regs.data_latency,
+base + L310_DATA_LATENCY_CTRL);
+ writel_relaxed(l2x0_saved_regs.filter_end,
+base + L310_ADDR_FILTER_END);
+ writel_relaxed(l2x0_saved_regs.filter_start,
+base + L310_ADDR_FILTER_START);
+
^^ The above change broke AM437xx. Looks like the change causes the
following behavior difference on AM437x. For some reason, touching any
of the above 4 registers(even with the values read from the same
registers) causes AM437x to go beserk. Comment the 4 writes and we
reach shell. looks like l2c310_resume is not invoked prior to this
series. :(.. now that we reuse that logic to actually do programming,
we start to see the problem.
one option might be to write only those registers that differ from
saved_registers (example: unmodified values dont need reprogramming).
Looks like the following also need addressing:
data-save is called twice (once more after l2cof_init)
l2c310_init_fns also needs l2c310_configure
will be nice to use l2x0_data only after we kmemdup data in __l2c_init
if you'd like to split this up in pieces, [1] might be nice - will go
good to change the pl310, aurora etc in each chunks to enable better
review.
[1]
https://github.com/nmenon/linux-2.6-playground/commits/temp/l2c-patch2-splitup
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: regression: OMAP4 (next-20141204) (bisect to: ARM: 8208/1: l2c: Refactor the driver to use commit-like)
On 12/23/2014 05:10 AM, Russell King - ARM Linux wrote: On Tue, Dec 23, 2014 at 12:00:00PM +0100, Marek Szyprowski wrote: I hope I did it right: https://lkml.org/lkml/2014/12/23/158 Please test, because I have no access to Omap hardware. Patch 1/8 looks like I'd expect it to. Nishanth, please test with your failing scenario, thanks. 3.19-rc1 1: am437x-sk: BOOT: PASS: http://slexy.org/raw/s2ARFeCcDp 2:am43xx-epos: BOOT: PASS: http://slexy.org/raw/s2Kzli0GYS 3: am43xx-gpevm: BOOT: PASS: http://slexy.org/raw/s2DMkJGmdF 4: pandaboard-es: BOOT: PASS: http://slexy.org/raw/s204jfptrr 5: pandaboard-vanilla: BOOT: PASS: http://slexy.org/raw/s2cbd82pMI 6:sdp4430: BOOT: PASS: http://slexy.org/raw/s21bzlzUNr TOTAL = 6 boards, Booted Boards = 6, No Boot boards = 0 against the patch series: (all am437x platforms fail) testing 1: am437x-sk: BOOT: FAIL: http://slexy.org/raw/s2yhDXyF7o 2:am43xx-epos: BOOT: FAIL: http://slexy.org/raw/s2m9cSdt55 3: am43xx-gpevm: BOOT: FAIL: http://slexy.org/raw/s2MqFFBuIl 4: pandaboard-es: BOOT: PASS: http://slexy.org/raw/s2XwggyB0a 5: pandaboard-vanilla: BOOT: PASS: http://slexy.org/raw/s25WDvtbob 6:sdp4430: BOOT: PASS: http://slexy.org/raw/s2gjynR1Co TOTAL = 6 boards, Booted Boards = 3, No Boot boards = 3 I am trying to understand what is different in AM437x except that it is a single A9 instead of OMAP4 style dual A9s. -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 2/8] ARM: l2c: Refactor the driver to use commit-like interface
On 12/23/2014 11:06 AM, Tony Lindgren wrote: * Marek Szyprowski m.szyprow...@samsung.com [141223 02:51]: From: Tomasz Figa t.f...@samsung.com Certain implementations of secure hypervisors (namely the one found on Samsung Exynos-based boards) do not provide access to individual L2C registers. This makes the .write_sec()-based interface insufficient and provoking ugly hacks. This patch is first step to make the driver not rely on availability of writes to individual registers. This is achieved by refactoring the driver to use a commit-like operation scheme: all register values are prepared first and stored in an instance of l2x0_regs struct and then a single callback is responsible to flush those values to the hardware. The first patch of the series applied things boot with no problem. But after applying this one I get the following on am437x: Unhandled fault: imprecise external abort (0xc06) at 0xb6f33884 Probably the same issue Nishanth mentioned. yep - just finished the bisect... came to the same conclusion.. c8c3a07fa6a8e9b27a1658e0d305b6f7e0fa068f is the first bad commit commit c8c3a07fa6a8e9b27a1658e0d305b6f7e0fa068f Author: Tomasz Figa t.f...@samsung.com Date: Tue Dec 23 11:48:30 2014 +0100 ARM: l2c: Refactor the driver to use commit-like interface Certain implementations of secure hypervisors (namely the one found on Samsung Exynos-based boards) do not provide access to individual L2C registers. This makes the .write_sec()-based interface insufficient and provoking ugly hacks. This patch is first step to make the driver not rely on availability of writes to individual registers. This is achieved by refactoring the driver to use a commit-like operation scheme: all register values are prepared first and stored in an instance of l2x0_regs struct and then a single callback is responsible to flush those values to the hardware. Signed-off-by: Tomasz Figa t.f...@samsung.com Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com :04 04 74c6c74a0dc0612d124cd759951adf2a1e4124ee 8082aabb474f8659231de744d87cd8dbd6dd79bb M arch $ git bisect log git bisect start # good: [97bf6af1f928216fd6c5a66e8a57bfa95a659672] Linux 3.19-rc1 git bisect good 97bf6af1f928216fd6c5a66e8a57bfa95a659672 # bad: [9afe195db6558621bd8bac379ed65ef121930684] ARM: dts: exynos4: Add nodes for L2 cache controller git bisect bad 9afe195db6558621bd8bac379ed65ef121930684 # bad: [0a89ef4dd870bbf692e30fef6c8182d7b8b42e17] ARM: l2c: Get outer cache .write_sec callback from mach_desc only if not NULL git bisect bad 0a89ef4dd870bbf692e30fef6c8182d7b8b42e17 # bad: [c8c3a07fa6a8e9b27a1658e0d305b6f7e0fa068f] ARM: l2c: Refactor the driver to use commit-like interface git bisect bad c8c3a07fa6a8e9b27a1658e0d305b6f7e0fa068f # good: [080ab387c653b8655dc1ee790658b618399db2aa] ARM: OMAP2+: use common l2cache initialization code git bisect good 080ab387c653b8655dc1ee790658b618399db2aa -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: regression: OMAP4 (next-20141204) (bisect to: ARM: 8208/1: l2c: Refactor the driver to use commit-like)
On Mon, Dec 22, 2014 at 11:04 AM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: Nishanth - can we push OMAP over to using the generic DT L2C initialisation (the one from init_IRQ in arch/arm/kernel/irq.c) and kill the SoC specific stuff in arch/arm/mach-omap2/omap4-common.c ? From what I can see, in the DT case, the only thing which is used there is the ioremap() to provide omap4_get_l2cache_base() with something to return. Everything else - the initialisation of the l2c_write_sec pointer, and the aux mask and values - can be specified via the machine_desc struct. I think this is what Marek proposed. I had requested for patches to be reposted with linux-omap in CC so that we can test and provide feedback. That only leaves the non-DT stuff to worry about this, and from what I understand, that's going to be removed soon. If we're going to keep the non-DT stuff, we should implement a new machine_desc hook for it instead of hijacking one of the existing callbacks. none of the PL310 support requires non-DT. PL310 is needed for OMAP4 and AM437x both of which are DT only. -- --- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: regression: OMAP4 (next-20141204) (bisect to: ARM: 8208/1: l2c: Refactor the driver to use commit-like)
On 10:13-20141205, Nishanth Menon wrote:
On 12/05/2014 10:10 AM, Nishanth Menon wrote:
next-20141204 fails to boot, but next-20141203 boots fine with
omap2plus_defconfig.
Panda-ES(4460):
https://github.com/nmenon/kernel-test-logs/blob/next-20141204/omap2plus_defconfig/pandaboard-es.txt
Panda(4430):
https://github.com/nmenon/kernel-test-logs/blob/next-20141204/omap2plus_defconfig/pandaboard-vanilla.txt
at the point of hang (JTAG):
pandaboard-es:
cpu0: http://slexy.org/view/s2eIFqkRd5
cpu1: http://slexy.org/view/s2Tysb6gpL
Case #1:
Disabling CPUIDLE allows boot to proceed. there does not seem to have
been any change in drivers/cpuidle and arch/arm/mach-omap2 w.r.t this.
Case #2: Reverting the following allows boot.
From next-20141204
10df7d5 ARM: 8211/1: l2c: Add support for overriding prefetch settings
revert this - boot still fails
d42ced0 ARM: 8210/1: l2c: Get outer cache .write_sec callback from
mach_desc only if not NULL
revert this - boot still fails
46b9af8 ARM: 8209/1: l2c: Add interface to ask hypervisor to configure L2C
revert this - boot still fails
c94e325 ARM: 8208/1: l2c: Refactor the driver to use commit-like
revert this - boot passed (first bad commit).
+ linux-samsung soc and updated Thomaz's mail ID (gmail now).
Spend a few mins trying to track this down and it does look like commit
c94e325 does a kmemdup for the data as part of l2x0_of_init-__l2c_init
This fails since the invocation is in early_init. doing it a bit later
as the following hack makes it work
diff --git a/arch/arm/mach-omap2/board-generic.c
b/arch/arm/mach-omap2/board-generic.c
index 608079a..0bc6bd9 100644
--- a/arch/arm/mach-omap2/board-generic.c
+++ b/arch/arm/mach-omap2/board-generic.c
@@ -170,12 +170,19 @@ static const char *const omap4_boards_compat[]
__initconst = {
NULL,
};
+
+static void tmp_init_irq(void)
+{
+ omap_l2_cache_init();
+ omap_gic_of_init();
+}
+
DT_MACHINE_START(OMAP4_DT, Generic OMAP4 (Flattened Device Tree))
.reserve= omap_reserve,
.smp= smp_ops(omap4_smp_ops),
.map_io = omap4_map_io,
.init_early = omap4430_init_early,
- .init_irq = omap_gic_of_init,
+ .init_irq = tmp_init_irq,
.init_machine = omap_generic_init,
.init_late = omap4430_init_late,
.init_time = omap4_local_timer_init,
diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
index 03cbb16..f97847d 100644
--- a/arch/arm/mach-omap2/io.c
+++ b/arch/arm/mach-omap2/io.c
@@ -627,7 +627,6 @@ void __init omap4430_init_early(void)
omap44xx_clockdomains_init();
omap44xx_hwmod_init();
omap_hwmod_init_postsetup();
- omap_l2_cache_init();
omap_clk_soc_init = omap4xxx_dt_clk_init;
}
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index e5948c5..0ca90db 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -848,8 +848,11 @@ static int __init __l2c_init(const struct l2c_init_data
*data,
* context from callers can access the structure.
*/
l2x0_data = kmemdup(data, sizeof(*data), GFP_KERNEL);
- if (!l2x0_data)
+ if (!l2x0_data) {
+ pr_err(%s no mem %d\n, __func__, sizeof(*data));
+ dump_stack();
return -ENOMEM;
+ }
/*
* Sanity check the aux values. aux_mask is the bits we preserve
@@ -1647,6 +1650,7 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
struct device_node *np;
struct resource res;
u32 cache_id, old_aux;
+ int r;
np = of_find_matching_node(NULL, l2x0_ids);
if (!np)
@@ -1693,6 +1697,8 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
else
cache_id = readl_relaxed(l2x0_base + L2X0_CACHE_ID);
- return __l2c_init(data, aux_val, aux_mask, cache_id);
+ r = __l2c_init(data, aux_val, aux_mask, cache_id);
+ pr_err(%s: %d\n, __func__, r);
+ return r;
}
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: regression: OMAP4 (next-20141204) (bisect to: ARM: 8208/1: l2c: Refactor the driver to use commit-like)
On 12/08/2014 06:22 AM, Russell King - ARM Linux wrote: On Mon, Dec 08, 2014 at 08:54:18PM +0900, Tomasz Figa wrote: 2014-12-06 1:23 GMT+09:00 Russell King - ARM Linux li...@arm.linux.org.uk: Given where we are in the cycle (-final likely this weekend) the only thing we can do right now is to drop the patch set; exynos (and mvebu) will have to wait another cycle until this patch set (hopefully in a revised form) can be merged. Or a fix could be queued on top of this. Since (I believe) this series has been queued for 3.19, we have 6 or 7 RC releases ahead, which could be used for the purpose of fixing things (as they are supposed to?). They were merged on 27th November, so they would've been in linux-next from about last Monday. Nishanth reported a failure on Friday, the For what ever it is worth, the l2c changes actually appeared on Thursday CST (next-20141204). Found the regression against next-20141203 tag and it took me a day to track it down (after looking at a few other regressions as well).. Anyways... -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: regression: OMAP4 (next-20141204) (bisect to: ARM: 8208/1: l2c: Refactor the driver to use commit-like)
On 12/05/2014 10:10 AM, Nishanth Menon wrote: next-20141204 fails to boot, but next-20141203 boots fine with omap2plus_defconfig. Panda-ES(4460): https://github.com/nmenon/kernel-test-logs/blob/next-20141204/omap2plus_defconfig/pandaboard-es.txt Panda(4430): https://github.com/nmenon/kernel-test-logs/blob/next-20141204/omap2plus_defconfig/pandaboard-vanilla.txt at the point of hang (JTAG): pandaboard-es: cpu0: http://slexy.org/view/s2eIFqkRd5 cpu1: http://slexy.org/view/s2Tysb6gpL Case #1: Disabling CPUIDLE allows boot to proceed. there does not seem to have been any change in drivers/cpuidle and arch/arm/mach-omap2 w.r.t this. Case #2: Reverting the following allows boot. From next-20141204 10df7d5 ARM: 8211/1: l2c: Add support for overriding prefetch settings revert this - boot still fails d42ced0 ARM: 8210/1: l2c: Get outer cache .write_sec callback from mach_desc only if not NULL revert this - boot still fails 46b9af8 ARM: 8209/1: l2c: Add interface to ask hypervisor to configure L2C revert this - boot still fails c94e325 ARM: 8208/1: l2c: Refactor the driver to use commit-like revert this - boot passed (first bad commit). + linux-samsung soc and updated Thomaz's mail ID (gmail now). -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM / OPP: Remove ARCH_HAS_OPP
On 06/06/2014 05:36 AM, Mark Brown wrote: [...] diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig index 0ba4826..524b027 100644 --- a/arch/arm/mach-omap2/Kconfig +++ b/arch/arm/mach-omap2/Kconfig @@ -12,7 +12,6 @@ config ARCH_OMAP3 bool TI OMAP3 depends on ARCH_MULTI_V7 select ARCH_OMAP2PLUS - select ARCH_HAS_OPP select ARM_CPU_SUSPEND if PM select OMAP_INTERCONNECT select PM_OPP if PM For OMAP portion: Acked-by: Nishanth Menon n...@ti.com -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
On 05/30/2014 01:55 PM, Mark Rutland wrote: On Fri, May 30, 2014 at 07:05:43PM +0100, Thomas Abraham wrote: Hi Mark, On Fri, May 30, 2014 at 6:38 PM, Mark Rutland mark.rutl...@arm.com wrote: Hi, Apologies for being somewhat late w.r.t. review on this. On Fri, May 30, 2014 at 10:01:17AM +0100, Thomas Abraham wrote: From: Thomas Abraham thomas...@samsung.com Add a new optional boost-frequency binding for specifying the frequencies usable in boost mode. Cc: Rob Herring robh...@kernel.org Cc: Pawel Moll pawel.m...@arm.com Cc: Mark Rutland mark.rutl...@arm.com Cc: Ian Campbell ijc+devicet...@hellion.org.uk Cc: Kumar Gala ga...@codeaurora.org Signed-off-by: Thomas Abraham thomas...@samsung.com Acked-by: Viresh Kumar viresh.ku...@linaro.org Acked-by: Nishanth Menon n...@ti.com Acked-by: Lukasz Majewski l.majew...@samsung.com --- .../devicetree/bindings/cpufreq/cpufreq-boost.txt | 38 1 file changed, 38 insertions(+) create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt new file mode 100644 index 000..63ed0fc --- /dev/null +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt @@ -0,0 +1,38 @@ +* Device tree binding for CPU boost frequency (aka over-clocking) + +Certain CPU's can be operated in optional 'boost' mode (or sometimes referred as Nit: CPUs (we're not greengrocers [1]) +overclocking) in which the CPU can operate at frequencies which are not +specified by the manufacturer as CPU's operating frequency. + +Optional Properties: +- boost-frequencies: list of frequencies in KHz to be used only in boost mode. + This list should be a subset of frequencies listed in operating-points + property. Refer to Documentation/devicetree/bindings/power/opp.txt for + details about operating-points property. What is 'boost-mode'? boost-mode activates additional one or more cpu clock speeds (which are not specified as operating frequency of the cpu by the manufacturer). The cpu is allowed to operate in these boost mode speeds when the boost mode is active. The boost mode speeds are usually undocumented. Some of the chip samples could be clocked in boost mode speeds and only such samples can be safely operated in boost mode. The mechanism of entry into and exit out of boost mode is outside the scope of this documentation. What are the limitations on boost frequencies? When is a CPU expected to go to these frequencies and for now long? When should I as a dt author place elements in boost-frequencies? I will add these details in the next revision of this patch. Cheers. Why are these in both operating-points and boost-frequencies? It'll be really easy to accidentally forget to mark something as a boost-frequency this way. Why not have a boost-points instead? Does boost-points mean a set of clock speeds which are not listed as part of operating-points property? If yes, that also is a possible implementation (it was implemented in one of the earlier version of this series). Could you confirm that you want the boost mode speeds to be exclusive of the speeds listed in operating-points? If these boost mode operating points are not generally advisable for use as the other operating-points are, then they should IMO been in an entirely separate property exclusive of (but in the same format as) the operating-points property, e.g. operating points = A B, C D; boost-points = E F; you are asking boost frequencies to remain for ever tied down to OPP style description. What we are trying to describe? What are my SoC's overclocked frequencies? That description can be used even in a system that does not use OPP style table (say ACPI based OPP tables or whatever acronymned table). Tying it down to operating points just because we have it today as a convenient description, is limiting. Further, if we decide to educate boost-frequencies to also indicate how long is it safe? That does indeed belong to boost-frequency description and not OPP description. Or if we decide to change operating-points description[1] in the future has an impact on boost-points description, when it should not have. Otherwise, without boost-mode support we have to parse the boost mode table to figure out which points to avoid. Or if someone typos a value That is OS usage of h/w description - yeah - in anycase, if OS has no ability to deal with boost-frequencies, it should skip it for sure. in either table we might go into a boost mode when we didn't want to! There are other ways to screw up device with dts typo. you could give a wrong voltage(extra 0?) and ensure you never use the chip ever again.. typos are dt bugs, we can do the best to write robust code to report them. [1] http://marc.info/?t=14005961858r=1w=2 -- Regards, Nishanth Menon
Re: [PATCH v6 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
On 05/30/2014 03:02 PM, Tomasz Figa wrote: On 30.05.2014 21:50, Nishanth Menon wrote: On 05/30/2014 01:55 PM, Mark Rutland wrote: On Fri, May 30, 2014 at 07:05:43PM +0100, Thomas Abraham wrote: Hi Mark, On Fri, May 30, 2014 at 6:38 PM, Mark Rutland mark.rutl...@arm.com wrote: Hi, Apologies for being somewhat late w.r.t. review on this. On Fri, May 30, 2014 at 10:01:17AM +0100, Thomas Abraham wrote: From: Thomas Abraham thomas...@samsung.com Add a new optional boost-frequency binding for specifying the frequencies usable in boost mode. Cc: Rob Herring robh...@kernel.org Cc: Pawel Moll pawel.m...@arm.com Cc: Mark Rutland mark.rutl...@arm.com Cc: Ian Campbell ijc+devicet...@hellion.org.uk Cc: Kumar Gala ga...@codeaurora.org Signed-off-by: Thomas Abraham thomas...@samsung.com Acked-by: Viresh Kumar viresh.ku...@linaro.org Acked-by: Nishanth Menon n...@ti.com Acked-by: Lukasz Majewski l.majew...@samsung.com --- .../devicetree/bindings/cpufreq/cpufreq-boost.txt | 38 1 file changed, 38 insertions(+) create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt new file mode 100644 index 000..63ed0fc --- /dev/null +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt @@ -0,0 +1,38 @@ +* Device tree binding for CPU boost frequency (aka over-clocking) + +Certain CPU's can be operated in optional 'boost' mode (or sometimes referred as Nit: CPUs (we're not greengrocers [1]) +overclocking) in which the CPU can operate at frequencies which are not +specified by the manufacturer as CPU's operating frequency. + +Optional Properties: +- boost-frequencies: list of frequencies in KHz to be used only in boost mode. + This list should be a subset of frequencies listed in operating-points + property. Refer to Documentation/devicetree/bindings/power/opp.txt for + details about operating-points property. What is 'boost-mode'? boost-mode activates additional one or more cpu clock speeds (which are not specified as operating frequency of the cpu by the manufacturer). The cpu is allowed to operate in these boost mode speeds when the boost mode is active. The boost mode speeds are usually undocumented. Some of the chip samples could be clocked in boost mode speeds and only such samples can be safely operated in boost mode. The mechanism of entry into and exit out of boost mode is outside the scope of this documentation. What are the limitations on boost frequencies? When is a CPU expected to go to these frequencies and for now long? When should I as a dt author place elements in boost-frequencies? I will add these details in the next revision of this patch. Cheers. Why are these in both operating-points and boost-frequencies? It'll be really easy to accidentally forget to mark something as a boost-frequency this way. Why not have a boost-points instead? Does boost-points mean a set of clock speeds which are not listed as part of operating-points property? If yes, that also is a possible implementation (it was implemented in one of the earlier version of this series). Could you confirm that you want the boost mode speeds to be exclusive of the speeds listed in operating-points? If these boost mode operating points are not generally advisable for use as the other operating-points are, then they should IMO been in an entirely separate property exclusive of (but in the same format as) the operating-points property, e.g. operating points = A B, C D; boost-points = E F; you are asking boost frequencies to remain for ever tied down to OPP style description. What we are trying to describe? What are my SoC's overclocked frequencies? That description can be used even in a system that does not use OPP style table (say ACPI based OPP tables or whatever acronymned table). Tying it down to operating points just because we have it today as a convenient description, is limiting. Further, if we decide to educate boost-frequencies to also indicate how long is it safe? That does indeed belong to boost-frequency description and not OPP description. Or if we decide to change operating-points description[1] in the future has an impact on boost-points description, when it should not have. Otherwise, without boost-mode support we have to parse the boost mode table to figure out which points to avoid. Or if someone typos a value That is OS usage of h/w description - yeah - in anycase, if OS has no ability to deal with boost-frequencies, it should skip it for sure. in either table we might go into a boost mode when we didn't want to! There are other ways to screw up device with dts typo. you could give a wrong voltage(extra 0?) and ensure you never use the chip ever again.. typos are dt bugs, we can do the best to write robust code
Re: [PATCH v6 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
On 05/30/2014 03:19 PM, Tomasz Figa wrote: On 30.05.2014 22:13, Nishanth Menon wrote: On 05/30/2014 03:02 PM, Tomasz Figa wrote: On 30.05.2014 21:50, Nishanth Menon wrote: On 05/30/2014 01:55 PM, Mark Rutland wrote: On Fri, May 30, 2014 at 07:05:43PM +0100, Thomas Abraham wrote: Hi Mark, On Fri, May 30, 2014 at 6:38 PM, Mark Rutland mark.rutl...@arm.com wrote: Hi, Apologies for being somewhat late w.r.t. review on this. On Fri, May 30, 2014 at 10:01:17AM +0100, Thomas Abraham wrote: From: Thomas Abraham thomas...@samsung.com Add a new optional boost-frequency binding for specifying the frequencies usable in boost mode. Cc: Rob Herring robh...@kernel.org Cc: Pawel Moll pawel.m...@arm.com Cc: Mark Rutland mark.rutl...@arm.com Cc: Ian Campbell ijc+devicet...@hellion.org.uk Cc: Kumar Gala ga...@codeaurora.org Signed-off-by: Thomas Abraham thomas...@samsung.com Acked-by: Viresh Kumar viresh.ku...@linaro.org Acked-by: Nishanth Menon n...@ti.com Acked-by: Lukasz Majewski l.majew...@samsung.com --- .../devicetree/bindings/cpufreq/cpufreq-boost.txt | 38 1 file changed, 38 insertions(+) create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt new file mode 100644 index 000..63ed0fc --- /dev/null +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt @@ -0,0 +1,38 @@ +* Device tree binding for CPU boost frequency (aka over-clocking) + +Certain CPU's can be operated in optional 'boost' mode (or sometimes referred as Nit: CPUs (we're not greengrocers [1]) +overclocking) in which the CPU can operate at frequencies which are not +specified by the manufacturer as CPU's operating frequency. + +Optional Properties: +- boost-frequencies: list of frequencies in KHz to be used only in boost mode. + This list should be a subset of frequencies listed in operating-points + property. Refer to Documentation/devicetree/bindings/power/opp.txt for + details about operating-points property. What is 'boost-mode'? boost-mode activates additional one or more cpu clock speeds (which are not specified as operating frequency of the cpu by the manufacturer). The cpu is allowed to operate in these boost mode speeds when the boost mode is active. The boost mode speeds are usually undocumented. Some of the chip samples could be clocked in boost mode speeds and only such samples can be safely operated in boost mode. The mechanism of entry into and exit out of boost mode is outside the scope of this documentation. What are the limitations on boost frequencies? When is a CPU expected to go to these frequencies and for now long? When should I as a dt author place elements in boost-frequencies? I will add these details in the next revision of this patch. Cheers. Why are these in both operating-points and boost-frequencies? It'll be really easy to accidentally forget to mark something as a boost-frequency this way. Why not have a boost-points instead? Does boost-points mean a set of clock speeds which are not listed as part of operating-points property? If yes, that also is a possible implementation (it was implemented in one of the earlier version of this series). Could you confirm that you want the boost mode speeds to be exclusive of the speeds listed in operating-points? If these boost mode operating points are not generally advisable for use as the other operating-points are, then they should IMO been in an entirely separate property exclusive of (but in the same format as) the operating-points property, e.g. operating points = A B, C D; boost-points = E F; you are asking boost frequencies to remain for ever tied down to OPP style description. What we are trying to describe? What are my SoC's overclocked frequencies? That description can be used even in a system that does not use OPP style table (say ACPI based OPP tables or whatever acronymned table). Tying it down to operating points just because we have it today as a convenient description, is limiting. Further, if we decide to educate boost-frequencies to also indicate how long is it safe? That does indeed belong to boost-frequency description and not OPP description. Or if we decide to change operating-points description[1] in the future has an impact on boost-points description, when it should not have. Otherwise, without boost-mode support we have to parse the boost mode table to figure out which points to avoid. Or if someone typos a value That is OS usage of h/w description - yeah - in anycase, if OS has no ability to deal with boost-frequencies, it should skip it for sure. in either table we might go into a boost mode when we didn't want to! There are other ways to screw up device with dts typo. you could give a wrong voltage(extra 0?) and ensure
Re: [PATCH v6 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
On 05/30/2014 03:45 PM, Rob Herring wrote: On Fri, May 30, 2014 at 3:33 PM, Nishanth Menon n...@ti.com wrote: On 05/30/2014 03:19 PM, Tomasz Figa wrote: On 30.05.2014 22:13, Nishanth Menon wrote: On 05/30/2014 03:02 PM, Tomasz Figa wrote: On 30.05.2014 21:50, Nishanth Menon wrote: On 05/30/2014 01:55 PM, Mark Rutland wrote: On Fri, May 30, 2014 at 07:05:43PM +0100, Thomas Abraham wrote: Hi Mark, On Fri, May 30, 2014 at 6:38 PM, Mark Rutland mark.rutl...@arm.com wrote: Hi, Apologies for being somewhat late w.r.t. review on this. [...] Why are these in both operating-points and boost-frequencies? It'll be really easy to accidentally forget to mark something as a boost-frequency this way. Why not have a boost-points instead? Does boost-points mean a set of clock speeds which are not listed as part of operating-points property? If yes, that also is a possible implementation (it was implemented in one of the earlier version of this series). Could you confirm that you want the boost mode speeds to be exclusive of the speeds listed in operating-points? If these boost mode operating points are not generally advisable for use as the other operating-points are, then they should IMO been in an entirely separate property exclusive of (but in the same format as) the operating-points property, e.g. operating points = A B, C D; boost-points = E F; you are asking boost frequencies to remain for ever tied down to OPP style description. What we are trying to describe? What are my SoC's overclocked frequencies? That description can be used even in a system that does not use OPP style table (say ACPI based OPP tables or whatever acronymned table). Tying it down to operating points just because we have it today as a convenient description, is limiting. Further, if we decide to educate boost-frequencies to also indicate how long is it safe? That does indeed belong to boost-frequency description and not OPP description. Or if we decide to change operating-points description[1] in the future has an impact on boost-points description, when it should not have. Otherwise, without boost-mode support we have to parse the boost mode table to figure out which points to avoid. Or if someone typos a value That is OS usage of h/w description - yeah - in anycase, if OS has no ability to deal with boost-frequencies, it should skip it for sure. in either table we might go into a boost mode when we didn't want to! There are other ways to screw up device with dts typo. you could give a wrong voltage(extra 0?) and ensure you never use the chip ever again.. typos are dt bugs, we can do the best to write robust code to report them. Typos are not the primary thing to worry about here. Adding boost frequencies to the list of primary operating points is flawed, because an OS that has no idea of boost mode will use them as normal operating points and this is not desired. That means we have an implementation bug in OS since it does not consider the complete hardware description that device tree is providing. An analogy will be a regulator compatible match being used but regulator-min-microvolt and regulator-max-microvolt being ignored by OS. No. The operating points bindings were defined far before this boost-frequency and so there is no requirement to support the latter. So, we dont add any new bindings ever again? /me blinks. *IF* we add a new property in the future, do we expect the new description to be supported in older kernel(which could not have known about it)? How far are we taking this ABI thing? Documentation/devicetree/bindings/ABI.txt states: 3) Bindings can be augmented, but the driver shouldn't break when given the old binding. ie. add additional properties, but don't change the meaning of an existing property. For drivers, default to the original behaviour when a newly added property is missing. we are not changing the meaning of existing property, we are augumenting it. You are changing the meaning of entries in that they can have additional data which changes their properties. If we accept the DT changes (as DT maintainers) and reject the kernel changes (as kernel maintainers), you would be left with a broken system. That is true and I agree. In my opinion, *IF* we are concerned about polluting operating-points description, why dont we enforce that the boost operating points should NOT be defined in the current operating-points description - and - just follow what Rob suggested and iMx already does - add such operating points from platform code. I believe I also said on this and other attempts to bandage up the existing OPP binding, that we should not change/append it, but define a new binding for OPPs that addresses this and other issues. Otherwise, I'm going to just NAK every incomplete OPP binding bandaid. Are we open to creating a completely mutually-exclusive binding and maintain the legacy one as legacy
Re: [PATCH v6 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
On 05/30/2014 03:43 PM, Tomasz Figa wrote: [...] OK, so you add overclocked frequencies to operating-points property, boost-frequency property, build a dtb, use it with a kernel that doesn't support boost-frequency and nicely overheat (and likely destroy) your board. I don't think this makes too much sense, sorry. Yes, that is unfortunately true as well :( Sigh. -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/2] cpufreq / OPP: Allow boost frequency to be looked up from device tree
On 05/29/2014 09:38 AM, Thomas Abraham wrote:
From: Thomas Abraham thomas...@samsung.com
Commit 6f19efc0 (cpufreq: Add boost frequency support in core) adds
support for CPU boost mode. This patch adds support for finding available
boost frequencies from device tree and marking them as usable in boost mode.
Cc: Nishanth Menon n...@ti.com
Cc: Lukasz Majewski l.majew...@samsung.com
Signed-off-by: Thomas Abraham thomas...@samsung.com
---
drivers/cpufreq/cpufreq_opp.c | 45
+
1 file changed, 45 insertions(+)
diff --git a/drivers/cpufreq/cpufreq_opp.c b/drivers/cpufreq/cpufreq_opp.c
index c0c6f4a..05fb115 100644
--- a/drivers/cpufreq/cpufreq_opp.c
+++ b/drivers/cpufreq/cpufreq_opp.c
@@ -19,6 +19,7 @@
#include linux/pm_opp.h
#include linux/rcupdate.h
#include linux/slab.h
+#include linux/of.h
/**
* dev_pm_opp_init_cpufreq_table() - create a cpufreq table for a device
@@ -51,6 +52,11 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
struct cpufreq_frequency_table *freq_table = NULL;
int i, max_opps, ret = 0;
unsigned long rate;
+#ifdef CONFIG_CPU_FREQ_BOOST_SW
+ struct cpufreq_frequency_table *ft;
+ int len, count;
+ u32 *boost_freqs = NULL;
+#endif
rcu_read_lock();
@@ -82,6 +88,45 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
*table = freq_table[0];
+#ifdef CONFIG_CPU_FREQ_BOOST_SW
+ if (!of_find_property(dev-of_node, boost-frequencies, len))
+ goto out;
+
+ if (!len || !IS_ALIGNED(len, sizeof(u32))) {
+ dev_err(dev, %s: invalid boost frequency\n, __func__);
+ ret = -EINVAL;
+ goto out;
+ }
+
+ boost_freqs = kmalloc(len, GFP_KERNEL);
+ if (!boost_freqs) {
+ dev_err(dev, %s: no memory for boost freq table\n, __func__);
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ count = len / sizeof(u32);
+ of_property_read_u32_array(dev-of_node, boost-frequencies,
+ boost_freqs, count);
+
+ for (i = 0; i count; i++) {
+ cpufreq_for_each_valid_entry(ft, *table) {
+ if (boost_freqs[i] == ft-frequency) {
+ ft-flags |= CPUFREQ_BOOST_FREQ;
+ pr_debug(%s: marked %d as boost frequency\n,
+ __func__, boost_freqs[i]);
+ break;
+ }
+ }
+
+ if (ft-frequency == CPUFREQ_TABLE_END)
+ dev_err(dev, %s: invalid boost frequency %d\n,
+ __func__, boost_freqs[i]);
+ }
+
+ kfree(boost_freqs);
+#endif
+
out:
rcu_read_unlock();
if (ret)
I suggest the following checkpatch --strict warnings should be fixed.
@@ -0,0 +1,10 @@
+CHECK: Alignment should match open parenthesis
+#65: FILE: drivers/cpufreq/cpufreq_opp.c:110:
++ of_property_read_u32_array(dev-of_node, boost-frequencies,
++ boost_freqs, count);
+CHECK: Alignment should match open parenthesis
+#72: FILE: drivers/cpufreq/cpufreq_opp.c:117:
++ pr_debug(%s: marked %d as boost
frequency\n,
++ __func__, boost_freqs[i]);
+If any of these errors are false positives, please report
+them to the maintainer, see CHECKPATCH in MAINTAINERS.
Otherwise,
For the entire series:
Acked-by: Nishanth Menon n...@ti.com
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] cpufreq / OPP: Allow boost frequency to be looked up from device tree
On 05/14/2014 01:24 AM, Viresh Kumar wrote: On 14 May 2014 11:39, Lukasz Majewski l.majew...@samsung.com wrote: I agree with Nishanth here, that point 1 (as described by Viresh at [*]) is a more scalable approach. The only reason why I wanted all that to be done at OPP level was to ensure if somebody else also needs it apart from cpufreq, they don't have to duplicate code and find it.. As it is present at a central place.. But if no other code is going to look for that, it may just be fine as is.. If we eventually have a need beyond cpufreq (say devfreq) with similar instances, then it makes sense to move it out to a generic place. Either way, code implementation/duplication is a OS problem - and should be looked at independent of the description in dts. If we feel the description is valid hardware description (which, personally, I do), then lets go to the next discussion point of where to put it - generic or cpufreq specific (here, I have no preference), and finally decide the implementation as necessary as a result of the description. -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
On Tue, May 13, 2014 at 8:03 PM, Thomas Abraham ta.oma...@gmail.com wrote: From: Thomas Abraham thomas...@samsung.com Add a new optional boost-frequency binding for specifying the frequencies usable in boost mode. Cc: Nishanth Menon n...@ti.com Cc: Lukasz Majewski l.majew...@samsung.com Cc: Rob Herring robh...@kernel.org Cc: Pawel Moll pawel.m...@arm.com Cc: Mark Rutland mark.rutl...@arm.com Cc: Ian Campbell ijc+devicet...@hellion.org.uk Cc: Kumar Gala ga...@codeaurora.org Signed-off-by: Thomas Abraham thomas...@samsung.com --- .../devicetree/bindings/cpufreq/cpufreq-boost.txt | 11 +++ 1 files changed, 11 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt new file mode 100644 index 000..d925e38 --- /dev/null +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt @@ -0,0 +1,11 @@ +* Device tree binding for CPU boost frequency (aka over-clocking) + +Certain CPU's can be operated in optional 'boost' mode (or sometimes referred as +overclocking) in which the CPU can operate in frequencies beyond the normal operate at? +operating conditions. normal operating conditions probably need a little bit of an expansion here perhaps? + +Optional Properties: +- boost-frequency: list of frequencies in KHz to be used only in boost mode. probably boost-frequencies? + This list should be a subset of frequencies listed in operating-points + property. Refer to Documentation/devicetree/bindings/power/opp.txt for + details about operating-points property. an example is expected here. personally, I think I understand the intent here, but as a hardware description, will let folks comment if it is acceptable. -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] cpufreq / OPP: Allow boost frequency to be looked up from device tree
On Tue, May 13, 2014 at 10:40 PM, Viresh Kumar viresh.ku...@linaro.org wrote:
On 14 May 2014 06:32, Thomas Abraham ta.oma...@gmail.com wrote:
[...]
+#ifdef CONFIG_CPU_FREQ_BOOST_SW
+ if (of_find_property(dev-of_node, boost-frequency, len)) {
Does this mean another block inside the cpu node ? Like this: ?
cpu@0 {
compatible = arm,cortex-a9;
reg = 0;
next-level-cache = L2;
operating-points =
/* kHzuV */
792000 110
396000 95
198000 85
;
boost-frequency =
792000
198000
;
};
I think it we might better add another field in the opp block as these
OPPs are rather boost one..
If we change the meaning to be:
operating-points =
/* kHzuV boost? */
792000 110 1
396000 950
We are adding a modifier to the OPP definition here and does imply
every platform which may or maynot require boost need to indicate
that - basically fails at least my least common description for a
generic definition. As I had indicated in other threads - we are back
to the discussion of additional properties to an OPP..
Option 1:
- describe modifiers separately (as in boost-frequencies) - that
operate based on data from opp table.
Option 2:
- keep adding to the description of opp as time goes by - every SoC
has it's own set of quirks that are needed for an OPP - I know that at
TI, we have certain OPPs that can only be enabled *if* custom
hardware driver is enabled, and similar stories. - yet another
example is enable the OPP if efuse X, bit Y is set.
Personally, looking at the various descriptions and bL, cpu-idle
states dependencies on OPPs, etc.. Option 2 is a non-scalable
approach.
[...]
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] PM / OPP: Remove cpufreq wrapper dependency on internal data organization
CPUFREQ custom functions for OPP (Operating Performance Points)
currently exist inside the OPP library. These custom functions currently
depend on internal data structures to pick up OPP information to create
the cpufreq table. For example, the cpufreq table is created precisely
in the same order of how OPP entries are stored inside the list implementation.
This kind of tight interdependency is purely artificial since the same
functionality can be achieved using the generic OPP functions
meant to do the same. This interdependency also limits the independent
modification of cpufreq and OPP library.
So use the generic dev_pm_opp_find_freq_ceil function that achieves the
table organization as we currently use.
As a result of this, we dont need to use the internal device_opp
structure anymore, and we hence we can switch over to rcu lock instead
of the mutex holding the internal list lock.
This breaking of dependency on internal data structure imposes no change
to usage of these.
NOTE: This change is a precursor to moving this cpufreq specific logic
out of the generic library into cpufreq.
Cc: Rafael J. Wysocki r...@rjwysocki.net
Cc: Viresh Kumar viresh.ku...@linaro.org
Cc: Kevin Hilman khil...@deeprootsystems.com
Cc: cpuf...@vger.kernel.org
Cc: linux-samsung-soc@vger.kernel.org
Cc: linux-o...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Nishanth Menon n...@ti.com
---
drivers/base/power/opp.c | 55 +++---
1 file changed, 28 insertions(+), 27 deletions(-)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index 2553867..38b43bb 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -617,53 +617,54 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_disable);
* the table if any of the mentioned functions have been invoked in the
interim.
*
* Locking: The internal device_opp and opp structures are RCU protected.
- * To simplify the logic, we pretend we are updater and hold relevant mutex
here
- * Callers should ensure that this function is *NOT* called under RCU
protection
- * or in contexts where mutex locking cannot be used.
+ * Since we just use the regular accessor functions to access the internal data
+ * structures, we use RCU read lock inside this function. As a result, users of
+ * this function DONOT need to use explicit locks for invoking.
*/
int dev_pm_opp_init_cpufreq_table(struct device *dev,
struct cpufreq_frequency_table **table)
{
- struct device_opp *dev_opp;
struct dev_pm_opp *opp;
- struct cpufreq_frequency_table *freq_table;
- int i = 0;
+ struct cpufreq_frequency_table *freq_table = NULL;
+ int i, max_opps, ret = 0;
+ unsigned long rate;
- /* Pretend as if I am an updater */
- mutex_lock(dev_opp_list_lock);
+ rcu_read_lock();
- dev_opp = find_device_opp(dev);
- if (IS_ERR(dev_opp)) {
- int r = PTR_ERR(dev_opp);
- mutex_unlock(dev_opp_list_lock);
- dev_err(dev, %s: Device OPP not found (%d)\n, __func__, r);
- return r;
+ max_opps = dev_pm_opp_get_opp_count(dev);
+ if (max_opps = 0) {
+ ret = max_opps ? max_opps : -ENODATA;
+ goto out;
}
- freq_table = kzalloc(sizeof(struct cpufreq_frequency_table) *
-(dev_pm_opp_get_opp_count(dev) + 1), GFP_KERNEL);
+ freq_table = kzalloc(sizeof(*freq_table) * (max_opps + 1), GFP_KERNEL);
if (!freq_table) {
- mutex_unlock(dev_opp_list_lock);
- dev_warn(dev, %s: Unable to allocate frequency table\n,
- __func__);
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto out;
}
- list_for_each_entry(opp, dev_opp-opp_list, node) {
- if (opp-available) {
- freq_table[i].driver_data = i;
- freq_table[i].frequency = opp-rate / 1000;
- i++;
+ for (i = 0, rate = 0; i max_opps; i++, rate++) {
+ /* find next rate */
+ opp = dev_pm_opp_find_freq_ceil(dev, rate);
+ if (IS_ERR(opp)) {
+ ret = PTR_ERR(opp);
+ goto out;
}
+ freq_table[i].driver_data = i;
+ freq_table[i].frequency = rate / 1000;
}
- mutex_unlock(dev_opp_list_lock);
freq_table[i].driver_data = i;
freq_table[i].frequency = CPUFREQ_TABLE_END;
*table = freq_table[0];
- return 0;
+out:
+ rcu_read_unlock();
+ if (ret)
+ kfree(freq_table);
+
+ return ret;
}
EXPORT_SYMBOL_GPL(dev_pm_opp_init_cpufreq_table);
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body
[PATCH 2/2] PM / OPP: Move cpufreq specific OPP functions out of generic OPP library
CPUFreq specific helper functions for OPP (Operating Performance Points)
now use generic OPP functions that allow CPUFreq to be be moved back
into CPUFreq framework. This allows for independent modifications
or future enhancements as needed isolated to just CPUFreq framework
alone.
Here, we just move relevant code and documentation to make this part of
CPUFreq infrastructure.
Cc: Rafael J. Wysocki r...@rjwysocki.net
Cc: Viresh Kumar viresh.ku...@linaro.org
Cc: Kevin Hilman khil...@deeprootsystems.com
Cc: cpuf...@vger.kernel.org
Cc: linux-samsung-soc@vger.kernel.org
Cc: linux-o...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Nishanth Menon n...@ti.com
---
List of files impacted by this change detected by:
# check if pm_opp.h header is needed anymore:
git grep dev_pm_opp_init_cpufreq_table .|cut -d ':' -f1|sort|uniq|xargs
grep dev_pm_opp |grep -v cpufreq
# check if any file is missing cpufreq.h header is needed anymore:
git grep dev_pm_opp_init_cpufreq_table .|cut -d ':' -f1|sort|uniq|xargs
grep cpufreq.h
Documentation/cpu-freq/core.txt | 29 +++
Documentation/power/opp.txt | 40 ++
drivers/base/power/opp.c| 92
drivers/cpufreq/Makefile|2 +
drivers/cpufreq/cpufreq_opp.c | 110 +++
include/linux/cpufreq.h | 21
include/linux/pm_opp.h | 20 ---
7 files changed, 167 insertions(+), 147 deletions(-)
create mode 100644 drivers/cpufreq/cpufreq_opp.c
diff --git a/Documentation/cpu-freq/core.txt b/Documentation/cpu-freq/core.txt
index 0060d76..70933ea 100644
--- a/Documentation/cpu-freq/core.txt
+++ b/Documentation/cpu-freq/core.txt
@@ -20,6 +20,7 @@ Contents:
-
1. CPUFreq core and interfaces
2. CPUFreq notifiers
+3. CPUFreq Table Generation with Operating Performance Point (OPP)
1. General Information
===
@@ -92,3 +93,31 @@ values:
cpu- number of the affected CPU
old- old frequency
new- new frequency
+
+3. CPUFreq Table Generation with Operating Performance Point (OPP)
+==
+For details about OPP, see Documentation/power/opp.txt
+
+dev_pm_opp_init_cpufreq_table - cpufreq framework typically is initialized with
+ cpufreq_frequency_table_cpuinfo which is provided with the list of
+ frequencies that are available for operation. This function provides
+ a ready to use conversion routine to translate the OPP layer's internal
+ information about the available frequencies into a format readily
+ providable to cpufreq.
+
+ WARNING: Do not use this function in interrupt context.
+
+ Example:
+soc_pm_init()
+{
+ /* Do things */
+ r = dev_pm_opp_init_cpufreq_table(dev, freq_table);
+ if (!r)
+ cpufreq_frequency_table_cpuinfo(policy, freq_table);
+ /* Do other things */
+}
+
+ NOTE: This function is available only if CONFIG_CPU_FREQ is enabled in
+ addition to CONFIG_PM_OPP.
+
+dev_pm_opp_free_cpufreq_table - Free up the table allocated by
dev_pm_opp_init_cpufreq_table
diff --git a/Documentation/power/opp.txt b/Documentation/power/opp.txt
index b8a907d..a9adad8 100644
--- a/Documentation/power/opp.txt
+++ b/Documentation/power/opp.txt
@@ -10,8 +10,7 @@ Contents
3. OPP Search Functions
4. OPP Availability Control Functions
5. OPP Data Retrieval Functions
-6. Cpufreq Table Generation
-7. Data Structures
+6. Data Structures
1. Introduction
===
@@ -72,7 +71,6 @@ operations until that OPP could be re-enabled if possible.
OPP library facilitates this concept in it's implementation. The following
operational functions operate only on available opps:
opp_find_freq_{ceil, floor}, dev_pm_opp_get_voltage, dev_pm_opp_get_freq,
dev_pm_opp_get_opp_count
-and dev_pm_opp_init_cpufreq_table
dev_pm_opp_find_freq_exact is meant to be used to find the opp pointer which
can then
be used for dev_pm_opp_enable/disable functions to make an opp available as
required.
@@ -96,10 +94,9 @@ using RCU read locks. The opp_find_freq_{exact,ceil,floor},
opp_get_{voltage, freq, opp_count} fall into this category.
opp_{add,enable,disable} are updaters which use mutex and implement it's own
-RCU locking mechanisms. dev_pm_opp_init_cpufreq_table acts as an updater and
uses
-mutex to implment RCU updater strategy. These functions should *NOT* be called
-under RCU locks and other contexts that prevent blocking functions in RCU or
-mutex operations from working.
+RCU locking mechanisms. These functions should *NOT* be called under RCU locks
+and other contexts that prevent blocking functions in RCU or mutex operations
+from working.
2. Initial OPP List Registration
[PATCH 0/2] PM / OPP: move cpufreq specific helpers out of OPP layer
CPUFreq usage of OPP should be independent of the ordering of type of data storage inside OPP layer. The current operations can equally be performed by generic operations. [RFC]: https://patchwork.kernel.org/patch/4100811/ Series based on: v3.15-rc1 Nishanth Menon (2): PM / OPP: Remove cpufreq wrapper dependency on internal data organization PM / OPP: Move cpufreq specific OPP functions out of generic OPP library Documentation/cpu-freq/core.txt | 29 +++ Documentation/power/opp.txt | 40 ++ drivers/base/power/opp.c| 91 drivers/cpufreq/Makefile|2 + drivers/cpufreq/cpufreq_opp.c | 110 +++ include/linux/cpufreq.h | 21 include/linux/pm_opp.h | 20 --- 7 files changed, 167 insertions(+), 146 deletions(-) create mode 100644 drivers/cpufreq/cpufreq_opp.c -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] PM / OPP: Remove cpufreq wrapper dependency on internal data organization
On Mon, May 5, 2014 at 9:23 AM, Viresh Kumar viresh.ku...@linaro.org wrote: What if opp is being added for some reason at the same time? I hope we can surely see some awkward results, maybe some NULL pointers invocations as well.. we wont - rcu operations ensure that. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc 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] PM / OPP: move cpufreq specific OPP functions out of generic OPP library
On Fri, May 2, 2014 at 12:22 AM, Viresh Kumar viresh.ku...@linaro.org wrote: On 2 May 2014 10:48, Nishanth Menon n...@ti.com wrote: On Thu, May 1, 2014 at 11:30 PM, Viresh Kumar viresh.ku...@linaro.org wrote: diff --git a/drivers/cpufreq/cpufreq_opp.c b/drivers/cpufreq/cpufreq_opp.c new file mode 100644 index 000..2602ff8 --- /dev/null +++ b/drivers/cpufreq/cpufreq_opp.c @@ -0,0 +1,102 @@ +/* + * Generic OPP Interface for CPUFREQ drivers + * + * Copyright (C) 2009-2014 Texas Instruments Incorporated. + * Nishanth Menon + * Romit Dasgupta + * Kevin Hilman + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ I hope you have just copy pasted routines to this file, and haven't done even the most minor modification in those, as its hard to review it. there is code replacement ofcourse - * the logic of walking down the list holding a mutex has been replaced with rcu locks, * instead of reading internal data structure and generating the list, use the existing search API that does exactly the same. * Documentation update for the same. Hmm, actually if I would have written this patch, then probably I would have done the same thing, but looking from the reviewers perspective, it would be much more easy if we can separate things into patches. So, maybe do these changes first in opp.c only and then finally a patch that just moves things around. Both are needed if you have to move the code out. functionally, both are equivalent That's an assumption and we never know when we might have screwed the code :) .. And so more careful review of those parts is required :) True. Will do the same as suggested for the formal series. Thanks for your feedback and review. Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH] PM / OPP: move cpufreq specific OPP functions out of generic OPP library
CPUFREQ specific functions for OPP (Operating Performance Points) can
be isolated to just cpufreq. This allows for independent modifications
as needed. The functionality desired by cpufreq can easily be provided
by existing functions and any future special handling needed for
cpufreq drivers can similarly be handled.
With this change the internal storage order of OPP entries are no
longer a limiting factor for how we need cpufreq table to look like.
Cc: Rafael J. Wysocki r...@rjwysocki.net
Cc: Viresh Kumar viresh.ku...@linaro.org
Cc: Kevin Hilman khil...@deeprootsystems.com
Cc: cpuf...@vger.kernel.org
Cc: linux-samsung-soc@vger.kernel.org
Cc: linux-o...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Nishanth Menon n...@ti.com
---
patch based on v3.15-rc1 tag.
Test log: OMAP5uEVM: http://slexy.org/view/s20WzLXI7K
drivers/base/power/opp.c | 91
drivers/cpufreq/Kconfig|5 ++
drivers/cpufreq/Makefile |2 +
drivers/cpufreq/arm_big_little.c |2 +-
drivers/cpufreq/arm_big_little_dt.c|2 +-
drivers/cpufreq/cpufreq-cpu0.c |3 +-
drivers/cpufreq/cpufreq_opp.c | 102
drivers/cpufreq/cpufreq_opp.h | 39
drivers/cpufreq/exynos5440-cpufreq.c |3 +-
drivers/cpufreq/imx6q-cpufreq.c|3 +-
drivers/cpufreq/omap-cpufreq.c |3 +-
drivers/cpufreq/vexpress-spc-cpufreq.c |2 +-
include/linux/pm_opp.h | 20 ---
13 files changed, 159 insertions(+), 118 deletions(-)
create mode 100644 drivers/cpufreq/cpufreq_opp.c
create mode 100644 drivers/cpufreq/cpufreq_opp.h
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index 2553867..d9e376a 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -15,7 +15,6 @@
#include linux/errno.h
#include linux/err.h
#include linux/slab.h
-#include linux/cpufreq.h
#include linux/device.h
#include linux/list.h
#include linux/rculist.h
@@ -596,96 +595,6 @@ int dev_pm_opp_disable(struct device *dev, unsigned long
freq)
}
EXPORT_SYMBOL_GPL(dev_pm_opp_disable);
-#ifdef CONFIG_CPU_FREQ
-/**
- * dev_pm_opp_init_cpufreq_table() - create a cpufreq table for a device
- * @dev: device for which we do this operation
- * @table: Cpufreq table returned back to caller
- *
- * Generate a cpufreq table for a provided device- this assumes that the
- * opp list is already initialized and ready for usage.
- *
- * This function allocates required memory for the cpufreq table. It is
- * expected that the caller does the required maintenance such as freeing
- * the table as required.
- *
- * Returns -EINVAL for bad pointers, -ENODEV if the device is not found,
-ENOMEM
- * if no memory available for the operation (table is not populated), returns 0
- * if successful and table is populated.
- *
- * WARNING: It is important for the callers to ensure refreshing their copy of
- * the table if any of the mentioned functions have been invoked in the
interim.
- *
- * Locking: The internal device_opp and opp structures are RCU protected.
- * To simplify the logic, we pretend we are updater and hold relevant mutex
here
- * Callers should ensure that this function is *NOT* called under RCU
protection
- * or in contexts where mutex locking cannot be used.
- */
-int dev_pm_opp_init_cpufreq_table(struct device *dev,
- struct cpufreq_frequency_table **table)
-{
- struct device_opp *dev_opp;
- struct dev_pm_opp *opp;
- struct cpufreq_frequency_table *freq_table;
- int i = 0;
-
- /* Pretend as if I am an updater */
- mutex_lock(dev_opp_list_lock);
-
- dev_opp = find_device_opp(dev);
- if (IS_ERR(dev_opp)) {
- int r = PTR_ERR(dev_opp);
- mutex_unlock(dev_opp_list_lock);
- dev_err(dev, %s: Device OPP not found (%d)\n, __func__, r);
- return r;
- }
-
- freq_table = kzalloc(sizeof(struct cpufreq_frequency_table) *
-(dev_pm_opp_get_opp_count(dev) + 1), GFP_KERNEL);
- if (!freq_table) {
- mutex_unlock(dev_opp_list_lock);
- dev_warn(dev, %s: Unable to allocate frequency table\n,
- __func__);
- return -ENOMEM;
- }
-
- list_for_each_entry(opp, dev_opp-opp_list, node) {
- if (opp-available) {
- freq_table[i].driver_data = i;
- freq_table[i].frequency = opp-rate / 1000;
- i++;
- }
- }
- mutex_unlock(dev_opp_list_lock);
-
- freq_table[i].driver_data = i;
- freq_table[i].frequency = CPUFREQ_TABLE_END;
-
- *table = freq_table[0];
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(dev_pm_opp_init_cpufreq_table
Re: [RFC PATCH] PM / OPP: move cpufreq specific OPP functions out of generic OPP library
On Thu, May 1, 2014 at 11:30 PM, Viresh Kumar viresh.ku...@linaro.org wrote: On 2 May 2014 06:36, Nishanth Menon n...@ti.com wrote: diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig index 1fbe11f..281ccfb 100644 --- a/drivers/cpufreq/Kconfig +++ b/drivers/cpufreq/Kconfig @@ -17,6 +17,11 @@ config CPU_FREQ if CPU_FREQ +config CPU_FREQ_PM_OPP + bool + depends on PM_OPP + default y + Don't need this ok. config CPU_FREQ_GOV_COMMON bool diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index 0dbb963..16eea68 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -1,5 +1,7 @@ # CPUfreq core obj-$(CONFIG_CPU_FREQ) += cpufreq.o freq_table.o +obj-$(CONFIG_CPU_FREQ_PM_OPP) += cpufreq_opp.o Just use: obj-$(CONFIG_PM_OPP) ok. + # CPUfreq stats obj-$(CONFIG_CPU_FREQ_STAT) += cpufreq_stats.o diff --git a/drivers/cpufreq/cpufreq_opp.c b/drivers/cpufreq/cpufreq_opp.c new file mode 100644 index 000..2602ff8 --- /dev/null +++ b/drivers/cpufreq/cpufreq_opp.c @@ -0,0 +1,102 @@ +/* + * Generic OPP Interface for CPUFREQ drivers + * + * Copyright (C) 2009-2014 Texas Instruments Incorporated. + * Nishanth Menon + * Romit Dasgupta + * Kevin Hilman + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ I hope you have just copy pasted routines to this file, and haven't done even the most minor modification in those, as its hard to review it. there is code replacement ofcourse - * the logic of walking down the list holding a mutex has been replaced with rcu locks, * instead of reading internal data structure and generating the list, use the existing search API that does exactly the same. * Documentation update for the same. Both are needed if you have to move the code out. functionally, both are equivalent +#include linux/slab.h Sure? That's it, nothing else required to compile this file independently? As a rule include all the files directly which might be required for compilation of this file and don't expect them to be included by some other header files indirectly. Alrite. will do, I try to trim the headers down to bare minimum, but will take care of it in the formal post. diff --git a/drivers/cpufreq/cpufreq_opp.h b/drivers/cpufreq/cpufreq_opp.h Two problems, driver may lie in arch/ as well, though we don't recommend them, secondly move these in cpufreq.h, don't need a header here for sure. There are none at the moment. ideally, we'd like to discourage folks putting cpufreq drivers in arch/ given the amount of effort you have undertaken in bringing them all here. but I have personally no strong objection to getting rid of the private header and using the generic cpufreq header. Otherwise, I assume you are ok with this approach and will post a formal patch by monday. Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/7] cpufreq: cpufreq-cpu0: allow use of optional boost mode frequencies
On Fri, Feb 7, 2014 at 11:19 PM, Thomas Abraham ta.oma...@gmail.com wrote: --- a/drivers/cpufreq/cpufreq-cpu0.c +++ b/drivers/cpufreq/cpufreq-cpu0.c @@ -195,6 +195,9 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev) transition_latency += ret * 1000; } + if (of_find_property(cpu_dev-of_node, boost-frequency, NULL)) + cpu0_cpufreq_driver.boost_supported = true; + might as well hide that under the opp api which returns a bool that says if boost is supported or not. that allows the parse to be isolated to a single location. This is specific to the cpufreq-cpu0 driver. Other cpufreq drivers might want to use other methods and additional constraints to determine if boost has to be enabled. struct cpufreq_driver::boost_supported is nothing CPU0 specifc - i am just saying that the same property boost-frequency parsed by in two places (dev_pm_opp_init_cpufreq_table and cpu0_cpufreq_probe) is a very bad idea. you could instead make int dev_pm_opp_init_cpufreq_table(struct device *dev, struct cpufreq_frequency_table **table) into: int dev_pm_opp_init_cpufreq_table(struct device *dev, struct cpufreq_frequency_table **table, bool *boost_supported); Then use boost_supported for anything of your driver's choice (including updating cpufreq_driver::boost_supported). That allows the property to be isolated into a single function, who can be modified without impacting other drivers at a later point in time. Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
On Fri, Feb 7, 2014 at 9:19 AM, Thomas Abraham ta.oma...@gmail.com wrote: From: Thomas Abraham thomas...@samsung.com Add a new optional boost-frequency binding for specifying the frequencies usable in boost mode. Cc: Nishanth Menon n...@ti.com Cc: Lukasz Majewski l.majew...@samsung.com Cc: Rob Herring robh...@kernel.org Cc: Pawel Moll pawel.m...@arm.com Cc: Mark Rutland mark.rutl...@arm.com Cc: Ian Campbell ijc+devicet...@hellion.org.uk Cc: Kumar Gala ga...@codeaurora.org Signed-off-by: Thomas Abraham thomas...@samsung.com --- Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt | 11 +++ 1 file changed, 11 insertions(+) create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt new file mode 100644 index 000..d925e38 --- /dev/null +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt @@ -0,0 +1,11 @@ +* Device tree binding for CPU boost frequency (aka over-clocking) + +Certain CPU's can be operated in optional 'boost' mode (or sometimes referred as +overclocking) in which the CPU can operate in frequencies beyond the normal +operating conditions. + +Optional Properties: +- boost-frequency: list of frequencies in KHz to be used only in boost mode. boost-frequencies? + This list should be a subset of frequencies listed in operating-points + property. Refer to Documentation/devicetree/bindings/power/opp.txt for + details about operating-points property. -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-pm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] PM / OPP: Allow boost frequency to be looked up from device tree
On Fri, Feb 7, 2014 at 9:19 AM, Thomas Abraham ta.oma...@gmail.com wrote:
From: Thomas Abraham thomas...@samsung.com
Commit 6f19efc0 (cpufreq: Add boost frequency support in core) adds
support for CPU boost mode. This patch adds support for finding available
boost frequencies from device tree and marking them as usable in boost mode.
Cc: Nishanth Menon n...@ti.com
Cc: Lukasz Majewski l.majew...@samsung.com
Signed-off-by: Thomas Abraham thomas...@samsung.com
---
drivers/base/power/opp.c | 34 +-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index fa41874..b636826 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -628,7 +628,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
struct device_opp *dev_opp;
struct dev_pm_opp *opp;
struct cpufreq_frequency_table *freq_table;
- int i = 0;
+ int i = 0, j, len, ret;
+ u32 *boost_freqs = NULL;
/* Pretend as if I am an updater */
mutex_lock(dev_opp_list_lock);
@@ -650,10 +651,35 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
return -ENOMEM;
}
+ if (of_find_property(dev-of_node, boost-frequency, len)) {
+ if (len == 0 || (len (sizeof(u32) - 1)) != 0) {
+ dev_err(dev, %s: invalid boost frequency\n,
__func__);
+ ret = -EINVAL;
+ goto err_boost;
+ }
+
+ boost_freqs = kzalloc(len, GFP_KERNEL);
+ if (!boost_freqs) {
+ dev_warn(dev, %s: no memory for boost freq table\n,
+ __func__);
+ ret = -ENOMEM;
+ goto err_boost;
+ }
+ of_property_read_u32_array(dev-of_node, boost-frequency,
+ boost_freqs, len / sizeof(u32));
+ }
+
list_for_each_entry(opp, dev_opp-opp_list, node) {
if (opp-available) {
freq_table[i].driver_data = i;
freq_table[i].frequency = opp-rate / 1000;
+ for (j = 0; j len / sizeof(u32) boost_freqs;
j++) {
+ if (boost_freqs[j] ==
freq_table[i].frequency) {
+ freq_table[i].driver_data =
+ CPUFREQ_BOOST_FREQ;
+ break;
+ }
+ }
What if any one of the boost_freqs are not contained in the enabled frequencies?
i++;
}
}
@@ -663,8 +689,14 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
freq_table[i].frequency = CPUFREQ_TABLE_END;
*table = freq_table[0];
+ kfree(boost_freqs);
return 0;
+
+err_boost:
+ kfree(freq_table);
+ mutex_unlock(dev_opp_list_lock);
+ return ret;
}
EXPORT_SYMBOL_GPL(dev_pm_opp_init_cpufreq_table);
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-pm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] PM / OPP: Allow boost frequency to be looked up from device tree
On 02/07/2014 09:38 AM, Thomas Abraham wrote:
[...]
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index fa41874..b636826 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -628,7 +628,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
struct device_opp *dev_opp;
struct dev_pm_opp *opp;
struct cpufreq_frequency_table *freq_table;
- int i = 0;
+ int i = 0, j, len, ret;
+ u32 *boost_freqs = NULL;
/* Pretend as if I am an updater */
mutex_lock(dev_opp_list_lock);
@@ -650,10 +651,35 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
return -ENOMEM;
}
+ if (of_find_property(dev-of_node, boost-frequency, len)) {
+ if (len == 0 || (len (sizeof(u32) - 1)) != 0) {
+ dev_err(dev, %s: invalid boost frequency\n,
__func__);
+ ret = -EINVAL;
+ goto err_boost;
+ }
+
+ boost_freqs = kzalloc(len, GFP_KERNEL);
+ if (!boost_freqs) {
+ dev_warn(dev, %s: no memory for boost freq
table\n,
+ __func__);
+ ret = -ENOMEM;
+ goto err_boost;
+ }
+ of_property_read_u32_array(dev-of_node, boost-frequency,
+ boost_freqs, len / sizeof(u32));
+ }
+
list_for_each_entry(opp, dev_opp-opp_list, node) {
if (opp-available) {
freq_table[i].driver_data = i;
freq_table[i].frequency = opp-rate / 1000;
+ for (j = 0; j len / sizeof(u32) boost_freqs;
j++) {
+ if (boost_freqs[j] ==
freq_table[i].frequency) {
+ freq_table[i].driver_data =
+ CPUFREQ_BOOST_FREQ;
+ break;
+ }
+ }
What if any one of the boost_freqs are not contained in the enabled
frequencies?
It is not used as a boost frequency because its corresponding voltage
is not known. If required a warning can be printed out for the same.
yes - that would be good, as it helps debug if there are developer
errors in dts.
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/7] cpufreq: cpufreq-cpu0: allow use of optional boost mode frequencies
On 02/07/2014 09:55 AM, Thomas Abraham wrote:
From: Thomas Abraham thomas...@samsung.com
Lookup for the optional boost-frequency property in cpu0 node and if
available, enable support for boost mode frequencies. The frequencies
usable in boost mode are determined while preparing the cpufreq table
from the list of operating points available.
In addition to this, enable the CPU_FREQ_BOOST_SW config option for
this driver by default. On platforms that do not support boost mode,
the boost mode frequencies will not be specified in cpu0 node and
hence the boost mode support will not be enabled. Since this driver
anyways depends on THERMAL config option, it is safe to enable
CPU_FREQ_BOOST_SW config option as default.
Cc: Shawn Guo shawn@linaro.org
Cc: Lukasz Majewski l.majew...@samsung.com
Signed-off-by: Thomas Abraham thomas...@samsung.com
---
Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt |2 ++
drivers/cpufreq/Kconfig|1 +
drivers/cpufreq/cpufreq-cpu0.c |3 +++
3 files changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
index f055515..60f321a 100644
--- a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
@@ -19,6 +19,8 @@ Optional properties:
- cooling-min-level:
- cooling-max-level:
Please refer to Documentation/devicetree/bindings/thermal/thermal.txt.
+- boost-frequency:
+ Please refer to
Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
Examples:
diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
index 4b029c0..52cc704 100644
--- a/drivers/cpufreq/Kconfig
+++ b/drivers/cpufreq/Kconfig
@@ -187,6 +187,7 @@ config GENERIC_CPUFREQ_CPU0
tristate Generic CPU0 cpufreq driver
depends on HAVE_CLK REGULATOR OF THERMAL CPU_THERMAL
select PM_OPP
+ select CPU_FREQ_BOOST_SW
help
This adds a generic cpufreq driver for CPU0 frequency management.
It supports both uniprocessor (UP) and symmetric multiprocessor (SMP)
diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index 0c12ffc..06539eb 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -195,6 +195,9 @@ static int cpu0_cpufreq_probe(struct platform_device
*pdev)
transition_latency += ret * 1000;
}
+ if (of_find_property(cpu_dev-of_node, boost-frequency, NULL))
+ cpu0_cpufreq_driver.boost_supported = true;
+
ret = cpufreq_register_driver(cpu0_cpufreq_driver);
if (ret) {
pr_err(failed register driver: %d\n, ret);
might as well hide that under the opp api which returns a bool that
says if boost is supported or not. that allows the parse to be
isolated to a single location.
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
On Fri, Feb 7, 2014 at 10:28 AM, Sudeep Holla sudeep.ho...@arm.com wrote: On 07/02/14 16:15, Sudeep Holla wrote: On 07/02/14 15:19, Thomas Abraham wrote: From: Thomas Abraham thomas...@samsung.com Add a new optional boost-frequency binding for specifying the frequencies usable in boost mode. Cc: Nishanth Menon n...@ti.com Cc: Lukasz Majewski l.majew...@samsung.com Cc: Rob Herring robh...@kernel.org Cc: Pawel Moll pawel.m...@arm.com Cc: Mark Rutland mark.rutl...@arm.com Cc: Ian Campbell ijc+devicet...@hellion.org.uk Cc: Kumar Gala ga...@codeaurora.org Signed-off-by: Thomas Abraham thomas...@samsung.com --- Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt | 11 +++ 1 file changed, 11 insertions(+) create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt new file mode 100644 index 000..d925e38 --- /dev/null +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt @@ -0,0 +1,11 @@ +* Device tree binding for CPU boost frequency (aka over-clocking) + +Certain CPU's can be operated in optional 'boost' mode (or sometimes referred as +overclocking) in which the CPU can operate in frequencies beyond the normal +operating conditions. + +Optional Properties: +- boost-frequency: list of frequencies in KHz to be used only in boost mode. + This list should be a subset of frequencies listed in operating-points + property. Refer to Documentation/devicetree/bindings/power/opp.txt for + details about operating-points property. Won't single entry for boost frequency suffice which would be the starting frequency in the boost range. IOW will there be OPP list with frequencies: A B C D, but only B and C are boost frequency. That seems little odd, unless it's some configuration chosen purely on software basis rather than hardware. For me B marks the beginning of over-clocking. Ah, I meant A B C D in the above example. Should'nt we let the SoC dts define that - traditionally, yes, but consider the following: A, B, C uses clk_parent X which describes B, C as overclocked. and say D uses clk_parent Y which is not over clocked, then you have the scenario that on the first look seems counter-intutive. Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla sudeep.ho...@arm.com wrote: Yes I thought of exactly similar clock setup, but was not convinced that it should be part of OPP. In that case it looks like we are trying to represent clock internals through some OPP bindings. And this series (rightly) does not make it an OPP behavior. instead all it does is to list the boost-frequencies and mark those in cpufreq table. the description is left to the dts and implementation to the clock drivers involved. Yes I think its counter-intuitive as it's visible to the userspace(list of frequencies and the boost parameters are exposed through sysfs) That will be a different problem - as currently every single frequency in the cpufreq list has ability to be marked as boost frequency - if userspace does not maintain that, then, IMHO, fix the userspace :D Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
On Fri, Feb 7, 2014 at 12:02 PM, Sudeep Holla sudeep.ho...@arm.com wrote: On 07/02/14 17:37, Nishanth Menon wrote: On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla sudeep.ho...@arm.com wrote: [...] Yes I think its counter-intuitive as it's visible to the userspace(list of frequencies and the boost parameters are exposed through sysfs) That will be a different problem - as currently every single frequency in the cpufreq list has ability to be marked as boost frequency - if userspace does not maintain that, then, IMHO, fix the userspace :D /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies gives the list of frequencies based on the state of the boost feature at anytime. Intuitively the list without boost shouldn't have any frequency above the range when it's enabled :), that's what I was referring to. So I am not talking about any issue with user-space maintenance. Fair enough - but i still think it has nothing to do with dt binding itself - and i think the discussion we've had should be good for the binding provided in this patch.. I hope.. if documentation needs a bit of better explanation to prevent a repeat of the same discussion at a later point of time, now might be a good time to add it in. Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
On Fri, Feb 7, 2014 at 11:37 AM, Nishanth Menon n...@ti.com wrote: On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla sudeep.ho...@arm.com wrote: Yes I thought of exactly similar clock setup, but was not convinced that it should be part of OPP. In that case it looks like we are trying to represent clock internals through some OPP bindings. And this series (rightly) does not make it an OPP behavior. instead all it does is to list the boost-frequencies and mark those in cpufreq table. the description is left to the dts and implementation to the clock drivers involved. One more thing, before I forget - currently dev_pm_opp_[init|free]_cpufreq_table is in drivers/base/power/opp.c - this probably should go away to drivers/cpufreq to keep opp.c independent of frameworks using it. i dont see any code that is introduced in the mentioned functions as being OPP behavior specific, instead, I consider them as cpufreq+opp behaviors, which this change fits into. Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/7] cpufreq: exynos: remove all exynos specific cpufreq driver support
On Wed, Feb 5, 2014 at 6:43 AM, Thomas Abraham ta.oma...@gmail.com wrote: Okay, thanks. Initially it looked like adding boost frequencies into operating-modes would convolute it but I guess I was wrong. So I will add support for looking up boost-frequencies property in dev_pm_opp_init_cpufreq_table function and mark the frequencies listed in this binding as CPUFREQ_BOOST_FREQ. I wonder if it is high time that we pop the cpufreq stuff out of opp.c other frameworks such as devfreq might choose to do things differently. Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] PM / OPP: Add support for 'boost' mode OPP
On 02/04/2014 03:41 AM, Thomas Abraham wrote:
From: Thomas Abraham thomas...@samsung.com
Commit 6f19efc0 (cpufreq: Add boost frequency support in core) adds
support for CPU boost mode. This patch adds support for finding available
boost OPPs from device tree and marking them as usable in boost mode.
Cc: Nishanth Menon n...@ti.com
Cc: Lukasz Majewski l.majew...@samsung.com
Signed-off-by: Thomas Abraham thomas...@samsung.com
---
Why is a cpufreq feature being pushed on to OPP library? you can very
well have a property boot-frequencies = a b c and be done with the
job.
I agree with Rob's comment that this is something we have to discuss
in wider add features to an OPP discussion[1].
[1] http://marc.info/?t=13910894641r=1w=2
drivers/base/power/opp.c | 69
+-
1 file changed, 56 insertions(+), 13 deletions(-)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index 2553867..de4d52d 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -62,6 +62,7 @@ struct dev_pm_opp {
struct list_head node;
bool available;
+ bool boost;
unsigned long rate;
unsigned long u_volt;
@@ -380,10 +381,12 @@ struct dev_pm_opp *dev_pm_opp_find_freq_floor(struct
device *dev,
EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_floor);
/**
- * dev_pm_opp_add() - Add an OPP table from a table definitions
+ * dev_pm_opp_add_flags() - Add an OPP to device OPP list with flags
* @dev: device for which we do this operation
* @freq:Frequency in Hz for this OPP
* @u_volt: Voltage in uVolts for this OPP
+ * @available: initial availability of the OPP with adding it to the
list.
+ * @boost: availability of this opp in controller's boost operating mode.
*
* This function adds an opp definition to the opp list and returns status.
* The opp is made available by default and it can be controlled using
@@ -395,7 +398,8 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_floor);
* that this function is *NOT* called under RCU protection or in contexts
where
* mutex cannot be locked.
*/
-int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long
u_volt)
+static int dev_pm_opp_add_flags(struct device *dev, unsigned long freq,
+ unsigned long u_volt, bool available, bool boost)
{
struct device_opp *dev_opp = NULL;
struct dev_pm_opp *opp, *new_opp;
@@ -441,7 +445,8 @@ int dev_pm_opp_add(struct device *dev, unsigned long
freq, unsigned long u_volt)
new_opp-dev_opp = dev_opp;
new_opp-rate = freq;
new_opp-u_volt = u_volt;
- new_opp-available = true;
+ new_opp-available = available;
+ new_opp-boost = boost;
/* Insert new OPP in order of increasing frequency */
head = dev_opp-opp_list;
@@ -462,6 +467,27 @@ int dev_pm_opp_add(struct device *dev, unsigned long
freq, unsigned long u_volt)
srcu_notifier_call_chain(dev_opp-head, OPP_EVENT_ADD, new_opp);
return 0;
}
+
+/**
+ * dev_pm_opp_add() - Add an OPP table from a table definitions
+ * @dev: device for which we do this operation
+ * @freq:Frequency in Hz for this OPP
+ * @u_volt: Voltage in uVolts for this OPP
+ *
+ * This function adds an opp definition to the opp list and returns status.
+ * The opp is made available by default and it can be controlled using
+ * dev_pm_opp_enable/disable functions.
+ *
+ * Locking: The internal device_opp and opp structures are RCU protected.
+ * Hence this function internally uses RCU updater strategy with mutex locks
+ * to keep the integrity of the internal data structures. Callers should
ensure
+ * that this function is *NOT* called under RCU protection or in contexts
where
+ * mutex cannot be locked.
+ */
+int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long
u_volt)
+{
+ return dev_pm_opp_add_flags(dev, freq, u_volt, true, false);
+}
EXPORT_SYMBOL_GPL(dev_pm_opp_add);
/**
@@ -651,7 +677,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
list_for_each_entry(opp, dev_opp-opp_list, node) {
if (opp-available) {
- freq_table[i].driver_data = i;
+ freq_table[i].driver_data =
+ opp-boost ? CPUFREQ_BOOST_FREQ : i;
freq_table[i].frequency = opp-rate / 1000;
i++;
}
@@ -701,19 +728,14 @@ struct srcu_notifier_head
*dev_pm_opp_get_notifier(struct device *dev)
}
#ifdef CONFIG_OF
-/**
- * of_init_opp_table() - Initialize opp table from device tree
- * @dev: device pointer used to lookup device OPPs.
- *
- * Register the initial OPP table with the OPP library for given device.
- */
-int of_init_opp_table(struct device *dev)
+static int of_parse_opp_table(struct device *dev, const char *prop_name
Re: [PATCH 1/2] PM / OPP: Add support for 'boost' mode OPP
On 02/04/2014 09:59 AM, Thomas Abraham wrote:
On Tue, Feb 4, 2014 at 8:45 PM, Nishanth Menon n...@ti.com wrote:
On 02/04/2014 03:41 AM, Thomas Abraham wrote:
From: Thomas Abraham thomas...@samsung.com
Commit 6f19efc0 (cpufreq: Add boost frequency support in core) adds
support for CPU boost mode. This patch adds support for finding available
boost OPPs from device tree and marking them as usable in boost mode.
Cc: Nishanth Menon n...@ti.com
Cc: Lukasz Majewski l.majew...@samsung.com
Signed-off-by: Thomas Abraham thomas...@samsung.com
---
Why is a cpufreq feature being pushed on to OPP library? you can very
well have a property boot-frequencies = a b c and be done with the
job.
The boost-opp was not limited to be a cpu/cpufreq only feature. Any
device (such as a bus) which has OPPs and if it can support optional
boost OPPs, can utilize this feature. The boost OPPs also require a
voltage to be associated with the frequency and hence the binding
boost-frequencies would not be suffice. The code changes in this patch
also do not have anything that is cpufreq specific.
if we have
operating-points = Fa Va
Fb Vb
Fc Vc
Fd Vd
;
boost-frequencies = Fc Fd;
you can easily pick up the voltages from the table.
The point being - there does not seem to be a need to modify the
existing definitions to introduce new OPP definitions.
a way to flip this over is to consider iMX6(see
arch/arm/mach-imx/mach-imx6q.) where OPP tuple Fd Vd can only be
enabled *iff* efuse register x has bit y set.
Would we want to introduce efuse offsets into OPP definitions? into
something like additional field definition 'efuse_controlled'?
I agree with Rob's comment that this is something we have to discuss
in wider add features to an OPP discussion[1].
Okay. I have read through the discussion in [1]. Thanks for the link.
Assuming that the current OPP tuple format will not change, I do not
feel the code changes in this patch will be hinder the outcome of the
discussion in [1].
The context of that discussion is to consider what is the data form we
consider OPP as? should we consider OPP as a data that is extensible
(as in phandle with properties that we add on) OR should we consider
key, value pair which provides a key (frequency) into another table
for other data (like efuse bit map, boost set etc..).
Both approaches I mentioned will work, and will take additional code
to make it happen. But having custom properties like this limits
extensibility - that is not scalable for other property definitions
we'd like to make in the future.
Regards,
Thomas.
[1] http://marc.info/?t=13910894641r=1w=2
drivers/base/power/opp.c | 69
+-
1 file changed, 56 insertions(+), 13 deletions(-)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index 2553867..de4d52d 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -62,6 +62,7 @@ struct dev_pm_opp {
struct list_head node;
bool available;
+ bool boost;
unsigned long rate;
unsigned long u_volt;
@@ -380,10 +381,12 @@ struct dev_pm_opp *dev_pm_opp_find_freq_floor(struct
device *dev,
EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_floor);
/**
- * dev_pm_opp_add() - Add an OPP table from a table definitions
+ * dev_pm_opp_add_flags() - Add an OPP to device OPP list with flags
* @dev: device for which we do this operation
* @freq:Frequency in Hz for this OPP
* @u_volt: Voltage in uVolts for this OPP
+ * @available: initial availability of the OPP with adding it to the
list.
+ * @boost: availability of this opp in controller's boost operating mode.
*
* This function adds an opp definition to the opp list and returns status.
* The opp is made available by default and it can be controlled using
@@ -395,7 +398,8 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_floor);
* that this function is *NOT* called under RCU protection or in contexts
where
* mutex cannot be locked.
*/
-int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long
u_volt)
+static int dev_pm_opp_add_flags(struct device *dev, unsigned long freq,
+ unsigned long u_volt, bool available, bool boost)
{
struct device_opp *dev_opp = NULL;
struct dev_pm_opp *opp, *new_opp;
@@ -441,7 +445,8 @@ int dev_pm_opp_add(struct device *dev, unsigned long
freq, unsigned long u_volt)
new_opp-dev_opp = dev_opp;
new_opp-rate = freq;
new_opp-u_volt = u_volt;
- new_opp-available = true;
+ new_opp-available = available;
+ new_opp-boost = boost;
/* Insert new OPP in order of increasing frequency */
head = dev_opp-opp_list;
@@ -462,6 +467,27 @@ int dev_pm_opp_add(struct device *dev, unsigned long
freq, unsigned long u_volt)
srcu_notifier_call_chain(dev_opp-head, OPP_EVENT_ADD, new_opp);
return 0
Re: [PATCH V3 0/6] cpufreq: suspend early/resume late
On 11/25/2013 08:11 AM, Viresh Kumar wrote:
This patchset adds cpufreq callbacks to dpm_{suspend|resume}() for handling
suspend/resume of cpufreq governors and core. This is required for early
suspend
and late resume of governors and cpufreq core.
There are multiple problems that are fixed by this patch:
- Nishanth Menon (TI) found an interesting problem on his platform, OMAP. His
board
wasn't working well with suspend/resume as calls for removing non-boot CPUs
was turning out into a call to drivers -target() which then tries to play
with regulators. But regulators and their I2C bus were already suspended and
this resulted in a failure. Many platforms have such problems, samsung,
tegra,
etc.. They solved it with driver specific PM notifiers where they used to
disable their driver's -target() routine. Most of these are updated in this
patchset to use new infrastructure.
- Lan Tianyu (Intel) Jinhyuk Choi (Broadcom) found another issue where
tunables configuration for clusters/sockets with non-boot CPUs was getting
lost after suspend/resume, as we were notifying governors with
CPUFREQ_GOV_POLICY_EXIT on removal of the last cpu for that policy and so
deallocating memory for tunables. This is also fixed with this patch as
don't
allow any operation on Governors during suspend/resume now.
So to solve these issues we introduce early suspend and late resume callbacks
which would remove need of cpufreq drivers to implement PM notifiers to
disable
transition after suspend and before resume.
@Nishanth: Can you please test V2 as well and confirm that suspend_noirq()
doesn't work for you. I am sure it will not, but would be better if you
confirm
that.
Viresh Kumar (6):
cpufreq: suspend governors on system suspend/hibernate
cpufreq: call driver's suspend/resume for each policy
patches 1-2,
Tested-by: Nishanth Menon n...@ti.com
http://pastebin.mozilla.org/3670932
Prior to these two patches: http://pastebin.mozilla.org/3670933
cpufreq driver used: cpufreq_cpu0
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] PM / OPP: rename functions to dev_pm_opp*
On 12:44-20130920, Viresh Kumar wrote:
On 20 September 2013 02:33, Nishanth Menon n...@ti.com wrote:
opp_get_opp_count
opp_find_freq_exact
opp_init_cpufreq_table
opp_free_cpufreq_table
The only problem I see is that routines names for few of them are getting
really long now.. Otherwise not much I could find...
I am open to suggestions if any one feels we can improve this better.
Though you had following changes, which you could have avoided in this
hard to review patchset:
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
new_opp = [-kzalloc(sizeof(struct
opp),-]{+kzalloc(sizeof(*new_opp),+} GFP_KERNEL);
new_opp = [-kmalloc(sizeof(struct
opp),-]{+kmalloc(sizeof(*new_opp),+} GFP_KERNEL);
It is almost impossible to catch these with naked eyes for such long
patches.. I took help of --word-diff though :)
I believe that change was from Patch #2[1]
yes, you are right, I had squashed this patch in to squelch checkpatch
warnings:
CHECK: Prefer kzalloc(sizeof(*new_opp)...) over kzalloc(sizeof(struct
dev_pm_opp)...)
#177: FILE: drivers/base/power/opp.c:406:
+ new_opp = kzalloc(sizeof(struct dev_pm_opp), GFP_KERNEL);
CHECK: Prefer kmalloc(sizeof(*new_opp)...) over kmalloc(sizeof(struct
dev_pm_opp)...)
#191: FILE: drivers/base/power/opp.c:495:
+ new_opp = kmalloc(sizeof(struct dev_pm_opp), GFP_KERNEL);
I had added a comment:
Minor checkpatch warning fixes as a result of this change was fixed as
well.
Would you suggest I split the change off to a separate patch or improve
the comment a little more?
If no one else sees these as problems then feel free to add my:
Acked-by: Viresh Kumar viresh.ku...@linaro.org
[1] https://patchwork.kernel.org/patch/2913551/
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] PM / OPP: rename functions to dev_pm_opp*
On 09/20/2013 11:51 AM, Viresh Kumar wrote: On 20 September 2013 18:08, Nishanth Menon n...@ti.com wrote: I am open to suggestions if any one feels we can improve this better. I didn't really had one.. I thought of pm_opp** instead of dev_pm_opp** though.. I had proposed this earlier, however, had gone with Rafael's suggestion [1] to have the right context to the usage. I believe that change was from Patch #2[1] Yeah.. I just replied on a single patch :) yes, you are right, I had squashed this patch in to squelch checkpatch warnings: I see.. I had added a comment: Minor checkpatch warning fixes as a result of this change was fixed as well. I really missed that.. no problems.. Would you suggest I split the change off to a separate patch or improve the comment a little more? don't really know, maybe leave those as is and let checkpatch warn you.. that is fair as well. i can split it off seperately in the next respin. [1] http://marc.info/?l=linaro-kernelm=137645747511725w=2 -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] PM / OPP: rename data structures to dev_pm equivalents
Since Operating Performance Points(OPP) data structures are specific
to device specific power management, be specific and rename opp_* data
structures in OPP library with dev_pm_opp_* equivalent.
Impacted structures are:
struct opp
enum opp_event
Minor checkpatch warning fixes as a result of this change was fixed as
well.
Reported-by: Randy Dunlap rdun...@infradead.org
Signed-off-by: Nishanth Menon n...@ti.com
---
Documentation/power/opp.txt |4 ++--
arch/arm/mach-omap2/pm.c |2 +-
drivers/base/power/opp.c | 41 ++
drivers/cpufreq/cpufreq-cpu0.c |4 ++--
drivers/cpufreq/exynos5440-cpufreq.c |2 +-
drivers/cpufreq/imx6q-cpufreq.c |4 ++--
drivers/cpufreq/omap-cpufreq.c |2 +-
drivers/devfreq/devfreq.c|9
drivers/devfreq/exynos/exynos4_bus.c |6 ++---
drivers/devfreq/exynos/exynos5_bus.c |6 ++---
include/linux/devfreq.h |4 ++--
include/linux/opp.h | 29 +---
12 files changed, 60 insertions(+), 53 deletions(-)
diff --git a/Documentation/power/opp.txt b/Documentation/power/opp.txt
index 185367b..7f67e3d 100644
--- a/Documentation/power/opp.txt
+++ b/Documentation/power/opp.txt
@@ -358,14 +358,14 @@ accessed by various functions as described above.
However, the structures
representing the actual OPPs and domains are internal to the OPP library itself
to allow for suitable abstraction reusable across systems.
-struct opp - The internal data structure of OPP library which is used to
+struct dev_pm_opp - The internal data structure of OPP library which is used to
represent an OPP. In addition to the freq, voltage, availability
information, it also contains internal book keeping information required
for the OPP library to operate on. Pointer to this structure is
provided back to the users such as SoC framework to be used as a
identifier for OPP in the interactions with OPP layer.
- WARNING: The struct opp pointer should not be parsed or modified by the
+ WARNING: The struct dev_pm_opp pointer should not be parsed or modified
by the
users. The defaults of for an instance is populated by dev_pm_opp_add,
but the
availability of the OPP can be modified by dev_pm_opp_enable/disable
functions.
diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index 937744c..92901bd 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -131,7 +131,7 @@ static int __init omap2_set_init_voltage(char *vdd_name,
char *clk_name,
{
struct voltagedomain *voltdm;
struct clk *clk;
- struct opp *opp;
+ struct dev_pm_opp *opp;
unsigned long freq, bootup_volt;
struct device *dev;
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index 00c6a44..693e14a 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -42,7 +42,7 @@
*/
/**
- * struct opp - Generic OPP description structure
+ * struct dev_pm_opp - Generic OPP description structure
* @node: opp list node. The nodes are maintained throughout the lifetime
* of boot. It is expected only an optimal set of OPPs are
* added to the library by the SoC framework.
@@ -59,7 +59,7 @@
*
* This structure stores the OPP information for a given device.
*/
-struct opp {
+struct dev_pm_opp {
struct list_head node;
bool available;
@@ -150,9 +150,9 @@ static struct device_opp *find_device_opp(struct device
*dev)
* prior to unlocking with rcu_read_unlock() to maintain the integrity of the
* pointer.
*/
-unsigned long dev_pm_opp_get_voltage(struct opp *opp)
+unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
{
- struct opp *tmp_opp;
+ struct dev_pm_opp *tmp_opp;
unsigned long v = 0;
tmp_opp = rcu_dereference(opp);
@@ -180,9 +180,9 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_get_voltage);
* prior to unlocking with rcu_read_unlock() to maintain the integrity of the
* pointer.
*/
-unsigned long dev_pm_opp_get_freq(struct opp *opp)
+unsigned long dev_pm_opp_get_freq(struct dev_pm_opp *opp)
{
- struct opp *tmp_opp;
+ struct dev_pm_opp *tmp_opp;
unsigned long f = 0;
tmp_opp = rcu_dereference(opp);
@@ -209,7 +209,7 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_get_freq);
int dev_pm_opp_get_opp_count(struct device *dev)
{
struct device_opp *dev_opp;
- struct opp *temp_opp;
+ struct dev_pm_opp *temp_opp;
int count = 0;
dev_opp = find_device_opp(dev);
@@ -254,11 +254,12 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_get_opp_count);
* under the locked area. The pointer returned must be used prior to unlocking
* with rcu_read_unlock() to maintain the integrity of the pointer.
*/
-struct opp *dev_pm_opp_find_freq_exact(struct device *dev, unsigned long freq
[PATCH 0/3] PM / OPP: rename to dev_pm_opp * equivalents
Hi, Based on [1], Randy rightly pointed out that OPP functions and data structures could be a bit specific to Power management. So, the following series is based on v3.12-rc1 tag. If folks like it broken in a different way, I am open to suggestions. Nishanth Menon (3): PM / OPP: rename functions to dev_pm_opp* PM / OPP: rename data structures to dev_pm equivalents PM / OPP: rename header to linux/pm_opp.h Documentation/power/opp.txt | 108 arch/arm/mach-imx/mach-imx6q.c |4 +- arch/arm/mach-omap2/board-omap3beagle.c | 10 +-- arch/arm/mach-omap2/omap-pm.h |2 +- arch/arm/mach-omap2/opp.c |6 +- arch/arm/mach-omap2/pm.c|8 +- drivers/base/power/opp.c| 115 - drivers/cpufreq/arm_big_little.c|8 +- drivers/cpufreq/arm_big_little_dt.c |2 +- drivers/cpufreq/cpufreq-cpu0.c | 24 +++--- drivers/cpufreq/exynos5440-cpufreq.c| 17 ++-- drivers/cpufreq/imx6q-cpufreq.c | 26 +++--- drivers/cpufreq/omap-cpufreq.c | 12 +-- drivers/devfreq/devfreq.c | 25 +++--- drivers/devfreq/exynos/exynos4_bus.c| 29 +++ drivers/devfreq/exynos/exynos5_bus.c| 28 +++ include/linux/devfreq.h |6 +- include/linux/opp.h | 134 - include/linux/pm_opp.h | 139 +++ 19 files changed, 357 insertions(+), 346 deletions(-) delete mode 100644 include/linux/opp.h create mode 100644 include/linux/pm_opp.h -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] PM / OPP: rename functions to dev_pm_opp*
Since Operating Performance Points(OPP) functions are specific to
device specific power management, be specific and rename opp_*
accessors in OPP library with dev_pm_opp_* equivalent.
Impacted functions are:
opp_get_voltage
opp_get_freq
opp_get_opp_count
opp_find_freq_exact
opp_find_freq_floor
opp_find_freq_ceil
opp_add
opp_enable
opp_disable
opp_get_notifier
opp_init_cpufreq_table
opp_free_cpufreq_table
Reported-by: Randy Dunlap rdun...@infradead.org
Signed-off-by: Nishanth Menon n...@ti.com
---
Documentation/power/opp.txt | 102 +++
arch/arm/mach-imx/mach-imx6q.c |2 +-
arch/arm/mach-omap2/board-omap3beagle.c |8 +--
arch/arm/mach-omap2/opp.c |4 +-
arch/arm/mach-omap2/pm.c|4 +-
drivers/base/power/opp.c| 82 -
drivers/cpufreq/arm_big_little.c|6 +-
drivers/cpufreq/cpufreq-cpu0.c | 18 +++---
drivers/cpufreq/exynos5440-cpufreq.c| 13 ++--
drivers/cpufreq/imx6q-cpufreq.c | 20 +++---
drivers/cpufreq/omap-cpufreq.c |8 +--
drivers/devfreq/devfreq.c | 14 ++---
drivers/devfreq/exynos/exynos4_bus.c| 21 ---
drivers/devfreq/exynos/exynos5_bus.c| 18 +++---
include/linux/opp.h | 50 +++
15 files changed, 187 insertions(+), 183 deletions(-)
diff --git a/Documentation/power/opp.txt b/Documentation/power/opp.txt
index 425c51d..185367b 100644
--- a/Documentation/power/opp.txt
+++ b/Documentation/power/opp.txt
@@ -71,14 +71,14 @@ operations until that OPP could be re-enabled if possible.
OPP library facilitates this concept in it's implementation. The following
operational functions operate only on available opps:
-opp_find_freq_{ceil, floor}, opp_get_voltage, opp_get_freq, opp_get_opp_count
-and opp_init_cpufreq_table
+opp_find_freq_{ceil, floor}, dev_pm_opp_get_voltage, dev_pm_opp_get_freq,
dev_pm_opp_get_opp_count
+and dev_pm_opp_init_cpufreq_table
-opp_find_freq_exact is meant to be used to find the opp pointer which can then
-be used for opp_enable/disable functions to make an opp available as required.
+dev_pm_opp_find_freq_exact is meant to be used to find the opp pointer which
can then
+be used for dev_pm_opp_enable/disable functions to make an opp available as
required.
WARNING: Users of OPP library should refresh their availability count using
-get_opp_count if opp_enable/disable functions are invoked for a device, the
+get_opp_count if dev_pm_opp_enable/disable functions are invoked for a device,
the
exact mechanism to trigger these or the notification mechanism to other
dependent subsystems such as cpufreq are left to the discretion of the SoC
specific framework which uses the OPP library. Similar care needs to be taken
@@ -96,24 +96,24 @@ using RCU read locks. The opp_find_freq_{exact,ceil,floor},
opp_get_{voltage, freq, opp_count} fall into this category.
opp_{add,enable,disable} are updaters which use mutex and implement it's own
-RCU locking mechanisms. opp_init_cpufreq_table acts as an updater and uses
+RCU locking mechanisms. dev_pm_opp_init_cpufreq_table acts as an updater and
uses
mutex to implment RCU updater strategy. These functions should *NOT* be called
under RCU locks and other contexts that prevent blocking functions in RCU or
mutex operations from working.
2. Initial OPP List Registration
-The SoC implementation calls opp_add function iteratively to add OPPs per
+The SoC implementation calls dev_pm_opp_add function iteratively to add OPPs
per
device. It is expected that the SoC framework will register the OPP entries
optimally- typical numbers range to be less than 5. The list generated by
registering the OPPs is maintained by OPP library throughout the device
operation. The SoC framework can subsequently control the availability of the
-OPPs dynamically using the opp_enable / disable functions.
+OPPs dynamically using the dev_pm_opp_enable / disable functions.
-opp_add - Add a new OPP for a specific domain represented by the device
pointer.
+dev_pm_opp_add - Add a new OPP for a specific domain represented by the device
pointer.
The OPP is defined using the frequency and voltage. Once added, the OPP
is assumed to be available and control of it's availability can be done
- with the opp_enable/disable functions. OPP library internally stores
+ with the dev_pm_opp_enable/disable functions. OPP library internally
stores
and manages this information in the opp struct. This function may be
used by SoC framework to define a optimal list as per the demands of
SoC usage environment.
@@ -124,7 +124,7 @@ opp_add - Add a new OPP for a specific domain represented
by the device pointer.
soc_pm_init()
{
/* Do things */
- r = opp_add(mpu_dev, 100, 90);
+ r
[PATCH 3/3] PM / OPP: rename header to linux/pm_opp.h
Since Operating Performance Points(OPP) functions are specific to
device specific power management, be specific and rename opp.h
to pm_opp.h
Reported-by: Randy Dunlap rdun...@infradead.org
Signed-off-by: Nishanth Menon n...@ti.com
---
Documentation/power/opp.txt |2 +-
arch/arm/mach-imx/mach-imx6q.c |2 +-
arch/arm/mach-omap2/board-omap3beagle.c |2 +-
arch/arm/mach-omap2/omap-pm.h |2 +-
arch/arm/mach-omap2/opp.c |2 +-
arch/arm/mach-omap2/pm.c|2 +-
drivers/base/power/opp.c|2 +-
drivers/cpufreq/arm_big_little.c|2 +-
drivers/cpufreq/arm_big_little_dt.c |2 +-
drivers/cpufreq/cpufreq-cpu0.c |2 +-
drivers/cpufreq/exynos5440-cpufreq.c|2 +-
drivers/cpufreq/imx6q-cpufreq.c |2 +-
drivers/cpufreq/omap-cpufreq.c |2 +-
drivers/devfreq/devfreq.c |2 +-
drivers/devfreq/exynos/exynos4_bus.c|2 +-
drivers/devfreq/exynos/exynos5_bus.c|4 ++--
include/linux/devfreq.h |2 +-
include/linux/{opp.h = pm_opp.h} |0
18 files changed, 18 insertions(+), 18 deletions(-)
rename include/linux/{opp.h = pm_opp.h} (100%)
diff --git a/Documentation/power/opp.txt b/Documentation/power/opp.txt
index 7f67e3d..b8a907d 100644
--- a/Documentation/power/opp.txt
+++ b/Documentation/power/opp.txt
@@ -42,7 +42,7 @@ We can represent these as three OPPs as the following {Hz,
uV} tuples:
OPP library provides a set of helper functions to organize and query the OPP
information. The library is located in drivers/base/power/opp.c and the header
-is located in include/linux/opp.h. OPP library can be enabled by enabling
+is located in include/linux/pm_opp.h. OPP library can be enabled by enabling
CONFIG_PM_OPP from power management menuconfig menu. OPP library depends on
CONFIG_PM as certain SoCs such as Texas Instrument's OMAP framework allows to
optionally boot at a certain OPP without needing cpufreq.
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index dfbf6f2..86dea10 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -25,7 +25,7 @@
#include linux/of_address.h
#include linux/of_irq.h
#include linux/of_platform.h
-#include linux/opp.h
+#include linux/pm_opp.h
#include linux/phy.h
#include linux/reboot.h
#include linux/regmap.h
diff --git a/arch/arm/mach-omap2/board-omap3beagle.c
b/arch/arm/mach-omap2/board-omap3beagle.c
index 33969e5..6432ab8 100644
--- a/arch/arm/mach-omap2/board-omap3beagle.c
+++ b/arch/arm/mach-omap2/board-omap3beagle.c
@@ -25,7 +25,7 @@
#include linux/gpio.h
#include linux/input.h
#include linux/gpio_keys.h
-#include linux/opp.h
+#include linux/pm_opp.h
#include linux/cpu.h
#include linux/mtd/mtd.h
diff --git a/arch/arm/mach-omap2/omap-pm.h b/arch/arm/mach-omap2/omap-pm.h
index 67faa7b..1d777e6 100644
--- a/arch/arm/mach-omap2/omap-pm.h
+++ b/arch/arm/mach-omap2/omap-pm.h
@@ -17,7 +17,7 @@
#include linux/device.h
#include linux/cpufreq.h
#include linux/clk.h
-#include linux/opp.h
+#include linux/pm_opp.h
/*
* agent_id values for use with omap_pm_set_min_bus_tput():
diff --git a/arch/arm/mach-omap2/opp.c b/arch/arm/mach-omap2/opp.c
index 7b04637..ec21e6e 100644
--- a/arch/arm/mach-omap2/opp.c
+++ b/arch/arm/mach-omap2/opp.c
@@ -17,7 +17,7 @@
* GNU General Public License for more details.
*/
#include linux/module.h
-#include linux/opp.h
+#include linux/pm_opp.h
#include linux/cpu.h
#include omap_device.h
diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index 92901bd..2f569b3 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -13,7 +13,7 @@
#include linux/init.h
#include linux/io.h
#include linux/err.h
-#include linux/opp.h
+#include linux/pm_opp.h
#include linux/export.h
#include linux/suspend.h
#include linux/cpu.h
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index 693e14a..fa41874 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -21,7 +21,7 @@
#include linux/list.h
#include linux/rculist.h
#include linux/rcupdate.h
-#include linux/opp.h
+#include linux/pm_opp.h
#include linux/of.h
#include linux/export.h
diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
index 9e82a9d..e010fb7 100644
--- a/drivers/cpufreq/arm_big_little.c
+++ b/drivers/cpufreq/arm_big_little.c
@@ -25,7 +25,7 @@
#include linux/cpumask.h
#include linux/export.h
#include linux/of_platform.h
-#include linux/opp.h
+#include linux/pm_opp.h
#include linux/slab.h
#include linux/topology.h
#include linux/types.h
diff --git a/drivers/cpufreq/arm_big_little_dt.c
b/drivers/cpufreq/arm_big_little_dt.c
index 480c0bd..8d9d591 100644
--- a/drivers/cpufreq/arm_big_little_dt.c
+++ b/drivers/cpufreq/arm_big_little_dt.c
@@ -24,7 +24,7 @@
#include linux/export.h
#include linux/module.h
#include linux
Re: [PATCH] cpufreq: exynos5440: Protect opp search calls with rcu lock
Daniel,
On Mon, Apr 15, 2013 at 1:24 AM, Amit Daniel Kachhap
amit.dan...@samsung.com wrote:
As per the OPP library documentation(Documentation/power/opp.txt) all
opp find/get calls should be protected by rcu locks.
Signed-off-by: Amit Daniel Kachhap amit.dan...@samsung.com
---
This patch is created against linux-next tree and is suggested by
Nishanth Menon. (https://lkml.org/lkml/2013/4/12/119)
drivers/cpufreq/exynos5440-cpufreq.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/cpufreq/exynos5440-cpufreq.c
b/drivers/cpufreq/exynos5440-cpufreq.c
index ead7ed4..0c74018 100644
--- a/drivers/cpufreq/exynos5440-cpufreq.c
+++ b/drivers/cpufreq/exynos5440-cpufreq.c
@@ -120,11 +120,13 @@ static int init_div_table(void)
int i = 0;
struct opp *opp;
+ rcu_read_lock();
for (i = 0; freq_tbl[i].frequency != CPUFREQ_TABLE_END; i++) {
opp = opp_find_freq_exact(dvfs_info-dev,
freq_tbl[i].frequency * 1000, true);
if (IS_ERR(opp)) {
+ rcu_read_unlock();
dev_err(dvfs_info-dev,
failed to find valid OPP for %u KHZ\n,
freq_tbl[i].frequency);
@@ -159,6 +161,7 @@ static int init_div_table(void)
__raw_writel(tmp, dvfs_info-base + XMU_PMU_P0_7 + 4 * i);
}
+ rcu_read_unlock();
Is it not possible to reduce the amount of code protected by RCU lock?
something like this:
+ rcu_read_lock();
opp = opp_find_freq_exact(dvfs_info-dev,
freq_tbl[i].frequency * 1000, true);
if (IS_ERR(opp)) {
+ rcu_read_unlock();
dev_err(dvfs_info-dev,
failed to find valid OPP for %u KHZ\n,
freq_tbl[i].frequency);
return PTR_ERR(opp);
}
+ volt_id = opp_get_voltage(opp);
+ rcu_unlock();
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH V6 1/4] cpufreq: exynos: Add cpufreq driver for exynos5440
Hi,
On Mon, Apr 8, 2013 at 4:57 AM, Amit Daniel Kachhap
amit.dan...@samsung.com wrote:
+
+static int init_div_table(void)
+{
+ struct cpufreq_frequency_table *freq_tbl = dvfs_info-freq_table;
+ unsigned int tmp, clk_div, ema_div, freq, volt_id;
+ int i = 0;
+ struct opp *opp;
+
+ for (i = 0; freq_tbl[i].frequency != CPUFREQ_TABLE_END; i++) {
+
+ opp = opp_find_freq_exact(dvfs_info-dev,
+ freq_tbl[i].frequency * 1000, true);
+ if (IS_ERR(opp)) {
+ dev_err(dvfs_info-dev,
+ failed to find valid OPP for %u KHZ\n,
+ freq_tbl[i].frequency);
+ return PTR_ERR(opp);
+ }
please use RCU read locks.
+
+ freq = freq_tbl[i].frequency / 1000; /* In MHZ */
+ clk_div = ((freq / CPU_DIV_FREQ_MAX) P0_7_CPUCLKDEV_MASK)
+P0_7_CPUCLKDEV_SHIFT;
+ clk_div |= ((freq / CPU_ATB_FREQ_MAX) P0_7_ATBCLKDEV_MASK)
+P0_7_ATBCLKDEV_SHIFT;
+ clk_div |= ((freq / CPU_DBG_FREQ_MAX) P0_7_CSCLKDEV_MASK)
+P0_7_CSCLKDEV_SHIFT;
+
+ /* Calculate EMA */
+ volt_id = opp_get_voltage(opp);
Please use RCU read locks as documented for OPP library.
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
