* [PATCH] arm: bcm2835: Add the PMU to the devicetree.
From: Florian Fainelli @ 2018-05-17 19:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <alpine.DEB.2.21.1805171530080.8529@macbook-air>
On 05/17/2018 12:31 PM, Vince Weaver wrote:
> On Thu, 17 May 2018, Vince Weaver wrote:
>
>> On Thu, 17 May 2018, Peter Zijlstra wrote:
>> with cortex-a7 now, would it be possible to later drop that if proper
>> cortex-a53 support is added to the armv7 pmu driver? Or would that lead
>> to all kinds of back-compatability mess?
>
> For what it's worth, the pi-foundation kernel bcm2710 device tree file
> does:
>
> arm-pmu {
> #ifdef RPI364
> compatible = "arm,armv8-pmuv3", "arm,cortex-a7-pmu";
> #else
> compatible = "arm,cortex-a7-pmu";
> #endif
> interrupt-parent = <&local_intc>;
> interrupts = <9>;
> };
>
>
> Which is probably where I was getting the arm,armv8-pmuv3 from in my
> original patch.
I thought somehow that Marc Z. had unified
arch/arm/kernel/perf_event_v7.c and arch/arm64/kernel/perf_event.c into
a common driver entry point under drivers/perf/arm_pmu.c but I don't see
it and after about 15 minutes looking at it, it does not look as trivial
as I though to separate out those files so the ARMv8 PMU description can
be moved into a generic location for instance.
FWIW, Broadcom STB chips, even when 64-bit capable or often used with an
32-bit ARM kernel, so having the ARMv8 PMUs work under a 32-bit ARM
kernel would be great. The downstream solution we have sued thus far is
to find the closest compatible string to represent those, which is not
great...
--
Florian
^ permalink raw reply
* [PATCH] mm/dmapool: localize page allocations
From: Sinan Kaya @ 2018-05-17 20:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180517194612.GG26718@bombadil.infradead.org>
On 5/17/2018 3:46 PM, Matthew Wilcox wrote:
>> Remember that the CPU core that is running this driver is most probably on
>> the same NUMA node as the device itself.
> Umm ... says who? If my process is running on NUMA node 5 and I submit
> an I/O, it should be allocating from a pool on node 5, not from a pool
> on whichever node the device is attached to.
OK, let's do an exercise. Maybe, I'm missing something in the big picture.
If a user process is running at node 5, it submits some work to the hardware
via block layer that is eventually invoked by syscall.
Whatever buffer process is using, it gets copied into the kernel space as
it is crossing a userspace/kernel space boundary.
Block layer packages a block request with the kernel pointers and makes a
request to the NVMe driver for consumption.
Last time I checked, dma_alloc_coherent() API uses the locality information
from the device not from the CPU for allocation.
While the metadata for dma_pool is pointing to the currently running CPU core,
the DMA buffer itself is created using the device node itself today without
my patch.
I would think that you actually want to run the process at the same NUMA node
as the CPU and device itself for performance reasons. Otherwise, performance
expectations should be low.
Even if user says please keep my process to a particular NUMA node,
we keep pointing to the memory on the other node today.
I don't know what is so special about memory on the default node. IMO, all memory
allocations used by a driver need to follow the device.
I wish I could do this in kmalloc(). devm_kmalloc() follows the device as another
example not CPU.
With these assumptions, even though user said please use the NUMA node from the
device, we still keep pointing to the default domain for pointers.
Isn't this wrong?
>
> If it actually makes a performance difference, then NVMe should allocate
> one pool per queue, rather than one pool per device like it currently
> does.
>
>> Also, if it was a one time init kind of thing, I'd say "yeah, leave it alone".
>> DMA pool is used by a wide range of drivers and it is used to allocate
>> fixed size buffers at runtime.
> * DMA Pool allocator
> *
> * Copyright 2001 David Brownell
> * Copyright 2007 Intel Corporation
> * Author: Matthew Wilcox <willy@linux.intel.com>
>
> I know what it's used for.
>
cool, good to know.
--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH net-next v3 00/10] net: mvpp2: phylink conversion
From: David Miller @ 2018-05-17 20:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180517082939.14598-1-antoine.tenart@bootlin.com>
From: Antoine Tenart <antoine.tenart@bootlin.com>
Date: Thu, 17 May 2018 10:29:29 +0200
> @Dave: patches 7 to 10 should go through the mvebu tree (Gregory in
> Cc.) to avoid any conflict with the other mvebu dt patches taken during
> this cycle.
>
> The series is based on today's net-next.
Ok, patches 1-6 applied to net-next, thank you.
^ permalink raw reply
* [PATCH] arm64: dts: renesas: r8a77980: add SMP support
From: Sergei Shtylyov @ 2018-05-17 20:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <14666252.G84Ipxe6EL@wasted.cogentembedded.com>
Add the device nodes for 3 more Cortex-A53 CPU cores; adjust the interrupt
delivery masks for the ARM GIC and Architectured Timer.
Based on the original (and large) patch by Vladimir Barinov.
Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
The patch is against the 'renesas-devel-20180516v2-v4.17-rc5' tag of Simon
Horman's 'renesas.git' repo. Tested successfully on the V3M Starter Kit board
(except offlining CPU0 hangs the kernel).
arch/arm64/boot/dts/renesas/r8a77980.dtsi | 40 ++++++++++++++++++++++++++----
1 file changed, 35 insertions(+), 5 deletions(-)
Index: renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
===================================================================
--- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
+++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
@@ -30,6 +30,36 @@
enable-method = "psci";
};
+ a53_1: cpu at 1 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53","arm,armv8";
+ reg = <1>;
+ clocks = <&cpg CPG_CORE R8A77980_CLK_Z2>;
+ power-domains = <&sysc R8A77980_PD_CA53_CPU1>;
+ next-level-cache = <&L2_CA53>;
+ enable-method = "psci";
+ };
+
+ a53_2: cpu at 2 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53","arm,armv8";
+ reg = <2>;
+ clocks = <&cpg CPG_CORE R8A77980_CLK_Z2>;
+ power-domains = <&sysc R8A77980_PD_CA53_CPU2>;
+ next-level-cache = <&L2_CA53>;
+ enable-method = "psci";
+ };
+
+ a53_3: cpu at 3 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53","arm,armv8";
+ reg = <3>;
+ clocks = <&cpg CPG_CORE R8A77980_CLK_Z2>;
+ power-domains = <&sysc R8A77980_PD_CA53_CPU3>;
+ next-level-cache = <&L2_CA53>;
+ enable-method = "psci";
+ };
+
L2_CA53: cache-controller {
compatible = "cache";
power-domains = <&sysc R8A77980_PD_CA53_SCU>;
@@ -408,7 +438,7 @@
<0x0 0xf1020000 0 0x20000>,
<0x0 0xf1040000 0 0x20000>,
<0x0 0xf1060000 0 0x20000>;
- interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(1) |
+ interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) |
IRQ_TYPE_LEVEL_HIGH)>;
clocks = <&cpg CPG_MOD 408>;
clock-names = "clk";
@@ -424,13 +454,13 @@
timer {
compatible = "arm,armv8-timer";
- interrupts-extended = <&gic GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(1) |
+ interrupts-extended = <&gic GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |
IRQ_TYPE_LEVEL_LOW)>,
- <&gic GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(1) |
+ <&gic GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) |
IRQ_TYPE_LEVEL_LOW)>,
- <&gic GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(1) |
+ <&gic GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) |
IRQ_TYPE_LEVEL_LOW)>,
- <&gic GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(1) |
+ <&gic GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) |
IRQ_TYPE_LEVEL_LOW)>;
};
};
^ permalink raw reply
* [PATCH] arm64: dts: renesas: r8a77980: add SMP support
From: Geert Uytterhoeven @ 2018-05-17 20:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <ec4c6905-23d1-8600-a1cc-4297fbd1ddaa@cogentembedded.com>
Hi Sergei,
On Thu, May 17, 2018 at 10:19 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Add the device nodes for 3 more Cortex-A53 CPU cores; adjust the interrupt
> delivery masks for the ARM GIC and Architectured Timer.
>
> Based on the original (and large) patch by Vladimir Barinov.
>
> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Thanks for your patch!
> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> +++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> @@ -30,6 +30,36 @@
> enable-method = "psci";
> };
>
> + a53_1: cpu at 1 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53","arm,armv8";
Please stop copying spaceless lists ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH] mm/dmapool: localize page allocations
From: Matthew Wilcox @ 2018-05-17 20:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <d49e594a-c18a-160f-ca4c-91520ff3b293@codeaurora.org>
On Thu, May 17, 2018 at 04:05:45PM -0400, Sinan Kaya wrote:
> On 5/17/2018 3:46 PM, Matthew Wilcox wrote:
> >> Remember that the CPU core that is running this driver is most probably on
> >> the same NUMA node as the device itself.
> > Umm ... says who? If my process is running on NUMA node 5 and I submit
> > an I/O, it should be allocating from a pool on node 5, not from a pool
> > on whichever node the device is attached to.
>
> OK, let's do an exercise. Maybe, I'm missing something in the big picture.
Sure.
> If a user process is running at node 5, it submits some work to the hardware
> via block layer that is eventually invoked by syscall.
>
> Whatever buffer process is using, it gets copied into the kernel space as
> it is crossing a userspace/kernel space boundary.
>
> Block layer packages a block request with the kernel pointers and makes a
> request to the NVMe driver for consumption.
>
> Last time I checked, dma_alloc_coherent() API uses the locality information
> from the device not from the CPU for allocation.
Yes, it does. I wonder why that is; it doesn't actually make any sense.
It'd be far more sensible to allocate it on memory local to the user
than memory local to the device.
> While the metadata for dma_pool is pointing to the currently running CPU core,
> the DMA buffer itself is created using the device node itself today without
> my patch.
Umm ... dma_alloc_coherent memory is for metadata about the transfer, not
for the memory used for the transaction.
> I would think that you actually want to run the process at the same NUMA node
> as the CPU and device itself for performance reasons. Otherwise, performance
> expectations should be low.
That's foolish. Consider a database appliance with four sockets, each
with its own memory and I/O devices attached. You can't tell the user
to shard the database into four pieces and have each socket only work on
the quarter of the database that's available to each socket. They may
as well buy four smaller machines. The point of buying a large NUMA
machine is to use all of it.
Let's try a different example. I have a four-socket system with one
NVMe device with lots of hardware queues. Each CPU has its own queue
assigned to it. If I allocate all the PRP metadata on the socket with
the NVMe device attached to it, I'm sending a lot of coherency traffic
in the direction of that socket, in addition to the actual data. If the
PRP lists are allocated randomly on the various sockets, the traffic
is heading all over the fabric. If the PRP lists are allocated on the
local socket, the only time those lists move off this node is when the
device requests them.
^ permalink raw reply
* [PATCH RFC] ARM: dts: add Raspberry Pi Compute Module and IO board
From: Stefan Wahren @ 2018-05-17 20:44 UTC (permalink / raw)
To: linux-arm-kernel
The Raspberry Pi Compute Module (CM1) is a SoM which contains a
BCM2835 processor, 512 MB RAM and a 4 GB eMMC. There is also a carrier
board which is called Compute Module IO Board.
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/bcm2835-rpi-cm1-io1.dts | 92 +++++++++++++++++++++++++++++++
arch/arm/boot/dts/bcm2835-rpi-cm1.dtsi | 34 ++++++++++++
3 files changed, 127 insertions(+)
create mode 100644 arch/arm/boot/dts/bcm2835-rpi-cm1-io1.dts
create mode 100644 arch/arm/boot/dts/bcm2835-rpi-cm1.dtsi
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index ec2024e..a9883e8 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -73,6 +73,7 @@ dtb-$(CONFIG_ARCH_BCM2835) += \
bcm2835-rpi-b-rev2.dtb \
bcm2835-rpi-b-plus.dtb \
bcm2835-rpi-a-plus.dtb \
+ bcm2835-rpi-cm1-io1.dtb \
bcm2836-rpi-2-b.dtb \
bcm2837-rpi-3-b.dtb \
bcm2837-rpi-3-b-plus.dtb \
diff --git a/arch/arm/boot/dts/bcm2835-rpi-cm1-io1.dts b/arch/arm/boot/dts/bcm2835-rpi-cm1-io1.dts
new file mode 100644
index 0000000..4d9aa22
--- /dev/null
+++ b/arch/arm/boot/dts/bcm2835-rpi-cm1-io1.dts
@@ -0,0 +1,92 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+#include "bcm2835-rpi-cm1.dtsi"
+#include "bcm283x-rpi-usb-host.dtsi"
+
+/ {
+ compatible = "raspberrypi,compute-module", "brcm,bcm2835";
+ model = "Raspberry Pi Compute Module IO board rev1";
+};
+
+&dsi1 {
+ status = "okay";
+};
+
+&gpio {
+ /*
+ * This is based on the official GPU firmware DT blob.
+ *
+ * Legend:
+ * "NC" = not connected (no rail from the SoC)
+ * "FOO" = GPIO line named "FOO" on the schematic
+ * "FOO_N" = GPIO line named "FOO" on schematic, active low
+ */
+ gpio-line-names = "GPIO0",
+ "GPIO1",
+ "GPIO2",
+ "GPIO3",
+ "GPIO4",
+ "GPIO5",
+ "GPIO6",
+ "GPIO7",
+ "GPIO8",
+ "GPIO9",
+ "GPIO10",
+ "GPIO11",
+ "GPIO12",
+ "GPIO13",
+ "GPIO14",
+ "GPIO15",
+ "GPIO16",
+ "GPIO17",
+ "GPIO18",
+ "GPIO19",
+ "GPIO20",
+ "GPIO21",
+ "GPIO22",
+ "GPIO23",
+ "GPIO24",
+ "GPIO25",
+ "GPIO26",
+ "GPIO27",
+ "GPIO28",
+ "GPIO29",
+ "GPIO30",
+ "GPIO31",
+ "GPIO32",
+ "GPIO33",
+ "GPIO34",
+ "GPIO35",
+ "GPIO36",
+ "GPIO37",
+ "GPIO38",
+ "GPIO39",
+ "GPIO40",
+ "GPIO41",
+ "GPIO42",
+ "GPIO43",
+ "GPIO44",
+ "GPIO45",
+ "HDMI_HPD_N",
+ /* Also used as ACT LED */
+ "EMMC_EN_N",
+ /* Used by eMMC */
+ "SD_CLK_R",
+ "SD_CMD_R",
+ "SD_DATA0_R",
+ "SD_DATA1_R",
+ "SD_DATA2_R",
+ "SD_DATA3_R";
+
+ pinctrl-0 = <&gpioout &alt0>;
+};
+
+&hdmi {
+ hpd-gpios = <&gpio 46 GPIO_ACTIVE_HIGH>;
+};
+
+&uart0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart0_gpio14>;
+ status = "okay";
+};
diff --git a/arch/arm/boot/dts/bcm2835-rpi-cm1.dtsi b/arch/arm/boot/dts/bcm2835-rpi-cm1.dtsi
new file mode 100644
index 0000000..ef22c2d
--- /dev/null
+++ b/arch/arm/boot/dts/bcm2835-rpi-cm1.dtsi
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+#include "bcm2835.dtsi"
+#include "bcm2835-rpi.dtsi"
+
+/ {
+ leds {
+ act {
+ gpios = <&gpio 47 GPIO_ACTIVE_LOW>;
+ };
+ };
+
+ reg_3v3: fixed-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "3V3";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ };
+
+ reg_1v8: fixed-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "1V8";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-always-on;
+ };
+};
+
+&sdhost {
+ non-removable;
+ vmmc-supply = <®_3v3>;
+ vqmmc-supply = <®_1v8>;
+};
--
2.7.4
^ permalink raw reply related
* [PATCH 1/2] regulator: add QCOM RPMh regulator driver
From: David Collins @ 2018-05-17 20:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180517060948.GI20254@sirena.org.uk>
On 05/16/2018 11:09 PM, Mark Brown wrote:
> On Tue, Apr 24, 2018 at 01:46:21PM -0700, David Collins wrote:
>> The RPMh hardware is aware of the parent-child connections between
>> regulators as well as minimum headroom to ensure stable LDO voltage output
>> for subregulated LDOs. The intention of having the headroom be a
>> configurable property for processors is to support usecases in which
>> subregulated LDO loads are particularly sensitive to noise and require
>> additional headroom. Such usecases are board dependent and beyond the
>> baseline configurations set in RPMh hardware.
>
> So the hardware implementation is some hard coding stuff that doesn't
> really adequately reflect reality? This seems unfortunate. However do
> we really need to tell the hardware about the fact that we're adding
> extra headroom - are there actual interactions with non-Linux things
> here?
The RPMh hardware is configured by the boot loader. The configuration
does reflect reality; however, it cannot handle all configurations at
initialization time. Specific headroom management typically comes up in
modem usecases for RF supplies that are sensitive to noise. This feature
allows RPMh masters (application processor, modem processor, etc) to make
requests only for the regulators that they directly care about without
having to worry about power grid parent-child details and setting the
voltage of parent regulators in order to ensure sufficient headroom.
If you really don't like having this feature present in the Linux RPMh
regulator driver, then I'd be ok removing it. It is not required for
SDM845 which the driver is initially targeting.
>>>> XOB managed regulators physically cannot change voltage. Therefore, do
>>>> you agree that it is reasonable to use fixed_uV for them? Note that I
>>>> removed init_data->constraints.apply_uV manipulation in version 2 of this
>>>> patch.
>
>>> If these regulators can't change voltage then surely we know what
>>> voltage they have without needing it to be specified in DT?
>
>> In the case of XOB managed LDO regulators, the LDOs physically can be
>> configured to different voltages by the bootloader. However, the RPMh
>> interface provides no mechanism for the application processor to read or
>> change that voltage. Therefore, we need a way to specify such voltages in
>> a board specific (as opposed to driver specific) manner (i.e. device tree).
>
> Is the kernel somehow prevented from varying these voltages?
Yes. Physically, there exists no RPMh register to read or write the
voltage of LDOs managed via XOB. Additionally, the kernel running on the
application processor is blocked from configuring the voltage via a direct
SPMI writes by access permissions that crash the system when violated.
Take care,
David
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH] mm/dmapool: localize page allocations
From: Sinan Kaya @ 2018-05-17 21:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180517204103.GJ26718@bombadil.infradead.org>
On 5/17/2018 4:41 PM, Matthew Wilcox wrote:
> Let's try a different example. I have a four-socket system with one
> NVMe device with lots of hardware queues. Each CPU has its own queue
> assigned to it. If I allocate all the PRP metadata on the socket with
> the NVMe device attached to it, I'm sending a lot of coherency traffic
> in the direction of that socket, in addition to the actual data. If the
> PRP lists are allocated randomly on the various sockets, the traffic
> is heading all over the fabric. If the PRP lists are allocated on the
> local socket, the only time those lists move off this node is when the
> device requests them.
So.., your reasoning is that you actually want to keep the memory as close
as possible to the CPU rather than the device itself. CPU would do
frequent updates the buffer until the point where it hands off the buffer
to the hardware. Device would fetch the memory via coherency when it needs
to consume the data but this would be a one time penalty.
It sounds logical to me. I was always told that you want to keep buffers
as close as possible to the device.
Maybe, it makes sense for things that device needs frequent access like
receive buffers.
If the majority user is CPU, then the buffer needs to be kept closer to
the CPU.
dma_alloc_coherent() is generally used for receiver buffer allocation in
network adapters in general. People allocate a chunk and then create a
queue that hardware owns for dumping events and data.
Since DMA pool is a generic API, we should maybe request where we want
to keep the buffers closer to and allocate buffers from the appropriate
NUMA node based on that.
--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH 1/2] dt-bindings: power: Add ZynqMP power domain bindings
From: Jolly Shah @ 2018-05-17 21:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <DM2PR0201MB07670C06F504F97EF4E4BAD4B8D00@DM2PR0201MB0767.namprd02.prod.outlook.com>
Hi Marek,
> -----Original Message-----
> From: linux-kernel-owner at vger.kernel.org [mailto:linux-kernel-
> owner at vger.kernel.org] On Behalf Of Jolly Shah
> Sent: Thursday, March 15, 2018 10:47 AM
> To: Marek Szyprowski <m.szyprowski@samsung.com>; Geert Uytterhoeven
> <geert@linux-m68k.org>; Rob Herring <robh@kernel.org>
> Cc: Matthias Brugger <matthias.bgg@gmail.com>; Andy Gross
> <andy.gross@linaro.org>; Shawn Guo <shawnguo@kernel.org>; Geert
> Uytterhoeven <geert+renesas@glider.be>; Bj?rn Andersson
> <bjorn.andersson@linaro.org>; sean.wang at mediatek.com; Michal Simek
> <michal.simek@xilinx.com>; Mark Rutland <mark.rutland@arm.com>; Rajan
> Vaja <RAJANV@xilinx.com>; open list:OPEN FIRMWARE AND FLATTENED
> DEVICE TREE BINDINGS <devicetree@vger.kernel.org>; Linux ARM <linux-arm-
> kernel at lists.infradead.org>; Linux Kernel Mailing List <linux-
> kernel at vger.kernel.org>
> Subject: RE: [PATCH 1/2] dt-bindings: power: Add ZynqMP power domain
> bindings
>
> [This sender failed our fraud detection checks and may not be who they appear
> to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing]
>
> Hi Rob, Geert, Marek,
>
> > -----Original Message-----
> > From: Marek Szyprowski [mailto:m.szyprowski at samsung.com]
> > Sent: Tuesday, March 06, 2018 12:06 AM
> > To: Geert Uytterhoeven <geert@linux-m68k.org>; Rob Herring
> > <robh@kernel.org>
> > Cc: Jolly Shah <JOLLYS@xilinx.com>; Matthias Brugger
> > <matthias.bgg@gmail.com>; Andy Gross <andy.gross@linaro.org>; Shawn
> > Guo <shawnguo@kernel.org>; Geert Uytterhoeven
> > <geert+renesas@glider.be>; Bj?rn Andersson
> > <bjorn.andersson@linaro.org>; sean.wang at mediatek.com; Michal Simek
> > <michal.simek@xilinx.com>; Mark Rutland <mark.rutland@arm.com>; Rajan
> > Vaja <RAJANV@xilinx.com>; open list:OPEN FIRMWARE AND FLATTENED
> DEVICE
> > TREE BINDINGS <devicetree@vger.kernel.org>; Linux ARM <linux-arm-
> > kernel at lists.infradead.org>; Linux Kernel Mailing List <linux-
> > kernel at vger.kernel.org>; Jolly Shah <JOLLYS@xilinx.com>
> > Subject: Re: [PATCH 1/2] dt-bindings: power: Add ZynqMP power domain
> > bindings
> >
> > Hi All,
> >
> > On 2018-03-06 08:59, Geert Uytterhoeven wrote:
> > > Hi Rob, Jolly,
> > >
> > > On Mon, Mar 5, 2018 at 11:39 PM, Rob Herring <robh@kernel.org> wrote:
> > >> On Tue, Feb 27, 2018 at 03:55:49PM -0800, Jolly Shah wrote:
> > >>> Add documentation to describe ZynqMP power domain bindings.
> > >>>
> > >>> Signed-off-by: Jolly Shah <jollys@xilinx.com>
> > >>> Signed-off-by: Rajan Vaja <rajanv@xilinx.com>
> > >>> ---
> > >>> .../devicetree/bindings/power/zynqmp-genpd.txt | 46
> > ++++++++++++++++++++++
> > >>> 1 file changed, 46 insertions(+)
> > >>> create mode 100644
> > >>> Documentation/devicetree/bindings/power/zynqmp-genpd.txt
> > >>>
> > >>> diff --git
> > >>> a/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
> > >>> b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
> > >>> new file mode 100644
> > >>> index 0000000..25f9711
> > >>> --- /dev/null
> > >>> +++ b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
> > >>> +This node contains a number of subnodes, each representing a
> > >>> +single PM domain that PM domain consumer devices reference.
> > >>> +
> > >>> +== PM Domain Nodes ==
> > >>> +
> > >>> +Required properties:
> > >>> + - #power-domain-cells: Number of cells in a PM domain specifier.
> > >>> +Must be
> > 0.
> > >>> + - pd-id: List of domain identifiers of as defined by platform firmware.
> > These
> > >>> + identifiers are passed to the PM firmware.
> > >>> +
> > >>> +Example:
> > >>> + zynqmp-genpd {
> > >>> + compatible = "xlnx,zynqmp-genpd";
> > >> What's the control interface for controlling the domains?
> > >>> +
> > >>> + pd_usb0: pd-usb0 {
> > >>> + pd-id = <22>;
> > >>> + #power-domain-cells = <0>;
> > >> There's no need for all these sub nodes. Make #power-domain-cells 1
> > >> and put the id in the cell value.
> > > That was my first reaction, too...
> > >>> + };
> > >>> +
> > >>> + pd_sata: pd-sata {
> > >>> + pd-id = <28>;
> > >>> + #power-domain-cells = <0>;
> > >>> + };
> > >>> +
> > >>> + pd_gpu: pd-gpu {
> > >>> + pd-id = <58 20 21>;
> > > ... until I saw the above.
> > > Controlling the GPU power area requires controlling 3 physical areas?
> > >
> > > However, doing it this way may bite you in the future, if a need
> > > arises to control a subset. And what about power up/down order?
> >
> > What about defining 3 separate domains and arranging them in
> > parent-child relationship? generic power domains already supports that
> > and this allows to nicely define the power on/off order.
> >
> > >>> + #power-domain-cells = <0x0>;
> > >>> + };
> > >>> + };
> >
>
> I agree it should be arranged in as parent child order to control subset or control
> order. Will incorporate those changes in next version.
>
> > Best regards
> > --
> > Marek Szyprowski, PhD
> > Samsung R&D Institute Poland
As suggested, I tried out parent, child approach. However what I found is Genpd core takes care of parent child dependencies for power on off routines only. In our case, We need them in attach-detach routines too. In that case, we need to handle dependencies manually for those routines. Please suggest better approach, if any.
Thanks,
Jolly Shah
^ permalink raw reply
* [PATCH 1/2] regulator: of: add property for allowed modes specification
From: Doug Anderson @ 2018-05-17 21:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <f3aa166ddd11c8f9813602be68aa5b10e5c90979.1526088289.git.collinsd@codeaurora.org>
Hi,
On Fri, May 11, 2018 at 6:46 PM, David Collins <collinsd@codeaurora.org> wrote:
> Add a common device tree property for regulator nodes to support
> the specification of allowed operating modes.
>
> Signed-off-by: David Collins <collinsd@codeaurora.org>
> ---
> Documentation/devicetree/bindings/regulator/regulator.txt | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
> index 2babe15b..c627aa0 100644
> --- a/Documentation/devicetree/bindings/regulator/regulator.txt
> +++ b/Documentation/devicetree/bindings/regulator/regulator.txt
> @@ -59,6 +59,11 @@ Optional properties:
> - regulator-initial-mode: initial operating mode. The set of possible operating
> modes depends on the capabilities of every hardware so each device binding
> documentation explains which values the regulator supports.
> +- regulator-allowed-modes: list of operating modes that software is allowed to
> + configure for the regulator at run-time. Elements may be specified in any
> + order. The set of possible operating modes depends on the capabilities of
> + every hardware so each device binding document explains which values the
> + regulator supports.
Looks sane to me. It might be interesting to be explicit about what
happens if "regulator-allowed-modes" doesn't include the mode that was
listed as "regulator-initial-mode". Does that mean that there's no
way to get back to "regulator-initial-mode" after it's been changed
once, or is it an error to not include the initial mode in the set of
allowed modes?
I'm not 100% sure if going to such detail is necessary though. Thus,
feel free to add:
Reviewed-by: Douglas Anderson <dianders@chromium.org>
^ permalink raw reply
* [PATCH 2/2] regulator: of: add support for allowed modes configuration
From: Doug Anderson @ 2018-05-17 21:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <58bb4f965515a67a6cbbc726b0d7b092c22b79d7.1526088289.git.collinsd@codeaurora.org>
Hi,
On Fri, May 11, 2018 at 6:46 PM, David Collins <collinsd@codeaurora.org> wrote:
> @@ -136,6 +136,33 @@ static void of_get_regulation_constraints(struct device_node *np,
> }
> }
>
> + len = of_property_count_elems_of_size(np, "regulator-allowed-modes",
> + sizeof(u32));
> + if (len > 0) {
> + if (desc && desc->of_map_mode) {
> + for (i = 0; i < len; i++) {
> + ret = of_property_read_u32_index(np,
> + "regulator-allowed-modes", i, &pval);
> + if (ret) {
> + pr_err("%s: couldn't read allowed modes index %d, ret=%d\n",
> + np->name, i, ret);
> + break;
> + }
> + mode = desc->of_map_mode(pval);
> + if (mode == REGULATOR_MODE_INVALID)
> + pr_err("%s: invalid regulator-allowed-modes element %u\n",
> + np->name, pval);
> + else
> + constraints->valid_modes_mask |= mode;
> + }
> + if (constraints->valid_modes_mask)
> + constraints->valid_ops_mask
> + |= REGULATOR_CHANGE_MODE;
Kinda calls into question the value of REGULATOR_CHANGE_MODE in the
valid_ops_mask if it's just set whenever valid_modes_mask is non-zero,
huh?
> + } else {
> + pr_warn("%s: mode mapping not defined\n", np->name);
> + }
> + }
> +
This patch seems good to me.
Reviewed-by: Douglas Anderson <dianders@chromium.org>
^ permalink raw reply
* [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: Doug Anderson @ 2018-05-17 21:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <41d5df73ddac772551d2966e0752bb0c357b1ded.1526088081.git.collinsd@codeaurora.org>
Hi,
On Fri, May 11, 2018 at 7:28 PM, David Collins <collinsd@codeaurora.org> wrote:
> +- qcom,regulator-initial-microvolt
> + Usage: optional; VRM regulators only
> + Value type: <u32>
> + Definition: Specifies the initial voltage in microvolts to request for a
> + VRM regulator.
Now that Mark has landed the patch adding support for the
-ENOTRECOVERABLE error code from get_voltage() / get_voltage_sel(), do
we still need the qcom,regulator-initial-microvolt property? If this
is really still needed, can it be moved to the regulator core?
> +- regulator-initial-mode
> + Usage: optional; VRM regulators only
> + Value type: <u32>
> + Definition: Specifies the initial mode to request for a VRM regulator.
> + Supported values are RPMH_REGULATOR_MODE_* which are defined
> + in [1] (i.e. 0 to 3). This property may be specified even
> + if the regulator-allow-set-load property is not specified.
Every time I read the above I wonder why you're documenting a standard
regulator regulator property in your bindings. ...then I realize it's
because you're doing it because you want to explicitly document what
the valid modes are. I wonder if it makes sense to just put a
reference somewhere else in this document to go look at the header
file where these are all nicely documented.
Speaking of documenting things like that, it might be worth finding
somewhere in this doc to mention that the "bob" regulator on PMI8998
can support "regulator-allow-bypass". That tidbit got lost when we
moved to the standard regulator bindings for bypass.
> +- qcom,allowed-drms-modes
> + Usage: required if regulator-allow-set-load is specified;
> + VRM regulators only
> + Value type: <prop-encoded-array>
> + Definition: A list of integers specifying the PMIC regulator modes which
> + can be configured at runtime based upon consumer load needs.
> + Supported values are RPMH_REGULATOR_MODE_* which are defined
> + in [1] (i.e. 0 to 3).
Why is this still here? You moved it to the core regulator framework,
right? It's still in your examples too. Shouldn't this be removed?
It looks like the driver still needs this and it needs to be an exact
duplicate of the common binding. That doesn't seem right...
> +- qcom,drms-mode-max-microamps
> + Usage: required if regulator-allow-set-load is specified;
> + VRM regulators only
> + Value type: <prop-encoded-array>
> + Definition: A list of integers specifying the maximum allowed load
> + current in microamps for each of the modes listed in
> + qcom,allowed-drms-modes (matched 1-to-1 in order). Elements
> + must be specified in order from lowest to highest value.
Any reason this can't go into the regulator core? You'd basically
just take the existing concept of rpmh_regulator_vrm_set_load() and
put it in the core.
> +- qcom,headroom-microvolt
> + Usage: optional; VRM regulators only
> + Value type: <u32>
> + Definition: Specifies the headroom voltage in microvolts to request for
> + a VRM regulator. RPMh hardware automatically ensures that
> + the parent of this regulator outputs a voltage high enough
> + to satisfy the requested headroom. Supported values are
> + 0 to 511000.
I'm curious: is this a voted-for value, or a global value?
Said another way: the whole point of RPMh is that there may be more
than one processor that needs the same rails, right? So the AP might
request 1.1 V for a rail and the modem might request 1.3 V. RPMh
would decide to pick the higher of those two (1.3 V), but if the modem
said it no longer needs the rail it will drop down to 1.1 V.
...and as an example of why the headroom needs to be in hardware, if
the source voltage was normally 1.4 V and the headroom was 200 mV then
the hardware would need to know to bump up the source voltage to 1.5V
during the period of of time that the modem wants the rail at 1.3V.
So my question is: do the AP and modem in the above situation
separately vote for headroom? How is it aggregated? ...or is it a
global value and this sets the headroom for all clients of RPMh? It
would be interesting to document this as it might help with figuring
out how this value should be set.
> diff --git a/include/dt-bindings/regulator/qcom,rpmh-regulator.h b/include/dt-bindings/regulator/qcom,rpmh-regulator.h
> new file mode 100644
> index 0000000..4378c4b
> --- /dev/null
> +++ b/include/dt-bindings/regulator/qcom,rpmh-regulator.h
> +/*
> + * These mode constants may be used for regulator-initial-mode and
> + * qcom,allowed-drms-modes properties of an RPMh regulator device tree node.
Technically also for your new "regulator-allowed-modes". Maybe just
say that they're used anywhere a regulator mode is needed in this
driver and give regulator-initial-mode as an example?
-Doug
^ permalink raw reply
* [PATCH v3 2/2] regulator: add QCOM RPMh regulator driver
From: Doug Anderson @ 2018-05-17 21:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <3a4195365ab0d252fdf064d2300f45b9b777991c.1526088081.git.collinsd@codeaurora.org>
Hi,
On Fri, May 11, 2018 at 7:28 PM, David Collins <collinsd@codeaurora.org> wrote:
> +static int rpmh_regulator_parse_vrm_modes(struct rpmh_vreg *vreg,
> + struct device *dev, struct device_node *node)
> +{
> + const char *prop;
> + int i, len, ret, mode;
> + u32 *buf;
> +
> + /* qcom,allowed-drms-modes is optional */
> + prop = "qcom,allowed-drms-modes";
As per comments in bindings patch: this is a duplicate of your new
attribute you added to the regulator core. Makes no sense to have a
private attribute too with the same value.
> + prop = "qcom,drms-mode-max-microamps";
As per comments in the bindings patch, I think we should move
"qcom,drms-mode-max-microamps" to the regulator core.
> + prop = "qcom,regulator-initial-microvolt";
As per comments in bindings patch: seems like we should get rid of
"qcom,regulator-initial-microvolt" or move to the core.
> + /*
> + * Default the voltage selector to an error value in the
> + * case that qcom,regulator-initial-microvolt is not
> + * specified in device tree since the true voltage is
> + * not known. Note that this value causes
> + * devm_regulator_register() to fail in the case that
> + * regulator-min-microvolt and regulator-max-microvolt
> + * are specified in device tree due to
> + * machine_constraints_voltage() bailing when the
> + * get_voltage_sel() callback returns this error value.
> + */
> + vreg->voltage_selector = -EINVAL;
As per comments in other threads, adjust this comment and use
-ENOTRECOVERABLE now.
NOTE: I think this driver is looking really good now. Hopefully the
above things should be quick to spin (even getting "max-microamps" in
the core should be quick I think) and we can get something landed!
:)
-Doug
^ permalink raw reply
* [PATCH 2/4] omap2: clockdomain: Inroduce cpu_pm notifiers for context save/restore
From: Tony Lindgren @ 2018-05-17 21:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526483821-25585-3-git-send-email-j-keerthy@ti.com>
* Keerthy <j-keerthy@ti.com> [180516 15:21]:
> Inroduce cpu_pm notifiers for context save/restore. This will be
> needed for am43xx family in case of rtc only mode with ddr in
> self-refresh.
...
> +static int cpu_notifier(struct notifier_block *nb, unsigned long cmd, void *v)
> +{
> + switch (cmd) {
> + case CPU_CLUSTER_PM_ENTER:
> + if (enable_off_mode)
> + clkdm_save_context();
> + break;
> + case CPU_CLUSTER_PM_EXIT:
> + if (enable_off_mode)
> + clkdm_restore_context();
> + break;
> + }
> +
> + return NOTIFY_OK;
> +}
Can you do this too only on suspend instead of the cpu notifier?
If you need to call this eventually from cpuidle also then yeah
the notifier makes sense.
Regards,
Tony
^ permalink raw reply
* [PATCH 4/4] omap2: powerdomain: Inroduce cpu_pm notifiers for context save/restore
From: Tony Lindgren @ 2018-05-17 21:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526483821-25585-5-git-send-email-j-keerthy@ti.com>
* Keerthy <j-keerthy@ti.com> [180516 15:21]:
> + /* Only AM43XX can lose pwrdm context during rtc-ddr suspend */
> + if (soc_is_am43xx()) {
> + nb.notifier_call = cpu_notifier;
> + cpu_pm_register_notifier(&nb);
> + }
> +
Hmm in patch 3/4 you add omap4_pwrdm_save_context(), I think
that we'd want to run with the notifier for cpuidle on omap4?
Regards,
Tony
^ permalink raw reply
* [PATCH] bus: ti-sysc: Fix optional clocks array access
From: Tony Lindgren @ 2018-05-17 22:14 UTC (permalink / raw)
To: linux-arm-kernel
We should be checking ddata->clocks[i] instead of clock_names[i]
for the optional clocks. Currently this just happens to work for
the typical case of one fck and one optional clock.
Fixes: 09dfe5810762 ("bus: ti-sysc: Add handling for clkctrl opt clocks")
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/bus/ti-sysc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -128,7 +128,7 @@ static int sysc_get_one_clock(struct sysc *ddata, const char *name)
if (index < 0) {
for (i = SYSC_OPTFCK0; i < SYSC_MAX_CLOCKS; i++) {
- if (!clock_names[i]) {
+ if (!ddata->clocks[i]) {
index = i;
break;
}
--
2.17.0
^ permalink raw reply
* [PATCH v7 2/2] drivers: soc: Add LLCC driver
From: Evan Green @ 2018-05-17 22:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526492623-20527-3-git-send-email-rishabhb@codeaurora.org>
On Wed, May 16, 2018 at 10:44 AM Rishabh Bhatnagar <rishabhb@codeaurora.org>
wrote:
> LLCC (Last Level Cache Controller) provides additional cache memory
> in the system. LLCC is partitioned into multiple slices and each
> slice gets its own priority, size, ID and other config parameters.
> LLCC driver programs these parameters for each slice. Clients that
> are assigned to use LLCC need to get information such size & ID of the
> slice they get and activate or deactivate the slice as needed. LLCC driver
> provides API for the clients to perform these operations.
> Signed-off-by: Channagoud Kadabi <ckadabi@codeaurora.org>
> Signed-off-by: Rishabh Bhatnagar <rishabhb@codeaurora.org>
> ---
> drivers/soc/qcom/Kconfig | 17 ++
> drivers/soc/qcom/Makefile | 2 +
> drivers/soc/qcom/llcc-sdm845.c | 106 ++++++++++++
> drivers/soc/qcom/llcc-slice.c | 337
+++++++++++++++++++++++++++++++++++++
> include/linux/soc/qcom/llcc-qcom.h | 180 ++++++++++++++++++++
> 5 files changed, 642 insertions(+)
> create mode 100644 drivers/soc/qcom/llcc-sdm845.c
> create mode 100644 drivers/soc/qcom/llcc-slice.c
> create mode 100644 include/linux/soc/qcom/llcc-qcom.h
Thanks Rishabh.
Reviewed-by: Evan Green <evgreen@chromium.org>
^ permalink raw reply
* [PATCH 0/4] Correct wlcore irqs for PM improvments for omap4 boards
From: Tony Lindgren @ 2018-05-17 23:35 UTC (permalink / raw)
To: linux-arm-kernel
Hi all,
Looks like we can get omap4 devices to do some PM with mainline kernel
mostly with just few dts tweaks. Mostly we just try to remove any GPIO
level interrupts and debounce clocks. But let's fix the wlcore irq for all
the boards as we know for sure it should be a level interrupt.
These changes allow pandaboard-es and duovero parlor to enter core retention
during idle, and droid4 to enter per retention during idle.
To have the devices enter idle, the UARTs need to be idled, and USB related
modules need to be unloaded or unbound. Additionally pandaboard-es somehow
needs also rmmod ti_soc_thermal which I don't quite understand.
On droid4, GPIO bank 1 needs to be somehow reset after init to not block
core retention. I also don't quite understand why this is, maybe because of
kexec booting?
For dealing with GPIO level interrupts, we're missing some GPIO PM runtime
patches that will be posted later after some more testing.
Regards,
Tony
Tony Lindgren (4):
ARM: OMAP2+: Make sure LOGICRETSTATE bits are not cleared
ARM: dts: Fix omap l4per idling with wlcore edge sensitive interrupt
ARM: dts: Configure duovero for to allow core retention during idle
omap4-droid4: Use software debounce for gpio-keys
arch/arm/boot/dts/am335x-baltos.dtsi | 2 +-
arch/arm/boot/dts/am335x-evm.dts | 2 +-
arch/arm/boot/dts/am335x-evmsk.dts | 2 +-
arch/arm/boot/dts/am437x-gp-evm.dts | 2 +-
arch/arm/boot/dts/logicpd-som-lv.dtsi | 2 +-
arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts | 2 +-
arch/arm/boot/dts/omap3-cm-t3517.dts | 2 +-
arch/arm/boot/dts/omap3-cm-t3730.dts | 2 +-
arch/arm/boot/dts/omap3-evm-common.dtsi | 2 +-
arch/arm/boot/dts/omap3-igep0020-rev-f.dts | 2 +-
arch/arm/boot/dts/omap3-igep0030-rev-g.dts | 2 +-
arch/arm/boot/dts/omap3-zoom3.dts | 2 +-
arch/arm/boot/dts/omap4-droid4-xt894.dts | 5 +++--
arch/arm/boot/dts/omap4-duovero-parlor.dts | 3 ++-
arch/arm/boot/dts/omap4-duovero.dtsi | 1 +
arch/arm/boot/dts/omap4-panda-common.dtsi | 2 +-
arch/arm/boot/dts/omap4-sdp.dts | 2 +-
arch/arm/boot/dts/omap4-var-som-om44-wlan.dtsi | 2 +-
arch/arm/boot/dts/omap5-board-common.dtsi | 2 +-
arch/arm/mach-omap2/pm44xx.c | 13 +++++++++++++
20 files changed, 35 insertions(+), 19 deletions(-)
--
2.17.0
^ permalink raw reply
* [PATCH 1/4] ARM: OMAP2+: Make sure LOGICRETSTATE bits are not cleared
From: Tony Lindgren @ 2018-05-17 23:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180517233542.30629-1-tony@atomide.com>
We may have LOGICRETSTATE cleared by the bootloader or kexec boot.
Currently this means we will see lost GPIO interrupts at least for
network interfaces such as wlcore and smsc911x if PER hits retention.
Let's fix the issue by making sure LOGICRETSTATE is set. Once we have
GPIOs working with wakeirqs then we should be able to clear it.
Cc: Keerthy <j-keerthy@ti.com>
Cc: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
arch/arm/mach-omap2/pm44xx.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/arm/mach-omap2/pm44xx.c b/arch/arm/mach-omap2/pm44xx.c
--- a/arch/arm/mach-omap2/pm44xx.c
+++ b/arch/arm/mach-omap2/pm44xx.c
@@ -131,6 +131,19 @@ static int __init pwrdms_setup(struct powerdomain *pwrdm, void *unused)
return 0;
}
+ /*
+ * Bootloader or kexec boot may have LOGICRETSTATE cleared
+ * for some domains. This is the case when kexec booting from
+ * Android kernels that support off mode for example.
+ * Make sure it's set at least for core and per, otherwise
+ * we currently will see lost GPIO interrupts for wlcore and
+ * smsc911x at least if per hits retention during idle.
+ */
+ if (!strncmp(pwrdm->name, "core", 4) ||
+ !strncmp(pwrdm->name, "l4per", 5) ||
+ !strncmp(pwrdm->name, "wkup", 4))
+ pwrdm_set_logic_retst(pwrdm, PWRDM_POWER_RET);
+
pwrst = kmalloc(sizeof(struct power_state), GFP_ATOMIC);
if (!pwrst)
return -ENOMEM;
--
2.17.0
^ permalink raw reply
* [PATCH 2/4] ARM: dts: Fix omap l4per idling with wlcore edge sensitive interrupt
From: Tony Lindgren @ 2018-05-17 23:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180517233542.30629-1-tony@atomide.com>
The wl1835mod.pdf data sheet says this pretty clearly for WL_IRQ line:
"WLAN SDIO out-of-band interrupt line. Set to rising edge (active high)
by default."
And it seems this interrupt can be optionally configured to use falling
edge too since commit bd763482c82e ("wl18xx: wlan_irq: support platform
dependent interrupt types").
On omaps, if the wlcore interrupt is configured as level instead of edge,
L4PER will stop doing hardware based idling after ifconfig wlan0 down is
done and the WL_EN line is pulled down.
The symptoms show up with L4PER status registers no longer showing the
IDLEST bits as 2 but as 0 for all the active GPIO banks and for
L4PER_CLKCTRL. Also the l4per_pwrdm RET count stops increasing in
the /sys/kernel/debug/pm_debug/count.
I'm guessing wlcore was at some point configured to use level interrupts
because of edge handling issues in gpio-omap. However, with the recent
fixes to gpio-omap the edge interrupts seem to be working just fine.
Let's fix the issue for all omap boards with wlcore interrupt set as level.
Cc: Dave Gerlach <d-gerlach@ti.com>
Cc: Eyal Reizer <eyalr@ti.com>
Cc: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: Nishanth Menon <nm@ti.com>
Cc: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
arch/arm/boot/dts/am335x-baltos.dtsi | 2 +-
arch/arm/boot/dts/am335x-evm.dts | 2 +-
arch/arm/boot/dts/am335x-evmsk.dts | 2 +-
arch/arm/boot/dts/am437x-gp-evm.dts | 2 +-
arch/arm/boot/dts/logicpd-som-lv.dtsi | 2 +-
arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts | 2 +-
arch/arm/boot/dts/omap3-cm-t3517.dts | 2 +-
arch/arm/boot/dts/omap3-cm-t3730.dts | 2 +-
arch/arm/boot/dts/omap3-evm-common.dtsi | 2 +-
arch/arm/boot/dts/omap3-igep0020-rev-f.dts | 2 +-
arch/arm/boot/dts/omap3-igep0030-rev-g.dts | 2 +-
arch/arm/boot/dts/omap3-zoom3.dts | 2 +-
arch/arm/boot/dts/omap4-droid4-xt894.dts | 2 +-
arch/arm/boot/dts/omap4-panda-common.dtsi | 2 +-
arch/arm/boot/dts/omap4-sdp.dts | 2 +-
arch/arm/boot/dts/omap4-var-som-om44-wlan.dtsi | 2 +-
arch/arm/boot/dts/omap5-board-common.dtsi | 2 +-
17 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/arch/arm/boot/dts/am335x-baltos.dtsi b/arch/arm/boot/dts/am335x-baltos.dtsi
--- a/arch/arm/boot/dts/am335x-baltos.dtsi
+++ b/arch/arm/boot/dts/am335x-baltos.dtsi
@@ -396,7 +396,7 @@
compatible = "ti,wl1835";
reg = <2>;
interrupt-parent = <&gpio3>;
- interrupts = <7 IRQ_TYPE_LEVEL_HIGH>;
+ interrupts = <7 IRQ_TYPE_EDGE_RISING>;
};
};
diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
--- a/arch/arm/boot/dts/am335x-evm.dts
+++ b/arch/arm/boot/dts/am335x-evm.dts
@@ -771,7 +771,7 @@
compatible = "ti,wl1835";
reg = <2>;
interrupt-parent = <&gpio3>;
- interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
+ interrupts = <17 IRQ_TYPE_EDGE_RISING>;
};
};
diff --git a/arch/arm/boot/dts/am335x-evmsk.dts b/arch/arm/boot/dts/am335x-evmsk.dts
--- a/arch/arm/boot/dts/am335x-evmsk.dts
+++ b/arch/arm/boot/dts/am335x-evmsk.dts
@@ -682,7 +682,7 @@
compatible = "ti,wl1271";
reg = <2>;
interrupt-parent = <&gpio0>;
- interrupts = <31 IRQ_TYPE_LEVEL_HIGH>; /* gpio 31 */
+ interrupts = <31 IRQ_TYPE_EDGE_RISING>; /* gpio 31 */
ref-clock-frequency = <38400000>;
};
};
diff --git a/arch/arm/boot/dts/am437x-gp-evm.dts b/arch/arm/boot/dts/am437x-gp-evm.dts
--- a/arch/arm/boot/dts/am437x-gp-evm.dts
+++ b/arch/arm/boot/dts/am437x-gp-evm.dts
@@ -790,7 +790,7 @@
compatible = "ti,wl1835";
reg = <2>;
interrupt-parent = <&gpio1>;
- interrupts = <23 IRQ_TYPE_LEVEL_HIGH>;
+ interrupts = <23 IRQ_TYPE_EDGE_RISING>;
};
};
diff --git a/arch/arm/boot/dts/logicpd-som-lv.dtsi b/arch/arm/boot/dts/logicpd-som-lv.dtsi
--- a/arch/arm/boot/dts/logicpd-som-lv.dtsi
+++ b/arch/arm/boot/dts/logicpd-som-lv.dtsi
@@ -113,7 +113,7 @@
compatible = "ti,wl1273";
reg = <2>;
interrupt-parent = <&gpio1>;
- interrupts = <2 IRQ_TYPE_LEVEL_HIGH>; /* gpio 2 */
+ interrupts = <2 IRQ_TYPE_EDGE_RISING>; /* gpio 2 */
ref-clock-frequency = <26000000>;
};
};
diff --git a/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts b/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
--- a/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
+++ b/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
@@ -48,7 +48,7 @@
compatible = "ti,wl1283";
reg = <2>;
interrupt-parent = <&gpio5>;
- interrupts = <24 IRQ_TYPE_LEVEL_HIGH>; /* gpio 152 */
+ interrupts = <24 IRQ_TYPE_EDGE_RISING>; /* gpio 152 */
ref-clock-frequency = <26000000>;
tcxo-clock-frequency = <26000000>;
};
diff --git a/arch/arm/boot/dts/omap3-cm-t3517.dts b/arch/arm/boot/dts/omap3-cm-t3517.dts
--- a/arch/arm/boot/dts/omap3-cm-t3517.dts
+++ b/arch/arm/boot/dts/omap3-cm-t3517.dts
@@ -141,7 +141,7 @@
compatible = "ti,wl1271";
reg = <2>;
interrupt-parent = <&gpio5>;
- interrupts = <17 IRQ_TYPE_LEVEL_HIGH>; /* gpio 145 */
+ interrupts = <17 IRQ_TYPE_EDGE_RISING>; /* gpio 145 */
ref-clock-frequency = <38400000>;
};
};
diff --git a/arch/arm/boot/dts/omap3-cm-t3730.dts b/arch/arm/boot/dts/omap3-cm-t3730.dts
--- a/arch/arm/boot/dts/omap3-cm-t3730.dts
+++ b/arch/arm/boot/dts/omap3-cm-t3730.dts
@@ -81,7 +81,7 @@
compatible = "ti,wl1271";
reg = <2>;
interrupt-parent = <&gpio5>;
- interrupts = <8 IRQ_TYPE_LEVEL_HIGH>; /* gpio 136 */
+ interrupts = <8 IRQ_TYPE_EDGE_RISING>; /* gpio 136 */
ref-clock-frequency = <38400000>;
};
};
diff --git a/arch/arm/boot/dts/omap3-evm-common.dtsi b/arch/arm/boot/dts/omap3-evm-common.dtsi
--- a/arch/arm/boot/dts/omap3-evm-common.dtsi
+++ b/arch/arm/boot/dts/omap3-evm-common.dtsi
@@ -133,7 +133,7 @@
compatible = "ti,wl1271";
reg = <2>;
interrupt-parent = <&gpio5>;
- interrupts = <21 IRQ_TYPE_LEVEL_HIGH>; /* gpio 149 */
+ interrupts = <21 IRQ_TYPE_EDGE_RISING>; /* gpio 149 */
ref-clock-frequency = <38400000>;
};
};
diff --git a/arch/arm/boot/dts/omap3-igep0020-rev-f.dts b/arch/arm/boot/dts/omap3-igep0020-rev-f.dts
--- a/arch/arm/boot/dts/omap3-igep0020-rev-f.dts
+++ b/arch/arm/boot/dts/omap3-igep0020-rev-f.dts
@@ -49,6 +49,6 @@
compatible = "ti,wl1835";
reg = <2>;
interrupt-parent = <&gpio6>;
- interrupts = <17 IRQ_TYPE_LEVEL_HIGH>; /* gpio 177 */
+ interrupts = <17 IRQ_TYPE_EDGE_RISING>; /* gpio 177 */
};
};
diff --git a/arch/arm/boot/dts/omap3-igep0030-rev-g.dts b/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
--- a/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
+++ b/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
@@ -71,6 +71,6 @@
compatible = "ti,wl1835";
reg = <2>;
interrupt-parent = <&gpio5>;
- interrupts = <8 IRQ_TYPE_LEVEL_HIGH>; /* gpio 136 */
+ interrupts = <8 IRQ_TYPE_EDGE_RISING>; /* gpio 136 */
};
};
diff --git a/arch/arm/boot/dts/omap3-zoom3.dts b/arch/arm/boot/dts/omap3-zoom3.dts
--- a/arch/arm/boot/dts/omap3-zoom3.dts
+++ b/arch/arm/boot/dts/omap3-zoom3.dts
@@ -202,7 +202,7 @@
compatible = "ti,wl1271";
reg = <2>;
interrupt-parent = <&gpio6>;
- interrupts = <2 IRQ_TYPE_LEVEL_HIGH>; /* gpio 162 */
+ interrupts = <2 IRQ_TYPE_EDGE_RISING>; /* gpio 162 */
ref-clock-frequency = <26000000>;
};
};
diff --git a/arch/arm/boot/dts/omap4-droid4-xt894.dts b/arch/arm/boot/dts/omap4-droid4-xt894.dts
--- a/arch/arm/boot/dts/omap4-droid4-xt894.dts
+++ b/arch/arm/boot/dts/omap4-droid4-xt894.dts
@@ -372,7 +372,7 @@
compatible = "ti,wl1285", "ti,wl1283";
reg = <2>;
interrupt-parent = <&gpio4>;
- interrupts = <4 IRQ_TYPE_LEVEL_HIGH>; /* gpio100 */
+ interrupts = <4 IRQ_TYPE_EDGE_RISING>; /* gpio100 */
ref-clock-frequency = <26000000>;
tcxo-clock-frequency = <26000000>;
};
diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi b/arch/arm/boot/dts/omap4-panda-common.dtsi
--- a/arch/arm/boot/dts/omap4-panda-common.dtsi
+++ b/arch/arm/boot/dts/omap4-panda-common.dtsi
@@ -464,7 +464,7 @@
compatible = "ti,wl1271";
reg = <2>;
interrupt-parent = <&gpio2>;
- interrupts = <21 IRQ_TYPE_LEVEL_HIGH>; /* gpio 53 */
+ interrupts = <21 IRQ_TYPE_EDGE_RISING>; /* gpio 53 */
ref-clock-frequency = <38400000>;
};
};
diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts
--- a/arch/arm/boot/dts/omap4-sdp.dts
+++ b/arch/arm/boot/dts/omap4-sdp.dts
@@ -493,7 +493,7 @@
compatible = "ti,wl1281";
reg = <2>;
interrupt-parent = <&gpio1>;
- interrupts = <21 IRQ_TYPE_LEVEL_HIGH>; /* gpio 53 */
+ interrupts = <21 IRQ_TYPE_EDGE_RISING>; /* gpio 53 */
ref-clock-frequency = <26000000>;
tcxo-clock-frequency = <26000000>;
};
diff --git a/arch/arm/boot/dts/omap4-var-som-om44-wlan.dtsi b/arch/arm/boot/dts/omap4-var-som-om44-wlan.dtsi
--- a/arch/arm/boot/dts/omap4-var-som-om44-wlan.dtsi
+++ b/arch/arm/boot/dts/omap4-var-som-om44-wlan.dtsi
@@ -72,7 +72,7 @@
compatible = "ti,wl1271";
reg = <2>;
interrupt-parent = <&gpio2>;
- interrupts = <9 IRQ_TYPE_LEVEL_HIGH>; /* gpio 41 */
+ interrupts = <9 IRQ_TYPE_EDGE_RISING>; /* gpio 41 */
ref-clock-frequency = <38400000>;
};
};
diff --git a/arch/arm/boot/dts/omap5-board-common.dtsi b/arch/arm/boot/dts/omap5-board-common.dtsi
--- a/arch/arm/boot/dts/omap5-board-common.dtsi
+++ b/arch/arm/boot/dts/omap5-board-common.dtsi
@@ -364,7 +364,7 @@
pinctrl-names = "default";
pinctrl-0 = <&wlcore_irq_pin>;
interrupt-parent = <&gpio1>;
- interrupts = <14 IRQ_TYPE_LEVEL_HIGH>; /* gpio 14 */
+ interrupts = <14 IRQ_TYPE_EDGE_RISING>; /* gpio 14 */
ref-clock-frequency = <26000000>;
};
};
--
2.17.0
^ permalink raw reply
* [PATCH 3/4] ARM: dts: Configure duovero for to allow core retention during idle
From: Tony Lindgren @ 2018-05-17 23:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180517233542.30629-1-tony@atomide.com>
By reconfiguring few GPIOs in the dts file we can make duovero parlor
hit retention during idle:
1. Let's a larger debounce value for gpio-keys
This will then make gpio-keys use software debounce and the GPIO
debounce clock is not enabled.
2. Let's allow WLAN suspend for mwifiex
This can be done just by adding keep-power-in-suspend.
3. Let's reconfigure smsc911x driver to use GPIO edge interrupt
This will allow using NFSroot while the system idles, and the kernel
has quite a few dts files with "smsc,lan9115" compatible using edge
interrupts.
Then to have the system hit core retention during idle, the UARTs
needs to be idled and USB modules need to be unloaded or unbound.
Cc: Ash Charles <ash@gumstix.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
arch/arm/boot/dts/omap4-duovero-parlor.dts | 3 ++-
arch/arm/boot/dts/omap4-duovero.dtsi | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/omap4-duovero-parlor.dts b/arch/arm/boot/dts/omap4-duovero-parlor.dts
--- a/arch/arm/boot/dts/omap4-duovero-parlor.dts
+++ b/arch/arm/boot/dts/omap4-duovero-parlor.dts
@@ -36,6 +36,7 @@
label = "button0";
linux,code = <BTN_0>;
gpios = <&gpio4 25 GPIO_ACTIVE_LOW>; /* gpio_121 */
+ debounce-interval = <10>;
wakeup-source;
};
};
@@ -140,7 +141,7 @@
ethernet at gpmc {
reg = <5 0 0xff>;
interrupt-parent = <&gpio2>;
- interrupts = <12 IRQ_TYPE_LEVEL_LOW>; /* gpio_44 */
+ interrupts = <12 IRQ_TYPE_EDGE_FALLING>; /* gpio_44 */
phy-mode = "mii";
diff --git a/arch/arm/boot/dts/omap4-duovero.dtsi b/arch/arm/boot/dts/omap4-duovero.dtsi
--- a/arch/arm/boot/dts/omap4-duovero.dtsi
+++ b/arch/arm/boot/dts/omap4-duovero.dtsi
@@ -248,6 +248,7 @@
ti,bus-width = <4>;
ti,non-removable;
cap-power-off-card;
+ keep-power-in-suspend;
};
&twl_usb_comparator {
--
2.17.0
^ permalink raw reply
* [PATCH 4/4] omap4-droid4: Use software debounce for gpio-keys
From: Tony Lindgren @ 2018-05-17 23:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180517233542.30629-1-tony@atomide.com>
The default value of 5ms will use GPIO hardware based debounce clocks
that will keep L4PER from idling consuming about extra 30mW.
Use a value of 10ms that is above the hardware debounce maximum of
7.95ms forcing software based debouncing.
This allows droid4 to enter PER retention during idle as long as UARTs
are idled and USB modules unloaded or unbound.
Note that there seems to be a pending issue with having droid 4 enter core
retention during idle where GPIO bank 1 needs to be reset late after init
for some reason to not block core retention. In addition to that, we are
also missing GPIO related PM runtime calls for omap4 that will be posted
separately.
Cc: Marcel Partap <mpartap@gmx.net>
Cc: Merlijn Wajer <merlijn@wizzup.org>
Cc: Michael Scott <hashcode0f@gmail.com>
Cc: NeKit <nekit1000@gmail.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Sebastian Reichel <sre@kernel.org>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
arch/arm/boot/dts/omap4-droid4-xt894.dts | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/omap4-droid4-xt894.dts b/arch/arm/boot/dts/omap4-droid4-xt894.dts
--- a/arch/arm/boot/dts/omap4-droid4-xt894.dts
+++ b/arch/arm/boot/dts/omap4-droid4-xt894.dts
@@ -128,6 +128,7 @@
gpios = <&gpio5 26 GPIO_ACTIVE_LOW>; /* gpio154 */
linux,code = <KEY_VOLUMEDOWN>;
linux,can-disable;
+ debounce-interval = <10>;
};
slider {
@@ -136,7 +137,7 @@
linux,input-type = <EV_SW>;
linux,code = <SW_KEYPAD_SLIDE>;
linux,can-disable;
-
+ debounce-interval = <10>;
};
};
--
2.17.0
^ permalink raw reply
* [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: David Collins @ 2018-05-18 0:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAD=FV=XegPeFR8pdZM-YiBBJjrbej=C+2biBt4zJKnE90uTYZA@mail.gmail.com>
On 05/17/2018 02:22 PM, Doug Anderson wrote:
> On Fri, May 11, 2018 at 7:28 PM, David Collins <collinsd@codeaurora.org> wrote:
>> +- qcom,regulator-initial-microvolt
>> + Usage: optional; VRM regulators only
>> + Value type: <u32>
>> + Definition: Specifies the initial voltage in microvolts to request for a
>> + VRM regulator.
>
> Now that Mark has landed the patch adding support for the
> -ENOTRECOVERABLE error code from get_voltage() / get_voltage_sel(), do
> we still need the qcom,regulator-initial-microvolt property?
Yes, this is still needed. The -ENOTRECOVERABLE patch ensures that
qcom-rpmh-regulator devices can be registered even if
qcom,regulator-initial-microvolt is not specified. However, that will
result in the regulators being configured for the minimum voltage
supported in the DT specified min/max range. The
qcom,regulator-initial-microvolt property allows us to set a specific
voltage that is larger than the min constraint.
> If this is really still needed, can it be moved to the regulator core?
I'm not opposed to the idea, but I think that Mark is [1]:
>> Do you have a preference for qcom,regulator-initial-microvolt vs a generic
>> framework supported regulator-initial-microvolt property for configuring a
>> specific voltage at registration time? We'll need to have support for one
>> or the other in order for the qcom_rpmh-regulator driver to be functional.
>
> This is basically specific to Qualcomm, I can't off hand think of any
> other devices with similar issues.
>> +- regulator-initial-mode
>> + Usage: optional; VRM regulators only
>> + Value type: <u32>
>> + Definition: Specifies the initial mode to request for a VRM regulator.
>> + Supported values are RPMH_REGULATOR_MODE_* which are defined
>> + in [1] (i.e. 0 to 3). This property may be specified even
>> + if the regulator-allow-set-load property is not specified.
>
> Every time I read the above I wonder why you're documenting a standard
> regulator regulator property in your bindings. ...then I realize it's
> because you're doing it because you want to explicitly document what
> the valid modes are. I wonder if it makes sense to just put a
> reference somewhere else in this document to go look at the header
> file where these are all nicely documented.
Isn't that what the [1] in the above snippet is currently doing. Further
down in qcom,rpmh-regulator.txt is this line:
+[1] include/dt-bindings/regulator/qcom,rpmh-regulator.h
> Speaking of documenting things like that, it might be worth finding
> somewhere in this doc to mention that the "bob" regulator on PMI8998
> can support "regulator-allow-bypass". That tidbit got lost when we
> moved to the standard regulator bindings for bypass.
I suppose that I could add something like this:
+- regulator-allow-bypass
+ Usage: optional; BOB type VRM regulators only
+ Value type: <empty>
+ Definition: See [2] for details.
...
+[2]: Documentation/devicetree/bindings/regulator.txt
However, I don't want the patch to get NACKed because it is defining a
property that is already defined in the common regulator.txt file.
>> +- qcom,allowed-drms-modes
>> + Usage: required if regulator-allow-set-load is specified;
>> + VRM regulators only
>> + Value type: <prop-encoded-array>
>> + Definition: A list of integers specifying the PMIC regulator modes which
>> + can be configured at runtime based upon consumer load needs.
>> + Supported values are RPMH_REGULATOR_MODE_* which are defined
>> + in [1] (i.e. 0 to 3).
>
> Why is this still here? You moved it to the core regulator framework,
> right? It's still in your examples too. Shouldn't this be removed?
> It looks like the driver still needs this and it needs to be an exact
> duplicate of the common binding. That doesn't seem right...
The qcom,allowed-drms-modes property supports a different feature than the
regulator-allowed-modes property accepted in [2]. The latter specifies
the modes that may be used at all (e.g. in regulator_set_mode() calls) and
it lists the mode values in an unordered fashion.
qcom,allowed-drms-modes defines a specific subset of the possible allowed
modes that should be set based on DRMS (e.g. in regulator_set_load()
calls). Its values are listed in a specific order and must match 1-to-1
with qcom,drms-mode-max-microamps entries.
It would probably be good to change the name of the property from
qcom,allowed-drms-modes to qcom,regulator-drms-modes.
>> +- qcom,drms-mode-max-microamps
>> + Usage: required if regulator-allow-set-load is specified;
>> + VRM regulators only
>> + Value type: <prop-encoded-array>
>> + Definition: A list of integers specifying the maximum allowed load
>> + current in microamps for each of the modes listed in
>> + qcom,allowed-drms-modes (matched 1-to-1 in order). Elements
>> + must be specified in order from lowest to highest value.
>
> Any reason this can't go into the regulator core? You'd basically
> just take the existing concept of rpmh_regulator_vrm_set_load() and
> put it in the core.
This could be implemented in the core via new constraint elements parsed
in of_regulator and a helper function to specify in regulator_ops.
However, I'm not sure about the wide-spread applicability of this feature.
I'd prefer to leave it in the driver unless Mark would like me to add it
into the core.
>> +- qcom,headroom-microvolt
>> + Usage: optional; VRM regulators only
>> + Value type: <u32>
>> + Definition: Specifies the headroom voltage in microvolts to request for
>> + a VRM regulator. RPMh hardware automatically ensures that
>> + the parent of this regulator outputs a voltage high enough
>> + to satisfy the requested headroom. Supported values are
>> + 0 to 511000.
>
> I'm curious: is this a voted-for value, or a global value?
>
> Said another way: the whole point of RPMh is that there may be more
> than one processor that needs the same rails, right? So the AP might
> request 1.1 V for a rail and the modem might request 1.3 V. RPMh
> would decide to pick the higher of those two (1.3 V), but if the modem
> said it no longer needs the rail it will drop down to 1.1 V.
>
> ...and as an example of why the headroom needs to be in hardware, if
> the source voltage was normally 1.4 V and the headroom was 200 mV then
> the hardware would need to know to bump up the source voltage to 1.5V
> during the period of of time that the modem wants the rail at 1.3V.
>
> So my question is: do the AP and modem in the above situation
> separately vote for headroom? How is it aggregated? ...or is it a
> global value and this sets the headroom for all clients of RPMh? It
> would be interesting to document this as it might help with figuring
> out how this value should be set.
The headroom voltage voting is supported in hardware per-regulator and
per-master (AP, modem, etc). The headroom voltage and output voltage are
each aggregated (using max) per-regulator across masters. If the
aggregated enable state for a regulator is on, then the aggregated output
voltage and headroom voltage are added together and applied as a min
constraint on the parent's output voltage (if there is a parent).
>> diff --git a/include/dt-bindings/regulator/qcom,rpmh-regulator.h b/include/dt-bindings/regulator/qcom,rpmh-regulator.h
>> new file mode 100644
>> index 0000000..4378c4b
>> --- /dev/null
>> +++ b/include/dt-bindings/regulator/qcom,rpmh-regulator.h
>> +/*
>> + * These mode constants may be used for regulator-initial-mode and
>> + * qcom,allowed-drms-modes properties of an RPMh regulator device tree node.
>
> Technically also for your new "regulator-allowed-modes". Maybe just
> say that they're used anywhere a regulator mode is needed in this
> driver and give regulator-initial-mode as an example?
Sure, I'll update this description.
Take care,
David
[1]: https://lkml.org/lkml/2018/4/24/960
[2]: https://lkml.org/lkml/2018/5/11/696
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH v3 2/2] regulator: add QCOM RPMh regulator driver
From: David Collins @ 2018-05-18 0:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAD=FV=UiZEFTAtO9C0UbRO=ow5=dv6zNG-XMqhHm1Fmp2GwVcA@mail.gmail.com>
On 05/17/2018 02:23 PM, Doug Anderson wrote:
> On Fri, May 11, 2018 at 7:28 PM, David Collins <collinsd@codeaurora.org> wrote:
>> + /*
>> + * Default the voltage selector to an error value in the
>> + * case that qcom,regulator-initial-microvolt is not
>> + * specified in device tree since the true voltage is
>> + * not known. Note that this value causes
>> + * devm_regulator_register() to fail in the case that
>> + * regulator-min-microvolt and regulator-max-microvolt
>> + * are specified in device tree due to
>> + * machine_constraints_voltage() bailing when the
>> + * get_voltage_sel() callback returns this error value.
>> + */
>> + vreg->voltage_selector = -EINVAL;
>
> As per comments in other threads, adjust this comment and use
> -ENOTRECOVERABLE now.
I'll make this change.
Take care,
David
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox