* [PATCH v2 1/2] arm64: dts: qcom: sc7180: Add sc7180-lazor-r2
@ 2020-11-06 0:37 Matthias Kaehlcke
2020-11-06 0:37 ` [PATCH v2 2/2] arm64: dts: qcom: sc7180-trogdor: Make pp3300_a the default supply for pp3300_hub Matthias Kaehlcke
2020-11-06 0:55 ` [PATCH v2 1/2] arm64: dts: qcom: sc7180: Add sc7180-lazor-r2 Doug Anderson
0 siblings, 2 replies; 6+ messages in thread
From: Matthias Kaehlcke @ 2020-11-06 0:37 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Rob Herring
Cc: linux-arm-msm, Douglas Anderson, devicetree, linux-kernel,
Matthias Kaehlcke
One important delta with respect to rev1 is a switch of the power
supply for the onboard USB hub from 'pp3300_l7c' to 'pp3300_a' + a
load switch. The actual regulator switch is done by the patch 'arm64:
dts: qcom: sc7180-trogdor: Make pp3300_a the default supply for
pp3300_hub', since it affects the entire trogdor platform. Here we
only add the .dts files for lazor rev2 and replace the generic
compatible entries in the rev1 .dts files.
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Changes in v2:
- patch added to the series
arch/arm64/boot/dts/qcom/Makefile | 3 +++
.../dts/qcom/sc7180-trogdor-lazor-r1-kb.dts | 4 ++--
.../dts/qcom/sc7180-trogdor-lazor-r1-lte.dts | 4 ++--
.../boot/dts/qcom/sc7180-trogdor-lazor-r1.dts | 4 ++--
.../dts/qcom/sc7180-trogdor-lazor-r2-kb.dts | 17 +++++++++++++++++
.../dts/qcom/sc7180-trogdor-lazor-r2-lte.dts | 18 ++++++++++++++++++
.../boot/dts/qcom/sc7180-trogdor-lazor-r2.dts | 15 +++++++++++++++
7 files changed, 59 insertions(+), 6 deletions(-)
create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r2-kb.dts
create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r2-lte.dts
create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r2.dts
diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index fb4631f898fd..3d72c7b63c79 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -26,6 +26,9 @@ dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-lazor-r0.dtb
dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-lazor-r1.dtb
dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-lazor-r1-kb.dtb
dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-lazor-r1-lte.dtb
+dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-lazor-r2.dtb
+dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-lazor-r2-kb.dtb
+dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-lazor-r2-lte.dtb
dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-r1.dtb
dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-r1-lte.dtb
dtb-$(CONFIG_ARCH_QCOM) += sdm630-sony-xperia-ganges-kirin.dtb
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1-kb.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1-kb.dts
index c3f426c3c30a..99f2c240c339 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1-kb.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1-kb.dts
@@ -8,8 +8,8 @@
#include "sc7180-trogdor-lazor-r1.dts"
/ {
- model = "Google Lazor (rev1+) with KB Backlight";
- compatible = "google,lazor-sku2", "qcom,sc7180";
+ model = "Google Lazor (rev1) with KB Backlight";
+ compatible = "google,lazor-rev1-sku2", "qcom,sc7180";
};
&keyboard_backlight {
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1-lte.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1-lte.dts
index 73e59cf7752a..4074c62207ce 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1-lte.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1-lte.dts
@@ -9,8 +9,8 @@
#include "sc7180-trogdor-lte-sku.dtsi"
/ {
- model = "Google Lazor (rev1+) with LTE";
- compatible = "google,lazor-sku0", "qcom,sc7180";
+ model = "Google Lazor (rev1) with LTE";
+ compatible = "google,lazor-rev1-sku0", "qcom,sc7180";
};
&keyboard_backlight {
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts
index 3151ae31c1cc..f6ee1beba458 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts
@@ -10,6 +10,6 @@
#include "sc7180-trogdor-lazor.dtsi"
/ {
- model = "Google Lazor (rev1+)";
- compatible = "google,lazor", "qcom,sc7180";
+ model = "Google Lazor (rev1)";
+ compatible = "google,lazor-rev1", "qcom,sc7180";
};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r2-kb.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r2-kb.dts
new file mode 100644
index 000000000000..7c3a702ef209
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r2-kb.dts
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Google Lazor board device tree source
+ *
+ * Copyright 2020 Google LLC.
+ */
+
+#include "sc7180-trogdor-lazor-r1.dts"
+
+/ {
+ model = "Google Lazor (rev2+) with KB Backlight";
+ compatible = "google,lazor-sku2", "qcom,sc7180";
+};
+
+&keyboard_backlight {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r2-lte.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r2-lte.dts
new file mode 100644
index 000000000000..e19bdfd329be
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r2-lte.dts
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Google Lazor board device tree source
+ *
+ * Copyright 2020 Google LLC.
+ */
+
+#include "sc7180-trogdor-lazor-r2.dts"
+#include "sc7180-trogdor-lte-sku.dtsi"
+
+/ {
+ model = "Google Lazor (rev2+) with LTE";
+ compatible = "google,lazor-sku0", "qcom,sc7180";
+};
+
+&keyboard_backlight {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r2.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r2.dts
new file mode 100644
index 000000000000..68c04f6dfc05
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r2.dts
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Google Lazor board device tree source
+ *
+ * Copyright 2020 Google LLC.
+ */
+
+/dts-v1/;
+
+#include "sc7180-trogdor-lazor.dtsi"
+
+/ {
+ model = "Google Lazor (rev2+)";
+ compatible = "google,lazor", "qcom,sc7180";
+};
--
2.29.1.341.ge80a0c044ae-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] arm64: dts: qcom: sc7180-trogdor: Make pp3300_a the default supply for pp3300_hub
2020-11-06 0:37 [PATCH v2 1/2] arm64: dts: qcom: sc7180: Add sc7180-lazor-r2 Matthias Kaehlcke
@ 2020-11-06 0:37 ` Matthias Kaehlcke
2020-11-06 1:05 ` Doug Anderson
2020-11-06 0:55 ` [PATCH v2 1/2] arm64: dts: qcom: sc7180: Add sc7180-lazor-r2 Doug Anderson
1 sibling, 1 reply; 6+ messages in thread
From: Matthias Kaehlcke @ 2020-11-06 0:37 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Rob Herring
Cc: linux-arm-msm, Douglas Anderson, devicetree, linux-kernel,
Matthias Kaehlcke
The trogdor design has two options for supplying the 'pp3300_hub' power
rail, it can be supplied by 'pp3300_l7c' or 'pp3300_a'. The 'pp3300_a'
path includes a load switch that can be controlled through GPIO84.
Initially trogdor boards used 'pp3300_l7c' to power the USB hub, newer
revisions (will) use 'pp3300_a' as supply for 'pp3300_hub'.
Add a DT node for the 'pp3300_a' path and a pinctrl entry for the GPIO.
Make this path the default and keep trogdor rev1, lazor rev0 and rev1
on 'pp3300_l7c'. These earlier revisions also allocated the GPIO to the
purpose of controlling the power switch, so there is no need to limit
the pinctrl config to newer revisions. Remove the platform-wide
'always-on' property from 'pp3300_l7c' and add it to the boards that
use this supply. Also delete the 'always-on' of 'pp3300_hub' for
these boards.
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Changes in v2:
- added 'always-on' and 'boot-on' properties for new 'pp3300_hub'
- removed platform-wide 'always-on' property for 'pp3300_l7c'
- added 'always-on' property to 'pp3300_l7c' for boards that still
use 'pp3300_l7c'
- delete 'always-on' property of 'pp3300_hub' for boards that still
use 'pp3300_l7c'
- got rid of 'pp3300_hub_7c' label, just use 'pp3300_l7c'
- fixed position of 'en_pp3300_hub' node to respect ordering
- updated commit message
.../boot/dts/qcom/sc7180-trogdor-lazor-r0.dts | 13 ++++++++
.../boot/dts/qcom/sc7180-trogdor-lazor-r1.dts | 13 ++++++++
.../arm64/boot/dts/qcom/sc7180-trogdor-r1.dts | 13 ++++++++
arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 33 ++++++++++++++++++-
4 files changed, 71 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts
index ae4c23a4fe65..1d809269e7ef 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts
@@ -14,6 +14,15 @@ / {
compatible = "google,lazor-rev0", "qcom,sc7180";
};
+&pp3300_hub {
+ /* pp3300_l7c is used to power the USB hub */
+ /delete-property/regulator-always-on;
+};
+
+&pp3300_l7c {
+ regulator-always-on;
+};
+
&sn65dsi86_out {
/*
* Lane 0 was incorrectly mapped on the cable, but we've now decided
@@ -22,3 +31,7 @@ &sn65dsi86_out {
*/
lane-polarities = <1 0>;
};
+
+&usb_hub {
+ vdd-supply = <&pp3300_l7c>;
+};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts
index f6ee1beba458..1d573523d6b6 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts
@@ -13,3 +13,16 @@ / {
model = "Google Lazor (rev1)";
compatible = "google,lazor-rev1", "qcom,sc7180";
};
+
+&pp3300_hub {
+ /* pp3300_l7c is used to power the USB hub */
+ /delete-property/regulator-always-on;
+};
+
+&pp3300_l7c {
+ regulator-always-on;
+};
+
+&usb_hub {
+ vdd-supply = <&pp3300_l7c>;
+};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts
index 0a281c24841c..6603f2102233 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts
@@ -58,10 +58,23 @@ ap_ts: touchscreen@10 {
};
};
+&pp3300_hub {
+ /* pp3300_l7c is used to power the USB hub */
+ /delete-property/regulator-always-on;
+};
+
+&pp3300_l7c {
+ regulator-always-on;
+};
+
&sdhc_2 {
status = "okay";
};
+&usb_hub {
+ vdd-supply = <&pp3300_l7c>;
+};
+
/* PINCTRL - board-specific pinctrl */
&tlmm {
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
index bf875589d364..50e733412a7f 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
@@ -174,6 +174,25 @@ pp3300_fp_tp: pp3300-fp-tp-regulator {
vin-supply = <&pp3300_a>;
};
+ pp3300_hub: pp3300-hub {
+ compatible = "regulator-fixed";
+ regulator-name = "pp3300_hub";
+
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+
+ gpio = <&tlmm 84 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ pinctrl-names = "default";
+ pinctrl-0 = <&en_pp3300_hub>;
+
+ /* AP turns on with en_pp3300_hub; always on for AP */
+ regulator-always-on;
+ regulator-boot-on;
+
+ vin-supply = <&pp3300_a>;
+ };
+
/* BOARD-SPECIFIC TOP LEVEL NODES */
backlight: backlight {
@@ -469,7 +488,6 @@ ppvar_l6c: ldo6 {
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
- pp3300_hub:
pp3300_l7c: ldo7 {
regulator-min-microvolt = <3304000>;
regulator-max-microvolt = <3304000>;
@@ -1164,6 +1182,19 @@ pinconf {
};
};
+ en_pp3300_hub: en-pp3300-hub {
+ pinmux {
+ pins = "gpio84";
+ function = "gpio";
+ };
+
+ pinconf {
+ pins = "gpio84";
+ drive-strength = <2>;
+ bias-disable;
+ };
+ };
+
fpmcu_boot0: fpmcu-boot0 {
pinmux {
pins = "gpio10";
--
2.29.1.341.ge80a0c044ae-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] arm64: dts: qcom: sc7180: Add sc7180-lazor-r2
2020-11-06 0:37 [PATCH v2 1/2] arm64: dts: qcom: sc7180: Add sc7180-lazor-r2 Matthias Kaehlcke
2020-11-06 0:37 ` [PATCH v2 2/2] arm64: dts: qcom: sc7180-trogdor: Make pp3300_a the default supply for pp3300_hub Matthias Kaehlcke
@ 2020-11-06 0:55 ` Doug Anderson
2020-11-06 2:04 ` Matthias Kaehlcke
1 sibling, 1 reply; 6+ messages in thread
From: Doug Anderson @ 2020-11-06 0:55 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: Andy Gross, Bjorn Andersson, Rob Herring, linux-arm-msm,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML
Hi,
On Thu, Nov 5, 2020 at 4:37 PM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> One important delta with respect to rev1 is a switch of the power
> supply for the onboard USB hub from 'pp3300_l7c' to 'pp3300_a' + a
> load switch. The actual regulator switch is done by the patch 'arm64:
> dts: qcom: sc7180-trogdor: Make pp3300_a the default supply for
> pp3300_hub', since it affects the entire trogdor platform. Here we
> only add the .dts files for lazor rev2 and replace the generic
> compatible entries in the rev1 .dts files.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>
> Changes in v2:
> - patch added to the series
>
> arch/arm64/boot/dts/qcom/Makefile | 3 +++
> .../dts/qcom/sc7180-trogdor-lazor-r1-kb.dts | 4 ++--
> .../dts/qcom/sc7180-trogdor-lazor-r1-lte.dts | 4 ++--
> .../boot/dts/qcom/sc7180-trogdor-lazor-r1.dts | 4 ++--
> .../dts/qcom/sc7180-trogdor-lazor-r2-kb.dts | 17 +++++++++++++++++
> .../dts/qcom/sc7180-trogdor-lazor-r2-lte.dts | 18 ++++++++++++++++++
> .../boot/dts/qcom/sc7180-trogdor-lazor-r2.dts | 15 +++++++++++++++
> 7 files changed, 59 insertions(+), 6 deletions(-)
So it's pretty unlikely that this change actually happened in "-rev2".
"-rev2" was a _very_ small batch of boards that I don't think made it
into too many people's hands. You probably want "-rev3".
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r2-kb.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r2-kb.dts
> new file mode 100644
> index 000000000000..7c3a702ef209
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r2-kb.dts
> @@ -0,0 +1,17 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Google Lazor board device tree source
> + *
> + * Copyright 2020 Google LLC.
> + */
> +
> +#include "sc7180-trogdor-lazor-r1.dts"
Should have been updated to not point to '-r1', no?
===
If you want to compare, you can also look at my (abandoned) CL:
https://crrev.com/c/2481550
...that forked out a "-rev3" to tag the WiFi slightly differently, but
we ended up abandoning it because we found a better way to handle the
WiFi stuff.
-Doug
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] arm64: dts: qcom: sc7180-trogdor: Make pp3300_a the default supply for pp3300_hub
2020-11-06 0:37 ` [PATCH v2 2/2] arm64: dts: qcom: sc7180-trogdor: Make pp3300_a the default supply for pp3300_hub Matthias Kaehlcke
@ 2020-11-06 1:05 ` Doug Anderson
2020-11-06 2:19 ` Matthias Kaehlcke
0 siblings, 1 reply; 6+ messages in thread
From: Doug Anderson @ 2020-11-06 1:05 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: Andy Gross, Bjorn Andersson, Rob Herring, linux-arm-msm,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML
Hi,
On Thu, Nov 5, 2020 at 4:37 PM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts
> index 0a281c24841c..6603f2102233 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts
> @@ -58,10 +58,23 @@ ap_ts: touchscreen@10 {
> };
> };
>
> +&pp3300_hub {
> + /* pp3300_l7c is used to power the USB hub */
> + /delete-property/regulator-always-on;
> +};
> +
> +&pp3300_l7c {
> + regulator-always-on;
Personally I always end up pairing "always-on" and "boot-on", but that
might just be superstition from many kernel versions ago when there
were weird quirks. The way you have it now you will sometimes have
"boot-on" but not "always-on". Probably what you have is fine,
though.
> +};
> +
> &sdhc_2 {
> status = "okay";
> };
>
> +&usb_hub {
> + vdd-supply = <&pp3300_l7c>;
> +};
> +
> /* PINCTRL - board-specific pinctrl */
>
> &tlmm {
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> index bf875589d364..50e733412a7f 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> @@ -174,6 +174,25 @@ pp3300_fp_tp: pp3300-fp-tp-regulator {
> vin-supply = <&pp3300_a>;
> };
>
> + pp3300_hub: pp3300-hub {
> + compatible = "regulator-fixed";
> + regulator-name = "pp3300_hub";
> +
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> +
> + gpio = <&tlmm 84 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + pinctrl-names = "default";
> + pinctrl-0 = <&en_pp3300_hub>;
> +
> + /* AP turns on with en_pp3300_hub; always on for AP */
Delete the above comment. It's obvious based on the properties in
this node. Other similar comments are useful because they describe
how the _EC_ turns on regulators and why a regulator that has an
enable still looks like an "always-on" regulator to the AP (because
it's always on whenever the AP is on).
If you want to add a comment, you could say:
/* Always on until we have a way to specify it can go off in suspend */
> @@ -469,7 +488,6 @@ ppvar_l6c: ldo6 {
> regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> };
>
> - pp3300_hub:
> pp3300_l7c: ldo7 {
> regulator-min-microvolt = <3304000>;
> regulator-max-microvolt = <3304000>;
Shouldn't you delete the "regulator-always-on;" from ldo7 since you're
adding it for all the older revs?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] arm64: dts: qcom: sc7180: Add sc7180-lazor-r2
2020-11-06 0:55 ` [PATCH v2 1/2] arm64: dts: qcom: sc7180: Add sc7180-lazor-r2 Doug Anderson
@ 2020-11-06 2:04 ` Matthias Kaehlcke
0 siblings, 0 replies; 6+ messages in thread
From: Matthias Kaehlcke @ 2020-11-06 2:04 UTC (permalink / raw)
To: Doug Anderson
Cc: Andy Gross, Bjorn Andersson, Rob Herring, linux-arm-msm,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML
On Thu, Nov 05, 2020 at 04:55:40PM -0800, Doug Anderson wrote:
> Hi,
>
> On Thu, Nov 5, 2020 at 4:37 PM Matthias Kaehlcke <mka@chromium.org> wrote:
> >
> > One important delta with respect to rev1 is a switch of the power
> > supply for the onboard USB hub from 'pp3300_l7c' to 'pp3300_a' + a
> > load switch. The actual regulator switch is done by the patch 'arm64:
> > dts: qcom: sc7180-trogdor: Make pp3300_a the default supply for
> > pp3300_hub', since it affects the entire trogdor platform. Here we
> > only add the .dts files for lazor rev2 and replace the generic
> > compatible entries in the rev1 .dts files.
> >
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> >
> > Changes in v2:
> > - patch added to the series
> >
> > arch/arm64/boot/dts/qcom/Makefile | 3 +++
> > .../dts/qcom/sc7180-trogdor-lazor-r1-kb.dts | 4 ++--
> > .../dts/qcom/sc7180-trogdor-lazor-r1-lte.dts | 4 ++--
> > .../boot/dts/qcom/sc7180-trogdor-lazor-r1.dts | 4 ++--
> > .../dts/qcom/sc7180-trogdor-lazor-r2-kb.dts | 17 +++++++++++++++++
> > .../dts/qcom/sc7180-trogdor-lazor-r2-lte.dts | 18 ++++++++++++++++++
> > .../boot/dts/qcom/sc7180-trogdor-lazor-r2.dts | 15 +++++++++++++++
> > 7 files changed, 59 insertions(+), 6 deletions(-)
>
> So it's pretty unlikely that this change actually happened in "-rev2".
> "-rev2" was a _very_ small batch of boards that I don't think made it
> into too many people's hands. You probably want "-rev3".
Ah right, now that you mention it ...
> > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r2-kb.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r2-kb.dts
> > new file mode 100644
> > index 000000000000..7c3a702ef209
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r2-kb.dts
> > @@ -0,0 +1,17 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Google Lazor board device tree source
> > + *
> > + * Copyright 2020 Google LLC.
> > + */
> > +
> > +#include "sc7180-trogdor-lazor-r1.dts"
>
> Should have been updated to not point to '-r1', no?
ack
> ===
>
> If you want to compare, you can also look at my (abandoned) CL:
> https://crrev.com/c/2481550
>
> ...that forked out a "-rev3" to tag the WiFi slightly differently, but
> we ended up abandoning it because we found a better way to handle the
> WiFi stuff.
Ok, thanks
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] arm64: dts: qcom: sc7180-trogdor: Make pp3300_a the default supply for pp3300_hub
2020-11-06 1:05 ` Doug Anderson
@ 2020-11-06 2:19 ` Matthias Kaehlcke
0 siblings, 0 replies; 6+ messages in thread
From: Matthias Kaehlcke @ 2020-11-06 2:19 UTC (permalink / raw)
To: Doug Anderson
Cc: Andy Gross, Bjorn Andersson, Rob Herring, linux-arm-msm,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML
On Thu, Nov 05, 2020 at 05:05:38PM -0800, Doug Anderson wrote:
> Hi,
>
> On Thu, Nov 5, 2020 at 4:37 PM Matthias Kaehlcke <mka@chromium.org> wrote:
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts
> > index 0a281c24841c..6603f2102233 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts
> > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts
> > @@ -58,10 +58,23 @@ ap_ts: touchscreen@10 {
> > };
> > };
> >
> > +&pp3300_hub {
> > + /* pp3300_l7c is used to power the USB hub */
> > + /delete-property/regulator-always-on;
> > +};
> > +
> > +&pp3300_l7c {
> > + regulator-always-on;
>
> Personally I always end up pairing "always-on" and "boot-on", but that
> might just be superstition from many kernel versions ago when there
> were weird quirks. The way you have it now you will sometimes have
> "boot-on" but not "always-on". Probably what you have is fine,
> though.
You are right, it makes a certain sense to have them paired, I'll change it
even though it leads to a few more entries.
> > +};
> > +
> > &sdhc_2 {
> > status = "okay";
> > };
> >
> > +&usb_hub {
> > + vdd-supply = <&pp3300_l7c>;
> > +};
> > +
> > /* PINCTRL - board-specific pinctrl */
> >
> > &tlmm {
> > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> > index bf875589d364..50e733412a7f 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> > @@ -174,6 +174,25 @@ pp3300_fp_tp: pp3300-fp-tp-regulator {
> > vin-supply = <&pp3300_a>;
> > };
> >
> > + pp3300_hub: pp3300-hub {
> > + compatible = "regulator-fixed";
> > + regulator-name = "pp3300_hub";
> > +
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > +
> > + gpio = <&tlmm 84 GPIO_ACTIVE_HIGH>;
> > + enable-active-high;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&en_pp3300_hub>;
> > +
> > + /* AP turns on with en_pp3300_hub; always on for AP */
>
> Delete the above comment. It's obvious based on the properties in
> this node. Other similar comments are useful because they describe
> how the _EC_ turns on regulators and why a regulator that has an
> enable still looks like an "always-on" regulator to the AP (because
> it's always on whenever the AP is on).
>
> If you want to add a comment, you could say:
>
> /* Always on until we have a way to specify it can go off in suspend */
ok
> > @@ -469,7 +488,6 @@ ppvar_l6c: ldo6 {
> > regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> > };
> >
> > - pp3300_hub:
> > pp3300_l7c: ldo7 {
> > regulator-min-microvolt = <3304000>;
> > regulator-max-microvolt = <3304000>;
>
> Shouldn't you delete the "regulator-always-on;" from ldo7 since you're
> adding it for all the older revs?
Indeed, that was the intention, it didn't blow up into my face during testing
since the downstream tree doesn't have it anymore.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-11-06 2:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-06 0:37 [PATCH v2 1/2] arm64: dts: qcom: sc7180: Add sc7180-lazor-r2 Matthias Kaehlcke
2020-11-06 0:37 ` [PATCH v2 2/2] arm64: dts: qcom: sc7180-trogdor: Make pp3300_a the default supply for pp3300_hub Matthias Kaehlcke
2020-11-06 1:05 ` Doug Anderson
2020-11-06 2:19 ` Matthias Kaehlcke
2020-11-06 0:55 ` [PATCH v2 1/2] arm64: dts: qcom: sc7180: Add sc7180-lazor-r2 Doug Anderson
2020-11-06 2:04 ` Matthias Kaehlcke
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).