devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] ARM: dts: qcom: msm8974: Add Samsung Galaxy S5 China support
@ 2024-01-21 15:39 Rong Zhang
  2024-01-21 15:39 ` [PATCH 1/4] ARM: dts: qcom: msm8974-samsung-klte: Add label on /i2c-gpio-led Rong Zhang
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Rong Zhang @ 2024-01-21 15:39 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Rong Zhang, linux-arm-msm, devicetree, linux-kernel,
	Icenowy Zheng

Samsung Galaxy S5 has some variants, currently, the only supported one
is klte. Samsung Galaxy S5 China (kltechn) is the China edition of
klte, and it has minor difference compared to klte. It can mostly work
with klte device tree, with only LEDs and WiFi not working.

This patchset adds support for kltechn by fixing up the GPIO pins for
the /i2c-gpio-led node (a corresponding label, "i2c_led_gpio", is also
added), and adding the brcm,board-type property in the wifi@1 node of
the klte device tree to allow loading the same firmware on all klte*
variants.

Rong Zhang (4):
  ARM: dts: qcom: msm8974-samsung-klte: Add label on /i2c-gpio-led
  ARM: dts: qcom: msm8974-samsung-klte: Pin brcm,board-type in wifi
  dt-bindings: arm: qcom: add Samsung Galaxy S5 China (kltechn)
  ARM: dts: qcom: msm8974: Add device tree for Samsung Galaxy S5 China

 Documentation/devicetree/bindings/arm/qcom.yaml  |  1 +
 arch/arm/boot/dts/qcom/Makefile                  |  1 +
 .../dts/qcom/qcom-msm8974pro-samsung-klte.dts    |  8 +++++++-
 .../dts/qcom/qcom-msm8974pro-samsung-kltechn.dts | 16 ++++++++++++++++
 4 files changed, 25 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-kltechn.dts


base-commit: 7a396820222d6d4c02057f41658b162bdcdadd0e
-- 
2.43.0


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

* [PATCH 1/4] ARM: dts: qcom: msm8974-samsung-klte: Add label on /i2c-gpio-led
  2024-01-21 15:39 [PATCH 0/4] ARM: dts: qcom: msm8974: Add Samsung Galaxy S5 China support Rong Zhang
@ 2024-01-21 15:39 ` Rong Zhang
  2024-01-22  9:48   ` Krzysztof Kozlowski
  2024-01-21 15:39 ` [PATCH 2/4] ARM: dts: qcom: msm8974-samsung-klte: Pin brcm,board-type in wifi Rong Zhang
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Rong Zhang @ 2024-01-21 15:39 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Rong Zhang, linux-arm-msm, devicetree, linux-kernel,
	Icenowy Zheng

Some variants of klte, e.g., the China edition (kltechn), have minor
differences to differentiate them from klte. This includes the GPIO pins
connected to /i2c-gpio-led.

A label is added on /i2c-gpio-led to allow DT of other variants to
reference it conveniently. Considering both LEDs and a GPIO expander are
connected to the node, it is named "i2c_led_gpio".

Signed-off-by: Rong Zhang <i@rong.moe>
---
 arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-klte.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-klte.dts b/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-klte.dts
index b93539e2b87e..013946ccda0f 100644
--- a/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-klte.dts
+++ b/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-klte.dts
@@ -77,7 +77,7 @@ touchkey@20 {
 		};
 	};
 
-	i2c-gpio-led {
+	i2c_led_gpio: i2c-gpio-led {
 		compatible = "i2c-gpio";
 		#address-cells = <1>;
 		#size-cells = <0>;
-- 
2.43.0


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

* [PATCH 2/4] ARM: dts: qcom: msm8974-samsung-klte: Pin brcm,board-type in wifi
  2024-01-21 15:39 [PATCH 0/4] ARM: dts: qcom: msm8974: Add Samsung Galaxy S5 China support Rong Zhang
  2024-01-21 15:39 ` [PATCH 1/4] ARM: dts: qcom: msm8974-samsung-klte: Add label on /i2c-gpio-led Rong Zhang
@ 2024-01-21 15:39 ` Rong Zhang
  2024-01-21 15:39 ` [PATCH 3/4] dt-bindings: arm: qcom: add Samsung Galaxy S5 China (kltechn) Rong Zhang
  2024-01-21 15:39 ` [PATCH 4/4] ARM: dts: qcom: msm8974: Add device tree for Samsung Galaxy S5 China Rong Zhang
  3 siblings, 0 replies; 15+ messages in thread
From: Rong Zhang @ 2024-01-21 15:39 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Rong Zhang, linux-arm-msm, devicetree, linux-kernel,
	Icenowy Zheng

klte variants, namely, klte*, have little difference in the WiFi part.

Pin brcm,board-type to "samsung,klte" to allow klte* load the same
firmware.

Signed-off-by: Rong Zhang <i@rong.moe>
---
 arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-klte.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-klte.dts b/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-klte.dts
index 013946ccda0f..9025345a1ab9 100644
--- a/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-klte.dts
+++ b/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-klte.dts
@@ -665,6 +665,12 @@ wifi@1 {
 		reg = <1>;
 		compatible = "brcm,bcm4329-fmac";
 
+		/*
+		 * This aims to allow other klte* variants to load the same firmware,
+		 * as klte variants have little differences in the wifi part.
+		 */
+		brcm,board-type = "samsung,klte";
+
 		interrupt-parent = <&tlmm>;
 		interrupts = <92 IRQ_TYPE_LEVEL_HIGH>;
 		interrupt-names = "host-wake";
-- 
2.43.0


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

* [PATCH 3/4] dt-bindings: arm: qcom: add Samsung Galaxy S5 China (kltechn)
  2024-01-21 15:39 [PATCH 0/4] ARM: dts: qcom: msm8974: Add Samsung Galaxy S5 China support Rong Zhang
  2024-01-21 15:39 ` [PATCH 1/4] ARM: dts: qcom: msm8974-samsung-klte: Add label on /i2c-gpio-led Rong Zhang
  2024-01-21 15:39 ` [PATCH 2/4] ARM: dts: qcom: msm8974-samsung-klte: Pin brcm,board-type in wifi Rong Zhang
@ 2024-01-21 15:39 ` Rong Zhang
  2024-01-22  9:47   ` Krzysztof Kozlowski
  2024-01-21 15:39 ` [PATCH 4/4] ARM: dts: qcom: msm8974: Add device tree for Samsung Galaxy S5 China Rong Zhang
  3 siblings, 1 reply; 15+ messages in thread
From: Rong Zhang @ 2024-01-21 15:39 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Rong Zhang, linux-arm-msm, devicetree, linux-kernel,
	Icenowy Zheng

Document Samsung Galaxy S5 China (kltechn) based on msm8974pro.

Signed-off-by: Rong Zhang <i@rong.moe>
---
 Documentation/devicetree/bindings/arm/qcom.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index 1a5fb889a444..2bd29a2399ad 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -223,6 +223,7 @@ properties:
               - fairphone,fp2
               - oneplus,bacon
               - samsung,klte
+              - samsung,kltechn
               - sony,xperia-castor
           - const: qcom,msm8974pro
           - const: qcom,msm8974
-- 
2.43.0


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

* [PATCH 4/4] ARM: dts: qcom: msm8974: Add device tree for Samsung Galaxy S5 China
  2024-01-21 15:39 [PATCH 0/4] ARM: dts: qcom: msm8974: Add Samsung Galaxy S5 China support Rong Zhang
                   ` (2 preceding siblings ...)
  2024-01-21 15:39 ` [PATCH 3/4] dt-bindings: arm: qcom: add Samsung Galaxy S5 China (kltechn) Rong Zhang
@ 2024-01-21 15:39 ` Rong Zhang
  2024-01-22  9:48   ` Krzysztof Kozlowski
  2024-01-22 10:50   ` Konrad Dybcio
  3 siblings, 2 replies; 15+ messages in thread
From: Rong Zhang @ 2024-01-21 15:39 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Rong Zhang, linux-arm-msm, devicetree, linux-kernel,
	Icenowy Zheng

This device has little difference compared to Samsung Galaxy S5 (klte),
so the device tree is based on qcom-msm8974pro-samsung-klte.dts. The
only difference is the gpio pins of i2c_led_gpio. With pins corrected,
the LEDs and WiFi are able to work properly.

Signed-off-by: Rong Zhang <i@rong.moe>
---
 arch/arm/boot/dts/qcom/Makefile                  |  1 +
 .../dts/qcom/qcom-msm8974pro-samsung-kltechn.dts | 16 ++++++++++++++++
 2 files changed, 17 insertions(+)
 create mode 100644 arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-kltechn.dts

diff --git a/arch/arm/boot/dts/qcom/Makefile b/arch/arm/boot/dts/qcom/Makefile
index 9cc1e14e6cd0..5d7a34adf826 100644
--- a/arch/arm/boot/dts/qcom/Makefile
+++ b/arch/arm/boot/dts/qcom/Makefile
@@ -44,6 +44,7 @@ dtb-$(CONFIG_ARCH_QCOM) += \
 	qcom-msm8974pro-fairphone-fp2.dtb \
 	qcom-msm8974pro-oneplus-bacon.dtb \
 	qcom-msm8974pro-samsung-klte.dtb \
+	qcom-msm8974pro-samsung-kltechn.dtb \
 	qcom-msm8974pro-sony-xperia-shinano-castor.dtb \
 	qcom-mdm9615-wp8548-mangoh-green.dtb \
 	qcom-sdx55-mtp.dtb \
diff --git a/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-kltechn.dts b/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-kltechn.dts
new file mode 100644
index 000000000000..5a8d59ea4439
--- /dev/null
+++ b/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-kltechn.dts
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "qcom-msm8974pro-samsung-klte.dts"
+
+/ {
+	model = "Samsung Galaxy S5 China";
+	compatible = "samsung,kltechn", "samsung,klte", "qcom,msm8974pro", "qcom,msm8974";
+};
+
+&i2c_led_gpio {
+	scl-gpios = <&tlmm 61 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+	sda-gpios = <&tlmm 60 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+};
+
+&i2c_led_gpioex_pins {
+	pins = "gpio60", "gpio61";
+};
-- 
2.43.0


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

* Re: [PATCH 3/4] dt-bindings: arm: qcom: add Samsung Galaxy S5 China (kltechn)
  2024-01-21 15:39 ` [PATCH 3/4] dt-bindings: arm: qcom: add Samsung Galaxy S5 China (kltechn) Rong Zhang
@ 2024-01-22  9:47   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2024-01-22  9:47 UTC (permalink / raw)
  To: Rong Zhang, Bjorn Andersson, Konrad Dybcio, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, Icenowy Zheng

On 21/01/2024 16:39, Rong Zhang wrote:
> Document Samsung Galaxy S5 China (kltechn) based on msm8974pro.
> 
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---
>  Documentation/devicetree/bindings/arm/qcom.yaml | 1 +
>  1 file changed, 1 insertion(+)


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

Best regards,
Krzysztof


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

* Re: [PATCH 1/4] ARM: dts: qcom: msm8974-samsung-klte: Add label on /i2c-gpio-led
  2024-01-21 15:39 ` [PATCH 1/4] ARM: dts: qcom: msm8974-samsung-klte: Add label on /i2c-gpio-led Rong Zhang
@ 2024-01-22  9:48   ` Krzysztof Kozlowski
  2024-01-22 14:54     ` Rong Zhang
  0 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2024-01-22  9:48 UTC (permalink / raw)
  To: Rong Zhang, Bjorn Andersson, Konrad Dybcio, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, Icenowy Zheng

On 21/01/2024 16:39, Rong Zhang wrote:
> Some variants of klte, e.g., the China edition (kltechn), have minor
> differences to differentiate them from klte. This includes the GPIO pins
> connected to /i2c-gpio-led.
> 
> A label is added on /i2c-gpio-led to allow DT of other variants to
> reference it conveniently. Considering both LEDs and a GPIO expander are
> connected to the node, it is named "i2c_led_gpio".
> 
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---
>  arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-klte.dts | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-klte.dts b/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-klte.dts
> index b93539e2b87e..013946ccda0f 100644
> --- a/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-klte.dts
> +++ b/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-klte.dts
> @@ -77,7 +77,7 @@ touchkey@20 {
>  		};
>  	};
>  
> -	i2c-gpio-led {
> +	i2c_led_gpio: i2c-gpio-led {

This does not make much sense on its own. 6 commit msg lines just to add
a label. Squash it.

Best regards,
Krzysztof


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

* Re: [PATCH 4/4] ARM: dts: qcom: msm8974: Add device tree for Samsung Galaxy S5 China
  2024-01-21 15:39 ` [PATCH 4/4] ARM: dts: qcom: msm8974: Add device tree for Samsung Galaxy S5 China Rong Zhang
@ 2024-01-22  9:48   ` Krzysztof Kozlowski
  2024-01-22 14:51     ` Rong Zhang
  2024-01-22 10:50   ` Konrad Dybcio
  1 sibling, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2024-01-22  9:48 UTC (permalink / raw)
  To: Rong Zhang, Bjorn Andersson, Konrad Dybcio, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, Icenowy Zheng

On 21/01/2024 16:39, Rong Zhang wrote:
> This device has little difference compared to Samsung Galaxy S5 (klte),
> so the device tree is based on qcom-msm8974pro-samsung-klte.dts. The
> only difference is the gpio pins of i2c_led_gpio. With pins corrected,
> the LEDs and WiFi are able to work properly.
> 
> Signed-off-by: Rong Zhang <i@rong.moe>


> diff --git a/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-kltechn.dts b/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-kltechn.dts
> new file mode 100644
> index 000000000000..5a8d59ea4439
> --- /dev/null
> +++ b/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-kltechn.dts
> @@ -0,0 +1,16 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include "qcom-msm8974pro-samsung-klte.dts"
> +
> +/ {
> +	model = "Samsung Galaxy S5 China";
> +	compatible = "samsung,kltechn", "samsung,klte", "qcom,msm8974pro", "qcom,msm8974";

That's not what you said in the binding.

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (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 4/4] ARM: dts: qcom: msm8974: Add device tree for Samsung Galaxy S5 China
  2024-01-21 15:39 ` [PATCH 4/4] ARM: dts: qcom: msm8974: Add device tree for Samsung Galaxy S5 China Rong Zhang
  2024-01-22  9:48   ` Krzysztof Kozlowski
@ 2024-01-22 10:50   ` Konrad Dybcio
  2024-01-22 14:01     ` Icenowy Zheng
  2024-01-22 17:37     ` Rong Zhang
  1 sibling, 2 replies; 15+ messages in thread
From: Konrad Dybcio @ 2024-01-22 10:50 UTC (permalink / raw)
  To: Rong Zhang, Bjorn Andersson, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, Icenowy Zheng

On 21.01.2024 16:39, Rong Zhang wrote:
> This device has little difference compared to Samsung Galaxy S5 (klte),
> so the device tree is based on qcom-msm8974pro-samsung-klte.dts. The
> only difference is the gpio pins of i2c_led_gpio. With pins corrected,
> the LEDs and WiFi are able to work properly.
> 
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---

Looks like you didn't change the brcm,board-type though?

[...]

> +++ b/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-kltechn.dts
> @@ -0,0 +1,16 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include "qcom-msm8974pro-samsung-klte.dts"

It's customary not to include .dts files, instead split the common parts
into e.g. qcom-msm8974pro-samsung-klte-common.dtsi and include this in
both the existing and the new one.

Konrad

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

* Re: [PATCH 4/4] ARM: dts: qcom: msm8974: Add device tree for Samsung Galaxy S5 China
  2024-01-22 10:50   ` Konrad Dybcio
@ 2024-01-22 14:01     ` Icenowy Zheng
  2024-01-22 17:37     ` Rong Zhang
  1 sibling, 0 replies; 15+ messages in thread
From: Icenowy Zheng @ 2024-01-22 14:01 UTC (permalink / raw)
  To: Konrad Dybcio, Rong Zhang, Bjorn Andersson, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel

在 2024-01-22星期一的 11:50 +0100,Konrad Dybcio写道:
> On 21.01.2024 16:39, Rong Zhang wrote:
> > This device has little difference compared to Samsung Galaxy S5
> > (klte),
> > so the device tree is based on qcom-msm8974pro-samsung-klte.dts.
> > The
> > only difference is the gpio pins of i2c_led_gpio. With pins
> > corrected,
> > the LEDs and WiFi are able to work properly.
> > 
> > Signed-off-by: Rong Zhang <i@rong.moe>
> > ---
> 
> Looks like you didn't change the brcm,board-type though?

This should be intentional to allow kltechn and klte to share Wi-Fi
NVRAM file.

> 
> [...]
> 
> > +++ b/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-kltechn.dts
> > @@ -0,0 +1,16 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include "qcom-msm8974pro-samsung-klte.dts"
> 
> It's customary not to include .dts files, instead split the common
> parts
> into e.g. qcom-msm8974pro-samsung-klte-common.dtsi and include this
> in
> both the existing and the new one.
> 
> Konrad


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

* Re: [PATCH 4/4] ARM: dts: qcom: msm8974: Add device tree for Samsung Galaxy S5 China
  2024-01-22  9:48   ` Krzysztof Kozlowski
@ 2024-01-22 14:51     ` Rong Zhang
  2024-01-22 15:17       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 15+ messages in thread
From: Rong Zhang @ 2024-01-22 14:51 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Bjorn Andersson, Konrad Dybcio, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, Icenowy Zheng

On Mon, 2024-01-22 at 10:48 +0100, Krzysztof Kozlowski wrote:
> On 21/01/2024 16:39, Rong Zhang wrote:
> > This device has little difference compared to Samsung Galaxy S5 (klte),
> > so the device tree is based on qcom-msm8974pro-samsung-klte.dts. The
> > only difference is the gpio pins of i2c_led_gpio. With pins corrected,
> > the LEDs and WiFi are able to work properly.
> > 
> > Signed-off-by: Rong Zhang <i@rong.moe>
> 
> 
> > diff --git a/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-kltechn.dts b/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-kltechn.dts
> > new file mode 100644
> > index 000000000000..5a8d59ea4439
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-kltechn.dts
> > @@ -0,0 +1,16 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include "qcom-msm8974pro-samsung-klte.dts"
> > +
> > +/ {
> > +	model = "Samsung Galaxy S5 China";
> > +	compatible = "samsung,kltechn", "samsung,klte", "qcom,msm8974pro", "qcom,msm8974";
> 
> That's not what you said in the binding.
> 
> It does not look like you tested the DTS against bindings. Please run
> `make dtbs_check W=1` (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).

Oops, I've forgot to run dtbs_check again after my final decision of
adding "samsung,klte". Thanks for pointing it out.

I added it because I thought the difference between klte and kltechn is
so tiny and I've seen some other dts doing that.

I've glanced similar dts. To solve this, I think we could either:
1. keep the dt-binding in [PATCH 3/4], and delete "samsung,klte" here
2. rewrite dt-binding like crystalfontz,cfa100{36,37,49,55,56,57,58}:

diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index 2bd29a2399ad..4979ccae2b64 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -223,11 +223,17 @@ properties:
               - fairphone,fp2
               - oneplus,bacon
               - samsung,klte
-              - samsung,kltechn
               - sony,xperia-castor
           - const: qcom,msm8974pro
           - const: qcom,msm8974
 
+      - items:
+          - enum:
+              - samsung,kltechn
+          - const: samsung,klte
+          - const: qcom,msm8974pro
+          - const: qcom,msm8974
+
       - items:
           - const: qcom,msm8916-mtp
           - const: qcom,msm8916-mtp/1

My preference is (2.) since other variants of klte may be added in the
future. I would like to hear your preferences.

Thanks,
Rong


> Best regards,
> Krzysztof
> 


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

* Re: [PATCH 1/4] ARM: dts: qcom: msm8974-samsung-klte: Add label on /i2c-gpio-led
  2024-01-22  9:48   ` Krzysztof Kozlowski
@ 2024-01-22 14:54     ` Rong Zhang
  0 siblings, 0 replies; 15+ messages in thread
From: Rong Zhang @ 2024-01-22 14:54 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Bjorn Andersson, Konrad Dybcio, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, Icenowy Zheng

On Mon, 2024-01-22 at 10:48 +0100, Krzysztof Kozlowski wrote:
> On 21/01/2024 16:39, Rong Zhang wrote:
> > Some variants of klte, e.g., the China edition (kltechn), have minor
> > differences to differentiate them from klte. This includes the GPIO pins
> > connected to /i2c-gpio-led.
> > 
> > A label is added on /i2c-gpio-led to allow DT of other variants to
> > reference it conveniently. Considering both LEDs and a GPIO expander are
> > connected to the node, it is named "i2c_led_gpio".
> > 
> > Signed-off-by: Rong Zhang <i@rong.moe>
> > ---
> >  arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-klte.dts | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-klte.dts b/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-klte.dts
> > index b93539e2b87e..013946ccda0f 100644
> > --- a/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-klte.dts
> > +++ b/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-klte.dts
> > @@ -77,7 +77,7 @@ touchkey@20 {
> >  		};
> >  	};
> >  
> > -	i2c-gpio-led {
> > +	i2c_led_gpio: i2c-gpio-led {
> 
> This does not make much sense on its own. 6 commit msg lines just to add
> a label. Squash it.

Will squash it into "[PATCH 4/4] ARM: dts: qcom: msm8974: Add device
tree for Samsung Galaxy S5 China".

Thanks,
Rong


> Best regards,
> Krzysztof
> 


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

* Re: [PATCH 4/4] ARM: dts: qcom: msm8974: Add device tree for Samsung Galaxy S5 China
  2024-01-22 14:51     ` Rong Zhang
@ 2024-01-22 15:17       ` Krzysztof Kozlowski
  2024-01-22 15:27         ` Rong Zhang
  0 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2024-01-22 15:17 UTC (permalink / raw)
  To: Rong Zhang, Bjorn Andersson, Konrad Dybcio, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, Icenowy Zheng

On 22/01/2024 15:51, Rong Zhang wrote:
> I've glanced similar dts. To solve this, I think we could either:
> 1. keep the dt-binding in [PATCH 3/4], and delete "samsung,klte" here
> 2. rewrite dt-binding like crystalfontz,cfa100{36,37,49,55,56,57,58}:
> 
> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
> index 2bd29a2399ad..4979ccae2b64 100644
> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
> @@ -223,11 +223,17 @@ properties:
>                - fairphone,fp2
>                - oneplus,bacon
>                - samsung,klte
> -              - samsung,kltechn
>                - sony,xperia-castor
>            - const: qcom,msm8974pro
>            - const: qcom,msm8974
>  
> +      - items:
> +          - enum:
> +              - samsung,kltechn
> +          - const: samsung,klte
> +          - const: qcom,msm8974pro
> +          - const: qcom,msm8974
> +
>        - items:
>            - const: qcom,msm8916-mtp
>            - const: qcom,msm8916-mtp/1
> 
> My preference is (2.) since other variants of klte may be added in the
> future. I would like to hear your preferences.

It depends whether the devices are compatible. IOW, entire klte DTB
should work fine on kltechn, just without few new devices.

Best regards,
Krzysztof


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

* Re: [PATCH 4/4] ARM: dts: qcom: msm8974: Add device tree for Samsung Galaxy S5 China
  2024-01-22 15:17       ` Krzysztof Kozlowski
@ 2024-01-22 15:27         ` Rong Zhang
  0 siblings, 0 replies; 15+ messages in thread
From: Rong Zhang @ 2024-01-22 15:27 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Bjorn Andersson, Konrad Dybcio, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, Icenowy Zheng

On Mon, 2024-01-22 at 16:17 +0100, Krzysztof Kozlowski wrote:
> On 22/01/2024 15:51, Rong Zhang wrote:
> > I've glanced similar dts. To solve this, I think we could either:
> > 1. keep the dt-binding in [PATCH 3/4], and delete "samsung,klte" here
> > 2. rewrite dt-binding like crystalfontz,cfa100{36,37,49,55,56,57,58}:
[...]
> > My preference is (2.) since other variants of klte may be added in the
> > future. I would like to hear your preferences.
> 
> It depends whether the devices are compatible. IOW, entire klte DTB
> should work fine on kltechn, just without few new devices.

Yes, they are compatible. I'd used the klte DTB on kltechn for a long
time before I made this patchset. It worked totally fine expect for
LEDs and WiFi, as I've said in the cover letter.

Thanks,
Rong


> Best regards,
> Krzysztof
> 


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

* Re: [PATCH 4/4] ARM: dts: qcom: msm8974: Add device tree for Samsung Galaxy S5 China
  2024-01-22 10:50   ` Konrad Dybcio
  2024-01-22 14:01     ` Icenowy Zheng
@ 2024-01-22 17:37     ` Rong Zhang
  1 sibling, 0 replies; 15+ messages in thread
From: Rong Zhang @ 2024-01-22 17:37 UTC (permalink / raw)
  To: Konrad Dybcio, Bjorn Andersson, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, Icenowy Zheng

On Mon, 2024-01-22 at 11:50 +0100, Konrad Dybcio wrote:
> Looks like you didn't change the brcm,board-type though?

In "[PATCH 2/4] ARM: dts: qcom: msm8974-samsung-klte: Pin brcm,board-
type in wifi":

   /*
    * This aims to allow other klte* variants to load the same firmware,
    * as klte variants have little differences in the wifi part.
    */

So it is intentional, in order to let them share the same FW, in
particular, the NVRAM file.

Without [PATCH 2/4] and with this [PATCH 4/4]:
- klte DT probes "brcmfmac*.samsung,klte.txt"
- kltechn DT probes "brcmfmac*.samsung,kltechn.txt", but never probes
"brcmfmac*.samsung,klte.txt"

With both [PATCH 2/4] and this [PATCH 4/4]:
- klte DT probes "brcmfmac*.samsung,klte.txt"
- kltechn DT probes "brcmfmac*.samsung,klte.txt"

I pinned "brcm,board-type" in the klte DT instead of the kltechn one
because other klte* variants are known to have little difference in the
WiFi part. By pinning it in the klte DT, future ports could be easier.

If you'd prefer not doing this, I am OK to drop [PATCH 2/4] in the v2
patchset.

FYI:

LineageOS considers all klte variants use "common" WiFi FW:
https://github.com/search?q=repo%3ALineageOS%2Fandroid_kernel_samsung_klte+CONFIG_BCMDHD_NVRAM_PATH&type=code
https://github.com/LineageOS/android_device_samsung_klte-common/blob/8f71a63415397def5ba886f4030b0d91e2253262/common-proprietary-files.txt#L251

I've tested the klte port of PostmarketOS (with klte FW and this
patchset) on kltechn, and it worked fine. For the FW used by
PostmarketOS, see also:
https://gitlab.com/postmarketOS/pmaports/-/blob/439227770ffcd32eb7e26436598c804dc35637ad/device/testing/firmware-samsung-klte/APKBUILD#L17


> > +++ b/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-kltechn.dts
> > @@ -0,0 +1,16 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include "qcom-msm8974pro-samsung-klte.dts"
> 
> It's customary not to include .dts files, instead split the common parts
> into e.g. qcom-msm8974pro-samsung-klte-common.dtsi and include this in
> both the existing and the new one.

At the very beginning, I thought including .dts could make the patchset
tiny (considering that the difference among klte* is also tiny).

I agree that splitting the common parts into a .dtsi will make things
more elegant and make klte* DTs consistent with the style of qcom DTs.

Will do in v2. Thanks for your advice.

Thanks,
Rong


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

end of thread, other threads:[~2024-01-22 17:38 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-21 15:39 [PATCH 0/4] ARM: dts: qcom: msm8974: Add Samsung Galaxy S5 China support Rong Zhang
2024-01-21 15:39 ` [PATCH 1/4] ARM: dts: qcom: msm8974-samsung-klte: Add label on /i2c-gpio-led Rong Zhang
2024-01-22  9:48   ` Krzysztof Kozlowski
2024-01-22 14:54     ` Rong Zhang
2024-01-21 15:39 ` [PATCH 2/4] ARM: dts: qcom: msm8974-samsung-klte: Pin brcm,board-type in wifi Rong Zhang
2024-01-21 15:39 ` [PATCH 3/4] dt-bindings: arm: qcom: add Samsung Galaxy S5 China (kltechn) Rong Zhang
2024-01-22  9:47   ` Krzysztof Kozlowski
2024-01-21 15:39 ` [PATCH 4/4] ARM: dts: qcom: msm8974: Add device tree for Samsung Galaxy S5 China Rong Zhang
2024-01-22  9:48   ` Krzysztof Kozlowski
2024-01-22 14:51     ` Rong Zhang
2024-01-22 15:17       ` Krzysztof Kozlowski
2024-01-22 15:27         ` Rong Zhang
2024-01-22 10:50   ` Konrad Dybcio
2024-01-22 14:01     ` Icenowy Zheng
2024-01-22 17:37     ` Rong Zhang

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