[PATCH 0/3] Initial support for LG Nexus 5 phone (hammerhead)

[PATCH 0/3] Initial support for LG Nexus 5 phone (hammerhead)

[Bhushan Shah]

This patchset adds support for LG Nexus 5 phone, codenamed hammerhead.

Initial version have following supported,

- Serial console over headphone jack
- pm8841 and pm8941 regulator nodes
- Hardware keys (volume_up and volume_down)

Values for the regulator nodes are taken from following files in downstream msm
kernel tree, branch android-msm-hammerhead-3.4-marshmallow-mr2

- arch/arm/boot/dts/msm8974-regulator.dtsi
- arch/arm/boot/dts/msm8974-hammerhead/msm8974-hammerhead.dtsi

Values are verified against kmsg from downstream kernel as well.

However, it seems regulator-max-microvolt settings are not respected, for example,

l19{
	regulator-min-microvolt = <3000000>;
	regulator-max-microvolt = <3300000>;
};

but in kmsg, it says,

[    1.753401] l19: supplied by vph-pwr                
[    1.757418] l19: Bringing 0uV into 3000000-3000000uV

Relavant line from kmsg of downstream kernel says,

[    0.255590] 8941_l19: 3000 <--> 3300 mV at 3300 mV normal idle

Is there anything wrong with dts?

Cc: Andy Gross <andy.gross@linaro.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: David Brown <david.brown@linaro.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Russell King <linux@armlinux.org.uk>
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-arm-msm@vger.kernel.org

Bhushan Shah (3):
  ARM: dts: qcom: Add initial DTS for LG Nexus 5 Phone
  ARM: dts: msm8974-hammerhead: Add regulator nodes for hammerhead
  ARM: dts: msm8974-hammerhead: Introduce gpio-keys nodes

 arch/arm/boot/dts/Makefile                         |   1 +
 .../dts/qcom-msm8974-lge-nexus5-hammerhead.dts     | 296 +++++++++++++++++++++
 2 files changed, 297 insertions(+)
 create mode 100644 arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts

-- 
2.9.0

[PATCH 0/3] Initial support for LG Nexus 5 phone (hammerhead)

[Bhushan Shah]

This patchset adds support for LG Nexus 5 phone, codenamed hammerhead.

Initial version have following supported,

- Serial console over headphone jack
- pm8841 and pm8941 regulator nodes
- Hardware keys (volume_up and volume_down)

Values for the regulator nodes are taken from following files in downstream msm
kernel tree, branch android-msm-hammerhead-3.4-marshmallow-mr2

- arch/arm/boot/dts/msm8974-regulator.dtsi
- arch/arm/boot/dts/msm8974-hammerhead/msm8974-hammerhead.dtsi

Values are verified against kmsg from downstream kernel as well.

However, it seems regulator-max-microvolt settings are not respected, for example,

l19{
	regulator-min-microvolt = <3000000>;
	regulator-max-microvolt = <3300000>;
};

but in kmsg, it says,

[    1.753401] l19: supplied by vph-pwr                
[    1.757418] l19: Bringing 0uV into 3000000-3000000uV

Relavant line from kmsg of downstream kernel says,

[    0.255590] 8941_l19: 3000 <--> 3300 mV at 3300 mV normal idle

Is there anything wrong with dts?

Cc: Andy Gross <andy.gross@linaro.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: David Brown <david.brown@linaro.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Russell King <linux@armlinux.org.uk>
Cc: devicetree at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-arm-msm at vger.kernel.org

Bhushan Shah (3):
  ARM: dts: qcom: Add initial DTS for LG Nexus 5 Phone
  ARM: dts: msm8974-hammerhead: Add regulator nodes for hammerhead
  ARM: dts: msm8974-hammerhead: Introduce gpio-keys nodes

 arch/arm/boot/dts/Makefile                         |   1 +
 .../dts/qcom-msm8974-lge-nexus5-hammerhead.dts     | 296 +++++++++++++++++++++
 2 files changed, 297 insertions(+)
 create mode 100644 arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts

-- 
2.9.0

[PATCH 1/3] ARM: dts: qcom: Add initial DTS for LG Nexus 5 Phone

[Bhushan Shah]

This DTS file have support LG Nexus 5 (codenamed hammerhead).
Initial version have support for just serial console over headphone
jack.

Cc: Andy Gross <andy.gross@linaro.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: David Brown <david.brown@linaro.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Russell King <linux@armlinux.org.uk>
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-arm-msm@vger.kernel.org
Signed-off-by: Bhushan Shah <bshah@kde.org>
---
 arch/arm/boot/dts/Makefile                         |  1 +
 .../dts/qcom-msm8974-lge-nexus5-hammerhead.dts     | 25 ++++++++++++++++++++++
 2 files changed, 26 insertions(+)
 create mode 100644 arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 414b427..13947ec 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -584,6 +584,7 @@ dtb-$(CONFIG_ARCH_QCOM) += \
 	qcom-ipq8064-ap148.dtb \
 	qcom-msm8660-surf.dtb \
 	qcom-msm8960-cdp.dtb \
+	qcom-msm8974-lge-nexus5-hammerhead.dtb \
 	qcom-msm8974-sony-xperia-honami.dtb
 dtb-$(CONFIG_ARCH_REALVIEW) += \
 	arm-realview-pb1176.dtb \
diff --git a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
new file mode 100644
index 0000000..88d494f
--- /dev/null
+++ b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
@@ -0,0 +1,25 @@
+#include "qcom-msm8974.dtsi"
+#include "qcom-pm8841.dtsi"
+#include "qcom-pm8941.dtsi"
+
+/ {
+	model = "LGE MSM 8974 HAMMERHEAD";
+	compatible = "qcom,msm8974";
+	qcom,msm-id = <126 150 0x20002 0xB>;
+
+	aliases {
+		serial0 = &blsp1_uart1;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+};
+
+&soc {
+
+	serial@f991d000 {
+		status = "ok";
+	};
+
+};
-- 
2.9.0

[PATCH 1/3] ARM: dts: qcom: Add initial DTS for LG Nexus 5 Phone

[Bhushan Shah]

This DTS file have support LG Nexus 5 (codenamed hammerhead).
Initial version have support for just serial console over headphone
jack.

Cc: Andy Gross <andy.gross@linaro.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: David Brown <david.brown@linaro.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Russell King <linux@armlinux.org.uk>
Cc: devicetree at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-arm-msm at vger.kernel.org
Signed-off-by: Bhushan Shah <bshah@kde.org>
---
 arch/arm/boot/dts/Makefile                         |  1 +
 .../dts/qcom-msm8974-lge-nexus5-hammerhead.dts     | 25 ++++++++++++++++++++++
 2 files changed, 26 insertions(+)
 create mode 100644 arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 414b427..13947ec 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -584,6 +584,7 @@ dtb-$(CONFIG_ARCH_QCOM) += \
 	qcom-ipq8064-ap148.dtb \
 	qcom-msm8660-surf.dtb \
 	qcom-msm8960-cdp.dtb \
+	qcom-msm8974-lge-nexus5-hammerhead.dtb \
 	qcom-msm8974-sony-xperia-honami.dtb
 dtb-$(CONFIG_ARCH_REALVIEW) += \
 	arm-realview-pb1176.dtb \
diff --git a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
new file mode 100644
index 0000000..88d494f
--- /dev/null
+++ b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
@@ -0,0 +1,25 @@
+#include "qcom-msm8974.dtsi"
+#include "qcom-pm8841.dtsi"
+#include "qcom-pm8941.dtsi"
+
+/ {
+	model = "LGE MSM 8974 HAMMERHEAD";
+	compatible = "qcom,msm8974";
+	qcom,msm-id = <126 150 0x20002 0xB>;
+
+	aliases {
+		serial0 = &blsp1_uart1;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+};
+
+&soc {
+
+	serial at f991d000 {
+		status = "ok";
+	};
+
+};
-- 
2.9.0

[PATCH 2/3] ARM: dts: msm8974-hammerhead: Add regulator nodes for hammerhead

[Bhushan Shah]

Cc: Andy Gross <andy.gross@linaro.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: David Brown <david.brown@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-soc@vger.kernel.org
Cc: devicetree@vger.kernel.org
Signed-off-by: Bhushan Shah <bshah@kde.org>
---
 .../dts/qcom-msm8974-lge-nexus5-hammerhead.dts     | 241 +++++++++++++++++++++
 1 file changed, 241 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
index 88d494f..29fa0bb 100644
--- a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
+++ b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
@@ -1,6 +1,8 @@
 #include "qcom-msm8974.dtsi"
 #include "qcom-pm8841.dtsi"
 #include "qcom-pm8941.dtsi"
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
 
 / {
 	model = "LGE MSM 8974 HAMMERHEAD";
@@ -14,6 +16,234 @@
 	chosen {
 		stdout-path = "serial0:115200n8";
 	};
+
+	smd {
+		rpm {
+			rpm_requests {
+				pm8841-regulators {
+					s1 {
+						regulator-min-microvolt = <675000>;
+						regulator-max-microvolt = <1050000>;
+					};
+
+					s2 {
+						regulator-min-microvolt = <500000>;
+						regulator-max-microvolt = <1050000>;
+					};
+
+					s3 {
+						regulator-min-microvolt = <1050000>;
+						regulator-max-microvolt = <1050000>;
+					};
+
+					s4 {
+						regulator-min-microvolt = <815000>;
+						regulator-max-microvolt = <900000>;
+					};
+				};
+
+				pm8941-regulators {
+					vdd_l1_l3-supply = <&pm8941_s1>;
+					vdd_l2_lvs1_2_3-supply = <&pm8941_s3>;
+					vdd_l4_l11-supply = <&pm8941_s1>;
+					vdd_l5_l7-supply = <&pm8941_s2>;
+					vdd_l6_l12_l14_l15-supply = <&pm8941_s2>;
+					vdd_l8_l16_l18_l19-supply = <&vph_pwr_reg>;
+					vdd_l9_l10_l17_l22-supply = <&vreg_boost>;
+					vdd_l13_l20_l23_l24-supply = <&vreg_boost>;
+					vdd_l21-supply = <&vreg_boost>;
+
+					s1 {
+						regulator-min-microvolt = <1300000>;
+						regulator-max-microvolt = <1300000>;
+
+						regulator-always-on;
+						regulator-boot-on;
+					};
+
+					s2 {
+						regulator-min-microvolt = <2150000>;
+						regulator-max-microvolt = <2150000>;
+
+						regulator-boot-on;
+					};
+
+					s3 {
+						regulator-min-microvolt = <1800000>;
+						regulator-max-microvolt = <1800000>;
+
+						regulator-always-on;
+						regulator-boot-on;
+					};
+
+					l1 {
+						regulator-min-microvolt = <1225000>;
+						regulator-max-microvolt = <1225000>;
+
+						regulator-always-on;
+						regulator-boot-on;
+					};
+
+					l2 {
+						regulator-min-microvolt = <1200000>;
+						regulator-max-microvolt = <1200000>;
+					};
+
+					l3 {
+						regulator-min-microvolt = <1225000>;
+						regulator-max-microvolt = <1225000>;
+					};
+
+					l4 {
+						regulator-min-microvolt = <1225000>;
+						regulator-max-microvolt = <1225000>;
+					};
+
+					l5 {
+						regulator-min-microvolt = <1800000>;
+						regulator-max-microvolt = <1800000>;
+					};
+
+					l6 {
+						regulator-min-microvolt = <1800000>;
+						regulator-max-microvolt = <1800000>;
+
+						regulator-boot-on;
+					};
+
+					l7 {
+						regulator-min-microvolt = <1800000>;
+						regulator-max-microvolt = <1800000>;
+
+						regulator-boot-on;
+					};
+
+					l8 {
+						regulator-min-microvolt = <1800000>;
+						regulator-max-microvolt = <1800000>;
+					};
+
+					l9 {
+						regulator-min-microvolt = <1800000>;
+						regulator-max-microvolt = <2950000>;
+					};
+
+					l10 {
+						regulator-min-microvolt = <1800000>;
+						regulator-max-microvolt = <2950000>;
+					};
+
+					l11 {
+						regulator-min-microvolt = <1300000>;
+						regulator-max-microvolt = <1300000>;
+					};
+
+					l12 {
+						regulator-min-microvolt = <1800000>;
+						regulator-max-microvolt = <1800000>;
+
+						regulator-always-on;
+						regulator-boot-on;
+					};
+
+					l13 {
+						regulator-min-microvolt = <1800000>;
+						regulator-max-microvolt = <2950000>;
+
+						regulator-boot-on;
+					};
+
+					l14 {
+						regulator-min-microvolt = <1800000>;
+						regulator-max-microvolt = <1800000>;
+					};
+
+					l15 {
+						regulator-min-microvolt = <2050000>;
+						regulator-max-microvolt = <2050000>;
+					};
+
+					l16 {
+						regulator-min-microvolt = <2700000>;
+						regulator-max-microvolt = <2700000>;
+					};
+
+					l17 {
+						regulator-min-microvolt = <2850000>;
+						regulator-max-microvolt = <2850000>;
+					};
+
+					l18 {
+						regulator-min-microvolt = <2850000>;
+						regulator-max-microvolt = <2850000>;
+					};
+
+					l19 {
+						regulator-min-microvolt = <3000000>;
+						regulator-max-microvolt = <3300000>;
+					};
+
+					l20 {
+						regulator-min-microvolt = <2950000>;
+						regulator-max-microvolt = <2950000>;
+
+						regulator-boot-on;
+					};
+
+					l21 {
+						regulator-min-microvolt = <2950000>;
+						regulator-max-microvolt = <2950000>;
+
+						regulator-boot-on;
+					};
+
+					l22 {
+						regulator-min-microvolt = <3000000>;
+						regulator-max-microvolt = <3300000>;
+					};
+
+					l23 {
+						regulator-min-microvolt = <3000000>;
+						regulator-max-microvolt = <3000000>;
+					};
+
+					l24 {
+						regulator-min-microvolt = <3075000>;
+						regulator-max-microvolt = <3075000>;
+
+						regulator-boot-on;
+					};
+				};
+			};
+		};
+	};
+
+	vreg_boost: vreg-boost {
+		compatible = "regulator-fixed";
+
+		regulator-name = "vreg-boost";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+
+		regulator-always-on;
+		regulator-boot-on;
+
+		gpio = <&pm8941_gpios 21 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&boost_bypass_n_pin>;
+	};
+
+	vph_pwr_reg: vph_pwr_reg {
+		compatible = "regulator-fixed";
+		regulator-name = "vph-pwr";
+
+		regulator-min-microvolt = <3600000>;
+		regulator-max-microvolt = <3600000>;
+
+		regulator-always-on;
+	};
 };
 
 &soc {
@@ -23,3 +253,14 @@
 	};
 
 };
+
+&spmi_bus {
+	pm8941@0 {
+		gpios@c000 {
+			boost_bypass_n_pin: boost-bypass {
+				pins = "gpio21";
+				function = "normal";
+			};
+		};
+	};
+};
-- 
2.9.0

[PATCH 3/3] ARM: dts: msm8974-hammerhead: Introduce gpio-keys nodes

[Bhushan Shah]

This introduces the gpio-keys node for keys of hammerhead and pinctrl
state associated with it.

Cc: Andy Gross <andy.gross@linaro.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: David Brown <david.brown@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-soc@vger.kernel.org
Cc: devicetree@vger.kernel.org
Signed-off-by: Bhushan Shah <bshah@kde.org>
---
 .../dts/qcom-msm8974-lge-nexus5-hammerhead.dts     | 30 ++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
index 29fa0bb..12b6a14 100644
--- a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
+++ b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
@@ -2,6 +2,7 @@
 #include "qcom-pm8841.dtsi"
 #include "qcom-pm8941.dtsi"
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
 #include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
 
 / {
@@ -252,6 +253,27 @@
 		status = "ok";
 	};
 
+	gpio-keys {
+		compatible = "gpio-keys";
+		input-name = "gpio-keys";
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&gpio_keys_pin_a>;
+
+		volume-up {
+			label = "volume_up";
+			gpios = <&pm8941_gpios 2 GPIO_ACTIVE_LOW>;
+			linux,input-type = <1>;
+			linux,code = <KEY_VOLUMEUP>;
+		};
+
+		volume-down {
+			label = "volume_down";
+			gpios = <&pm8941_gpios 3 GPIO_ACTIVE_LOW>;
+			linux,input-type = <1>;
+			linux,code = <KEY_VOLUMEDOWN>;
+		};
+	};
 };
 
 &spmi_bus {
@@ -261,6 +283,14 @@
 				pins = "gpio21";
 				function = "normal";
 			};
+
+			gpio_keys_pin_a: gpio-keys-active {
+				pins = "gpio2", "gpio3";
+				function = "normal";
+
+				bias-pull-up;
+				power-source = <PM8941_GPIO_S3>;
+			};
 		};
 	};
 };
-- 
2.9.0

Re: [PATCH 2/3] ARM: dts: msm8974-hammerhead: Add regulator nodes for hammerhead

[Arnd Bergmann]

On Sunday, July 17, 2016 4:22:07 PM CEST Bhushan Shah wrote:
> +
> +       smd {
> +               rpm {
> +                       rpm_requests {
> +                               pm8841-regulators {
> +                                       s1 {
> +                                               regulator-min-microvolt = <675000>;
> +                                               regulator-max-microvolt = <1050000>;
> +                                       };

Maybe add a label at either the rpm_requests or the pm8841-regulators
node so you can add properties in the leaf nodes withoutout having to
specify the whole path?

	Arnd

Re: [PATCH 2/3] ARM: dts: msm8974-hammerhead: Add regulator nodes for hammerhead

[Bhushan Shah]

On Sun, Jul 17, 2016 at 09:21:48PM +0200, Arnd Bergmann wrote:
> On Sunday, July 17, 2016 4:22:07 PM CEST Bhushan Shah wrote:
> > +
> > +       smd {
> > +               rpm {
> > +                       rpm_requests {
> > +                               pm8841-regulators {
> > +                                       s1 {
> > +                                               regulator-min-microvolt = <675000>;
> > +                                               regulator-max-microvolt = <1050000>;
> > +                                       };
> 
> Maybe add a label at either the rpm_requests or the pm8841-regulators
> node so you can add properties in the leaf nodes withoutout having to
> specify the whole path?

Sure, I will adjust patch.
> 	Arnd

Re: [PATCH 2/3] ARM: dts: msm8974-hammerhead: Add regulator nodes for hammerhead

[Bjorn Andersson]

On Sun, Jul 17, 2016 at 8:34 PM, Bhushan Shah <bshah-RoXCvvDuEio@public.gmane.org> wrote:
> On Sun, Jul 17, 2016 at 09:21:48PM +0200, Arnd Bergmann wrote:
>> On Sunday, July 17, 2016 4:22:07 PM CEST Bhushan Shah wrote:
>> > +
>> > +       smd {
>> > +               rpm {
>> > +                       rpm_requests {
>> > +                               pm8841-regulators {
>> > +                                       s1 {
>> > +                                               regulator-min-microvolt = <675000>;
>> > +                                               regulator-max-microvolt = <1050000>;
>> > +                                       };
>>
>> Maybe add a label at either the rpm_requests or the pm8841-regulators
>> node so you can add properties in the leaf nodes withoutout having to
>> specify the whole path?
>
> Sure, I will adjust patch.

Please don't. After running into several cases where this would end us
up in having a multitude of nodes each describing just a snippet of
each level we decided not to do so in the general case for the
Qualcomm boards.

There are a few where we apparently ended up doing so anyways, but for
all other cases of regulators we express the full tree in the dts, so
please follow that so we don't mix the styles too much.

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] ARM: dts: msm8974-hammerhead: Add regulator nodes for hammerhead

[Arnd Bergmann]

On Sunday, July 17, 2016 8:44:01 PM CEST Bjorn Andersson wrote:
> On Sun, Jul 17, 2016 at 8:34 PM, Bhushan Shah <bshah-RoXCvvDuEio@public.gmane.org> wrote:
> > On Sun, Jul 17, 2016 at 09:21:48PM +0200, Arnd Bergmann wrote:
> >> On Sunday, July 17, 2016 4:22:07 PM CEST Bhushan Shah wrote:
> >> > +
> >> > +       smd {
> >> > +               rpm {
> >> > +                       rpm_requests {
> >> > +                               pm8841-regulators {
> >> > +                                       s1 {
> >> > +                                               regulator-min-microvolt = <675000>;
> >> > +                                               regulator-max-microvolt = <1050000>;
> >> > +                                       };
> >>
> >> Maybe add a label at either the rpm_requests or the pm8841-regulators
> >> node so you can add properties in the leaf nodes withoutout having to
> >> specify the whole path?
> >
> > Sure, I will adjust patch.
> 
> Please don't. After running into several cases where this would end us
> up in having a multitude of nodes each describing just a snippet of
> each level we decided not to do so in the general case for the
> Qualcomm boards.
> 
> There are a few where we apparently ended up doing so anyways, but for
> all other cases of regulators we express the full tree in the dts, so
> please follow that so we don't mix the styles too much.

Ok, then how about this instead:

/smd/rpm/rpm_requests/pm8841-regulators {
	s1 {
		regulator-min-microvolt = <675000>;
		regulator-max-microvolt = <1050000>;
	};

	...
};

That avoids the ridiculous intendation level but uses no labels.

	Arnd



--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] ARM: dts: msm8974-hammerhead: Add regulator nodes for hammerhead

[Bjorn Andersson]

On Mon 18 Jul 00:44 PDT 2016, Arnd Bergmann wrote:

> On Sunday, July 17, 2016 8:44:01 PM CEST Bjorn Andersson wrote:
> > On Sun, Jul 17, 2016 at 8:34 PM, Bhushan Shah <bshah@kde.org> wrote:
> > > On Sun, Jul 17, 2016 at 09:21:48PM +0200, Arnd Bergmann wrote:
> > >> On Sunday, July 17, 2016 4:22:07 PM CEST Bhushan Shah wrote:
> > >> > +
> > >> > +       smd {
> > >> > +               rpm {
> > >> > +                       rpm_requests {
> > >> > +                               pm8841-regulators {
> > >> > +                                       s1 {
> > >> > +                                               regulator-min-microvolt = <675000>;
> > >> > +                                               regulator-max-microvolt = <1050000>;
> > >> > +                                       };
> > >>
> > >> Maybe add a label at either the rpm_requests or the pm8841-regulators
> > >> node so you can add properties in the leaf nodes withoutout having to
> > >> specify the whole path?
> > >
> > > Sure, I will adjust patch.
> > 
> > Please don't. After running into several cases where this would end us
> > up in having a multitude of nodes each describing just a snippet of
> > each level we decided not to do so in the general case for the
> > Qualcomm boards.
> > 
> > There are a few where we apparently ended up doing so anyways, but for
> > all other cases of regulators we express the full tree in the dts, so
> > please follow that so we don't mix the styles too much.
> 
> Ok, then how about this instead:
> 
> /smd/rpm/rpm_requests/pm8841-regulators {

The problem I have with this is that in the dtsi we have properties and
other nodes under each one of these, hence we end up with completely
different overall structure depending on if I look in the dtsi or the
dts.

The problem I have with it in the dts is that we have properties and
nodes under "smd" and "rpm_requests". So siblings are no longer grouped
together.


I have a hard time finding my way through flattened trees, often spread out
over multiple files, that I need to puzzle together in my head. Perhaps
there are better ways to keep this comprehensible, without maintaining
the structure.

> 	s1 {
> 		regulator-min-microvolt = <675000>;
> 		regulator-max-microvolt = <1050000>;
> 	};
> 
> 	...
> };
> 
> That avoids the ridiculous intendation level but uses no labels.
> 

I do share your dislike of the indentation level.


I do have a few other concerns about style and scalability in other
places. How about we follow how I've done this in the other files for
now (i.e.  keep the structure of the patch as is) and sit down at LAS16
to discuss what to do about this?

Regards,
Bjorn

Re: [PATCH 1/3] ARM: dts: qcom: Add initial DTS for LG Nexus 5 Phone

[Bjorn Andersson]

On Sun 17 Jul 03:52 PDT 2016, Bhushan Shah wrote:

[..]
> diff --git a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
> new file mode 100644
> index 0000000..88d494f
> --- /dev/null
> +++ b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
> @@ -0,0 +1,25 @@
> +#include "qcom-msm8974.dtsi"
> +#include "qcom-pm8841.dtsi"
> +#include "qcom-pm8941.dtsi"
> +
> +/ {
> +	model = "LGE MSM 8974 HAMMERHEAD";
> +	compatible = "qcom,msm8974";

We should have a "lg,hammerhead" here as well, prior to "qcom,msm8974".
Preferably some sort of "product family" definition, if we know of any
to reduce the risk of conflicts with any other hammerheads from LGE.

> +	qcom,msm-id = <126 150 0x20002 0xB>;

We've decided against defining qcom,msm-id in the dts files, so please
drop this.

Instead use dtbTool from git://codeaurora.org/quic/kernel/skales which
should inject the correct information based on the compatible.

> +
> +	aliases {
> +		serial0 = &blsp1_uart1;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +};
> +
> +&soc {
> +

As you're dropping msm-id, please also drop this empty line.

> +	serial@f991d000 {
> +		status = "ok";
> +	};
> +

Dito.

> +};

Regards,
Bjorn

[PATCH 1/3] ARM: dts: qcom: Add initial DTS for LG Nexus 5 Phone

[Bjorn Andersson]

On Sun 17 Jul 03:52 PDT 2016, Bhushan Shah wrote:

[..]
> diff --git a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
> new file mode 100644
> index 0000000..88d494f
> --- /dev/null
> +++ b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
> @@ -0,0 +1,25 @@
> +#include "qcom-msm8974.dtsi"
> +#include "qcom-pm8841.dtsi"
> +#include "qcom-pm8941.dtsi"
> +
> +/ {
> +	model = "LGE MSM 8974 HAMMERHEAD";
> +	compatible = "qcom,msm8974";

We should have a "lg,hammerhead" here as well, prior to "qcom,msm8974".
Preferably some sort of "product family" definition, if we know of any
to reduce the risk of conflicts with any other hammerheads from LGE.

> +	qcom,msm-id = <126 150 0x20002 0xB>;

We've decided against defining qcom,msm-id in the dts files, so please
drop this.

Instead use dtbTool from git://codeaurora.org/quic/kernel/skales which
should inject the correct information based on the compatible.

> +
> +	aliases {
> +		serial0 = &blsp1_uart1;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +};
> +
> +&soc {
> +

As you're dropping msm-id, please also drop this empty line.

> +	serial at f991d000 {
> +		status = "ok";
> +	};
> +

Dito.

> +};

Regards,
Bjorn

Re: [PATCH 2/3] ARM: dts: msm8974-hammerhead: Add regulator nodes for hammerhead

[Bjorn Andersson]

On Sun 17 Jul 03:52 PDT 2016, Bhushan Shah wrote:

[..]
> diff --git a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
> index 88d494f..29fa0bb 100644
> --- a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
> +++ b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
> @@ -1,6 +1,8 @@
>  #include "qcom-msm8974.dtsi"
>  #include "qcom-pm8841.dtsi"
>  #include "qcom-pm8941.dtsi"
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
>  
>  / {
>  	model = "LGE MSM 8974 HAMMERHEAD";
> @@ -14,6 +16,234 @@
>  	chosen {
>  		stdout-path = "serial0:115200n8";
>  	};
> +
> +	smd {
> +		rpm {
> +			rpm_requests {
> +				pm8841-regulators {
[..]
> +				};
> +
> +				pm8941-regulators {
> +					vdd_l1_l3-supply = <&pm8941_s1>;
> +					vdd_l2_lvs1_2_3-supply = <&pm8941_s3>;
> +					vdd_l4_l11-supply = <&pm8941_s1>;
> +					vdd_l5_l7-supply = <&pm8941_s2>;
> +					vdd_l6_l12_l14_l15-supply = <&pm8941_s2>;
> +					vdd_l8_l16_l18_l19-supply = <&vph_pwr_reg>;
> +					vdd_l9_l10_l17_l22-supply = <&vreg_boost>;
> +					vdd_l13_l20_l23_l24-supply = <&vreg_boost>;
> +					vdd_l21-supply = <&vreg_boost>;
> +
[..]
> +	};
> +
> +	vreg_boost: vreg-boost {
> +		compatible = "regulator-fixed";
> +
> +		regulator-name = "vreg-boost";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;

vreg_boost is not to be confused with the boost regulator (PM8941 S4),
this boot regulator is responsible for keeping voltage high enough for
e.g. eMMC operation during voltage drops of the battery during high
current operations. I have it listed in the Sony file as 3.15V, which
seems more reasonable (I can't find the number in the Qcom docs right
now).

As this has turned out to be part of the Qualcomm platform rather than
something Sony specific I believe you should add a patch before this one
that moves the vreg-boost node from the honami.dts to the msm8974.dtsi.

> +
> +		regulator-always-on;
> +		regulator-boot-on;
> +
> +		gpio = <&pm8941_gpios 21 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&boost_bypass_n_pin>;
> +	};
> +
> +	vph_pwr_reg: vph_pwr_reg {

This is also Qualcomm generic, I just skipped it in the Honami case. But
please add this to the msm8974.dtsi instead.

You are not allowed to use _ in node names (but should do so in label
names), also just to follow naming please name this:

vreg_vph_pwr: vreg-vph-pwr { ... };

> +		compatible = "regulator-fixed";
> +		regulator-name = "vph-pwr";
> +
> +		regulator-min-microvolt = <3600000>;
> +		regulator-max-microvolt = <3600000>;
> +
> +		regulator-always-on;
> +	};
>  };
>  
>  &soc {
> @@ -23,3 +253,14 @@
>  	};
>  
>  };
> +
> +&spmi_bus {
> +	pm8941@0 {
> +		gpios@c000 {
> +			boost_bypass_n_pin: boost-bypass {
> +				pins = "gpio21";
> +				function = "normal";
> +			};
> +		};

This is also part of the platform, so move it from honami.dts to
qcom-pm8941.dtsi when you move the vreg above.

> +	};
> +};

Regards,
Bjorn

Re: [PATCH 3/3] ARM: dts: msm8974-hammerhead: Introduce gpio-keys nodes

[Bjorn Andersson]

On Sun 17 Jul 03:52 PDT 2016, Bhushan Shah wrote:

> This introduces the gpio-keys node for keys of hammerhead and pinctrl
> state associated with it.
> 
> Cc: Andy Gross <andy.gross-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Reviewed-by: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Regards,
Bjorn

> Cc: David Brown <david.brown-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> Cc: Russell King <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
> Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Bhushan Shah <bshah-RoXCvvDuEio@public.gmane.org>
> ---
>  .../dts/qcom-msm8974-lge-nexus5-hammerhead.dts     | 30 ++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
> index 29fa0bb..12b6a14 100644
> --- a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
> +++ b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
> @@ -2,6 +2,7 @@
>  #include "qcom-pm8841.dtsi"
>  #include "qcom-pm8941.dtsi"
>  #include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
>  #include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
>  
>  / {
> @@ -252,6 +253,27 @@
>  		status = "ok";
>  	};
>  
> +	gpio-keys {
> +		compatible = "gpio-keys";
> +		input-name = "gpio-keys";
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&gpio_keys_pin_a>;
> +
> +		volume-up {
> +			label = "volume_up";
> +			gpios = <&pm8941_gpios 2 GPIO_ACTIVE_LOW>;
> +			linux,input-type = <1>;
> +			linux,code = <KEY_VOLUMEUP>;
> +		};
> +
> +		volume-down {
> +			label = "volume_down";
> +			gpios = <&pm8941_gpios 3 GPIO_ACTIVE_LOW>;
> +			linux,input-type = <1>;
> +			linux,code = <KEY_VOLUMEDOWN>;
> +		};
> +	};
>  };
>  
>  &spmi_bus {
> @@ -261,6 +283,14 @@
>  				pins = "gpio21";
>  				function = "normal";
>  			};
> +
> +			gpio_keys_pin_a: gpio-keys-active {
> +				pins = "gpio2", "gpio3";
> +				function = "normal";
> +
> +				bias-pull-up;
> +				power-source = <PM8941_GPIO_S3>;
> +			};
>  		};
>  	};
>  };
> -- 
> 2.9.0
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] ARM: dts: msm8974-hammerhead: Add regulator nodes for hammerhead

[Arnd Bergmann]

On Monday, July 18, 2016 10:11:55 AM CEST Bjorn Andersson wrote:
> On Mon 18 Jul 00:44 PDT 2016, Arnd Bergmann wrote:
> > On Sunday, July 17, 2016 8:44:01 PM CEST Bjorn Andersson wrote:
> > Ok, then how about this instead:
> > 
> > /smd/rpm/rpm_requests/pm8841-regulators {
> 
> The problem I have with this is that in the dtsi we have properties and
> other nodes under each one of these, hence we end up with completely
> different overall structure depending on if I look in the dtsi or the
> dts.
> 
> The problem I have with it in the dts is that we have properties and
> nodes under "smd" and "rpm_requests". So siblings are no longer grouped
> together.
> 
> 
> I have a hard time finding my way through flattened trees, often spread out
> over multiple files, that I need to puzzle together in my head. Perhaps
> there are better ways to keep this comprehensible, without maintaining
> the structure.
> 
> > 	s1 {
> > 		regulator-min-microvolt = <675000>;
> > 		regulator-max-microvolt = <1050000>;
> > 	};
> > 
> > 	...
> > };
> > 
> > That avoids the ridiculous intendation level but uses no labels.
> > 
> 
> I do share your dislike of the indentation level.
> 
> 
> I do have a few other concerns about style and scalability in other
> places. How about we follow how I've done this in the other files for
> now (i.e.  keep the structure of the patch as is) and sit down at LAS16
> to discuss what to do about this?

Fair enough, let's do that for now.

	Arnd