devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Add no-esim sku for sc7180-lazor family and new board version
@ 2023-08-04  9:58 Sheng-Liang Pan
  2023-08-04  9:58 ` [PATCH v2 1/3] dt-bindings: arm: qcom: add sc7180-lazor board bindings Sheng-Liang Pan
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Sheng-Liang Pan @ 2023-08-04  9:58 UTC (permalink / raw)
  To: LKML
  Cc: dianders, Sheng-Liang Pan, Andy Gross, Bjorn Andersson,
	Conor Dooley, Konrad Dybcio, Krzysztof Kozlowski, Rob Herring,
	cros-qcom-dts-watchers, devicetree, linux-arm-msm

for audio codec ALC5682i-VS.

Changes in v2:
- add new entry rev9 with Parade bridge chip
- correct newly create dts files

Sheng-Liang Pan (3):
  dt-bindings: arm: qcom: add sc7180-lazor board bindings
  arm64: dts: qcom: sc7180: Add sku_id for lazor/limozeen
  arm64: dts: qcom: sc7180: Add board id for lazor/limozeen

 .../devicetree/bindings/arm/qcom.yaml         | 55 ++++++++++++++++++
 arch/arm64/boot/dts/qcom/Makefile             |  5 ++
 ...sc7180-trogdor-lazor-limozeen-nots-r10.dts | 40 +++++++++++++
 .../sc7180-trogdor-lazor-limozeen-nots-r9.dts |  4 +-
 .../sc7180-trogdor-lazor-limozeen-r10.dts     | 56 +++++++++++++++++++
 .../qcom/sc7180-trogdor-lazor-limozeen-r9.dts |  4 +-
 .../dts/qcom/sc7180-trogdor-lazor-r10-kb.dts  | 34 +++++++++++
 .../dts/qcom/sc7180-trogdor-lazor-r10-lte.dts | 38 +++++++++++++
 .../dts/qcom/sc7180-trogdor-lazor-r10.dts     | 30 ++++++++++
 .../dts/qcom/sc7180-trogdor-lazor-r9-kb.dts   |  4 +-
 .../dts/qcom/sc7180-trogdor-lazor-r9-lte.dts  |  4 +-
 .../boot/dts/qcom/sc7180-trogdor-lazor-r9.dts |  4 +-
 12 files changed, 268 insertions(+), 10 deletions(-)
 create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r10.dts
 create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-r10.dts
 create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10-kb.dts
 create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10-lte.dts
 create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10.dts

-- 
2.34.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/3] dt-bindings: arm: qcom: add sc7180-lazor board bindings
  2023-08-04  9:58 [PATCH v2 0/3] Add no-esim sku for sc7180-lazor family and new board version Sheng-Liang Pan
@ 2023-08-04  9:58 ` Sheng-Liang Pan
  2023-08-04 16:30   ` Doug Anderson
  2023-08-07  6:32   ` Krzysztof Kozlowski
  2023-08-04  9:58 ` [PATCH v2 2/3] arm64: dts: qcom: sc7180: Add sku_id for lazor/limozeen Sheng-Liang Pan
  2023-08-04  9:58 ` [PATCH v2 3/3] arm64: dts: qcom: sc7180: Add board id " Sheng-Liang Pan
  2 siblings, 2 replies; 15+ messages in thread
From: Sheng-Liang Pan @ 2023-08-04  9:58 UTC (permalink / raw)
  To: LKML
  Cc: dianders, Sheng-Liang Pan, Andy Gross, Bjorn Andersson,
	Conor Dooley, Konrad Dybcio, Krzysztof Kozlowski, Rob Herring,
	devicetree, linux-arm-msm

Introduce more sc7180-lazor sku and board version configuration,
add no-eSIM SKU 10 for Lazor, no-eSIM SKU 15 and 18 for Limozeen,
add new board version 10 for audio codec ALC5682i-VS.

Signed-off-by: Sheng-Liang Pan <sheng-liang.pan@quanta.corp-partner.google.com>
---

Changes in v2:
- add new entry rev9 with Parade bridge chip

 .../devicetree/bindings/arm/qcom.yaml         | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index 450f616774e0..dce7b771a280 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -470,6 +470,11 @@ properties:
           - const: google,lazor-rev8
           - const: qcom,sc7180
 
+      - description: Acer Chromebook Spin 513 Parade bridge chip (rev9)
+        items:
+          - const: google,lazor-rev9
+          - const: qcom,sc7180
+
       - description: Acer Chromebook Spin 513 (newest rev)
         items:
           - const: google,lazor
@@ -491,6 +496,11 @@ properties:
           - const: google,lazor-rev8-sku2
           - const: qcom,sc7180
 
+      - description: Acer Chromebook Spin 513 Parade bridge chip with KB Backlight (rev9)
+        items:
+          - const: google,lazor-rev9-sku2
+          - const: qcom,sc7180
+
       - description: Acer Chromebook Spin 513 with KB Backlight (newest rev)
         items:
           - const: google,lazor-sku2
@@ -512,11 +522,26 @@ properties:
           - const: google,lazor-rev8-sku0
           - const: qcom,sc7180
 
+      - description: Acer Chromebook Spin 513 Parade bridge chip with LTE (rev9)
+        items:
+          - const: google,lazor-rev9-sku0
+          - const: qcom,sc7180
+
       - description: Acer Chromebook Spin 513 with LTE (newest rev)
         items:
           - const: google,lazor-sku0
           - const: qcom,sc7180
 
+      - description: Acer Chromebook Spin 513 Parade bridge chip with LTE no-esim (rev9)
+        items:
+          - const: google,lazor-rev9-sku10
+          - const: qcom,sc7180
+
+      - description: Acer Chromebook Spin 513 with LTE no-esim (newest rev)
+        items:
+          - const: google,lazor-sku10
+          - const: qcom,sc7180
+
       - description: Acer Chromebook 511 (rev4 - rev8)
         items:
           - const: google,lazor-rev4-sku4
@@ -526,6 +551,11 @@ properties:
           - const: google,lazor-rev8-sku4
           - const: qcom,sc7180
 
+      - description: Acer Chromebook 511 Parade bridge chip (rev9)
+        items:
+          - const: google,lazor-rev9-sku4
+          - const: qcom,sc7180
+
       - description: Acer Chromebook 511 (newest rev)
         items:
           - const: google,lazor-sku4
@@ -545,11 +575,36 @@ properties:
           - const: google,lazor-rev8-sku6
           - const: qcom,sc7180
 
+      - description: Acer Chromebook 511 Parade bridge chip without Touchscreen (rev9)
+        items:
+          - const: google,lazor-rev9-sku6
+          - const: qcom,sc7180
+
       - description: Acer Chromebook 511 without Touchscreen (newest rev)
         items:
           - const: google,lazor-sku6
           - const: qcom,sc7180
 
+      - description: Acer Chromebook 511 Parade bridge chip no-esim (rev9)
+        items:
+          - const: google,lazor-rev9-sku15
+          - const: qcom,sc7180
+
+      - description: Acer Chromebook 511 no-esim (newest rev)
+        items:
+          - const: google,lazor-sku15
+          - const: qcom,sc7180
+
+      - description: Acer Chromebook 511 Parade bridge chip without Touchscreen no-esim (rev9)
+        items:
+          - const: google,lazor-rev9-sku18
+          - const: qcom,sc7180
+
+      - description: Acer Chromebook 511 without Touchscreen no-esim (newest rev)
+        items:
+          - const: google,lazor-sku18
+          - const: qcom,sc7180
+
       - description: Google Mrbland with AUO panel (rev0)
         items:
           - const: google,mrbland-rev0-sku0
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 2/3] arm64: dts: qcom: sc7180: Add sku_id for lazor/limozeen
  2023-08-04  9:58 [PATCH v2 0/3] Add no-esim sku for sc7180-lazor family and new board version Sheng-Liang Pan
  2023-08-04  9:58 ` [PATCH v2 1/3] dt-bindings: arm: qcom: add sc7180-lazor board bindings Sheng-Liang Pan
@ 2023-08-04  9:58 ` Sheng-Liang Pan
  2023-08-07  6:35   ` Krzysztof Kozlowski
  2023-08-04  9:58 ` [PATCH v2 3/3] arm64: dts: qcom: sc7180: Add board id " Sheng-Liang Pan
  2 siblings, 1 reply; 15+ messages in thread
From: Sheng-Liang Pan @ 2023-08-04  9:58 UTC (permalink / raw)
  To: LKML
  Cc: dianders, Sheng-Liang Pan, Andy Gross, Bjorn Andersson,
	Conor Dooley, Konrad Dybcio, Krzysztof Kozlowski, Rob Herring,
	cros-qcom-dts-watchers, devicetree, linux-arm-msm

SKU ID 10: Lazor LTE+Wifi, no-esim (Strapped 0 X 0)
SKU ID 15: Limozeen LTE+Wifi, TS, no esim (Strapped 1 X 0)
SKU ID 18: Limozeen LTE+Wifi, no TS, no esim (Strapped X 0 0)

Even though the "no esim" boards are strapped differently than
ones that have an esim, the esim isn't represented in the
device tree so the same device tree can be used for LTE w/ esim
and LTE w/out esim.

Signed-off-by: Sheng-Liang Pan <sheng-liang.pan@quanta.corp-partner.google.com>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
---

(no changes since v1)

 .../boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r9.dts     | 2 +-
 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-r9.dts   | 2 +-
 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-lte.dts        | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r9.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r9.dts
index 913b5fc3ba76..cef57c15b70b 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r9.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r9.dts
@@ -14,7 +14,7 @@
 
 / {
 	model = "Google Lazor Limozeen without Touchscreen (rev9+)";
-	compatible = "google,lazor-sku6", "qcom,sc7180";
+	compatible = "google,lazor-sku6", "google,lazor-sku18", "qcom,sc7180";
 };
 
 /delete-node/&ap_ts;
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-r9.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-r9.dts
index 15d77dc5f956..2038a82bc0e7 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-r9.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-r9.dts
@@ -14,7 +14,7 @@
 
 / {
 	model = "Google Lazor Limozeen (rev9+)";
-	compatible = "google,lazor-sku4", "qcom,sc7180";
+	compatible = "google,lazor-sku4", "google,lazor-sku15", "qcom,sc7180";
 };
 
 /delete-node/&ap_ts;
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-lte.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-lte.dts
index 38027f13b9d0..438ab9cd3389 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-lte.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-lte.dts
@@ -14,7 +14,7 @@
 
 / {
 	model = "Google Lazor (rev9+) with LTE";
-	compatible = "google,lazor-sku0", "qcom,sc7180";
+	compatible = "google,lazor-sku0", "google,lazor-sku10", "qcom,sc7180";
 };
 
 &ap_sar_sensor_i2c {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 3/3] arm64: dts: qcom: sc7180: Add board id for lazor/limozeen
  2023-08-04  9:58 [PATCH v2 0/3] Add no-esim sku for sc7180-lazor family and new board version Sheng-Liang Pan
  2023-08-04  9:58 ` [PATCH v2 1/3] dt-bindings: arm: qcom: add sc7180-lazor board bindings Sheng-Liang Pan
  2023-08-04  9:58 ` [PATCH v2 2/3] arm64: dts: qcom: sc7180: Add sku_id for lazor/limozeen Sheng-Liang Pan
@ 2023-08-04  9:58 ` Sheng-Liang Pan
  2023-08-04 16:30   ` Doug Anderson
  2023-08-07  6:34   ` Krzysztof Kozlowski
  2 siblings, 2 replies; 15+ messages in thread
From: Sheng-Liang Pan @ 2023-08-04  9:58 UTC (permalink / raw)
  To: LKML
  Cc: dianders, Sheng-Liang Pan, Andy Gross, Bjorn Andersson,
	Conor Dooley, Konrad Dybcio, Krzysztof Kozlowski, Rob Herring,
	cros-qcom-dts-watchers, devicetree, linux-arm-msm

add BRD_ID(0, Z, 0) = 10 for new board with ALC5682i-VS

Signed-off-by: Sheng-Liang Pan <sheng-liang.pan@quanta.corp-partner.google.com>
---

Changes in v2:
- correct newly create dts files

 arch/arm64/boot/dts/qcom/Makefile             |  5 ++
 ...sc7180-trogdor-lazor-limozeen-nots-r10.dts | 40 +++++++++++++
 .../sc7180-trogdor-lazor-limozeen-nots-r9.dts |  4 +-
 .../sc7180-trogdor-lazor-limozeen-r10.dts     | 56 +++++++++++++++++++
 .../qcom/sc7180-trogdor-lazor-limozeen-r9.dts |  4 +-
 .../dts/qcom/sc7180-trogdor-lazor-r10-kb.dts  | 34 +++++++++++
 .../dts/qcom/sc7180-trogdor-lazor-r10-lte.dts | 38 +++++++++++++
 .../dts/qcom/sc7180-trogdor-lazor-r10.dts     | 30 ++++++++++
 .../dts/qcom/sc7180-trogdor-lazor-r9-kb.dts   |  4 +-
 .../dts/qcom/sc7180-trogdor-lazor-r9-lte.dts  |  4 +-
 .../boot/dts/qcom/sc7180-trogdor-lazor-r9.dts |  4 +-
 11 files changed, 213 insertions(+), 10 deletions(-)
 create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r10.dts
 create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-r10.dts
 create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10-kb.dts
 create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10-lte.dts
 create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10.dts

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 337abc4ceb17..73e745fb1ff0 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -109,11 +109,16 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-lazor-r3-lte.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-lazor-r9.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-lazor-r9-kb.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-lazor-r9-lte.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-lazor-r10.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-lazor-r10-kb.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-lazor-r10-lte.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-lazor-limozeen-r4.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-lazor-limozeen-r9.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-lazor-limozeen-r10.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-lazor-limozeen-nots-r4.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-lazor-limozeen-nots-r5.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-lazor-limozeen-nots-r9.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-lazor-limozeen-nots-r10.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-pazquel-lte-parade.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-pazquel-lte-ti.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-pazquel-parade.dtb
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r10.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r10.dts
new file mode 100644
index 000000000000..fe634e2caf3b
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r10.dts
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Google Lazor Limozeen board device tree source
+ *
+ * Copyright 2023 Google LLC.
+ */
+
+/dts-v1/;
+
+#include "sc7180-trogdor.dtsi"
+#include "sc7180-trogdor-parade-ps8640.dtsi"
+#include "sc7180-trogdor-lazor.dtsi"
+#include "sc7180-trogdor-lte-sku.dtsi"
+
+/ {
+	model = "Google Lazor Limozeen without Touchscreen (rev10+)";
+	compatible = "google,lazor-sku6", "google,lazor-sku18", "qcom,sc7180";
+};
+
+/delete-node/&ap_ts;
+
+&panel {
+	compatible = "edp-panel";
+};
+
+&sdhc_2 {
+	status = "okay";
+};
+
+&alc5682 {
+	compatible = "realtek,rt5682s";
+	/delete-property/ VBAT-supply;
+	realtek,dmic1-clk-pin = <2>;
+	realtek,dmic-clk-rate-hz = <2048000>;
+};
+
+&sound {
+	compatible = "google,sc7180-trogdor";
+	model = "sc7180-rt5682s-max98357a-1mic";
+};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r9.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r9.dts
index cef57c15b70b..e3f1f30a7fc3 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r9.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r9.dts
@@ -13,8 +13,8 @@
 #include "sc7180-trogdor-lte-sku.dtsi"
 
 / {
-	model = "Google Lazor Limozeen without Touchscreen (rev9+)";
-	compatible = "google,lazor-sku6", "google,lazor-sku18", "qcom,sc7180";
+	model = "Google Lazor Limozeen without Touchscreen (rev9)";
+	compatible = "google,lazor-rev9-sku6", "google,lazor-rev9-sku18", "qcom,sc7180";
 };
 
 /delete-node/&ap_ts;
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-r10.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-r10.dts
new file mode 100644
index 000000000000..7b97963cce6e
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-r10.dts
@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Google Lazor Limozeen board device tree source
+ *
+ * Copyright 2023 Google LLC.
+ */
+
+/dts-v1/;
+
+#include "sc7180-trogdor.dtsi"
+#include "sc7180-trogdor-parade-ps8640.dtsi"
+#include "sc7180-trogdor-lazor.dtsi"
+#include "sc7180-trogdor-lte-sku.dtsi"
+
+/ {
+	model = "Google Lazor Limozeen (rev10+)";
+	compatible = "google,lazor-sku4", "google,lazor-sku15", "qcom,sc7180";
+};
+
+/delete-node/&ap_ts;
+
+&ap_ts_pen_1v8 {
+	ap_ts: touchscreen@10 {
+		compatible = "elan,ekth3500";
+		reg = <0x10>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&ts_int_l>, <&ts_reset_l>;
+
+		interrupt-parent = <&tlmm>;
+		interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
+
+		vcc33-supply = <&pp3300_ts>;
+
+		reset-gpios = <&tlmm 8 GPIO_ACTIVE_LOW>;
+	};
+};
+
+&panel {
+	compatible = "auo,b116xa01";
+};
+
+&sdhc_2 {
+	status = "okay";
+};
+
+&alc5682 {
+	compatible = "realtek,rt5682s";
+	/delete-property/ VBAT-supply;
+	realtek,dmic1-clk-pin = <2>;
+	realtek,dmic-clk-rate-hz = <2048000>;
+};
+
+&sound {
+	compatible = "google,sc7180-trogdor";
+	model = "sc7180-rt5682s-max98357a-1mic";
+};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-r9.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-r9.dts
index 2038a82bc0e7..3b6053f3e62b 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-r9.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-r9.dts
@@ -13,8 +13,8 @@
 #include "sc7180-trogdor-lte-sku.dtsi"
 
 / {
-	model = "Google Lazor Limozeen (rev9+)";
-	compatible = "google,lazor-sku4", "google,lazor-sku15", "qcom,sc7180";
+	model = "Google Lazor Limozeen (rev9)";
+	compatible = "google,lazor-rev9-sku4", "google,lazor-rev9-sku15", "qcom,sc7180";
 };
 
 /delete-node/&ap_ts;
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10-kb.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10-kb.dts
new file mode 100644
index 000000000000..6f98f61ac567
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10-kb.dts
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Google Lazor board device tree source
+ *
+ * Copyright 2023 Google LLC.
+ */
+
+/dts-v1/;
+
+#include "sc7180-trogdor.dtsi"
+#include "sc7180-trogdor-parade-ps8640.dtsi"
+#include "sc7180-trogdor-lazor.dtsi"
+#include "sc7180-lite.dtsi"
+
+/ {
+	model = "Google Lazor (rev10+) with KB Backlight";
+	compatible = "google,lazor-sku2", "qcom,sc7180";
+};
+
+&keyboard_backlight {
+	status = "okay";
+};
+
+&alc5682 {
+	compatible = "realtek,rt5682s";
+	/delete-property/ VBAT-supply;
+	realtek,dmic1-clk-pin = <2>;
+	realtek,dmic-clk-rate-hz = <2048000>;
+};
+
+&sound {
+	compatible = "google,sc7180-trogdor";
+	model = "sc7180-rt5682s-max98357a-1mic";
+};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10-lte.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10-lte.dts
new file mode 100644
index 000000000000..03d0d0af367c
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10-lte.dts
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Google Lazor board device tree source
+ *
+ * Copyright 2023 Google LLC.
+ */
+
+/dts-v1/;
+
+#include "sc7180-trogdor.dtsi"
+#include "sc7180-trogdor-parade-ps8640.dtsi"
+#include "sc7180-trogdor-lazor.dtsi"
+#include "sc7180-trogdor-lte-sku.dtsi"
+
+/ {
+	model = "Google Lazor (rev10+) with LTE";
+	compatible = "google,lazor-sku0", "google,lazor-sku10", "qcom,sc7180";
+};
+
+&ap_sar_sensor_i2c {
+	status = "okay";
+};
+
+&keyboard_backlight {
+	status = "okay";
+};
+
+&alc5682 {
+	compatible = "realtek,rt5682s";
+	/delete-property/ VBAT-supply;
+	realtek,dmic1-clk-pin = <2>;
+	realtek,dmic-clk-rate-hz = <2048000>;
+};
+
+&sound {
+	compatible = "google,sc7180-trogdor";
+	model = "sc7180-rt5682s-max98357a-1mic";
+};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10.dts
new file mode 100644
index 000000000000..5a58e94c228e
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10.dts
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Google Lazor board device tree source
+ *
+ * Copyright 2023 Google LLC.
+ */
+
+/dts-v1/;
+
+#include "sc7180-trogdor.dtsi"
+#include "sc7180-trogdor-parade-ps8640.dtsi"
+#include "sc7180-trogdor-lazor.dtsi"
+#include "sc7180-lite.dtsi"
+
+/ {
+	model = "Google Lazor (rev10+)";
+	compatible = "google,lazor", "qcom,sc7180";
+};
+
+&alc5682 {
+	compatible = "realtek,rt5682s";
+	/delete-property/ VBAT-supply;
+	realtek,dmic1-clk-pin = <2>;
+	realtek,dmic-clk-rate-hz = <2048000>;
+};
+
+&sound {
+	compatible = "google,sc7180-trogdor";
+	model = "sc7180-rt5682s-max98357a-1mic";
+};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-kb.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-kb.dts
index 960f7b7ce094..f74a1985cac6 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-kb.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-kb.dts
@@ -13,8 +13,8 @@
 #include "sc7180-lite.dtsi"
 
 / {
-	model = "Google Lazor (rev9+) with KB Backlight";
-	compatible = "google,lazor-sku2", "qcom,sc7180";
+	model = "Google Lazor (rev9) with KB Backlight";
+	compatible = "google,lazor-rev9-sku2", "qcom,sc7180";
 };
 
 &keyboard_backlight {
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-lte.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-lte.dts
index 438ab9cd3389..15dcf95da311 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-lte.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-lte.dts
@@ -13,8 +13,8 @@
 #include "sc7180-trogdor-lte-sku.dtsi"
 
 / {
-	model = "Google Lazor (rev9+) with LTE";
-	compatible = "google,lazor-sku0", "google,lazor-sku10", "qcom,sc7180";
+	model = "Google Lazor (rev9) with LTE";
+	compatible = "google,lazor-rev9-sku0", "google,lazor-rev9-sku10", "qcom,sc7180";
 };
 
 &ap_sar_sensor_i2c {
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9.dts
index 56dd222650d3..44f61bc56f81 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9.dts
@@ -13,6 +13,6 @@
 #include "sc7180-lite.dtsi"
 
 / {
-	model = "Google Lazor (rev9+)";
-	compatible = "google,lazor", "qcom,sc7180";
+	model = "Google Lazor (rev9)";
+	compatible = "google,lazor-rev9", "qcom,sc7180";
 };
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: qcom: sc7180: Add board id for lazor/limozeen
  2023-08-04  9:58 ` [PATCH v2 3/3] arm64: dts: qcom: sc7180: Add board id " Sheng-Liang Pan
@ 2023-08-04 16:30   ` Doug Anderson
  2023-08-07  6:34   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 15+ messages in thread
From: Doug Anderson @ 2023-08-04 16:30 UTC (permalink / raw)
  To: Sheng-Liang Pan
  Cc: LKML, Andy Gross, Bjorn Andersson, Conor Dooley, Konrad Dybcio,
	Krzysztof Kozlowski, Rob Herring, cros-qcom-dts-watchers,
	devicetree, linux-arm-msm

Hi,

On Fri, Aug 4, 2023 at 2:58 AM Sheng-Liang Pan
<sheng-liang.pan@quanta.corp-partner.google.com> wrote:
>
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r10.dts
> @@ -0,0 +1,40 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Google Lazor Limozeen board device tree source
> + *
> + * Copyright 2023 Google LLC.
> + */
> +
> +/dts-v1/;
> +
> +#include "sc7180-trogdor.dtsi"
> +#include "sc7180-trogdor-parade-ps8640.dtsi"
> +#include "sc7180-trogdor-lazor.dtsi"
> +#include "sc7180-trogdor-lte-sku.dtsi"
> +
> +/ {
> +       model = "Google Lazor Limozeen without Touchscreen (rev10+)";
> +       compatible = "google,lazor-sku6", "google,lazor-sku18", "qcom,sc7180";
> +};
> +
> +/delete-node/&ap_ts;
> +
> +&panel {
> +       compatible = "edp-panel";
> +};
> +
> +&sdhc_2 {
> +       status = "okay";
> +};
> +
> +&alc5682 {
> +       compatible = "realtek,rt5682s";
> +       /delete-property/ VBAT-supply;
> +       realtek,dmic1-clk-pin = <2>;
> +       realtek,dmic-clk-rate-hz = <2048000>;
> +};
> +
> +&sound {
> +       compatible = "google,sc7180-trogdor";
> +       model = "sc7180-rt5682s-max98357a-1mic";
> +};

The sort ordering of the nodes above is still wrong. It should be
alphabetical. AKA "alc5682", "panel", "sdhc_2", "sound".

Once that's fixed, feel free to include:

Reviewed-by: Douglas Anderson <dianders@chromium.org>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/3] dt-bindings: arm: qcom: add sc7180-lazor board bindings
  2023-08-04  9:58 ` [PATCH v2 1/3] dt-bindings: arm: qcom: add sc7180-lazor board bindings Sheng-Liang Pan
@ 2023-08-04 16:30   ` Doug Anderson
  2023-08-07 13:37     ` Konrad Dybcio
  2023-08-07  6:32   ` Krzysztof Kozlowski
  1 sibling, 1 reply; 15+ messages in thread
From: Doug Anderson @ 2023-08-04 16:30 UTC (permalink / raw)
  To: Sheng-Liang Pan
  Cc: LKML, Andy Gross, Bjorn Andersson, Conor Dooley, Konrad Dybcio,
	Krzysztof Kozlowski, Rob Herring, devicetree, linux-arm-msm

Hi,

On Fri, Aug 4, 2023 at 2:58 AM Sheng-Liang Pan
<sheng-liang.pan@quanta.corp-partner.google.com> wrote:
>
> Introduce more sc7180-lazor sku and board version configuration,
> add no-eSIM SKU 10 for Lazor, no-eSIM SKU 15 and 18 for Limozeen,
> add new board version 10 for audio codec ALC5682i-VS.
>
> Signed-off-by: Sheng-Liang Pan <sheng-liang.pan@quanta.corp-partner.google.com>
> ---
>
> Changes in v2:
> - add new entry rev9 with Parade bridge chip
>
>  .../devicetree/bindings/arm/qcom.yaml         | 55 +++++++++++++++++++
>  1 file changed, 55 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
> index 450f616774e0..dce7b771a280 100644
> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
> @@ -470,6 +470,11 @@ properties:
>            - const: google,lazor-rev8
>            - const: qcom,sc7180
>
> +      - description: Acer Chromebook Spin 513 Parade bridge chip (rev9)

You probably didn't need to include "Parade bridge chip" in the
description since that's implied by "rev9"


> @@ -512,11 +522,26 @@ properties:
>            - const: google,lazor-rev8-sku0
>            - const: qcom,sc7180
>
> +      - description: Acer Chromebook Spin 513 Parade bridge chip with LTE (rev9)
> +        items:
> +          - const: google,lazor-rev9-sku0
> +          - const: qcom,sc7180
> +
>        - description: Acer Chromebook Spin 513 with LTE (newest rev)
>          items:
>            - const: google,lazor-sku0
>            - const: qcom,sc7180
>
> +      - description: Acer Chromebook Spin 513 Parade bridge chip with LTE no-esim (rev9)
> +        items:
> +          - const: google,lazor-rev9-sku10
> +          - const: qcom,sc7180

The no-eSIM and normal LTE should be combined into one, just like they
are in your device tree. If you look at patch #3, you can see
"sc7180-trogdor-lazor-r9-lte.dts" contains:

compatible = "google,lazor-rev9-sku0", "google,lazor-rev9-sku10", "qcom,sc7180";

You need to have a single entry here that matches that. That means one
entry that has both rev9-sku0 and rev9-sku10.

You should be running "make dtbs_check" at the end of your series. As
Krzysztof would say, (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions)


-Doug

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/3] dt-bindings: arm: qcom: add sc7180-lazor board bindings
  2023-08-04  9:58 ` [PATCH v2 1/3] dt-bindings: arm: qcom: add sc7180-lazor board bindings Sheng-Liang Pan
  2023-08-04 16:30   ` Doug Anderson
@ 2023-08-07  6:32   ` Krzysztof Kozlowski
  2023-08-15 20:41     ` Doug Anderson
  1 sibling, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2023-08-07  6:32 UTC (permalink / raw)
  To: Sheng-Liang Pan, LKML
  Cc: dianders, Andy Gross, Bjorn Andersson, Conor Dooley,
	Konrad Dybcio, Krzysztof Kozlowski, Rob Herring, devicetree,
	linux-arm-msm

On 04/08/2023 11:58, Sheng-Liang Pan wrote:
> Introduce more sc7180-lazor sku and board version configuration,
> add no-eSIM SKU 10 for Lazor, no-eSIM SKU 15 and 18 for Limozeen,
> add new board version 10 for audio codec ALC5682i-VS.
> 
> Signed-off-by: Sheng-Liang Pan <sheng-liang.pan@quanta.corp-partner.google.com>
> ---
> 
> Changes in v2:
> - add new entry rev9 with Parade bridge chip
> 
>  .../devicetree/bindings/arm/qcom.yaml         | 55 +++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
> index 450f616774e0..dce7b771a280 100644
> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
> @@ -470,6 +470,11 @@ properties:
>            - const: google,lazor-rev8
>            - const: qcom,sc7180
>  
> +      - description: Acer Chromebook Spin 513 Parade bridge chip (rev9)
> +        items:
> +          - const: google,lazor-rev9
> +          - const: qcom,sc7180
> +
>        - description: Acer Chromebook Spin 513 (newest rev)
>          items:
>            - const: google,lazor
> @@ -491,6 +496,11 @@ properties:
>            - const: google,lazor-rev8-sku2
>            - const: qcom,sc7180
>  
> +      - description: Acer Chromebook Spin 513 Parade bridge chip with KB Backlight (rev9)
> +        items:
> +          - const: google,lazor-rev9-sku2
> +          - const: qcom,sc7180
> +
>        - description: Acer Chromebook Spin 513 with KB Backlight (newest rev)
>          items:
>            - const: google,lazor-sku2
> @@ -512,11 +522,26 @@ properties:
>            - const: google,lazor-rev8-sku0
>            - const: qcom,sc7180
>  
> +      - description: Acer Chromebook Spin 513 Parade bridge chip with LTE (rev9)
> +        items:
> +          - const: google,lazor-rev9-sku0
> +          - const: qcom,sc7180
> +
>        - description: Acer Chromebook Spin 513 with LTE (newest rev)
>          items:
>            - const: google,lazor-sku0
>            - const: qcom,sc7180
>  
> +      - description: Acer Chromebook Spin 513 Parade bridge chip with LTE no-esim (rev9)
> +        items:
> +          - const: google,lazor-rev9-sku10
> +          - const: qcom,sc7180
> +
> +      - description: Acer Chromebook Spin 513 with LTE no-esim (newest rev)
> +        items:
> +          - const: google,lazor-sku10
> +          - const: qcom,sc7180
> +
>        - description: Acer Chromebook 511 (rev4 - rev8)
>          items:
>            - const: google,lazor-rev4-sku4
> @@ -526,6 +551,11 @@ properties:
>            - const: google,lazor-rev8-sku4
>            - const: qcom,sc7180
>  
> +      - description: Acer Chromebook 511 Parade bridge chip (rev9)
> +        items:
> +          - const: google,lazor-rev9-sku4
> +          - const: qcom,sc7180
> +
>        - description: Acer Chromebook 511 (newest rev)
>          items:
>            - const: google,lazor-sku4
> @@ -545,11 +575,36 @@ properties:
>            - const: google,lazor-rev8-sku6
>            - const: qcom,sc7180
>  
> +      - description: Acer Chromebook 511 Parade bridge chip without Touchscreen (rev9)
> +        items:
> +          - const: google,lazor-rev9-sku6
> +          - const: qcom,sc7180
> +
>        - description: Acer Chromebook 511 without Touchscreen (newest rev)
>          items:
>            - const: google,lazor-sku6
>            - const: qcom,sc7180
>  
> +      - description: Acer Chromebook 511 Parade bridge chip no-esim (rev9)
> +        items:
> +          - const: google,lazor-rev9-sku15
> +          - const: qcom,sc7180
> +
> +      - description: Acer Chromebook 511 no-esim (newest rev)
> +        items:
> +          - const: google,lazor-sku15
> +          - const: qcom,sc7180
> +
> +      - description: Acer Chromebook 511 Parade bridge chip without Touchscreen no-esim (rev9)
> +        items:
> +          - const: google,lazor-rev9-sku18
> +          - const: qcom,sc7180
> +
> +      - description: Acer Chromebook 511 without Touchscreen no-esim (newest rev)
> +        items:
> +          - const: google,lazor-sku18

All of these entries (existing and new) should be just one entry with:
 - enum:
     - ....
     - ....
 - const: qcom,sc7180

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: qcom: sc7180: Add board id for lazor/limozeen
  2023-08-04  9:58 ` [PATCH v2 3/3] arm64: dts: qcom: sc7180: Add board id " Sheng-Liang Pan
  2023-08-04 16:30   ` Doug Anderson
@ 2023-08-07  6:34   ` Krzysztof Kozlowski
  2023-08-15 21:10     ` Doug Anderson
  1 sibling, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2023-08-07  6:34 UTC (permalink / raw)
  To: Sheng-Liang Pan, LKML
  Cc: dianders, Andy Gross, Bjorn Andersson, Conor Dooley,
	Konrad Dybcio, Krzysztof Kozlowski, Rob Herring,
	cros-qcom-dts-watchers, devicetree, linux-arm-msm

On 04/08/2023 11:58, Sheng-Liang Pan wrote:
> add BRD_ID(0, Z, 0) = 10 for new board with ALC5682i-VS
> 
> Signed-off-by: Sheng-Liang Pan <sheng-liang.pan@quanta.corp-partner.google.com>
> ---
> 
> Changes in v2:
> - correct newly create dts files
> 


> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10.dts
> new file mode 100644
> index 000000000000..5a58e94c228e
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10.dts
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Google Lazor board device tree source
> + *
> + * Copyright 2023 Google LLC.
> + */
> +
> +/dts-v1/;
> +
> +#include "sc7180-trogdor.dtsi"
> +#include "sc7180-trogdor-parade-ps8640.dtsi"
> +#include "sc7180-trogdor-lazor.dtsi"
> +#include "sc7180-lite.dtsi"
> +
> +/ {
> +	model = "Google Lazor (rev10+)";
> +	compatible = "google,lazor", "qcom,sc7180";
> +};
> +
> +&alc5682 {
> +	compatible = "realtek,rt5682s";
> +	/delete-property/ VBAT-supply;

No, don't delete properties. First of all, why you do not have this
supply here? I doubt it... Especially that this DTS has vbat-supply
regulator!

Second, define the properties where applicable instead.

> +	realtek,dmic1-clk-pin = <2>;
> +	realtek,dmic-clk-rate-hz = <2048000>;
> +};
> +
> +&sound {
> +	compatible = "google,sc7180-trogdor";
> +	model = "sc7180-rt5682s-max98357a-1mic";
> +};
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-kb.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-kb.dts
> index 960f7b7ce094..f74a1985cac6 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-kb.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-kb.dts
> @@ -13,8 +13,8 @@
>  #include "sc7180-lite.dtsi"
>  
>  / {
> -	model = "Google Lazor (rev9+) with KB Backlight";
> -	compatible = "google,lazor-sku2", "qcom,sc7180";
> +	model = "Google Lazor (rev9) with KB Backlight";
> +	compatible = "google,lazor-rev9-sku2", "qcom,sc7180";
>  };
>  
>  &keyboard_backlight {
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-lte.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-lte.dts
> index 438ab9cd3389..15dcf95da311 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-lte.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-lte.dts
> @@ -13,8 +13,8 @@
>  #include "sc7180-trogdor-lte-sku.dtsi"
>  
>  / {
> -	model = "Google Lazor (rev9+) with LTE";
> -	compatible = "google,lazor-sku0", "google,lazor-sku10", "qcom,sc7180";
> +	model = "Google Lazor (rev9) with LTE";
> +	compatible = "google,lazor-rev9-sku0", "google,lazor-rev9-sku10", "qcom,sc7180";


This is not what your binding is saying. You just introduced new
dtbs_check warnings :(

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).


Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/3] arm64: dts: qcom: sc7180: Add sku_id for lazor/limozeen
  2023-08-04  9:58 ` [PATCH v2 2/3] arm64: dts: qcom: sc7180: Add sku_id for lazor/limozeen Sheng-Liang Pan
@ 2023-08-07  6:35   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2023-08-07  6:35 UTC (permalink / raw)
  To: Sheng-Liang Pan, LKML
  Cc: dianders, Andy Gross, Bjorn Andersson, Conor Dooley,
	Konrad Dybcio, Krzysztof Kozlowski, Rob Herring,
	cros-qcom-dts-watchers, devicetree, linux-arm-msm

On 04/08/2023 11:58, Sheng-Liang Pan wrote:
> SKU ID 10: Lazor LTE+Wifi, no-esim (Strapped 0 X 0)
> SKU ID 15: Limozeen LTE+Wifi, TS, no esim (Strapped 1 X 0)
> SKU ID 18: Limozeen LTE+Wifi, no TS, no esim (Strapped X 0 0)
> 
> Even though the "no esim" boards are strapped differently than
> ones that have an esim, the esim isn't represented in the
> device tree so the same device tree can be used for LTE w/ esim
> and LTE w/out esim.
> 
> Signed-off-by: Sheng-Liang Pan <sheng-liang.pan@quanta.corp-partner.google.com>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> ---
> 
> (no changes since v1)
> 
>  .../boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r9.dts     | 2 +-
>  arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-r9.dts   | 2 +-
>  arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-lte.dts        | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r9.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r9.dts
> index 913b5fc3ba76..cef57c15b70b 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r9.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r9.dts
> @@ -14,7 +14,7 @@
>  
>  / {
>  	model = "Google Lazor Limozeen without Touchscreen (rev9+)";
> -	compatible = "google,lazor-sku6", "qcom,sc7180";
> +	compatible = "google,lazor-sku6", "google,lazor-sku18", "qcom,sc7180";

NAK, you really did not test it. At all.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/3] dt-bindings: arm: qcom: add sc7180-lazor board bindings
  2023-08-04 16:30   ` Doug Anderson
@ 2023-08-07 13:37     ` Konrad Dybcio
  0 siblings, 0 replies; 15+ messages in thread
From: Konrad Dybcio @ 2023-08-07 13:37 UTC (permalink / raw)
  To: Doug Anderson, Sheng-Liang Pan
  Cc: LKML, Andy Gross, Bjorn Andersson, Conor Dooley,
	Krzysztof Kozlowski, Rob Herring, devicetree, linux-arm-msm

On 4.08.2023 18:30, Doug Anderson wrote:
> Hi,
> 
> On Fri, Aug 4, 2023 at 2:58 AM Sheng-Liang Pan
> <sheng-liang.pan@quanta.corp-partner.google.com> wrote:
>>
>> Introduce more sc7180-lazor sku and board version configuration,
>> add no-eSIM SKU 10 for Lazor, no-eSIM SKU 15 and 18 for Limozeen,
>> add new board version 10 for audio codec ALC5682i-VS.
>>
>> Signed-off-by: Sheng-Liang Pan <sheng-liang.pan@quanta.corp-partner.google.com>
>> ---
[...]

> 
> You should be running "make dtbs_check" at the end of your series. As
Ideally on each patch separately, with:

# or you can set git config alias.last describe --abbrev=0, thx Bjorn
git rebase -i $(git describe --abbrev=0) 
replace 'pick' -> 'edit'
while (rebase is ongoing, can probably check retval of git rebase --continue)
do "make ... dtbs check something.."

as otherwise you may slip in a breakage inbetween that you fix later on

Konrad

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/3] dt-bindings: arm: qcom: add sc7180-lazor board bindings
  2023-08-07  6:32   ` Krzysztof Kozlowski
@ 2023-08-15 20:41     ` Doug Anderson
  2023-08-15 20:46       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 15+ messages in thread
From: Doug Anderson @ 2023-08-15 20:41 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Sheng-Liang Pan, LKML, Andy Gross, Bjorn Andersson, Conor Dooley,
	Konrad Dybcio, Krzysztof Kozlowski, Rob Herring, devicetree,
	linux-arm-msm

Hi,

On Sun, Aug 6, 2023 at 11:32 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 04/08/2023 11:58, Sheng-Liang Pan wrote:
> > Introduce more sc7180-lazor sku and board version configuration,
> > add no-eSIM SKU 10 for Lazor, no-eSIM SKU 15 and 18 for Limozeen,
> > add new board version 10 for audio codec ALC5682i-VS.
> >
> > Signed-off-by: Sheng-Liang Pan <sheng-liang.pan@quanta.corp-partner.google.com>
> > ---
> >
> > Changes in v2:
> > - add new entry rev9 with Parade bridge chip
> >
> >  .../devicetree/bindings/arm/qcom.yaml         | 55 +++++++++++++++++++
> >  1 file changed, 55 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
> > index 450f616774e0..dce7b771a280 100644
> > --- a/Documentation/devicetree/bindings/arm/qcom.yaml
> > +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
> > @@ -470,6 +470,11 @@ properties:
> >            - const: google,lazor-rev8
> >            - const: qcom,sc7180
> >
> > +      - description: Acer Chromebook Spin 513 Parade bridge chip (rev9)
> > +        items:
> > +          - const: google,lazor-rev9
> > +          - const: qcom,sc7180
> > +
> >        - description: Acer Chromebook Spin 513 (newest rev)
> >          items:
> >            - const: google,lazor
> > @@ -491,6 +496,11 @@ properties:
> >            - const: google,lazor-rev8-sku2
> >            - const: qcom,sc7180
> >
> > +      - description: Acer Chromebook Spin 513 Parade bridge chip with KB Backlight (rev9)
> > +        items:
> > +          - const: google,lazor-rev9-sku2
> > +          - const: qcom,sc7180
> > +
> >        - description: Acer Chromebook Spin 513 with KB Backlight (newest rev)
> >          items:
> >            - const: google,lazor-sku2
> > @@ -512,11 +522,26 @@ properties:
> >            - const: google,lazor-rev8-sku0
> >            - const: qcom,sc7180
> >
> > +      - description: Acer Chromebook Spin 513 Parade bridge chip with LTE (rev9)
> > +        items:
> > +          - const: google,lazor-rev9-sku0
> > +          - const: qcom,sc7180
> > +
> >        - description: Acer Chromebook Spin 513 with LTE (newest rev)
> >          items:
> >            - const: google,lazor-sku0
> >            - const: qcom,sc7180
> >
> > +      - description: Acer Chromebook Spin 513 Parade bridge chip with LTE no-esim (rev9)
> > +        items:
> > +          - const: google,lazor-rev9-sku10
> > +          - const: qcom,sc7180
> > +
> > +      - description: Acer Chromebook Spin 513 with LTE no-esim (newest rev)
> > +        items:
> > +          - const: google,lazor-sku10
> > +          - const: qcom,sc7180
> > +
> >        - description: Acer Chromebook 511 (rev4 - rev8)
> >          items:
> >            - const: google,lazor-rev4-sku4
> > @@ -526,6 +551,11 @@ properties:
> >            - const: google,lazor-rev8-sku4
> >            - const: qcom,sc7180
> >
> > +      - description: Acer Chromebook 511 Parade bridge chip (rev9)
> > +        items:
> > +          - const: google,lazor-rev9-sku4
> > +          - const: qcom,sc7180
> > +
> >        - description: Acer Chromebook 511 (newest rev)
> >          items:
> >            - const: google,lazor-sku4
> > @@ -545,11 +575,36 @@ properties:
> >            - const: google,lazor-rev8-sku6
> >            - const: qcom,sc7180
> >
> > +      - description: Acer Chromebook 511 Parade bridge chip without Touchscreen (rev9)
> > +        items:
> > +          - const: google,lazor-rev9-sku6
> > +          - const: qcom,sc7180
> > +
> >        - description: Acer Chromebook 511 without Touchscreen (newest rev)
> >          items:
> >            - const: google,lazor-sku6
> >            - const: qcom,sc7180
> >
> > +      - description: Acer Chromebook 511 Parade bridge chip no-esim (rev9)
> > +        items:
> > +          - const: google,lazor-rev9-sku15
> > +          - const: qcom,sc7180
> > +
> > +      - description: Acer Chromebook 511 no-esim (newest rev)
> > +        items:
> > +          - const: google,lazor-sku15
> > +          - const: qcom,sc7180
> > +
> > +      - description: Acer Chromebook 511 Parade bridge chip without Touchscreen no-esim (rev9)
> > +        items:
> > +          - const: google,lazor-rev9-sku18
> > +          - const: qcom,sc7180
> > +
> > +      - description: Acer Chromebook 511 without Touchscreen no-esim (newest rev)
> > +        items:
> > +          - const: google,lazor-sku18
>
> All of these entries (existing and new) should be just one entry with:
>  - enum:
>      - ....
>      - ....
>  - const: qcom,sc7180

I believe we've had this discussion before. At least twice. Here's a
link to the last time you said "Ah, OK, I guess this is fine".

https://lore.kernel.org/r/d3d4d90b-85b5-5ad9-78e6-5a074c21af4f@linaro.org

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/3] dt-bindings: arm: qcom: add sc7180-lazor board bindings
  2023-08-15 20:41     ` Doug Anderson
@ 2023-08-15 20:46       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2023-08-15 20:46 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Sheng-Liang Pan, LKML, Andy Gross, Bjorn Andersson, Conor Dooley,
	Konrad Dybcio, Krzysztof Kozlowski, Rob Herring, devicetree,
	linux-arm-msm

On 15/08/2023 22:41, Doug Anderson wrote:
> Hi,
> 
> On Sun, Aug 6, 2023 at 11:32 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 04/08/2023 11:58, Sheng-Liang Pan wrote:
>>> Introduce more sc7180-lazor sku and board version configuration,
>>> add no-eSIM SKU 10 for Lazor, no-eSIM SKU 15 and 18 for Limozeen,
>>> add new board version 10 for audio codec ALC5682i-VS.
>>>
>>> Signed-off-by: Sheng-Liang Pan <sheng-liang.pan@quanta.corp-partner.google.com>
>>> ---
>>>
>>> Changes in v2:
>>> - add new entry rev9 with Parade bridge chip
>>>
>>>  .../devicetree/bindings/arm/qcom.yaml         | 55 +++++++++++++++++++
>>>  1 file changed, 55 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
>>> index 450f616774e0..dce7b771a280 100644
>>> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
>>> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
>>> @@ -470,6 +470,11 @@ properties:
>>>            - const: google,lazor-rev8
>>>            - const: qcom,sc7180
>>>
>>> +      - description: Acer Chromebook Spin 513 Parade bridge chip (rev9)
>>> +        items:
>>> +          - const: google,lazor-rev9
>>> +          - const: qcom,sc7180
>>> +
>>>        - description: Acer Chromebook Spin 513 (newest rev)
>>>          items:
>>>            - const: google,lazor
>>> @@ -491,6 +496,11 @@ properties:
>>>            - const: google,lazor-rev8-sku2
>>>            - const: qcom,sc7180
>>>
>>> +      - description: Acer Chromebook Spin 513 Parade bridge chip with KB Backlight (rev9)
>>> +        items:
>>> +          - const: google,lazor-rev9-sku2
>>> +          - const: qcom,sc7180
>>> +
>>>        - description: Acer Chromebook Spin 513 with KB Backlight (newest rev)
>>>          items:
>>>            - const: google,lazor-sku2
>>> @@ -512,11 +522,26 @@ properties:
>>>            - const: google,lazor-rev8-sku0
>>>            - const: qcom,sc7180
>>>
>>> +      - description: Acer Chromebook Spin 513 Parade bridge chip with LTE (rev9)
>>> +        items:
>>> +          - const: google,lazor-rev9-sku0
>>> +          - const: qcom,sc7180
>>> +
>>>        - description: Acer Chromebook Spin 513 with LTE (newest rev)
>>>          items:
>>>            - const: google,lazor-sku0
>>>            - const: qcom,sc7180
>>>
>>> +      - description: Acer Chromebook Spin 513 Parade bridge chip with LTE no-esim (rev9)
>>> +        items:
>>> +          - const: google,lazor-rev9-sku10
>>> +          - const: qcom,sc7180
>>> +
>>> +      - description: Acer Chromebook Spin 513 with LTE no-esim (newest rev)
>>> +        items:
>>> +          - const: google,lazor-sku10
>>> +          - const: qcom,sc7180
>>> +
>>>        - description: Acer Chromebook 511 (rev4 - rev8)
>>>          items:
>>>            - const: google,lazor-rev4-sku4
>>> @@ -526,6 +551,11 @@ properties:
>>>            - const: google,lazor-rev8-sku4
>>>            - const: qcom,sc7180
>>>
>>> +      - description: Acer Chromebook 511 Parade bridge chip (rev9)
>>> +        items:
>>> +          - const: google,lazor-rev9-sku4
>>> +          - const: qcom,sc7180
>>> +
>>>        - description: Acer Chromebook 511 (newest rev)
>>>          items:
>>>            - const: google,lazor-sku4
>>> @@ -545,11 +575,36 @@ properties:
>>>            - const: google,lazor-rev8-sku6
>>>            - const: qcom,sc7180
>>>
>>> +      - description: Acer Chromebook 511 Parade bridge chip without Touchscreen (rev9)
>>> +        items:
>>> +          - const: google,lazor-rev9-sku6
>>> +          - const: qcom,sc7180
>>> +
>>>        - description: Acer Chromebook 511 without Touchscreen (newest rev)
>>>          items:
>>>            - const: google,lazor-sku6
>>>            - const: qcom,sc7180
>>>
>>> +      - description: Acer Chromebook 511 Parade bridge chip no-esim (rev9)
>>> +        items:
>>> +          - const: google,lazor-rev9-sku15
>>> +          - const: qcom,sc7180
>>> +
>>> +      - description: Acer Chromebook 511 no-esim (newest rev)
>>> +        items:
>>> +          - const: google,lazor-sku15
>>> +          - const: qcom,sc7180
>>> +
>>> +      - description: Acer Chromebook 511 Parade bridge chip without Touchscreen no-esim (rev9)
>>> +        items:
>>> +          - const: google,lazor-rev9-sku18
>>> +          - const: qcom,sc7180
>>> +
>>> +      - description: Acer Chromebook 511 without Touchscreen no-esim (newest rev)
>>> +        items:
>>> +          - const: google,lazor-sku18
>>
>> All of these entries (existing and new) should be just one entry with:
>>  - enum:
>>      - ....
>>      - ....
>>  - const: qcom,sc7180
> 
> I believe we've had this discussion before. At least twice. Here's a
> link to the last time you said "Ah, OK, I guess this is fine".
> 
> https://lore.kernel.org/r/d3d4d90b-85b5-5ad9-78e6-5a074c21af4f@linaro.org

Current solutions brings descriptions, so it has some benefits... I
guess this is fine for third time. Maybe this time I will remember...
Although I keep coming with several same questions to all SoC
DTS/bindings which do things differently than common case. I will be
coming because that's the price of doing things awkwardly or differently
than everyone else, e.g. overriding by full DT path like in Tegra.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: qcom: sc7180: Add board id for lazor/limozeen
  2023-08-07  6:34   ` Krzysztof Kozlowski
@ 2023-08-15 21:10     ` Doug Anderson
  2023-08-15 21:22       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 15+ messages in thread
From: Doug Anderson @ 2023-08-15 21:10 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Sheng-Liang Pan, LKML, Andy Gross, Bjorn Andersson, Conor Dooley,
	Konrad Dybcio, Krzysztof Kozlowski, Rob Herring,
	cros-qcom-dts-watchers, devicetree, linux-arm-msm

Hi,

On Sun, Aug 6, 2023 at 11:34 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 04/08/2023 11:58, Sheng-Liang Pan wrote:
> > add BRD_ID(0, Z, 0) = 10 for new board with ALC5682i-VS
> >
> > Signed-off-by: Sheng-Liang Pan <sheng-liang.pan@quanta.corp-partner.google.com>
> > ---
> >
> > Changes in v2:
> > - correct newly create dts files
> >
>
>
> > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10.dts
> > new file mode 100644
> > index 000000000000..5a58e94c228e
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10.dts
> > @@ -0,0 +1,30 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Google Lazor board device tree source
> > + *
> > + * Copyright 2023 Google LLC.
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include "sc7180-trogdor.dtsi"
> > +#include "sc7180-trogdor-parade-ps8640.dtsi"
> > +#include "sc7180-trogdor-lazor.dtsi"
> > +#include "sc7180-lite.dtsi"
> > +
> > +/ {
> > +     model = "Google Lazor (rev10+)";
> > +     compatible = "google,lazor", "qcom,sc7180";
> > +};
> > +
> > +&alc5682 {
> > +     compatible = "realtek,rt5682s";
> > +     /delete-property/ VBAT-supply;
>
> No, don't delete properties. First of all, why you do not have this
> supply here? I doubt it... Especially that this DTS has vbat-supply
> regulator!
>
> Second, define the properties where applicable instead.

It looks like v3 is out, but responding here since it looks like
Sheng-Liang didn't make any changes in v3 but also didn't respond and
explain why he didn't make any changes. Sheng-Liang: for future
reference you should make sure to address comments folks have on the
list. If your new version takes their feedback into account then
there's no reason to just respond with "Done", but if (like in this
case) you ignored feedback you need to say why.

In this case the extra "/delete-property/" is needed to pass bindings
checks. Specifically this revision of the board replaces the "rt5682i"
with the newer "rt5682s". This new codec is _almost_ a drop-in
replacement for the old codec with just a few tiny changes. One such
change is that the new codec doesn't need a "VBAT-supply".

Since most trogdor devices have the older "rt5682i" codec, the default
in "sc7180-trogdor.dtsi" specifies the properties for that codec. Only
the handful of boards that have been spun to use the new codec have an
override like this. You can see that the override done here matches
the one done in a few other trogdor boards. A good grep is:

git grep -A4 realtek,rt5682s -- arch/arm64/boot/dts/qcom/sc7180-*

Ironically, that grep finds that "sc7180-trogdor-pazquel360.dtsi" is
missing the "/delete-property/" which I'm fairly certain means that
it's giving a validation warning today.

I'm happy to have a bikeshed discussion about doing this better. In a
previous reply [1] I suggested that it's probably time to move the
"realtek,rt5682s" snippet to something like
"sc7180-trogdor-rt5682s-sku.dtsi". Then we could include it in the
devices and avoid duplicating this bit of dts. I didn't insist on it,
but if you feel strongly then maybe Sheng-Liang could add that to his
series? Once done, we could have further bikeshed discussions about
whether we should continue to use the "/delete-property/" solution or
if we have to also create a "sc7180-trogdor-rt5682i-sku.dtsi" and
force all older SKUs to include that. Personally I don't hate this
"/delete-property/" but I don't care a whole lot either way.

[1] https://lore.kernel.org/r/CAD=FV=XRq8ymnPrMPCa=c7PkSH+Kj9aG29_hCjCNSL3fY-qaGg@mail.gmail.com


-Doug

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: qcom: sc7180: Add board id for lazor/limozeen
  2023-08-15 21:10     ` Doug Anderson
@ 2023-08-15 21:22       ` Krzysztof Kozlowski
       [not found]         ` <eb082e10-efc2-0f5f-95e1-4d2707c87c59@linaro.org/>
  0 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2023-08-15 21:22 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Sheng-Liang Pan, LKML, Andy Gross, Bjorn Andersson, Conor Dooley,
	Konrad Dybcio, Krzysztof Kozlowski, Rob Herring,
	cros-qcom-dts-watchers, devicetree, linux-arm-msm

On 15/08/2023 23:10, Doug Anderson wrote:
> Hi,
> 
> On Sun, Aug 6, 2023 at 11:34 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 04/08/2023 11:58, Sheng-Liang Pan wrote:
>>> add BRD_ID(0, Z, 0) = 10 for new board with ALC5682i-VS
>>>
>>> Signed-off-by: Sheng-Liang Pan <sheng-liang.pan@quanta.corp-partner.google.com>
>>> ---
>>>
>>> Changes in v2:
>>> - correct newly create dts files
>>>
>>
>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10.dts
>>> new file mode 100644
>>> index 000000000000..5a58e94c228e
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10.dts
>>> @@ -0,0 +1,30 @@
>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>> +/*
>>> + * Google Lazor board device tree source
>>> + *
>>> + * Copyright 2023 Google LLC.
>>> + */
>>> +
>>> +/dts-v1/;
>>> +
>>> +#include "sc7180-trogdor.dtsi"
>>> +#include "sc7180-trogdor-parade-ps8640.dtsi"
>>> +#include "sc7180-trogdor-lazor.dtsi"
>>> +#include "sc7180-lite.dtsi"
>>> +
>>> +/ {
>>> +     model = "Google Lazor (rev10+)";
>>> +     compatible = "google,lazor", "qcom,sc7180";
>>> +};
>>> +
>>> +&alc5682 {
>>> +     compatible = "realtek,rt5682s";
>>> +     /delete-property/ VBAT-supply;
>>
>> No, don't delete properties. First of all, why you do not have this
>> supply here? I doubt it... Especially that this DTS has vbat-supply
>> regulator!
>>
>> Second, define the properties where applicable instead.
> 
> It looks like v3 is out, but responding here since it looks like
> Sheng-Liang didn't make any changes in v3 but also didn't respond and
> explain why he didn't make any changes. Sheng-Liang: for future
> reference you should make sure to address comments folks have on the
> list. If your new version takes their feedback into account then
> there's no reason to just respond with "Done", but if (like in this
> case) you ignored feedback you need to say why.
> 
> In this case the extra "/delete-property/" is needed to pass bindings
> checks. Specifically this revision of the board replaces the "rt5682i"
> with the newer "rt5682s". This new codec is _almost_ a drop-in
> replacement for the old codec with just a few tiny changes. One such
> change is that the new codec doesn't need a "VBAT-supply".
> 
> Since most trogdor devices have the older "rt5682i" codec, the default
> in "sc7180-trogdor.dtsi" specifies the properties for that codec. Only
> the handful of boards that have been spun to use the new codec have an
> override like this. You can see that the override done here matches
> the one done in a few other trogdor boards. A good grep is:
> 
> git grep -A4 realtek,rt5682s -- arch/arm64/boot/dts/qcom/sc7180-*
> 
> Ironically, that grep finds that "sc7180-trogdor-pazquel360.dtsi" is
> missing the "/delete-property/" which I'm fairly certain means that
> it's giving a validation warning today.
> 
> I'm happy to have a bikeshed discussion about doing this better. In a
> previous reply [1] I suggested that it's probably time to move the
> "realtek,rt5682s" snippet to something like
> "sc7180-trogdor-rt5682s-sku.dtsi". Then we could include it in the
> devices and avoid duplicating this bit of dts. I didn't insist on it,
> but if you feel strongly then maybe Sheng-Liang could add that to his
> series? Once done, we could have further bikeshed discussions about
> whether we should continue to use the "/delete-property/" solution or
> if we have to also create a "sc7180-trogdor-rt5682i-sku.dtsi" and
> force all older SKUs to include that. Personally I don't hate this
> "/delete-property/" but I don't care a whole lot either way.

Thanks for explanation. I vote against /delete-property/ because it is
error-prone and a bit confusing. The same with overriding compatibles -
if possible, should be avoided. sc7180-trogdor-pazquel360.dtsi is doing
both, but that's not the pattern I find easy to read.

I accept overriding supplies or pins, because these differ per board.
But if common DTSI defines compatible, then it is common for everyone or
it is not really part of common DTSI.

IOW, the common DTSI should be more like a SoC DTSI - have only parts
present there. I simplify here, because obviously SoC is a real thing
piece of hardware and common board DTSI is not. It's just an
abstraction... but anyway if different boards use different codecs, then
I would say it is not part of common platform.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: qcom: sc7180: Add board id for lazor/limozeen
       [not found]           ` <20230816095910.41305-1-sheng-liang.pan@quanta.corp-partner.google.com>
@ 2023-08-16 18:31             ` Doug Anderson
  0 siblings, 0 replies; 15+ messages in thread
From: Doug Anderson @ 2023-08-16 18:31 UTC (permalink / raw)
  To: Sheng-Liang Pan
  Cc: krzysztof.kozlowski, agross, andersson, conor+dt,
	cros-qcom-dts-watchers, devicetree, konrad.dybcio,
	krzysztof.kozlowski+dt, linux-arm-msm, linux-kernel, robh+dt

Hi,

On Wed, Aug 16, 2023 at 2:59 AM Sheng-Liang Pan
<sheng-liang.pan@quanta.corp-partner.google.com> wrote:
>
> > On 15/08/2023 23:10, Doug Anderson wrote:
> >> Hi,
> >>
> >> On Sun, Aug 6, 2023 at 11:34 PM Krzysztof Kozlowski
> >> <krzysztof.kozlowski@linaro.org> wrote:
> >>>
> >>> On 04/08/2023 11:58, Sheng-Liang Pan wrote:
> >>>> add BRD_ID(0, Z, 0) = 10 for new board with ALC5682i-VS
> >>>>
> >>>> Signed-off-by: Sheng-Liang Pan <sheng-liang.pan@quanta.corp-partner.google.com>
> >>>> ---
> >>>>
> >>>> Changes in v2:
> >>>> - correct newly create dts files
> >>>>
> >>>
> >>>
> >>>> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10.dts
> >>>> new file mode 100644
> >>>> index 000000000000..5a58e94c228e
> >>>> --- /dev/null
> >>>> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10.dts
> >>>> @@ -0,0 +1,30 @@
> >>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> >>>> +/*
> >>>> + * Google Lazor board device tree source
> >>>> + *
> >>>> + * Copyright 2023 Google LLC.
> >>>> + */
> >>>> +
> >>>> +/dts-v1/;
> >>>> +
> >>>> +#include "sc7180-trogdor.dtsi"
> >>>> +#include "sc7180-trogdor-parade-ps8640.dtsi"
> >>>> +#include "sc7180-trogdor-lazor.dtsi"
> >>>> +#include "sc7180-lite.dtsi"
> >>>> +
> >>>> +/ {
> >>>> +     model = "Google Lazor (rev10+)";
> >>>> +     compatible = "google,lazor", "qcom,sc7180";
> >>>> +};
> >>>> +
> >>>> +&alc5682 {
> >>>> +     compatible = "realtek,rt5682s";
> >>>> +     /delete-property/ VBAT-supply;
> >>>
> >>> No, don't delete properties. First of all, why you do not have this
> >>> supply here? I doubt it... Especially that this DTS has vbat-supply
> >>> regulator!
> >>>
> >>> Second, define the properties where applicable instead.
> >>
> >> It looks like v3 is out, but responding here since it looks like
> >> Sheng-Liang didn't make any changes in v3 but also didn't respond and
> >> explain why he didn't make any changes. Sheng-Liang: for future
> >> reference you should make sure to address comments folks have on the
> >> list. If your new version takes their feedback into account then
> >> there's no reason to just respond with "Done", but if (like in this
> >> case) you ignored feedback you need to say why.
> >>
> >> In this case the extra "/delete-property/" is needed to pass bindings
> >> checks. Specifically this revision of the board replaces the "rt5682i"
> >> with the newer "rt5682s". This new codec is _almost_ a drop-in
> >> replacement for the old codec with just a few tiny changes. One such
> >> change is that the new codec doesn't need a "VBAT-supply".
> >>
> >> Since most trogdor devices have the older "rt5682i" codec, the default
> >> in "sc7180-trogdor.dtsi" specifies the properties for that codec. Only
> >> the handful of boards that have been spun to use the new codec have an
> >> override like this. You can see that the override done here matches
> >> the one done in a few other trogdor boards. A good grep is:
> >>
> >> git grep -A4 realtek,rt5682s -- arch/arm64/boot/dts/qcom/sc7180-*
> >>
> >> Ironically, that grep finds that "sc7180-trogdor-pazquel360.dtsi" is
> >> missing the "/delete-property/" which I'm fairly certain means that
> >> it's giving a validation warning today.
> >>
> >> I'm happy to have a bikeshed discussion about doing this better. In a
> >> previous reply [1] I suggested that it's probably time to move the
> >> "realtek,rt5682s" snippet to something like
> >> "sc7180-trogdor-rt5682s-sku.dtsi". Then we could include it in the
> >> devices and avoid duplicating this bit of dts. I didn't insist on it,
> >> but if you feel strongly then maybe Sheng-Liang could add that to his
> >> series? Once done, we could have further bikeshed discussions about
> >> whether we should continue to use the "/delete-property/" solution or
> >> if we have to also create a "sc7180-trogdor-rt5682i-sku.dtsi" and
> >> force all older SKUs to include that. Personally I don't hate this
> >> "/delete-property/" but I don't care a whole lot either way.
> >
> > Thanks for explanation. I vote against /delete-property/ because it is
> > error-prone and a bit confusing. The same with overriding compatibles -
> > if possible, should be avoided. sc7180-trogdor-pazquel360.dtsi is doing
> > both, but that's not the pattern I find easy to read.

OK, I tried it. I'm on the fence but don't object to it landing [1]


> > I accept overriding supplies or pins, because these differ per board.
> > But if common DTSI defines compatible, then it is common for everyone or
> > it is not really part of common DTSI.
> >
> > IOW, the common DTSI should be more like a SoC DTSI - have only parts
> > present there. I simplify here, because obviously SoC is a real thing
> > piece of hardware and common board DTSI is not. It's just an
> > abstraction... but anyway if different boards use different codecs, then
> > I would say it is not part of common platform.
> >
> > Best regards,
> > Krzysztof
> >
> >
> Thank Doug's explain, as Doug says, we need "/delete-property/" to pass binding checks.
> I read from https://lore.kernel.org/all/20221102182002.255282-9-nfraprado@collabora.com/ which removed VBAT-supply;
>
> I'd like to know what I can do for our project. Please advise.

I've posted a series which I think will help [2] [1]. Assuming those
look good, your action items would be:

1. If they look good, you could provide "Reviewed-by" and/or
"Tested-by" tags on my patches.

2. You can send a new version of your patches based atop mine. You'd
want to note in the cover letter and/or "after the cut" in the patch
that your patches depend on mine.

NOTE: there's no reason that the cleanup patches needed to be posted
by me. As you get more familiar with upstream kernel development, you
should be able to write similar patches yourself and include them in
your series. It's perfectly OK to "cleanup" other boards as part of
your series.


[1] https://lore.kernel.org/r/20230816112143.2.I29a5a330b6994afca81871f74bbacaf55b155937@changeid
[2] https://lore.kernel.org/r/20230816112143.1.I7227efd47e0dc42b6ff243bd22aa1a3e01923220@changeid

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2023-08-16 18:32 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-04  9:58 [PATCH v2 0/3] Add no-esim sku for sc7180-lazor family and new board version Sheng-Liang Pan
2023-08-04  9:58 ` [PATCH v2 1/3] dt-bindings: arm: qcom: add sc7180-lazor board bindings Sheng-Liang Pan
2023-08-04 16:30   ` Doug Anderson
2023-08-07 13:37     ` Konrad Dybcio
2023-08-07  6:32   ` Krzysztof Kozlowski
2023-08-15 20:41     ` Doug Anderson
2023-08-15 20:46       ` Krzysztof Kozlowski
2023-08-04  9:58 ` [PATCH v2 2/3] arm64: dts: qcom: sc7180: Add sku_id for lazor/limozeen Sheng-Liang Pan
2023-08-07  6:35   ` Krzysztof Kozlowski
2023-08-04  9:58 ` [PATCH v2 3/3] arm64: dts: qcom: sc7180: Add board id " Sheng-Liang Pan
2023-08-04 16:30   ` Doug Anderson
2023-08-07  6:34   ` Krzysztof Kozlowski
2023-08-15 21:10     ` Doug Anderson
2023-08-15 21:22       ` Krzysztof Kozlowski
     [not found]         ` <eb082e10-efc2-0f5f-95e1-4d2707c87c59@linaro.org/>
     [not found]           ` <20230816095910.41305-1-sheng-liang.pan@quanta.corp-partner.google.com>
2023-08-16 18:31             ` Doug Anderson

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).