* [PATCH 3/3] ARM: dts: imx6dl-riotboard: fix OTG regulator polarity
From: Alexander Kurz @ 2018-05-26 18:30 UTC (permalink / raw)
To: Shawn Guo; +Cc: Fabio Estevam, devicetree, Alexander Kurz, linux-arm-kernel
In-Reply-To: <20180526183053.14029-1-akurz@blala.de>
USB OTG power is switched on when the GPIO is pulled low.
Signed-off-by: Alexander Kurz <akurz@blala.de>
---
arch/arm/boot/dts/imx6dl-riotboard.dts | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/imx6dl-riotboard.dts b/arch/arm/boot/dts/imx6dl-riotboard.dts
index a52e05934e3a..ef3b270c15e5 100644
--- a/arch/arm/boot/dts/imx6dl-riotboard.dts
+++ b/arch/arm/boot/dts/imx6dl-riotboard.dts
@@ -70,8 +70,7 @@
regulator-name = "usb_otg_vbus";
regulator-min-microvolt = <5000000>;
regulator-max-microvolt = <5000000>;
- gpio = <&gpio3 22 0>;
- enable-active-high;
+ gpio = <&gpio3 22 GPIO_ACTIVE_LOW>;
};
};
--
2.11.0
^ permalink raw reply related
* [PATCH 2/3] ARM: dts: imx6qdl-wandboard: enable USB OTG
From: Alexander Kurz @ 2018-05-26 18:30 UTC (permalink / raw)
To: Shawn Guo; +Cc: Fabio Estevam, devicetree, Alexander Kurz, linux-arm-kernel
In-Reply-To: <20180526183053.14029-1-akurz@blala.de>
Enable USB OTG (dual-role) on the Wandboard.
Note, that the USB_OTG_VBUS current is quite limited due to a 22R resistor
in the power line. Hence, the overcurrent signal of 1A will never be
triggered on this board.
Signed-off-by: Alexander Kurz <akurz@blala.de>
---
arch/arm/boot/dts/imx6qdl-wandboard.dtsi | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/imx6qdl-wandboard.dtsi b/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
index ff7e824dfd28..4ac8ee355c42 100644
--- a/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
@@ -48,6 +48,16 @@
regulator-max-microvolt = <3300000>;
regulator-always-on;
};
+
+ reg_usb_otg_vbus: fixedregulator@2 {
+ compatible = "regulator-fixed";
+ regulator-name = "usb_otg_vbus";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usbotgvbus>;
+ gpio = <&gpio3 22 GPIO_ACTIVE_LOW>;
+ };
};
&audmux {
@@ -162,6 +172,12 @@
>;
};
+ pinctrl_usbotgvbus: usbotgvbusgrp {
+ fsl,pins = <
+ MX6QDL_PAD_EIM_D22__GPIO3_IO22 0x130b0
+ >;
+ };
+
pinctrl_usdhc1: usdhc1grp {
fsl,pins = <
MX6QDL_PAD_SD1_CMD__SD1_CMD 0x17059
@@ -236,10 +252,11 @@
};
&usbotg {
+ vbus-supply = <®_usb_otg_vbus>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_usbotg>;
disable-over-current;
- dr_mode = "peripheral";
+ dr_mode = "otg";
status = "okay";
};
--
2.11.0
^ permalink raw reply related
* [PATCH 1/3] ARM: dts: imx6 wandboard and riotboard: remove regulators bus node
From: Alexander Kurz @ 2018-05-26 18:30 UTC (permalink / raw)
To: Shawn Guo; +Cc: Fabio Estevam, devicetree, Alexander Kurz, linux-arm-kernel
In-Reply-To: <20180526183053.14029-1-akurz@blala.de>
To match the convention, move regulator-fixed nodes directly into
the root node.
Signed-off-by: Alexander Kurz <akurz@blala.de>
---
arch/arm/boot/dts/imx6dl-riotboard.dts | 55 +++++++++++++-------------------
arch/arm/boot/dts/imx6qdl-wandboard.dtsi | 40 ++++++++++-------------
2 files changed, 39 insertions(+), 56 deletions(-)
diff --git a/arch/arm/boot/dts/imx6dl-riotboard.dts b/arch/arm/boot/dts/imx6dl-riotboard.dts
index 2e98c92adff7..a52e05934e3a 100644
--- a/arch/arm/boot/dts/imx6dl-riotboard.dts
+++ b/arch/arm/boot/dts/imx6dl-riotboard.dts
@@ -19,38 +19,6 @@
reg = <0x10000000 0x40000000>;
};
- regulators {
- compatible = "simple-bus";
- #address-cells = <1>;
- #size-cells = <0>;
-
- reg_2p5v: regulator@0 {
- compatible = "regulator-fixed";
- reg = <0>;
- regulator-name = "2P5V";
- regulator-min-microvolt = <2500000>;
- regulator-max-microvolt = <2500000>;
- };
-
- reg_3p3v: regulator@1 {
- compatible = "regulator-fixed";
- reg = <1>;
- regulator-name = "3P3V";
- regulator-min-microvolt = <3300000>;
- regulator-max-microvolt = <3300000>;
- };
-
- reg_usb_otg_vbus: regulator@2 {
- compatible = "regulator-fixed";
- reg = <2>;
- regulator-name = "usb_otg_vbus";
- regulator-min-microvolt = <5000000>;
- regulator-max-microvolt = <5000000>;
- gpio = <&gpio3 22 0>;
- enable-active-high;
- };
- };
-
leds {
compatible = "gpio-leds";
pinctrl-names = "default";
@@ -82,6 +50,29 @@
mux-int-port = <1>;
mux-ext-port = <3>;
};
+
+ reg_2p5v: fixedregulator@0 {
+ compatible = "regulator-fixed";
+ regulator-name = "2P5V";
+ regulator-min-microvolt = <2500000>;
+ regulator-max-microvolt = <2500000>;
+ };
+
+ reg_3p3v: fixedregulator@1 {
+ compatible = "regulator-fixed";
+ regulator-name = "3P3V";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ };
+
+ reg_usb_otg_vbus: fixedregulator@2 {
+ compatible = "regulator-fixed";
+ regulator-name = "usb_otg_vbus";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ gpio = <&gpio3 22 0>;
+ enable-active-high;
+ };
};
&audmux {
diff --git a/arch/arm/boot/dts/imx6qdl-wandboard.dtsi b/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
index ed96d7b5feab..ff7e824dfd28 100644
--- a/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-wandboard.dtsi
@@ -12,30 +12,6 @@
#include <dt-bindings/gpio/gpio.h>
/ {
- regulators {
- compatible = "simple-bus";
- #address-cells = <1>;
- #size-cells = <0>;
-
- reg_2p5v: regulator@0 {
- compatible = "regulator-fixed";
- reg = <0>;
- regulator-name = "2P5V";
- regulator-min-microvolt = <2500000>;
- regulator-max-microvolt = <2500000>;
- regulator-always-on;
- };
-
- reg_3p3v: regulator@1 {
- compatible = "regulator-fixed";
- reg = <1>;
- regulator-name = "3P3V";
- regulator-min-microvolt = <3300000>;
- regulator-max-microvolt = <3300000>;
- regulator-always-on;
- };
- };
-
sound {
compatible = "fsl,imx6-wandboard-sgtl5000",
"fsl,imx-audio-sgtl5000";
@@ -56,6 +32,22 @@
spdif-controller = <&spdif>;
spdif-out;
};
+
+ reg_2p5v: fixedregulator@0 {
+ compatible = "regulator-fixed";
+ regulator-name = "2P5V";
+ regulator-min-microvolt = <2500000>;
+ regulator-max-microvolt = <2500000>;
+ regulator-always-on;
+ };
+
+ reg_3p3v: fixedregulator@1 {
+ compatible = "regulator-fixed";
+ regulator-name = "3P3V";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ };
};
&audmux {
--
2.11.0
^ permalink raw reply related
* ARM: dts: imx6: enable OTG on wandboard and riotboard
From: Alexander Kurz @ 2018-05-26 18:30 UTC (permalink / raw)
To: Shawn Guo; +Cc: Fabio Estevam, devicetree, linux-arm-kernel
Both boards wandboard and riotboard feature an USB OTG connector.
Change the DT regulator nodes to match the convention, fix the
GPIO regulator polatity for the riotboard and add enable OTG
including the regulator infrastructure on the riotboard.
^ permalink raw reply
* Re: [PATCH 3/6] arm64: dts: hisilicon: Add missing cooling device properties for CPUs
From: Wei Xu @ 2018-05-26 18:21 UTC (permalink / raw)
To: Viresh Kumar, arm, Rob Herring, Mark Rutland, Catalin Marinas,
Will Deacon
Cc: Vincent Guittot, ionela.voinescu, Daniel Lezcano, chris.redpath,
linux-arm-kernel, devicetree, linux-kernel
In-Reply-To: <5B09A0B4.2080400@hisilicon.com>
Hi Viresh,
On 2018/5/26 19:00, Wei Xu wrote:
> Hi Viresh,
>
> On 2018/5/25 6:40, Viresh Kumar wrote:
>> The cooling device properties, like "#cooling-cells" and
>> "dynamic-power-coefficient", should either be present for all the CPUs
>> of a cluster or none. If these are present only for a subset of CPUs of
>> a cluster then things will start falling apart as soon as the CPUs are
>> brought online in a different order. For example, this will happen
>> because the operating system looks for such properties in the CPU node
>> it is trying to bring up, so that it can register a cooling device.
>>
>> Add such missing properties.
>>
>> Do minor rearrangement as well to keep ordering consistent.
>>
>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>
> Thanks!
> Applied to the hisilicon fix tree.
Sorry for the noise!
It seems this patch is still under discussion.
I will drop it firstly.
Best Regards,
Wei
>
> Best Regards,
> Wei
>
>> ---
>> arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 16 +++++++++++++++-
>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>> index 586b281cd531..247024df714f 100644
>> --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>> +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>> @@ -88,8 +88,8 @@
>> next-level-cache = <&CLUSTER0_L2>;
>> clocks = <&stub_clock 0>;
>> operating-points-v2 = <&cpu_opp_table>;
>> - #cooling-cells = <2>; /* min followed by max */
>> cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
>> + #cooling-cells = <2>; /* min followed by max */
>> dynamic-power-coefficient = <311>;
>> };
>>
>> @@ -101,6 +101,8 @@
>> next-level-cache = <&CLUSTER0_L2>;
>> operating-points-v2 = <&cpu_opp_table>;
>> cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
>> + #cooling-cells = <2>; /* min followed by max */
>> + dynamic-power-coefficient = <311>;
>> };
>>
>> cpu2: cpu@2 {
>> @@ -111,6 +113,8 @@
>> next-level-cache = <&CLUSTER0_L2>;
>> operating-points-v2 = <&cpu_opp_table>;
>> cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
>> + #cooling-cells = <2>; /* min followed by max */
>> + dynamic-power-coefficient = <311>;
>> };
>>
>> cpu3: cpu@3 {
>> @@ -121,6 +125,8 @@
>> next-level-cache = <&CLUSTER0_L2>;
>> operating-points-v2 = <&cpu_opp_table>;
>> cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
>> + #cooling-cells = <2>; /* min followed by max */
>> + dynamic-power-coefficient = <311>;
>> };
>>
>> cpu4: cpu@100 {
>> @@ -131,6 +137,8 @@
>> next-level-cache = <&CLUSTER1_L2>;
>> operating-points-v2 = <&cpu_opp_table>;
>> cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
>> + #cooling-cells = <2>; /* min followed by max */
>> + dynamic-power-coefficient = <311>;
>> };
>>
>> cpu5: cpu@101 {
>> @@ -141,6 +149,8 @@
>> next-level-cache = <&CLUSTER1_L2>;
>> operating-points-v2 = <&cpu_opp_table>;
>> cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
>> + #cooling-cells = <2>; /* min followed by max */
>> + dynamic-power-coefficient = <311>;
>> };
>>
>> cpu6: cpu@102 {
>> @@ -151,6 +161,8 @@
>> next-level-cache = <&CLUSTER1_L2>;
>> operating-points-v2 = <&cpu_opp_table>;
>> cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
>> + #cooling-cells = <2>; /* min followed by max */
>> + dynamic-power-coefficient = <311>;
>> };
>>
>> cpu7: cpu@103 {
>> @@ -161,6 +173,8 @@
>> next-level-cache = <&CLUSTER1_L2>;
>> operating-points-v2 = <&cpu_opp_table>;
>> cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
>> + #cooling-cells = <2>; /* min followed by max */
>> + dynamic-power-coefficient = <311>;
>> };
>>
>> CLUSTER0_L2: l2-cache0 {
>>
^ permalink raw reply
* Re: [PATCH 3/6] arm64: dts: hisilicon: Add missing cooling device properties for CPUs
From: Wei Xu @ 2018-05-26 18:00 UTC (permalink / raw)
To: Viresh Kumar, arm, Rob Herring, Mark Rutland, Catalin Marinas,
Will Deacon
Cc: Vincent Guittot, ionela.voinescu, Daniel Lezcano, chris.redpath,
linux-arm-kernel, devicetree, linux-kernel
In-Reply-To: <0754957a2c3842cf4e36fa27231d327fd8d6d499.1527225682.git.viresh.kumar@linaro.org>
Hi Viresh,
On 2018/5/25 6:40, Viresh Kumar wrote:
> The cooling device properties, like "#cooling-cells" and
> "dynamic-power-coefficient", should either be present for all the CPUs
> of a cluster or none. If these are present only for a subset of CPUs of
> a cluster then things will start falling apart as soon as the CPUs are
> brought online in a different order. For example, this will happen
> because the operating system looks for such properties in the CPU node
> it is trying to bring up, so that it can register a cooling device.
>
> Add such missing properties.
>
> Do minor rearrangement as well to keep ordering consistent.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Thanks!
Applied to the hisilicon fix tree.
Best Regards,
Wei
> ---
> arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> index 586b281cd531..247024df714f 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> @@ -88,8 +88,8 @@
> next-level-cache = <&CLUSTER0_L2>;
> clocks = <&stub_clock 0>;
> operating-points-v2 = <&cpu_opp_table>;
> - #cooling-cells = <2>; /* min followed by max */
> cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> + #cooling-cells = <2>; /* min followed by max */
> dynamic-power-coefficient = <311>;
> };
>
> @@ -101,6 +101,8 @@
> next-level-cache = <&CLUSTER0_L2>;
> operating-points-v2 = <&cpu_opp_table>;
> cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> + #cooling-cells = <2>; /* min followed by max */
> + dynamic-power-coefficient = <311>;
> };
>
> cpu2: cpu@2 {
> @@ -111,6 +113,8 @@
> next-level-cache = <&CLUSTER0_L2>;
> operating-points-v2 = <&cpu_opp_table>;
> cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> + #cooling-cells = <2>; /* min followed by max */
> + dynamic-power-coefficient = <311>;
> };
>
> cpu3: cpu@3 {
> @@ -121,6 +125,8 @@
> next-level-cache = <&CLUSTER0_L2>;
> operating-points-v2 = <&cpu_opp_table>;
> cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> + #cooling-cells = <2>; /* min followed by max */
> + dynamic-power-coefficient = <311>;
> };
>
> cpu4: cpu@100 {
> @@ -131,6 +137,8 @@
> next-level-cache = <&CLUSTER1_L2>;
> operating-points-v2 = <&cpu_opp_table>;
> cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> + #cooling-cells = <2>; /* min followed by max */
> + dynamic-power-coefficient = <311>;
> };
>
> cpu5: cpu@101 {
> @@ -141,6 +149,8 @@
> next-level-cache = <&CLUSTER1_L2>;
> operating-points-v2 = <&cpu_opp_table>;
> cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> + #cooling-cells = <2>; /* min followed by max */
> + dynamic-power-coefficient = <311>;
> };
>
> cpu6: cpu@102 {
> @@ -151,6 +161,8 @@
> next-level-cache = <&CLUSTER1_L2>;
> operating-points-v2 = <&cpu_opp_table>;
> cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> + #cooling-cells = <2>; /* min followed by max */
> + dynamic-power-coefficient = <311>;
> };
>
> cpu7: cpu@103 {
> @@ -161,6 +173,8 @@
> next-level-cache = <&CLUSTER1_L2>;
> operating-points-v2 = <&cpu_opp_table>;
> cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> + #cooling-cells = <2>; /* min followed by max */
> + dynamic-power-coefficient = <311>;
> };
>
> CLUSTER0_L2: l2-cache0 {
>
^ permalink raw reply
* [PATCH v1] media: dt: bindings: tegra-vde: Document new optional Memory Client reset property
From: Dmitry Osipenko @ 2018-05-26 14:33 UTC (permalink / raw)
To: Rob Herring, Thierry Reding
Cc: linux-tegra, linux-media, devicetree, linux-kernel
Recently binding of the Memory Controller has been extended, exposing
the Memory Client reset controls and hence it is now a reset controller.
Tegra video-decoder device is among the Memory Controller reset users,
document the new optional VDE HW reset property.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
.../devicetree/bindings/media/nvidia,tegra-vde.txt | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/nvidia,tegra-vde.txt b/Documentation/devicetree/bindings/media/nvidia,tegra-vde.txt
index 470237ed6fe5..7302e949e662 100644
--- a/Documentation/devicetree/bindings/media/nvidia,tegra-vde.txt
+++ b/Documentation/devicetree/bindings/media/nvidia,tegra-vde.txt
@@ -27,9 +27,15 @@ Required properties:
- sxe
- clocks : Must include the following entries:
- vde
-- resets : Must include the following entries:
+- resets : Must contain an entry for each entry in reset-names.
+- reset-names : Should include the following entries:
- vde
+Optional properties:
+- resets : Must contain an entry for each entry in reset-names.
+- reset-names : Must include the following entries:
+ - mc
+
Example:
video-codec@6001a000 {
@@ -51,5 +57,6 @@ video-codec@6001a000 {
<GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>; /* SXE interrupt */
interrupt-names = "sync-token", "bsev", "sxe";
clocks = <&tegra_car TEGRA20_CLK_VDE>;
- resets = <&tegra_car 61>;
+ reset-names = "vde", "mc";
+ resets = <&tegra_car 61>, <&mc TEGRA20_MC_RESET_VDE>;
};
--
2.17.0
^ permalink raw reply related
* Re: [PATCH v7 5/5] clk: renesas: Renesas R9A06G032 clock driver
From: kbuild test robot @ 2018-05-26 13:45 UTC (permalink / raw)
Cc: kbuild-all, linux-renesas-soc, Simon Horman, phil.edworthy,
Michel Pollet, Michel Pollet, Michael Turquette, Stephen Boyd,
Rob Herring, Mark Rutland, Geert Uytterhoeven, linux-clk,
devicetree, linux-kernel
In-Reply-To: <1527154169-32380-6-git-send-email-michel.pollet@bp.renesas.com>
Hi Michel,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on renesas-drivers/clk-renesas]
[also build test WARNING on v4.17-rc6]
[cannot apply to robh/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Michel-Pollet/dt-bindings-Add-the-r9a06g032-sysctrl-h-file/20180526-154235
base: https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git clk-renesas
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> drivers/clk/renesas/r9a06g032-clocks.c:430:22: sparse: cast removes address space of expression
>> drivers/clk/renesas/r9a06g032-clocks.c:431:30: sparse: incorrect type in argument 1 (different address spaces) @@ expected unsigned int [noderef] [usertype] <asn:2>*reg @@ got eref] [usertype] <asn:2>*reg @@
drivers/clk/renesas/r9a06g032-clocks.c:431:30: expected unsigned int [noderef] [usertype] <asn:2>*reg
drivers/clk/renesas/r9a06g032-clocks.c:431:30: got unsigned int [usertype] *reg
drivers/clk/renesas/r9a06g032-clocks.c:516:22: sparse: cast removes address space of expression
drivers/clk/renesas/r9a06g032-clocks.c:528:38: sparse: incorrect type in argument 2 (different address spaces) @@ expected unsigned int [noderef] [usertype] <asn:2>*reg @@ got eref] [usertype] <asn:2>*reg @@
drivers/clk/renesas/r9a06g032-clocks.c:528:38: expected unsigned int [noderef] [usertype] <asn:2>*reg
drivers/clk/renesas/r9a06g032-clocks.c:528:38: got unsigned int [usertype] *reg
vim +430 drivers/clk/renesas/r9a06g032-clocks.c
421
422 #define to_r9a06g032_divider(_hw) \
423 container_of(_hw, struct r9a06g032_clk_div, hw)
424
425 static unsigned long r9a06g032_divider_recalc_rate(
426 struct clk_hw *hw,
427 unsigned long parent_rate)
428 {
429 struct r9a06g032_clk_div *clk = to_r9a06g032_divider(hw);
> 430 u32 *reg = ((u32 *)clk->clocks->reg) + clk->reg;
> 431 long div = clk_readl(reg);
432
433 if (div < clk->min)
434 div = clk->min;
435 else if (div > clk->max)
436 div = clk->max;
437 return DIV_ROUND_UP(parent_rate, div);
438 }
439
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* Re: [PATCH v3 03/16] dt-bindings: qcom_nandc: make nand-ecc-strength optional
From: Miquel Raynal @ 2018-05-26 8:42 UTC (permalink / raw)
To: Abhishek Sahu
Cc: Boris Brezillon, David Woodhouse, Brian Norris, Marek Vasut,
Richard Weinberger, Cyrille Pitchen, linux-arm-msm, linux-kernel,
linux-mtd, Andy Gross, Archit Taneja, Rob Herring, Mark Rutland,
devicetree
In-Reply-To: <1527250904-21988-4-git-send-email-absahu@codeaurora.org>
Hi Abhishek,
On Fri, 25 May 2018 17:51:31 +0530, Abhishek Sahu
<absahu@codeaurora.org> wrote:
> If nand-ecc-strength specified in DT, then controller will use
> this ECC strength otherwise ECC strength will be calculated
> according to chip requirement and available OOB size.
>
> Signed-off-by: Abhishek Sahu <absahu@codeaurora.org>
> ---
> * Changes from v2:
> NONE
>
> * Changes from v1:
> NEW PATCH
>
> Documentation/devicetree/bindings/mtd/qcom_nandc.txt | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt
> index 73d336be..f246aa0 100644
> --- a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt
> +++ b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt
> @@ -45,11 +45,13 @@ Required properties:
> number (e.g., 0, 1, 2, etc.)
> - #address-cells: see partition.txt
> - #size-cells: see partition.txt
> -- nand-ecc-strength: see nand.txt
> - nand-ecc-step-size: must be 512. see nand.txt for more details.
I think you can squash the two dt-bindings commits as they are tightly
related to each other.
>
> Optional properties:
> - nand-bus-width: see nand.txt
> +- nand-ecc-strength: see nand.txt. If not specified, then ECC strength will
> + be used according to chip requirement and available
> + OOB size.
>
> Each nandcs device node may optionally contain a 'partitions' sub-node, which
> further contains sub-nodes describing the flash partition mapping. See
--
Miquel Raynal, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH 1/6] arm64: dts: amlogic: Add missing cooling device properties for CPUs
From: Neil Armstrong @ 2018-05-26 8:37 UTC (permalink / raw)
To: Olof Johansson, Viresh Kumar
Cc: Mark Rutland, devicetree, Vincent Guittot, arm, Catalin Marinas,
Daniel Lezcano, Will Deacon, linux-kernel, Rob Herring,
Kevin Hilman, Carlo Caione, chris.redpath, linux-amlogic,
ionela.voinescu, linux-arm-kernel
In-Reply-To: <20180525211025.c73zdcdtyuvlewng@localhost>
Hi,
On 25/05/2018 23:10, Olof Johansson wrote:
> On Fri, May 25, 2018 at 11:10:01AM +0530, Viresh Kumar wrote:
>> The cooling device properties, like "#cooling-cells" and
>> "dynamic-power-coefficient", should either be present for all the CPUs
>> of a cluster or none. If these are present only for a subset of CPUs of
>> a cluster then things will start falling apart as soon as the CPUs are
>> brought online in a different order. For example, this will happen
>> because the operating system looks for such properties in the CPU node
>> it is trying to bring up, so that it can register a cooling device.
>>
>> Add such missing properties.
>
> This seems awkward compared to just having one cooling-cells in the /cpus node
> instead.
>
> What's it used for? I don't see any properties in the device nodes on meson-gxm
> that have any cooling-foo cells in them? So why should #cooling-cells be
> needed?
There is no reason to have the cooling-cells on these other CPUs, the DVFS is
controlled on the first CPU of each cluster, here cpu0 and cpu4 and only
cpu0 and cpu4 are used as cooling-cells.
Neil
>
>
> -Olof
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* Re: [PATCH 0/3] Dts nodes for Keystone2 hw_rng driver
From: santosh.shilimkar @ 2018-05-26 4:43 UTC (permalink / raw)
To: Vitaly Andrianov, ssantosh, robh+dt, mark.rutland,
linux-arm-kernel, devicetree, linux-kernel
Cc: m-karicheri2
In-Reply-To: <1527167538-29837-1-git-send-email-vitalya@ti.com>
On 5/24/18 6:12 AM, Vitaly Andrianov wrote:
> This series adds dts nodes for Keystone2 hw_rng driver
>
> Vitaly Andrianov (3):
> ARM: dts: k2hk: add dts node for k2hk hw_rng driver
> ARM: dts: k2l: add dts node for k2l hw_rng driver
> ARM: dts: k2e: add dts node for k2e hw_rng driver
>
Looks good. Will queue this up for 4.19. Thanks !!
Regards,
Santosh
^ permalink raw reply
* Re: [PATCH v2 13/40] vfio: Add support for Shared Virtual Addressing
From: Xu Zaibo @ 2018-05-26 3:53 UTC (permalink / raw)
To: Jean-Philippe Brucker,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org
Cc: ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org,
ilias.apalodimas-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
Will Deacon, okaya-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
rgummal-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org,
rfranz-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org,
liguozhu-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org,
christian.koenig-5C7GfCeVMHo@public.gmane.org
In-Reply-To: <fea420ff-e738-e2ed-ab1e-a3f4dde4b6ef-5wv7dgnIgG8@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 5992 bytes --]
Hi,
On 2018/5/25 17:47, Jean-Philippe Brucker wrote:
> On 25/05/18 03:39, Xu Zaibo wrote:
>> Hi,
>>
>> On 2018/5/24 23:04, Jean-Philippe Brucker wrote:
>>> On 24/05/18 13:35, Xu Zaibo wrote:
>>>>> Right, sva_init() must be called once for any device that intends to use
>>>>> bind(). For the second process though, group->sva_enabled will be true
>>>>> so we won't call sva_init() again, only bind().
>>>> Well, while I create mediated devices based on one parent device to support multiple
>>>> processes(a new process will create a new 'vfio_group' for the corresponding mediated device,
>>>> and 'sva_enabled' cannot work any more), in fact, *sva_init and *sva_shutdown are basically
>>>> working on parent device, so, as a result, I just only need sva initiation and shutdown on the
>>>> parent device only once. So I change the two as following:
>>>>
>>>> @@ -551,8 +565,18 @@ int iommu_sva_device_init(struct device *dev, unsigned long features,
>>>> if (features & ~IOMMU_SVA_FEAT_IOPF)
>>>> return -EINVAL;
>>>>
>>>> + /* If already exists, do nothing */
>>>> + mutex_lock(&dev->iommu_param->lock);
>>>> + if (dev->iommu_param->sva_param) {
>>>> + mutex_unlock(&dev->iommu_param->lock);
>>>> + return 0;
>>>> + }
>>>> + mutex_unlock(&dev->iommu_param->lock);
>>>>
>>>> if (features & IOMMU_SVA_FEAT_IOPF) {
>>>> ret = iommu_register_device_fault_handler(dev, iommu_queue_iopf,
>>>>
>>>>
>>>> @@ -621,6 +646,14 @@ int iommu_sva_device_shutdown(struct device *dev)
>>>> if (!domain)
>>>> return -ENODEV;
>>>>
>>>> + /* If any other process is working on the device, shut down does nothing. */
>>>> + mutex_lock(&dev->iommu_param->lock);
>>>> + if (!list_empty(&dev->iommu_param->sva_param->mm_list)) {
>>>> + mutex_unlock(&dev->iommu_param->lock);
>>>> + return 0;
>>>> + }
>>>> + mutex_unlock(&dev->iommu_param->lock);
>>> I don't think iommu-sva.c is the best place for this, it's probably
>>> better to implement an intermediate layer (the mediating driver), that
>>> calls iommu_sva_device_init() and iommu_sva_device_shutdown() once. Then
>>> vfio-pci would still call these functions itself, but for mdev the
>>> mediating driver keeps a refcount of groups, and calls device_shutdown()
>>> only when freeing the last mdev.
>>>
>>> A device driver (non mdev in this example) expects to be able to free
>>> all its resources after sva_device_shutdown() returns. Imagine the
>>> mm_list isn't empty (mm_exit() is running late), and instead of waiting
>>> in unbind_dev_all() below, we return 0 immediately. Then the calling
>>> driver frees its resources, and the mm_exit callback along with private
>>> data passed to bind() disappear. If a mm_exit() is still running in
>>> parallel, then it will try to access freed data and corrupt memory. So
>>> in this function if mm_list isn't empty, the only thing we can do is wait.
>>>
>> I still don't understand why we should 'unbind_dev_all', is it possible
>> to do a 'unbind_dev_pasid'?
> Not in sva_device_shutdown(), it needs to clean up everything. For
> example you want to physically unplug the device, or assign it to a VM.
> To prevent any leak sva_device_shutdown() needs to remove all bonds. In
> theory there shouldn't be any, since either the driver did unbind_dev(),
> or all process exited. This is a safety net.
>
>> Then we can do other things instead of waiting that user may not like. :)
> They may not like it, but it's for their own good :) At the moment we're
> waiting that:
>
> * All exit_mm() callback for this device have finished. If we don't wait
> then the caller will free the private data passed to bind and the
> mm_exit() callback while they are still being used.
>
> * All page requests targeting this device are dealt with. If we don't
> wait then some requests, that are lingering in the IOMMU PRI queue,
> may hit the next contexts bound to this device, possibly in a
> different VM. It may not be too risky (though probably exploitable in
> some way), but is incredibly messy.
>
> All of this is bounded in time, and normally should be over pretty fast
> unless the device driver's exit_mm() does something strange. If the
> driver did the right thing, there shouldn't be any wait here (although
> there may be one in unbind_dev() for the same reasons - prevent use
> after free).
>
I guess there may be some misunderstandings :).
From the current patches, '/iommu_sva_device_shutdown/' is called by
'/vfio_iommu_sva_shutdown/', which
is mainly used by '/vfio_iommu_type1_detach_group/' that is finally
called by processes' release of vfio facilitiy
automatically or called by 'VFIO_GROUP_UNSET_CONTAINER' manually in the
processes.
So, just image that 2 processes is working on the device with IOPF
feature, and the 2 do the following to enable SVA:
/ 1.open("/dev/vfio/vfio") ;//
//
// 2.open the group of the devcie by calling open("/dev/vfio/x"), but
now, //
// I think the second processes will fail to open the group because
current VFIO thinks that one group only can be used by one process/vm,
at present, I use mediated device to create more groups on the
parent device to support multiple processes;//
/ //
/ 3.VFIO_GROUP_SET_CONTAINER;/
/ 4.VFIO_SET_IOMMU;
/
/// 5.VFIO_IOMMU_BIND;
6.Do some works with the hardware working unit filled by PASID on the
device;
7.//VFIO_IOMMU_UNBIND;
//***8.VFIO_GROUP_UNSET_CONTAINER;---here, have to sleep to wait another
process to finish works of the step 6;
* 9. close(group); close(containner);
/So, my idea is: If it is possible to just release the params or
facilities that only belong to the current process while the process
shutdown the device,
and while the last process releases them all. Then, as in the above step
8, we
don't need to wait, or maybe wait for a very long time if the other
process is doing lots of work on the device.
Thanks
Zaibo
>
[-- Attachment #1.2: Type: text/html, Size: 46082 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces
From: Kenneth Lee @ 2018-05-26 2:24 UTC (permalink / raw)
To: Jonathan Cameron
Cc: xieyisheng1@huawei.com, liubo95@huawei.com, kvm@vger.kernel.org,
linux-pci@vger.kernel.org, xuzaibo@huawei.com, Will Deacon,
okaya@codeaurora.org, linux-mm@kvack.org, liguozhu,
yi.l.liu@intel.com, ashok.raj@intel.com, Jean-Philippe Brucker,
tn@semihalf.com, joro@8bytes.org,
iommu@lists.linux-foundation.org, bharatku@xilinx.com,
linux-acpi@vger.kernel.org, liudongdong3@huawei.com,
rfranz@cavium.com, devicetree@vger.kernel.org,
kevin.tian@intel.com, Jacob Pan, alex.williamson@redhat.com,
rgummal@xilinx.com, thunder.leizhen@huawei.com,
linux-arm-kernel@lists.infradead.org,
shunyong.yang@hxt-semitech.com, Robin Murphy, Ilias Apalodimas,
jcrouse@codeaurora.org, robdclark@gmail.com, dwmw2@infradead.org,
christian.koenig@amd.com, nwatters@codeaurora.org,
baolu.lu@linux.intel.com
In-Reply-To: <20180525093959.000040a7@huawei.com>
On Fri, May 25, 2018 at 09:39:59AM +0100, Jonathan Cameron wrote:
> Date: Fri, 25 May 2018 09:39:59 +0100
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> To: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> CC: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>,
> "xieyisheng1@huawei.com" <xieyisheng1@huawei.com>, "kvm@vger.kernel.org"
> <kvm@vger.kernel.org>, "linux-pci@vger.kernel.org"
> <linux-pci@vger.kernel.org>, "xuzaibo@huawei.com" <xuzaibo@huawei.com>,
> Will Deacon <Will.Deacon@arm.com>, "okaya@codeaurora.org"
> <okaya@codeaurora.org>, "linux-mm@kvack.org" <linux-mm@kvack.org>,
> "yi.l.liu@intel.com" <yi.l.liu@intel.com>, "ashok.raj@intel.com"
> <ashok.raj@intel.com>, "tn@semihalf.com" <tn@semihalf.com>,
> "joro@8bytes.org" <joro@8bytes.org>, "robdclark@gmail.com"
> <robdclark@gmail.com>, "bharatku@xilinx.com" <bharatku@xilinx.com>,
> "linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
> "liudongdong3@huawei.com" <liudongdong3@huawei.com>, "rfranz@cavium.com"
> <rfranz@cavium.com>, "devicetree@vger.kernel.org"
> <devicetree@vger.kernel.org>, "kevin.tian@intel.com"
> <kevin.tian@intel.com>, Jacob Pan <jacob.jun.pan@linux.intel.com>,
> "alex.williamson@redhat.com" <alex.williamson@redhat.com>,
> "rgummal@xilinx.com" <rgummal@xilinx.com>, "thunder.leizhen@huawei.com"
> <thunder.leizhen@huawei.com>, "linux-arm-kernel@lists.infradead.org"
> <linux-arm-kernel@lists.infradead.org>, "shunyong.yang@hxt-semitech.com"
> <shunyong.yang@hxt-semitech.com>, "dwmw2@infradead.org"
> <dwmw2@infradead.org>, "liubo95@huawei.com" <liubo95@huawei.com>,
> "jcrouse@codeaurora.org" <jcrouse@codeaurora.org>,
> "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>,
> Robin Murphy <Robin.Murphy@arm.com>, "christian.koenig@amd.com"
> <christian.koenig@amd.com>, "nwatters@codeaurora.org"
> <nwatters@codeaurora.org>, "baolu.lu@linux.intel.com"
> <baolu.lu@linux.intel.com>, liguozhu@hisilicon.com,
> kenneth-lee-2012@foxmail.com
> Subject: Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces
> Message-ID: <20180525093959.000040a7@huawei.com>
>
> +CC Kenneth Lee
>
> On Fri, 25 May 2018 09:33:11 +0300
> Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote:
>
> > On Thu, May 24, 2018 at 04:04:39PM +0100, Jean-Philippe Brucker wrote:
> > > On 24/05/18 12:50, Ilias Apalodimas wrote:
> > > >> Interesting, I hadn't thought about this use-case before. At first I
> > > >> thought you were talking about mdev devices assigned to VMs, but I think
> > > >> you're referring to mdevs assigned to userspace drivers instead? Out of
> > > >> curiosity, is it only theoretical or does someone actually need this?
> > > >
> > > > There has been some non upstreamed efforts to have mdev and produce userspace
> > > > drivers. Huawei is using it on what they call "wrapdrive" for crypto devices and
> > > > we did a proof of concept for ethernet interfaces. At the time we choose not to
> > > > involve the IOMMU for the reason you mentioned, but having it there would be
> > > > good.
> > >
> > > I'm guessing there were good reasons to do it that way but I wonder, is
> > > it not simpler to just have the kernel driver create a /dev/foo, with a
> > > standard ioctl/mmap/poll interface? Here VFIO adds a layer of
> > > indirection, and since the mediating driver has to implement these
> > > operations already, what is gained?
> > The best reason i can come up with is "common code". You already have one API
> > doing that for you so we replicate it in a /dev file?
> > The mdev approach still needs extentions to support what we tried to do (i.e
> > mdev bus might need yo have access on iommu_ops), but as far as i undestand it's
> > a possible case.
Hi, Jean, Please allow me to share my understanding here:
https://zhuanlan.zhihu.com/p/35489035
The reason we do not use the /dev/foo scheme is that the devices to be
shared are programmable accelerators. We cannot fix up the kernel driver for them.
> > >
> > > Thanks,
> > > Jean
>
>
--
-Kenneth Lee (Hisilicon)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces
From: Kenneth Lee @ 2018-05-26 2:24 UTC (permalink / raw)
To: Jonathan Cameron
Cc: kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
xuzaibo-hv44wF8Li93QT0dZR+AlfA@public.gmane.org, Will Deacon,
okaya-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org,
liguozhu-C8/M+/jPZTeaMJb+Lgu22Q,
ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
rfranz-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
rgummal-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Ilias Apalodimas, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org,
christian.koenig-5C7GfCeVMHo@public.gmane.org
In-Reply-To: <20180525093959.000040a7-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
On Fri, May 25, 2018 at 09:39:59AM +0100, Jonathan Cameron wrote:
> Date: Fri, 25 May 2018 09:39:59 +0100
> From: Jonathan Cameron <Jonathan.Cameron-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> To: Ilias Apalodimas <ilias.apalodimas-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> CC: Jean-Philippe Brucker <jean-philippe.brucker-5wv7dgnIgG8@public.gmane.org>,
> "xieyisheng1-hv44wF8Li93QT0dZR+AlfA@public.gmane.org" <xieyisheng1-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>, "kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
> <kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
> <linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "xuzaibo-hv44wF8Li93QT0dZR+AlfA@public.gmane.org" <xuzaibo-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>,
> Will Deacon <Will.Deacon-5wv7dgnIgG8@public.gmane.org>, "okaya-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org"
> <okaya-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>, "linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org" <linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org>,
> "yi.l.liu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org" <yi.l.liu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>, "ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org"
> <ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>, "tn-nYOzD4b6Jr9Wk0Htik3J/w@public.gmane.org" <tn-nYOzD4b6Jr9Wk0Htik3J/w@public.gmane.org>,
> "joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org" <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>, "robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org"
> <robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, "bharatku-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org" <bharatku-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>,
> "linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
> "liudongdong3-hv44wF8Li93QT0dZR+AlfA@public.gmane.org" <liudongdong3-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>, "rfranz-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org"
> <rfranz-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>, "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
> <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "kevin.tian-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org"
> <kevin.tian-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>, Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>,
> "alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org" <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
> "rgummal-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org" <rgummal-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>, "thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org"
> <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>, "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
> <linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>, "shunyong.yang-PT9Dzx9SjPiXmMXjJBpWqg@public.gmane.org"
> <shunyong.yang-PT9Dzx9SjPiXmMXjJBpWqg@public.gmane.org>, "dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org"
> <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>, "liubo95-hv44wF8Li93QT0dZR+AlfA@public.gmane.org" <liubo95-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>,
> "jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org" <jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>,
> "iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org" <iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>,
> Robin Murphy <Robin.Murphy-5wv7dgnIgG8@public.gmane.org>, "christian.koenig-5C7GfCeVMHo@public.gmane.org"
> <christian.koenig-5C7GfCeVMHo@public.gmane.org>, "nwatters-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org"
> <nwatters-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>, "baolu.lu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org"
> <baolu.lu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>, liguozhu-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org,
> kenneth-lee-2012-H32Fclmsjq1BDgjK7y7TUQ@public.gmane.org
> Subject: Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces
> Message-ID: <20180525093959.000040a7-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>
> +CC Kenneth Lee
>
> On Fri, 25 May 2018 09:33:11 +0300
> Ilias Apalodimas <ilias.apalodimas-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>
> > On Thu, May 24, 2018 at 04:04:39PM +0100, Jean-Philippe Brucker wrote:
> > > On 24/05/18 12:50, Ilias Apalodimas wrote:
> > > >> Interesting, I hadn't thought about this use-case before. At first I
> > > >> thought you were talking about mdev devices assigned to VMs, but I think
> > > >> you're referring to mdevs assigned to userspace drivers instead? Out of
> > > >> curiosity, is it only theoretical or does someone actually need this?
> > > >
> > > > There has been some non upstreamed efforts to have mdev and produce userspace
> > > > drivers. Huawei is using it on what they call "wrapdrive" for crypto devices and
> > > > we did a proof of concept for ethernet interfaces. At the time we choose not to
> > > > involve the IOMMU for the reason you mentioned, but having it there would be
> > > > good.
> > >
> > > I'm guessing there were good reasons to do it that way but I wonder, is
> > > it not simpler to just have the kernel driver create a /dev/foo, with a
> > > standard ioctl/mmap/poll interface? Here VFIO adds a layer of
> > > indirection, and since the mediating driver has to implement these
> > > operations already, what is gained?
> > The best reason i can come up with is "common code". You already have one API
> > doing that for you so we replicate it in a /dev file?
> > The mdev approach still needs extentions to support what we tried to do (i.e
> > mdev bus might need yo have access on iommu_ops), but as far as i undestand it's
> > a possible case.
Hi, Jean, Please allow me to share my understanding here:
https://zhuanlan.zhihu.com/p/35489035
The reason we do not use the /dev/foo scheme is that the devices to be
shared are programmable accelerators. We cannot fix up the kernel driver for them.
> > >
> > > Thanks,
> > > Jean
>
>
--
-Kenneth Lee (Hisilicon)
^ permalink raw reply
* Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces
From: Kenneth Lee @ 2018-05-26 2:24 UTC (permalink / raw)
To: Jonathan Cameron
Cc: xieyisheng1@huawei.com, liubo95@huawei.com, kvm@vger.kernel.org,
linux-pci@vger.kernel.org, xuzaibo@huawei.com, Will Deacon,
okaya@codeaurora.org, linux-mm@kvack.org, liguozhu,
yi.l.liu@intel.com, ashok.raj@intel.com, Jean-Philippe Brucker,
tn@semihalf.com, joro@8bytes.org,
iommu@lists.linux-foundation.org, bharatku@xilinx.com,
linux-acpi@vger.kernel.org, liudongdong3@huawei.com,
rfranz@cavium.com, devic
In-Reply-To: <20180525093959.000040a7@huawei.com>
On Fri, May 25, 2018 at 09:39:59AM +0100, Jonathan Cameron wrote:
> Date: Fri, 25 May 2018 09:39:59 +0100
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> To: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> CC: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>,
> "xieyisheng1@huawei.com" <xieyisheng1@huawei.com>, "kvm@vger.kernel.org"
> <kvm@vger.kernel.org>, "linux-pci@vger.kernel.org"
> <linux-pci@vger.kernel.org>, "xuzaibo@huawei.com" <xuzaibo@huawei.com>,
> Will Deacon <Will.Deacon@arm.com>, "okaya@codeaurora.org"
> <okaya@codeaurora.org>, "linux-mm@kvack.org" <linux-mm@kvack.org>,
> "yi.l.liu@intel.com" <yi.l.liu@intel.com>, "ashok.raj@intel.com"
> <ashok.raj@intel.com>, "tn@semihalf.com" <tn@semihalf.com>,
> "joro@8bytes.org" <joro@8bytes.org>, "robdclark@gmail.com"
> <robdclark@gmail.com>, "bharatku@xilinx.com" <bharatku@xilinx.com>,
> "linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
> "liudongdong3@huawei.com" <liudongdong3@huawei.com>, "rfranz@cavium.com"
> <rfranz@cavium.com>, "devicetree@vger.kernel.org"
> <devicetree@vger.kernel.org>, "kevin.tian@intel.com"
> <kevin.tian@intel.com>, Jacob Pan <jacob.jun.pan@linux.intel.com>,
> "alex.williamson@redhat.com" <alex.williamson@redhat.com>,
> "rgummal@xilinx.com" <rgummal@xilinx.com>, "thunder.leizhen@huawei.com"
> <thunder.leizhen@huawei.com>, "linux-arm-kernel@lists.infradead.org"
> <linux-arm-kernel@lists.infradead.org>, "shunyong.yang@hxt-semitech.com"
> <shunyong.yang@hxt-semitech.com>, "dwmw2@infradead.org"
> <dwmw2@infradead.org>, "liubo95@huawei.com" <liubo95@huawei.com>,
> "jcrouse@codeaurora.org" <jcrouse@codeaurora.org>,
> "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>,
> Robin Murphy <Robin.Murphy@arm.com>, "christian.koenig@amd.com"
> <christian.koenig@amd.com>, "nwatters@codeaurora.org"
> <nwatters@codeaurora.org>, "baolu.lu@linux.intel.com"
> <baolu.lu@linux.intel.com>, liguozhu@hisilicon.com,
> kenneth-lee-2012@foxmail.com
> Subject: Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces
> Message-ID: <20180525093959.000040a7@huawei.com>
>
> +CC Kenneth Lee
>
> On Fri, 25 May 2018 09:33:11 +0300
> Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote:
>
> > On Thu, May 24, 2018 at 04:04:39PM +0100, Jean-Philippe Brucker wrote:
> > > On 24/05/18 12:50, Ilias Apalodimas wrote:
> > > >> Interesting, I hadn't thought about this use-case before. At first I
> > > >> thought you were talking about mdev devices assigned to VMs, but I think
> > > >> you're referring to mdevs assigned to userspace drivers instead? Out of
> > > >> curiosity, is it only theoretical or does someone actually need this?
> > > >
> > > > There has been some non upstreamed efforts to have mdev and produce userspace
> > > > drivers. Huawei is using it on what they call "wrapdrive" for crypto devices and
> > > > we did a proof of concept for ethernet interfaces. At the time we choose not to
> > > > involve the IOMMU for the reason you mentioned, but having it there would be
> > > > good.
> > >
> > > I'm guessing there were good reasons to do it that way but I wonder, is
> > > it not simpler to just have the kernel driver create a /dev/foo, with a
> > > standard ioctl/mmap/poll interface? Here VFIO adds a layer of
> > > indirection, and since the mediating driver has to implement these
> > > operations already, what is gained?
> > The best reason i can come up with is "common code". You already have one API
> > doing that for you so we replicate it in a /dev file?
> > The mdev approach still needs extentions to support what we tried to do (i.e
> > mdev bus might need yo have access on iommu_ops), but as far as i undestand it's
> > a possible case.
Hi, Jean, Please allow me to share my understanding here:
https://zhuanlan.zhihu.com/p/35489035
The reason we do not use the /dev/foo scheme is that the devices to be
shared are programmable accelerators. We cannot fix up the kernel driver for them.
> > >
> > > Thanks,
> > > Jean
>
>
--
-Kenneth Lee (Hisilicon)
^ permalink raw reply
* [PATCH v2] ARM: dts: imx51-zii-rdu1: Make sure SD1_WP is low
From: Andrey Smirnov @ 2018-05-26 2:12 UTC (permalink / raw)
To: Shawn Guo
Cc: Andrey Smirnov, Nikita Yushchenko, Fabio Estevam, Lucas Stach,
Chris Healy, Rob Herring, linux-arm-kernel, devicetree,
linux-kernel
Make sure that MX51_PAD_GPIO1_1 does not remain configure as
ALT0/SD1_WP (it is out of reset). This is needed because of external
pull-up resistor attached to that pad that, when left unchanged, will
drive SD1_WP high preventing eSDHC1/eMMC from working correctly.
To fix that add a pinmux configuration line configureing the pad to
function as a GPIO. While we are at it, add a corresponding
output-high GPIO hog in an effort to minimize current consumption.
Cc: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
Changes since [v1]:
- Switched GPIO hog to be output-high
- Removed whitespace damage
[v1] lkml.kernel.org/r/20180525030153.15986-1-andrew.smirnov@gmail.com
arch/arm/boot/dts/imx51-zii-rdu1.dts | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/arch/arm/boot/dts/imx51-zii-rdu1.dts b/arch/arm/boot/dts/imx51-zii-rdu1.dts
index df9eca94d812..1e343f35a9d9 100644
--- a/arch/arm/boot/dts/imx51-zii-rdu1.dts
+++ b/arch/arm/boot/dts/imx51-zii-rdu1.dts
@@ -476,6 +476,17 @@
status = "okay";
};
+&gpio1 {
+ unused-sd3-wp-gpio {
+ /*
+ * See pinctrl_esdhc1 below for more details on this
+ */
+ gpio-hog;
+ gpios = <1 GPIO_ACTIVE_HIGH>;
+ output-high;
+ };
+};
+
&i2c2 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_i2c2>;
@@ -660,6 +671,23 @@
MX51_PAD_SD1_DATA1__SD1_DATA1 0x20d5
MX51_PAD_SD1_DATA2__SD1_DATA2 0x20d5
MX51_PAD_SD1_DATA3__SD1_DATA3 0x20d5
+ /*
+ * GPIO1_1 is not directly used by eSDHC1 in
+ * any capacity, but earlier versions of RDU1
+ * used that pin as WP GPIO for eSDHC3 and
+ * because of that that pad has an external
+ * pull-up resistor. This is problematic
+ * because out of reset the pad is configured
+ * as ALT0 which serves as SD1_WP, which, when
+ * pulled high by and external pull-up, will
+ * inhibit execution of any write request to
+ * attached eMMC device.
+ *
+ * To avoid this problem we configure the pad
+ * to ALT1/GPIO and avoid driving SD1_WP
+ * signal high.
+ */
+ MX51_PAD_GPIO1_1__GPIO1_1 0x0000
>;
};
--
2.17.0
^ permalink raw reply related
* Re: [PATCH v2 5/6] soc: qcom: rpmh powerdomain driver
From: David Collins @ 2018-05-26 1:08 UTC (permalink / raw)
To: Rajendra Nayak, viresh.kumar, sboyd, andy.gross, ulf.hansson
Cc: devicetree, linux-arm-msm, linux-kernel, Lina Iyer
In-Reply-To: <20180525100121.28214-6-rnayak@codeaurora.org>
Hello Rajendra,
On 05/25/2018 03:01 AM, Rajendra Nayak wrote:
> The RPMh powerdomain driver aggregates the corner votes from various
s/powerdomain/power domain/
This applies to all instances in this patch. "Power domain" seems to be
the preferred spelling in the kernel.
> consumers for the ARC resources and communicates it to RPMh.
>
> We also add data for all powerdomains on sdm845 as part of the patch.
> The driver can be extended to support other SoCs which support RPMh
>
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
> .../devicetree/bindings/power/qcom,rpmhpd.txt | 65 ++++
> drivers/soc/qcom/Kconfig | 9 +
> drivers/soc/qcom/Makefile | 1 +
> drivers/soc/qcom/rpmhpd.c | 360 ++++++++++++++++++
> 4 files changed, 435 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/power/qcom,rpmhpd.txt
> create mode 100644 drivers/soc/qcom/rpmhpd.c
...
> +++ b/Documentation/devicetree/bindings/power/qcom,rpmhpd.txt
I think that this binding documentation should be in a patch separate from
the driver.
> @@ -0,0 +1,65 @@
> +Qualcomm RPMh Powerdomains
s/Qualcomm/Qualcomm Technologies, Inc./
> +
> +* For RPMh powerdomains, we communicate a performance state to RPMh
Does this line need to start with '*'?
> +which then translates it into a corresponding voltage on a rail
> +
> +Required Properties:
> + - compatible: Should be one of the following
> + * qcom,sdm845-rpmhpd: RPMh powerdomain for the sdm845 family of SoC
> + - power-domain-cells: number of cells in power domain specifier
> + must be 1
> + - operating-points-v2: Phandle to the OPP table for the power-domain.
> + Refer to Documentation/devicetree/bindings/power/power_domain.txt
> + and Documentation/devicetree/bindings/opp/qcom-opp.txt for more details
> +
> +Example:
> +
> + rpmhpd: power-controller {
> + compatible = "qcom,sdm845-rpmhpd";
> + #power-domain-cells = <1>;
> + operating-points-v2 = <&rpmhpd_opp_table>,
> + <&rpmhpd_opp_table>,
> + <&rpmhpd_opp_table>,
> + <&rpmhpd_opp_table>,
> + <&rpmhpd_opp_table>,
> + <&rpmhpd_opp_table>,
> + <&rpmhpd_opp_table>,
> + <&rpmhpd_opp_table>,
> + <&rpmhpd_opp_table>;
Can this be changed to simply:
operating-points-v2 = <&rpmhpd_opp_table>;
The opp binding documentation [1] states that this should be ok:
If only one phandle is available, then the same OPP table will be used
for all power domains provided by the power domain provider.
> + };
> +
> + rpmhpd_opp_table: opp-table {
> + compatible = "operating-points-v2-qcom-level", "operating-points-v2";
> +
> + rpmhpd_opp1: opp@1 {
Is there any significance to the 1 through 8 values in these OPP table
nodes? If not, then could this be changed to something like:
rpmhpd_retention: opp@16 {
...
rpmhpd_turbo_l1: opp@416 {
> + qcom-corner = <16>;
s/qcom-corner/qcom,level/g
> + };
> +
> + rpmhpd_opp2: opp@2 {
> + qcom-corner = <48>;
> + };
> +
> + rpmhpd_opp3: opp@3 {
> + qcom-corner = <64>;
> + };
> +
> + rpmhpd_opp4: opp@4 {
> + qcom-corner = <128>;
> + };
> +
> + rpmhpd_opp5: opp@5 {
> + qcom-corner = <192>;
> + };
> +
> + rpmhpd_opp6: opp@6 {
> + qcom-corner = <256>;
> + };
> +
> + rpmhpd_opp7: opp@7 {
> + qcom-corner = <320>;
> + };
Can you please add 336 and 384 to your example? 384 at least should be
present as it corresponds to the Turbo level which all supplies support.
> + rpmhpd_opp8: opp@8 {
> + qcom-corner = <416>;
> + };
> + };
How are consumers of these power domains supposed to identify which domain
within <&rpmhpd> to use (e.g. VDD_CX vs VDD_MX)? If the answer is a plain
integer index, then could you please add per-platform #define constants in
a DT header file which explicitly define the meaning for each index?
How do consumers of these power domains identify which level they want to
set for a specific power domain (e.g. Nominal vs Turbo)?
Would it be helpful to provide a DT header file with #define constants for
the cross-platform sparse level mapping? This is done in [2] for the
downstream rpmh-regulator driver that handles ARC managed regulators.
Would it be ok to add some consumer DT nodes in this binding file example
so that it is clear how consumers interact with the rpmhpd?
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index a7a405178967..1faed239701d 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -74,6 +74,15 @@ config QCOM_RMTFS_MEM
>
> Say y here if you intend to boot the modem remoteproc.
>
> +config QCOM_RPMHPD
> + tristate "Qualcomm RPMh Powerdomain driver"
s/Qualcomm/Qualcomm Technologies, Inc./
> + depends on QCOM_RPMH && QCOM_COMMAND_DB
> + help
> + QCOM RPMh powerdomain driver to support powerdomain with
> + performance states. The driver communicates a performance state
> + value to RPMh which then translates it into corresponding voltage
> + for the voltage rail.
> +
> config QCOM_RPMPD
> tristate "Qualcomm RPM Powerdomain driver"
> depends on MFD_QCOM_RPM && QCOM_SMD_RPM
> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> index 9550c170de93..499513f63bef 100644
> --- a/drivers/soc/qcom/Makefile
> +++ b/drivers/soc/qcom/Makefile
> @@ -16,3 +16,4 @@ obj-$(CONFIG_QCOM_SMSM) += smsm.o
> obj-$(CONFIG_QCOM_WCNSS_CTRL) += wcnss_ctrl.o
> obj-$(CONFIG_QCOM_APR) += apr.o
> obj-$(CONFIG_QCOM_RPMPD) += rpmpd.o
> +obj-$(CONFIG_QCOM_RPMHPD) += rpmhpd.o
> diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c
> new file mode 100644
> index 000000000000..a79f094ad326
> --- /dev/null
> +++ b/drivers/soc/qcom/rpmhpd.c
> @@ -0,0 +1,360 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + */
Minor: The copyright comment could be made into a single line comment.
> +
> +#include <linux/err.h>
> +#include <linux/export.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/pm_domain.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_opp.h>
> +#include <soc/qcom/cmd-db.h>
> +#include <soc/qcom/rpmh.h>
> +
> +#define domain_to_rpmhpd(domain) container_of(domain, struct rpmhpd, pd)
> +
> +#define DEFINE_RPMHPD_AO(_platform, _name, _active) \
> + static struct rpmhpd _platform##_##_active; \
> + static struct rpmhpd _platform##_##_name = { \
> + .pd = { .name = #_name, }, \
> + .peer = &_platform##_##_active, \
> + .res_name = #_name".lvl", \
> + }; \
> + static struct rpmhpd _platform##_##_active = { \
> + .pd = { .name = #_active, }, \
> + .peer = &_platform##_##_name, \
> + .active_only = true, \
> + .res_name = #_name".lvl", \
> + }
> +
> +#define DEFINE_RPMHPD(_platform, _name) \
> + static struct rpmhpd _platform##_##_name = { \
> + .pd = { .name = #_name, }, \
> + .res_name = #_name".lvl", \
> + }
> +
> +/*
> + * This is the number of bytes used for each command DB aux data entry of an
> + * ARC resource.
> + */
> +#define RPMH_ARC_LEVEL_SIZE 2
> +#define RPMH_ARC_MAX_LEVELS 16
> +
> +struct rpmhpd {
> + struct device *dev;
> + struct generic_pm_domain pd;
> + struct rpmhpd *peer;
> + const bool active_only;
> + unsigned long corner;
Does this actually need to be unsigned long? It looks like unsigned int
is being passed in from rpmhpd_set_performance(). Also, hlvl values sent
to RPMh will only every be in the range 0 - 15.
If you change the type here, then can you also please change long to int
in to_active_sleep() and rpmhpd_aggregate_corner() below?
> + u32 level[RPMH_ARC_MAX_LEVELS];
> + int level_count;
> + bool enabled;
> + const char *res_name;
> + u32 addr;
> +};
Can you please indent the fields of this struct to the same column with tabs?
> +
> +struct rpmhpd_desc {
> + struct rpmhpd **rpmhpds;
> + size_t num_pds;
> +};
This struct could be removed and the per-platform arrays could instead be
NULL terminated.
> +
> +static DEFINE_MUTEX(rpmhpd_lock);
> +
> +/* sdm845 RPMh powerdomains */
> +DEFINE_RPMHPD(sdm845, ebi);
> +DEFINE_RPMHPD_AO(sdm845, mx, mx_ao);
> +DEFINE_RPMHPD_AO(sdm845, cx, cx_ao);
> +DEFINE_RPMHPD(sdm845, lmx);
> +DEFINE_RPMHPD(sdm845, lcx);
> +DEFINE_RPMHPD(sdm845, gfx);
> +DEFINE_RPMHPD(sdm845, mss);
> +
> +static struct rpmhpd *sdm845_rpmhpds[] = {
> + [0] = &sdm845_ebi,
If you are going to explicitly index these elements, then can you please
use #define constants from a DT header file that specifies meaningful
names? The existing 0-8 indexing is no better than implicit indexing.
> + [1] = &sdm845_mx,
> + [2] = &sdm845_mx_ao,
> + [3] = &sdm845_cx,
> + [4] = &sdm845_cx_ao,
> + [5] = &sdm845_lmx,
> + [6] = &sdm845_lcx,
> + [7] = &sdm845_gfx,
> + [8] = &sdm845_mss,
> +};
> +
> +static const struct rpmhpd_desc sdm845_desc = {
> + .rpmhpds = sdm845_rpmhpds,
> + .num_pds = ARRAY_SIZE(sdm845_rpmhpds),
> +};
> +
> +static const struct of_device_id rpmhpd_match_table[] = {
> + { .compatible = "qcom,sdm845-rpmhpd", .data = &sdm845_desc },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, rpmhpd_match_table);
> +
> +static int rpmhpd_send_corner(struct rpmhpd *pd, int state, unsigned int corner)
> +{
> + struct tcs_cmd cmd = {
> + .addr = pd->addr,
> + .data = corner,
> + };
> +
> + return rpmh_write(pd->dev, state, &cmd, 1);
This can be optimized by calling rpmh_write_async() whenever the corner
being sent is smaller than the last value sent. That way, no time is
wasted waiting for an ACK when decreasing voltage. Would you mind adding
the necessary check and previous request caching for this?
> +};
> +
> +static void to_active_sleep(struct rpmhpd *pd, unsigned long corner,
> + unsigned long *active, unsigned long *sleep)
> +{
> + *active = corner;
> +
> + if (pd->active_only)
> + *sleep = 0;
> + else
> + *sleep = *active;
> +}
> +
> +static int rpmhpd_aggregate_corner(struct rpmhpd *pd)
> +{
> + int ret;
> + struct rpmhpd *peer = pd->peer;
> + unsigned long active_corner, sleep_corner;
> + unsigned long this_corner = 0, this_sleep_corner = 0;
> + unsigned long peer_corner = 0, peer_sleep_corner = 0;
s/this_corner/this_active_corner/
s/peer_corner/peer_active_corner/
This is more consistent and I think that it makes the code a little more
readable.
> +
> + to_active_sleep(pd, pd->corner, &this_corner, &this_sleep_corner);
> +
> + if (peer && peer->enabled)
> + to_active_sleep(peer, peer->corner, &peer_corner,
> + &peer_sleep_corner);
> +
> + active_corner = max(this_corner, peer_corner);
> +
> + ret = rpmhpd_send_corner(pd, RPMH_ACTIVE_ONLY_STATE, active_corner);
> + if (ret)
> + return ret;
> +
> + sleep_corner = max(this_sleep_corner, peer_sleep_corner);
> +
> + return rpmhpd_send_corner(pd, RPMH_SLEEP_STATE, sleep_corner);
> +}
This aggregation function as well as the rpmhpd_send_corner() calls below
are not sufficient for RPMh. There are 3 states that must all be used:
RPMH_ACTIVE_ONLY_STATE, RPMH_WAKE_ONLY_STATE, and RPMH_SLEEP_STATE. The
naming is somewhat confusing as rpmhpd is defining a different concept of
active-only.
For power domains without active-only or peers, only
RPMH_ACTIVE_ONLY_STATE should be used. This instructs RPMh to issue the
request immediately.
For power domains with active-only, requests will need to be made for all
three. active_corner would be sent for both RPMH_ACTIVE_ONLY_STATE (so
that the request takes effect immediately) and RPMH_WAKE_ONLY_STATE (so
that the request is inserted into the wake TCS). sleep_corner would be
sent for RPMH_SLEEP_STATE (so that the request is inserted into the sleep
TCS).
You can see how this is handled in the RPMh clock driver in patch [3].
You may be able to get away with using only RPMH_SLEEP_STATE and
RPMH_ACTIVE_ONLY_STATE assuming that you issue the RPMH_SLEEP_STATE
request first due to the rpmh driver caching behavior added in the
cache_rpm_request() function in [4]. However, could you please confirm
with Lina that this usage will continue to work in the future? I'm not
sure what guarantees are made at the rpmh API level.
> +
> +static int rpmhpd_power_on(struct generic_pm_domain *domain)
> +{
> + int ret = 0;
> + struct rpmhpd *pd = domain_to_rpmhpd(domain);
Minor: It might look a little nicer to list 'pd' definition first amongst
the local variables in this function as well as those below.
> +
> + mutex_lock(&rpmhpd_lock);
> +
> + pd->enabled = true;
> +
> + if (pd->corner)
> + ret = rpmhpd_aggregate_corner(pd);
> +
> + mutex_unlock(&rpmhpd_lock);
> +
> + return ret;
> +}
> +
> +static int rpmhpd_power_off(struct generic_pm_domain *domain)
> +{
> + int ret;
> + struct rpmhpd *pd = domain_to_rpmhpd(domain);
> +
> + mutex_lock(&rpmhpd_lock);
> +
> + if (pd->level[0] == 0) {
> + ret = rpmhpd_send_corner(pd, RPMH_ACTIVE_ONLY_STATE, 0);
> + if (ret)
> + return ret;
> +
> + ret = rpmhpd_send_corner(pd, RPMH_SLEEP_STATE, 0);
> + if (ret)
> + return ret;
> + }
> +
> + pd->enabled = false;
> +
> + mutex_unlock(&rpmhpd_lock);
> +
> + return 0;
> +}
> +
> +static int rpmhpd_set_performance(struct generic_pm_domain *domain,
> + unsigned int state)
> +{
> + int ret = 0;
> + struct rpmhpd *pd = domain_to_rpmhpd(domain);
> +
> + mutex_lock(&rpmhpd_lock);
> +
> + pd->corner = state;
What is the numbering space for 'state'? I assume that it is a vlvl value
corresponding to qcom,level in a DT OPP table node. If so, additional
logic is required.
When using RPMh, the platform and supply independent vlvl sparse numbering
space is used by consumers so that they can always have consistent values.
However, the actual requests sent to RPMh ARC must be in the hlvl
numbering space (i.e. 0 - 15(max)). In the case of this driver, the
acceptable hlvl values for a given power domain are 0 to pd->level_count - 1.
I suspect that you need to add something like this here:
int i;
for (i = 0; i < pd->level_count; i++)
if (state <= pd->level[i])
break;
if (i == pd->level_count) {
ret = -EINVAL;
dev_err(pd->dev, "invalid state=%u for domain %s",
state, pd->pd.name);
goto out;
}
pd->corner = i;
Note that a given power domain will likely not support all of the vlvl
values listed in the DT OPP table nodes.
> +
> + if (!pd->enabled)
> + goto out;
> +
> + ret = rpmhpd_aggregate_corner(pd);
> +out:
> + mutex_unlock(&rpmhpd_lock);
> +
> + return ret;
> +}
> +
> +static unsigned int rpmhpd_get_performance(struct generic_pm_domain *genpd,
> + struct dev_pm_opp *opp)> +{
> + struct device_node *np;
> + unsigned int corner = 0;
> +
> + np = dev_pm_opp_get_of_node(opp);
> + if (of_property_read_u32(np, "qcom,level", &corner)) {
> + pr_err("%s: missing 'qcom,level' property\n", __func__);
> + return 0;
Why return 0 instead of an error?
> + }
> +
> + of_node_put(np);
> +
> + return corner;
> +}
Is there an API to determine the currently operating performance state of
a given power domain? Is this information accessible from userspace? We
will definitely need this for general debugging.
> +
> +static int rpmhpd_update_level_mapping(struct rpmhpd *rpmhpd)
> +{
> + u8 *buf;
This could be changed to the following in order to remove the need for
kzalloc() and kfree() calls below:
u8 buf[RPMH_ARC_MAX_LEVELS * RPMH_ARC_LEVEL_SIZE];
> + int i, j, len, ret;
> +
> + len = cmd_db_read_aux_data_len(rpmhpd->res_name);
> + if (len <= 0)
> + return len;
> +
A check like this is needed here:
if (len > RPMH_ARC_MAX_LEVELS * RPMH_ARC_LEVEL_SIZE)
return -EINVAL;
> + buf = kzalloc(len, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + ret = cmd_db_read_aux_data(rpmhpd->res_name, buf, len);
> + if (ret < 0)
> + return ret;
> +
> + rpmhpd->level_count = len / RPMH_ARC_LEVEL_SIZE;
> +
> + for (i = 0; i < rpmhpd->level_count; i++) {
If you make buf a fixed size array, then rpmhpd->level[i] = 0; is needed
here or a memset() outside of the for loop.
> + for (j = 0; j < RPMH_ARC_LEVEL_SIZE; j++)
> + rpmhpd->level[i] |=
> + buf[i * RPMH_ARC_LEVEL_SIZE + j] << (8 * j);
> +
> + /*
> + * The AUX data may be zero padded. These 0 valued entries at
> + * the end of the map must be ignored.
> + */
> + if (i > 0 && rpmhpd->level[i] == 0) {
> + rpmhpd->level_count = i;
> + break;
> + }
> + pr_dbg("%s: ARC hlvl=%2d --> vlvl=%4u\n", rpmhpd->res_name, i,
> + rpmhpd->level[i]);
> + }
> +
> + kfree(buf);
> + return 0;
> +}
> +
> +static int rpmhpd_probe(struct platform_device *pdev)
> +{
> + int i, ret;
> + size_t num;
> + struct genpd_onecell_data *data;
> + struct rpmhpd **rpmhpds;
> + const struct rpmhpd_desc *desc;
> +
> + desc = of_device_get_match_data(&pdev->dev);
> + if (!desc)
> + return -EINVAL;
> +
> + rpmhpds = desc->rpmhpds;
> + num = desc->num_pds;
> +
> + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->domains = devm_kcalloc(&pdev->dev, num, sizeof(*data->domains),
> + GFP_KERNEL);
> + data->num_domains = num;
> +
> + ret = cmd_db_ready();
> + if (ret) {
> + if (ret != -EPROBE_DEFER)
> + dev_err(&pdev->dev, "Command DB unavailable, ret=%d\n",
> + ret);
> + return ret;
> + }
> +
> + for (i = 0; i < num; i++) {
> + if (!rpmhpds[i])
> + continue;
Why is this check needed?
> +
> + rpmhpds[i]->dev = &pdev->dev;
> + rpmhpds[i]->addr = cmd_db_read_addr(rpmhpds[i]->res_name);
> + if (!rpmhpds[i]->addr) {
> + dev_err(&pdev->dev, "Could not find RPMh address for resource %s\n",
> + rpmhpds[i]->res_name);
> + return -ENODEV;
> + }
> +
> + ret = cmd_db_read_slave_id(rpmhpds[i]->res_name);
> + if (ret != CMD_DB_HW_ARC) {
> + dev_err(&pdev->dev, "RPMh slave ID mismatch\n");
> + return -EINVAL;
> + }
> +
> + ret = rpmhpd_update_level_mapping(rpmhpds[i]);
> + if (ret)
> + return ret;
> +
> + rpmhpds[i]->pd.power_off = rpmhpd_power_off;
> + rpmhpds[i]->pd.power_on = rpmhpd_power_on;
> + rpmhpds[i]->pd.set_performance_state = rpmhpd_set_performance;
> + rpmhpds[i]->pd.opp_to_performance_state = rpmhpd_get_performance;
> + pm_genpd_init(&rpmhpds[i]->pd, NULL, true);
> +
> + data->domains[i] = &rpmhpds[i]->pd;
> + }
> +
> + return of_genpd_add_provider_onecell(pdev->dev.of_node, data);
> +}
> +
> +static int rpmhpd_remove(struct platform_device *pdev)
> +{
> + of_genpd_del_provider(pdev->dev.of_node);
> + return 0;
> +}
> +
> +static struct platform_driver rpmhpd_driver = {
> + .driver = {
> + .name = "qcom-rpmhpd",
> + .of_match_table = rpmhpd_match_table,
> + },
> + .probe = rpmhpd_probe,
> + .remove = rpmhpd_remove,
> +};
> +
> +static int __init rpmhpd_init(void)
> +{
> + return platform_driver_register(&rpmhpd_driver);
> +}
> +core_initcall(rpmhpd_init);
> +
> +static void __exit rpmhpd_exit(void)
> +{
> + platform_driver_unregister(&rpmhpd_driver);
> +}
> +module_exit(rpmhpd_exit);
> +
> +MODULE_DESCRIPTION("Qualcomm RPMh Power Domain Driver");
s/Qualcomm/Qualcomm Technologies, Inc./
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:qcom-rpmhpd");
Thanks,
David
[1]:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/opp/opp.txt?h=v4.17-rc6#n49
[2]:
https://source.codeaurora.org/quic/la/kernel/msm-4.9/tree/include/dt-bindings/regulator/qcom,rpmh-regulator.h?h=msm-4.9
[3]: https://lkml.org/lkml/2018/5/9/54
[4]: https://lkml.org/lkml/2018/5/24/536
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v2 07/40] iommu: Add a page fault handler
From: Jacob Pan @ 2018-05-26 0:35 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
xuzaibo-hv44wF8Li93QT0dZR+AlfA@public.gmane.org,
ilias.apalodimas-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
Will Deacon,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
okaya-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
rgummal-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org,
rfranz-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org,
dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org,
christian.koenig-5C7GfCeVMHo@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <bdf9f221-ab97-2168-d072-b7f6a0dba840-5wv7dgnIgG8@public.gmane.org>
On Thu, 24 May 2018 12:44:38 +0100
Jean-Philippe Brucker <jean-philippe.brucker-5wv7dgnIgG8@public.gmane.org> wrote:
> On 23/05/18 00:35, Jacob Pan wrote:
> >>>> + /* Insert *before* the last fault */
> >>>> + list_move(&fault->head, &group->faults);
> >>>> + }
> >>>> +
> >>> If you already sorted the group list with last fault at the end,
> >>> why do you need a separate entry to track the last fault?
> >>
> >> Not sure I understand your question, sorry. Do you mean why the
> >> iopf_group.last_fault? Just to avoid one more kzalloc.
> >>
> > kind of :) what i thought was that why can't the last_fault
> > naturally be the last entry in your group fault list? then there is
> > no need for special treatment in terms of allocation of the last
> > fault. just my preference.
>
> But we need a kzalloc for the last fault anyway, so I thought I'd just
> piggy-back on the group allocation. And if I don't add the last fault
> at the end of group->faults, then it's iopf_handle_group that requires
> special treatment. I'm still not sure I understood your question
> though, could you send me a patch that does it?
>
> >>>> +
> >>>> + queue->flush(queue->flush_arg, dev);
> >>>> +
> >>>> + /*
> >>>> + * No need to clear the partial list. All PRGs
> >>>> containing the PASID that
> >>>> + * needs to be decommissioned are whole (the device
> >>>> driver made sure of
> >>>> + * it before this function was called). They have been
> >>>> submitted to the
> >>>> + * queue by the above flush().
> >>>> + */
> >>> So you are saying device driver need to make sure LPIG PRQ is
> >>> submitted in the flush call above such that partial list is
> >>> cleared?
> >>
> >> Not exactly, it's the IOMMU driver that makes sure all LPIG in its
> >> queues are submitted by the above flush call. In more details the
> >> flow is:
> >>
> >> * Either device driver calls unbind()/sva_device_shutdown(), or the
> >> process exits.
> >> * If the device driver called, then it already told the device to
> >> stop using the PASID. Otherwise we use the mm_exit() callback to
> >> tell the device driver to stop using the PASID.
Sorry I still need more clarification. For the PASID termination
initiated by vfio unbind, I don't see device driver given a chance to
stop PASID. Seems just call __iommu_sva_unbind_device() which already
assume device stopped issuing DMA with the PASID.
So it is the vfio unbind caller responsible for doing driver callback
to stop DMA on a given PASID?
> >> * In either case, when receiving a stop request from the driver,
> >> the device sends the LPIGs to the IOMMU queue.
> >> * Then, the flush call above ensures that the IOMMU reports the
> >> LPIG with iommu_report_device_fault.
> >> * While submitting all LPIGs for this PASID to the work queue,
> >> ipof_queue_fault also picked up all partial faults, so the partial
> >> list is clean.
> >>
> >> Maybe I should improve this comment?
> >>
> > thanks for explaining. LPIG submission is done by device
> > asynchronously w.r.t. driver stopping/decommission PASID.
>
> Hmm, it should really be synchronous, otherwise there is no way to
> know when the PASID can be decommissioned. We need a guarantee such
> as the one in 6.20.1 of the PCIe spec, "Managing PASID TLP Prefix
> Usage":
>
> "When the stop request mechanism indicates completion, the Function
> has:
> * Completed all Non-Posted Requests associated with this PASID.
> * Flushed to the host all Posted Requests addressing host memory in
> all TCs that were used by the PASID."
>
> That's in combination with "The function shall [...] finish
> transmitting any multi-page Page Request Messages for this PASID
> (i.e. send the Page Request Message with the L bit Set)." from the
> ATS spec.
>
I am not contesting on the device side, what I meant was from the
host IOMMU driver perspective, LPIG is received via IOMMU host queue,
therefore asynchronous. Not sure about ARM, but on VT-d LPIG submission
could meet queue full condition. So per VT-d spec, iommu will generate a
successful auto response to the device. At this point, assume we
already stopped the given PASID on the device, there might not be
another LPIG sent for the device. Therefore, you could have a partial
list. I think we can just drop the requests in the partial list for
that PASID until the PASID gets re-allocated.
> (If I remember correctly a PRI Page Request is a Posted Request.) Only
> after this stop request completes can the driver call unbind(), or
> return from exit_mm(). Then we know that if there was a LPIG for that
> PASID, it is in the IOMMU's PRI queue (or already completed) once we
> call flush().
>
agreed.
> > so if we were to use this
> > flow on vt-d, which does not stall page request queue, then we
> > should use the iommu model specific flush() callback to ensure LPIG
> > is received? There could be queue full condition and retry. I am
> > just trying to understand how and where we can make sure LPIG is
> > received and all groups are whole.
>
> For SMMU in patch 30, the flush() callback waits until the PRI queue
> is empty or, when the PRI thread is running in parallel, until the
> thread has done a full circle (handled as many faults as the queue
> size). It's really unpleasant to implement because the flush()
> callback takes a lock to inspect the hardware state, but I don't
> think we have a choice.
>
yes, vt-d has similar situation in page request queue. one option is to
track queue head (SW update) to make sure one complete cycle when queue
tail(HW update) crosses. Or we(suggested by Ashok Raj) can take a
snapshot of the entire queue and process (drops PRQs belong to the
terminated PASID) without holding the queue.
Thanks,
Jacob
^ permalink raw reply
* Re: [PATCH] arm64: dts: msm8916: fix Coresight ETF graph connections
From: Andy Gross @ 2018-05-25 22:46 UTC (permalink / raw)
To: Mathieu Poirier
Cc: devicetree, Rob Herring, linux-arm-msm, David Brown,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Ivan T. Ivanov, Ivan T . Ivanov
In-Reply-To: <CANLsYkz9tAH7mazz2LNCtoYsdH4f5bKfGWEnJHsgH0mK6vNMEg@mail.gmail.com>
On Thu, May 24, 2018 at 10:32:46AM -0600, Mathieu Poirier wrote:
<snip>
> >
> > Hi Rob,
> >
> > I no longer have access to this hardware and documentation.
> > I am sure that Mathieu and friends will take care for verification
> > of this patch :-)
>
> The code triggers on the "slave-mode" property rather than the labels,
> so this patch has no effect on how a path is established. I've tested
> this on a 410c and things look good.
>
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> Tested-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Thanks for verifying this Matthew. I'll put this in for a fixes as I just sent
out my last set for 4.18.
Regards,
Andy
^ permalink raw reply
* Re: [PATCH v2 2/6] dt-bindings: opp: Introduce qcom-opp bindings
From: David Collins @ 2018-05-25 22:33 UTC (permalink / raw)
To: Rajendra Nayak, viresh.kumar, sboyd, andy.gross, ulf.hansson
Cc: devicetree, linux-arm-msm, linux-kernel
In-Reply-To: <20180525100121.28214-3-rnayak@codeaurora.org>
Hello Rajendra,
On 05/25/2018 03:01 AM, Rajendra Nayak wrote:
> On Qualcomm platforms, an OPP node needs to describe an
s/Qualcomm/Qualcomm Technologies, Inc./
> additional level/corner value that is then communicated to
> a remote microprocessor by the CPU, which then takes some
> actions (like adjusting voltage values across various rails)
> based on the value passed.
>
> Describe these bindings in the qcom-opp bindings document.
>
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
> .../devicetree/bindings/opp/qcom-opp.txt | 25 +++++++++++++++++++
> 1 file changed, 25 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/opp/qcom-opp.txt
>
> diff --git a/Documentation/devicetree/bindings/opp/qcom-opp.txt b/Documentation/devicetree/bindings/opp/qcom-opp.txt
> new file mode 100644
> index 000000000000..db4d970c7ec7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/opp/qcom-opp.txt
> @@ -0,0 +1,25 @@
> +Qualcomm OPP bindings to descibe OPP nodes with corner/level values
s/Qualcomm/Qualcomm Technologies, Inc./ ?
s/descibe/describe/
> +
> +OPP tables for devices on Qualcomm platforms require an additional
s/Qualcomm/Qualcomm Technologies, Inc./
> +platform specific corner/level value to be specified.
> +This value is passed on to the RPM (Resource Power Manager) by
Do you want to mention RPMh here as well?
> +the CPU, which then takes the necessary actions to set a voltage
> +rail to an appropriate voltage based on the value passed.
> +
> +The bindings are based on top of the operating-points-v2 bindings
> +described in Documentation/devicetree/bindings/opp/opp.txt
> +Additional properties are described below.
> +
> +* OPP Table Node
> +
> +Required properties:
> +- compatible: Allow OPPs to express their compatibility. It should be:
> + "operating-points-v2-qcom-level"
> +
> +* OPP Node
> +
> +Required properties:
> +- qcom,level: On Qualcomm platforms an OPP node can describe a positive value
s/Qualcomm/Qualcomm Technologies, Inc./
> +representing a corner/level that's communicated with a remote microprocessor
> +(usually called the RPM) which then translates it into a certain voltage on
> +a voltage rail.
Should these lines be indented 2 spaces as is the case for the compatible
property above?
Could you add an example for both RPM and RPMh in this binding file?
Thanks,
David
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH 2/2] arm64: dts: Add the rtc enable clock for watchdog
From: Olof Johansson @ 2018-05-25 22:29 UTC (permalink / raw)
To: Baolin Wang
Cc: robh+dt, mark.rutland, catalin.marinas, will.deacon, arnd,
orsonzhai, zhang.lyra, devicetree, arm, linux-arm-kernel,
linux-kernel, broonie
In-Reply-To: <bc450ea82fe7fbec7e7f74c5cdc8a37d202fe526.1523846102.git.baolin.wang@linaro.org>
On Mon, Apr 16, 2018 at 10:40:04AM +0800, Baolin Wang wrote:
> Add the rtc enable clock for watchdog controller to make it work well.
>
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
APplied with "sprd: whale2:" added to the patch subject.
-Olof
^ permalink raw reply
* Re: [PATCH 1/2] arm64: dts: Add GPIO and GPIO keys device nodes
From: Olof Johansson @ 2018-05-25 22:29 UTC (permalink / raw)
To: Baolin Wang
Cc: robh+dt, mark.rutland, catalin.marinas, will.deacon, arnd,
orsonzhai, zhang.lyra, devicetree, arm, linux-arm-kernel,
linux-kernel, broonie
In-Reply-To: <fcea1874b6e3d0243684c133c7b34f2384341a32.1523846102.git.baolin.wang@linaro.org>
On Mon, Apr 16, 2018 at 10:40:03AM +0800, Baolin Wang wrote:
> This patch adds device nodes to enable one GPIO controller located on
> digital chip, 2 EIC (external interrupt controller) controllers loacted
> on PMIC and digital chip for Spreadtrum SC9860 platform.
>
> Moreover this patch adds 3 GPIO keys relied on EIC controller to support
> power key and volume up/down keys.
>
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
Applied with "sprd:" added to the subject.
-Olof
^ permalink raw reply
* Re: [PATCH] arm64: dts: sprd: fix typo in 'remote-endpoint'
From: Olof Johansson @ 2018-05-25 22:11 UTC (permalink / raw)
To: Chunyan Zhang
Cc: Rob Herring, DTML, Baolin Wang, arm@kernel.org, Orson Zhai,
Linux ARM
In-Reply-To: <CAAfSe-vCb7uXOV5GCg_EMEb6VC=oiofSggWHhWbEzpKWfODVZw@mail.gmail.com>
On Fri, May 25, 2018 at 10:36:45AM +0800, Chunyan Zhang wrote:
> Hi Arnd, Olof
>
> Could you please take this patch through arm-soc git?
Applied.
-Olof
^ permalink raw reply
* Re: [PATCH 1/6] arm64: dts: amlogic: Add missing cooling device properties for CPUs
From: Olof Johansson @ 2018-05-25 21:10 UTC (permalink / raw)
To: Viresh Kumar
Cc: arm, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
Carlo Caione, Kevin Hilman, Vincent Guittot, ionela.voinescu,
Daniel Lezcano, chris.redpath, devicetree, linux-arm-kernel,
linux-amlogic, linux-kernel
In-Reply-To: <2a2eb28da9fecf129f6bc0ab3d3748d9f4d25a29.1527225682.git.viresh.kumar@linaro.org>
On Fri, May 25, 2018 at 11:10:01AM +0530, Viresh Kumar wrote:
> The cooling device properties, like "#cooling-cells" and
> "dynamic-power-coefficient", should either be present for all the CPUs
> of a cluster or none. If these are present only for a subset of CPUs of
> a cluster then things will start falling apart as soon as the CPUs are
> brought online in a different order. For example, this will happen
> because the operating system looks for such properties in the CPU node
> it is trying to bring up, so that it can register a cooling device.
>
> Add such missing properties.
This seems awkward compared to just having one cooling-cells in the /cpus node
instead.
What's it used for? I don't see any properties in the device nodes on meson-gxm
that have any cooling-foo cells in them? So why should #cooling-cells be
needed?
-Olof
^ permalink raw reply
* Re: [PATCH V1 3/3] mmc: host: Register changes for sdcc V5
From: Evan Green @ 2018-05-25 20:46 UTC (permalink / raw)
To: vviswana
Cc: adrian.hunter, Ulf Hansson, robh+dt, mark.rutland, linux-mmc,
linux-kernel, shawn.lin, linux-arm-msm, georgi.djakov, devicetree,
asutoshd, stummala, venkatg, jeremymc, Bjorn Andersson, riteshh,
vbadigan, Doug Anderson, sayalil
In-Reply-To: <0baf8584-7833-0917-4432-363b0ee31bbc@codeaurora.org>
On Thu, May 24, 2018 at 6:01 AM Vijay Viswanath <vviswana@codeaurora.org>
wrote:
> On 5/22/2018 11:42 PM, Evan Green wrote:
> > Hi Vijay. Thanks for this patch.
> >
> > On Thu, May 17, 2018 at 3:30 AM Vijay Viswanath <vviswana@codeaurora.org
> > wrote:
> >
> >> From: Sayali Lokhande <sayalil@codeaurora.org>
> >
...
> > Nit: host->ioaddr + msm_offset->core_dll_config might benefit from
having
> > its own local, since you use it so much in this function. Same goes for
> > where I've noted below...
> >
> core_dll_config is very much used. But having a local for it feels like
> a bad idea. As different versions come up, the most used register may
> change. So it would be better to stick to a consistent approach to
> accessing every register.
I generally optimize for readability, rather than find/replace-ability. In
my opinion, it's distracting to see that expression copy/pasted so many
times in the same function. But ultimately this is a style preference, so
if you decide not to do it, I'll live.
> >
> >> + msm_offset->core_pwrctl_status),
> >> + msm_host->var_ops->msm_readl_relaxed(host,
> >> + msm_offset->core_pwrctl_mask),
> >> + msm_host->var_ops->msm_readl_relaxed(host,
> >> + msm_offset->core_pwrctl_ctl));
> >
> > I think the idea of function pointers is fine, but overall the use of
them
> > everywhere sure is ugly. It makes it really hard to actually see what's
> > happening. I wonder if things might look a lot cleaner with a helper
> > function here. Then instead of:
> >
> > msm_host->var_ops->msm_readl_relaxed(host, msm_offset->core_pwrctl_ctl);
> >
> > You could have
> >
> > msm_core_read(host, msm_offset->core_pwrctl_ctl);
> >
> if we use a helper function, then we will have to pass msm_host into it
> as well. Otherwise there would be the hassle of deriving msm_host
> address from sdhci_host.
> How about using a MACRO here instead for readability ?
The deriving part in the helper would likely get inlined and shared by the
compiler among all call-sites within a function. But yes, a macro would
work too.
Thanks Vijay,
Evan
^ 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