[PATCH v2 1/3] dt-bindings: vendor-prefixes: add thwc

[PATCH v2 1/3] dt-bindings: vendor-prefixes: add thwc

[Yang Xiwen]

Shenzhen Tong Heng Wei Chuang Technology Co., Ltd. (hereinafter referred
to as "Tong Heng Wei Chuang") is a focus on wireless communications
equipment brand manufacturers.

Link: http://www.szthwc.com/en/about.html
Signed-off-by: Yang Xiwen <forbidden405@foxmail.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 3893c45963a26..8d4c1e5c0d21a 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1335,6 +1335,8 @@ patternProperties:
     description: Tecon Microprocessor Technologies, LLC.
   "^topeet,.*":
     description: Topeet
+  "^thwc,.*":
+    description: Shenzhen Tong Heng Wei Chuang Technology Co., Ltd.
   "^topic,.*":
     description: Topic Embedded Systems
   "^toppoly,.*":
-- 
2.39.0

[PATCH v2 2/3] dt-bindings: qcom: Document msm8916-thwc-uf896 and ufi001c

[Yang Xiwen]

Document the new thwc,uf896/ufi001c device tree bindings used in their device
trees.

Signed-off-by: Yang Xiwen <forbidden405@foxmail.com>
---
 Documentation/devicetree/bindings/arm/qcom.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index 65ef7e442da0d..18070574848ae 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -209,6 +209,8 @@ properties:
               - samsung,grandmax
               - samsung,j5
               - samsung,serranove
+              - thwc,uf896
+              - thwc,ufi001c
               - wingtech,wt88047
           - const: qcom,msm8916
 
-- 
2.39.0

[PATCH v2 3/3] arm64: dts: qcom: msm8916-thwc: Add initial device tree for Tong Heng Wei Chuang Wifi/LTE dongle UFI-001C and uf896

[Yang Xiwen]

This commit adds support for the ufi-001C and uf896 WiFi/LTE dongle made by
Tong Heng Wei Chuang based on MSM8916.
uf896 is another variant for the usb stick. The board design
differs by using different gpios for the keys and leds.

Note: The original firmware does not support 64-bit OS. It is necessary
to flash 64-bit TZ firmware to boot arm64.

Currently supported:
- All CPU cores
- Buttons
- LEDs
- Modem
- SDHC
- USB Device Mode
- UART

Co-developed-by: Jaime Breva <jbreva@nayarsystems.com>
Signed-off-by: Jaime Breva <jbreva@nayarsystems.com>
Co-developed-by: Nikita Travkin <nikita@trvn.ru>
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
Signed-off-by: Yang Xiwen <forbidden405@foxmail.com>
---
 arch/arm64/boot/dts/qcom/Makefile             |   2 +
 .../boot/dts/qcom/msm8916-thwc-uf896.dts      |  41 +++
 .../boot/dts/qcom/msm8916-thwc-ufi001c.dts    |  39 +++
 arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi     | 246 ++++++++++++++++++
 4 files changed, 328 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/msm8916-thwc-uf896.dts
 create mode 100644 arch/arm64/boot/dts/qcom/msm8916-thwc-ufi001c.dts
 create mode 100644 arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index b42362c7be1bf..523dbe6d06be8 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -21,6 +21,8 @@ dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-samsung-e7.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-samsung-grandmax.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-samsung-j5.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-samsung-serranove.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-thwc-uf896.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-thwc-ufi001c.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-wingtech-wt88047.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8953-motorola-potter.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8953-xiaomi-daisy.dtb
diff --git a/arch/arm64/boot/dts/qcom/msm8916-thwc-uf896.dts b/arch/arm64/boot/dts/qcom/msm8916-thwc-uf896.dts
new file mode 100644
index 0000000000000..79447ca48dd3a
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8916-thwc-uf896.dts
@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/dts-v1/;
+
+#include "msm8916-ufi.dtsi"
+
+/ {
+	model = "uf896 4G Modem Stick";
+	compatible = "thwc,uf896", "qcom,msm8916";
+};
+
+&button_restart {
+	gpios = <&msmgpio 35 GPIO_ACTIVE_LOW>;
+};
+
+&led_r {
+	gpios = <&msmgpio 82 GPIO_ACTIVE_HIGH>;
+};
+
+&led_g {
+	gpios = <&msmgpio 83 GPIO_ACTIVE_HIGH>;
+	function = LED_FUNCTION_WLAN;
+};
+
+&led_b {
+	gpios = <&msmgpio 81 GPIO_ACTIVE_HIGH>;
+	function = LED_FUNCTION_WAN;
+};
+
+&button_default {
+	pins = "gpio35";
+	bias-pull-up;
+};
+
+&gpio_leds_default {
+	pins = "gpio81", "gpio82", "gpio83";
+};
+
+&sim_ctrl_default {
+	pins = "gpio1", "gpio2";
+};
diff --git a/arch/arm64/boot/dts/qcom/msm8916-thwc-ufi001c.dts b/arch/arm64/boot/dts/qcom/msm8916-thwc-ufi001c.dts
new file mode 100644
index 0000000000000..700cf81cbf8c0
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8916-thwc-ufi001c.dts
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/dts-v1/;
+
+#include "msm8916-ufi.dtsi"
+
+/ {
+	model = "ufi-001c/ufi-001b 4G Modem Stick";
+	compatible = "thwc,ufi001c", "qcom,msm8916";
+};
+
+&button_restart {
+	gpios = <&msmgpio 37 GPIO_ACTIVE_HIGH>;
+};
+
+&led_r {
+	gpios = <&msmgpio 22 GPIO_ACTIVE_HIGH>;
+};
+
+&led_g {
+	gpios = <&msmgpio 21 GPIO_ACTIVE_HIGH>;
+};
+
+&led_b {
+	gpios = <&msmgpio 20 GPIO_ACTIVE_HIGH>;
+};
+
+&button_default {
+	pins = "gpio37";
+	bias-pull-down;
+};
+
+&gpio_leds_default {
+	pins = "gpio20", "gpio21", "gpio22";
+};
+
+&sim_ctrl_default {
+	pins = "gpio1", "gpio2";
+};
diff --git a/arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi b/arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi
new file mode 100644
index 0000000000000..286768d0fbe70
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi
@@ -0,0 +1,246 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include "msm8916-pm8916.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/leds/common.h>
+
+/ {
+	chassis-type = "embedded";
+
+	aliases {
+		serial0 = &blsp1_uart2;
+	};
+
+	chosen {
+		stdout-path = "serial0";
+	};
+
+	reserved-memory {
+		mpss_mem: mpss@86800000 {
+			reg = <0x0 0x86800000 0x0 0x5500000>;
+			no-map;
+		};
+
+		gps_mem: gps@8bd00000 {
+			reg = <0x0 0x8bd00000 0x0 0x200000>;
+			no-map;
+		};
+	};
+
+	gpio-keys {
+		compatible = "gpio-keys";
+
+		pinctrl-0 = <&button_default>;
+		pinctrl-names = "default";
+
+		label = "GPIO Buttons";
+
+		/* gpio is board-specific */
+		button_restart: button-restart {
+			label = "Restart";
+			linux,code = <KEY_RESTART>;
+		};
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		pinctrl-0 = <&gpio_leds_default>;
+		pinctrl-names = "default";
+
+		/* GPIOs are board-specific */
+		led_r: led-r {
+			color = <LED_COLOR_ID_RED>;
+			default-state = "on";
+			function = LED_FUNCTION_INDICATOR;
+		};
+
+		led_g: led-g {
+			color = <LED_COLOR_ID_GREEN>;
+			default-state = "off";
+			function = LED_FUNCTION_INDICATOR;
+		};
+
+		led_b: led-b {
+			color = <LED_COLOR_ID_BLUE>;
+			default-state = "off";
+			function = LED_FUNCTION_INDICATOR;
+		};
+	};
+};
+
+&bam_dmux {
+	status = "okay";
+};
+
+&bam_dmux_dma {
+	status = "okay";
+};
+
+&blsp1_uart2 {
+	status = "okay";
+};
+
+/* Remove &dsi_phy0 from clocks to make sure that gcc probes with display disabled */
+&gcc {
+	clocks = <&xo_board>, <&sleep_clk>, <0>, <0>, <0>, <0>, <0>;
+};
+
+&mpss {
+	pinctrl-0 = <&sim_ctrl_default>;
+	pinctrl-names = "default";
+
+	status = "okay";
+};
+
+&pm8916_usbin {
+	status = "okay";
+};
+
+&pronto {
+	status = "okay";
+};
+
+&sdhc_1 {
+	pinctrl-0 = <&sdc1_clk_on &sdc1_cmd_on &sdc1_data_on>;
+	pinctrl-1 = <&sdc1_clk_off &sdc1_cmd_off &sdc1_data_off>;
+	pinctrl-names = "default", "sleep";
+
+	status = "okay";
+};
+
+&usb {
+	extcon = <&pm8916_usbin>;
+	dr_mode = "peripheral";
+
+	status = "okay";
+};
+
+&usb_hs_phy {
+	extcon = <&pm8916_usbin>;
+};
+
+&smd_rpm_regulators {
+	vdd_l1_l2_l3-supply = <&pm8916_s3>;
+	vdd_l4_l5_l6-supply = <&pm8916_s4>;
+	vdd_l7-supply = <&pm8916_s4>;
+
+	s3 {
+		regulator-min-microvolt = <1200000>;
+		regulator-max-microvolt = <1300000>;
+	};
+
+	s4 {
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <2100000>;
+	};
+
+	l1 {
+		regulator-min-microvolt = <1225000>;
+		regulator-max-microvolt = <1225000>;
+	};
+
+	l2 {
+		regulator-min-microvolt = <1200000>;
+		regulator-max-microvolt = <1200000>;
+	};
+
+	l4 {
+		regulator-min-microvolt = <2050000>;
+		regulator-max-microvolt = <2050000>;
+	};
+
+	l5 {
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+	};
+
+	l6 {
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+	};
+
+	l7 {
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+	};
+
+	l8 {
+		regulator-min-microvolt = <2850000>;
+		regulator-max-microvolt = <2900000>;
+	};
+
+	l9 {
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+	};
+
+	l10 {
+		regulator-min-microvolt = <2700000>;
+		regulator-max-microvolt = <2800000>;
+	};
+
+	l11 {
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <2950000>;
+		regulator-system-load = <200000>;
+		regulator-allow-set-load;
+	};
+
+	l12 {
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <2950000>;
+	};
+
+	l13 {
+		regulator-min-microvolt = <3075000>;
+		regulator-max-microvolt = <3075000>;
+	};
+
+	l14 {
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+	};
+
+	l15 {
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+	};
+
+	l16 {
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+	};
+
+	l17 {
+		regulator-min-microvolt = <2850000>;
+		regulator-max-microvolt = <2850000>;
+	};
+
+	l18 {
+		regulator-min-microvolt = <2700000>;
+		regulator-max-microvolt = <2700000>;
+	};
+};
+
+&msmgpio {
+	/* pins are board-specific */
+	button_default: button-default-state {
+		function = "gpio";
+		drive-strength = <2>;
+	};
+
+	gpio_leds_default: gpio-leds-default-state {
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	sim_ctrl_default: sim-ctrl-default-state {
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+		output-low;
+	};
+};
-- 
2.39.0

Re: [PATCH v2 1/3] dt-bindings: vendor-prefixes: add thwc

[Krzysztof Kozlowski]

On 11/01/2023 15:13, Yang Xiwen wrote:
> Shenzhen Tong Heng Wei Chuang Technology Co., Ltd. (hereinafter referred
> to as "Tong Heng Wei Chuang") is a focus on wireless communications
> equipment brand manufacturers.
> 
> Link: http://www.szthwc.com/en/about.html
> Signed-off-by: Yang Xiwen <forbidden405@foxmail.com>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index 3893c45963a26..8d4c1e5c0d21a 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -1335,6 +1335,8 @@ patternProperties:
>      description: Tecon Microprocessor Technologies, LLC.
>    "^topeet,.*":
>      description: Topeet
> +  "^thwc,.*":
> +    description: Shenzhen Tong Heng Wei Chuang Technology Co., Ltd.

These are ordered by prefix, so you need to move it up.

Best regards,
Krzysztof

Re: [PATCH v2 2/3] dt-bindings: qcom: Document msm8916-thwc-uf896 and ufi001c

[Krzysztof Kozlowski]

On 11/01/2023 15:13, Yang Xiwen wrote:
> Document the new thwc,uf896/ufi001c device tree bindings used in their device
> trees.
> 
> Signed-off-by: Yang Xiwen <forbidden405@foxmail.com>

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

Best regards,
Krzysztof

Re: [PATCH v2 3/3] arm64: dts: qcom: msm8916-thwc: Add initial device tree for Tong Heng Wei Chuang Wifi/LTE dongle UFI-001C and uf896

[Konrad Dybcio]

On 11.01.2023 15:13, Yang Xiwen wrote:
> This commit adds support for the ufi-001C and uf896 WiFi/LTE dongle made by
> Tong Heng Wei Chuang based on MSM8916.
> uf896 is another variant for the usb stick. The board design
> differs by using different gpios for the keys and leds.
> 
> Note: The original firmware does not support 64-bit OS. It is necessary
> to flash 64-bit TZ firmware to boot arm64.
> 
> Currently supported:
> - All CPU cores
> - Buttons
> - LEDs
> - Modem
> - SDHC
> - USB Device Mode
> - UART
> 
> Co-developed-by: Jaime Breva <jbreva@nayarsystems.com>
> Signed-off-by: Jaime Breva <jbreva@nayarsystems.com>
> Co-developed-by: Nikita Travkin <nikita@trvn.ru>
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> Signed-off-by: Yang Xiwen <forbidden405@foxmail.com>
> ---
>  arch/arm64/boot/dts/qcom/Makefile             |   2 +
>  .../boot/dts/qcom/msm8916-thwc-uf896.dts      |  41 +++
>  .../boot/dts/qcom/msm8916-thwc-ufi001c.dts    |  39 +++
>  arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi     | 246 ++++++++++++++++++
>  4 files changed, 328 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/msm8916-thwc-uf896.dts
>  create mode 100644 arch/arm64/boot/dts/qcom/msm8916-thwc-ufi001c.dts
>  create mode 100644 arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi
> 
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index b42362c7be1bf..523dbe6d06be8 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -21,6 +21,8 @@ dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-samsung-e7.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-samsung-grandmax.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-samsung-j5.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-samsung-serranove.dtb
> +dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-thwc-uf896.dtb
> +dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-thwc-ufi001c.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-wingtech-wt88047.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8953-motorola-potter.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8953-xiaomi-daisy.dtb
> diff --git a/arch/arm64/boot/dts/qcom/msm8916-thwc-uf896.dts b/arch/arm64/boot/dts/qcom/msm8916-thwc-uf896.dts
> new file mode 100644
> index 0000000000000..79447ca48dd3a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8916-thwc-uf896.dts
> @@ -0,0 +1,41 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/dts-v1/;
> +
> +#include "msm8916-ufi.dtsi"
> +
> +/ {
> +	model = "uf896 4G Modem Stick";
> +	compatible = "thwc,uf896", "qcom,msm8916";
> +};
> +
> +&button_restart {
> +	gpios = <&msmgpio 35 GPIO_ACTIVE_LOW>;
> +};
> +
> +&led_r {
> +	gpios = <&msmgpio 82 GPIO_ACTIVE_HIGH>;
> +};
> +
> +&led_g {
> +	gpios = <&msmgpio 83 GPIO_ACTIVE_HIGH>;
> +	function = LED_FUNCTION_WLAN;
This is a software property, maybe it would be worth
commonizing?

> +};
> +
> +&led_b {
> +	gpios = <&msmgpio 81 GPIO_ACTIVE_HIGH>;
> +	function = LED_FUNCTION_WAN;
> +};
> +
> +&button_default {
> +	pins = "gpio35";
> +	bias-pull-up;
> +};
> +
> +&gpio_leds_default {
> +	pins = "gpio81", "gpio82", "gpio83";
> +};
> +
> +&sim_ctrl_default {
> +	pins = "gpio1", "gpio2";
> +};
> diff --git a/arch/arm64/boot/dts/qcom/msm8916-thwc-ufi001c.dts b/arch/arm64/boot/dts/qcom/msm8916-thwc-ufi001c.dts
> new file mode 100644
> index 0000000000000..700cf81cbf8c0
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8916-thwc-ufi001c.dts
> @@ -0,0 +1,39 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/dts-v1/;
> +
> +#include "msm8916-ufi.dtsi"
> +
> +/ {
> +	model = "ufi-001c/ufi-001b 4G Modem Stick";
> +	compatible = "thwc,ufi001c", "qcom,msm8916";
> +};
> +
> +&button_restart {
> +	gpios = <&msmgpio 37 GPIO_ACTIVE_HIGH>;
> +};
> +
> +&led_r {
> +	gpios = <&msmgpio 22 GPIO_ACTIVE_HIGH>;
> +};
> +
> +&led_g {
> +	gpios = <&msmgpio 21 GPIO_ACTIVE_HIGH>;
> +};
> +
> +&led_b {
> +	gpios = <&msmgpio 20 GPIO_ACTIVE_HIGH>;
> +};
> +
> +&button_default {
> +	pins = "gpio37";
> +	bias-pull-down;
> +};
> +
> +&gpio_leds_default {
> +	pins = "gpio20", "gpio21", "gpio22";
> +};
> +
> +&sim_ctrl_default {
> +	pins = "gpio1", "gpio2";
Identical between both boards, move to common please.

> +};
> diff --git a/arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi b/arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi
> new file mode 100644
> index 0000000000000..286768d0fbe70
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi
> @@ -0,0 +1,246 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include "msm8916-pm8916.dtsi"
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/leds/common.h>
> +
> +/ {
> +	chassis-type = "embedded";
> +
> +	aliases {
> +		serial0 = &blsp1_uart2;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0";
> +	};
> +
> +	reserved-memory {
> +		mpss_mem: mpss@86800000 {
> +			reg = <0x0 0x86800000 0x0 0x5500000>;
> +			no-map;
> +		};
> +
> +		gps_mem: gps@8bd00000 {
> +			reg = <0x0 0x8bd00000 0x0 0x200000>;
> +			no-map;
> +		};
> +	};
> +
> +	gpio-keys {
> +		compatible = "gpio-keys";
> +
> +		pinctrl-0 = <&button_default>;
> +		pinctrl-names = "default";
> +
> +		label = "GPIO Buttons";
> +
> +		/* gpio is board-specific */
GPIO is an acronym, please make it uppercase, as you
did 10 lines below..

> +		button_restart: button-restart {
> +			label = "Restart";
> +			linux,code = <KEY_RESTART>;
> +		};
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +
> +		pinctrl-0 = <&gpio_leds_default>;
> +		pinctrl-names = "default";
> +
> +		/* GPIOs are board-specific */
> +		led_r: led-r {
> +			color = <LED_COLOR_ID_RED>;
> +			default-state = "on";
> +			function = LED_FUNCTION_INDICATOR;
> +		};
> +
> +		led_g: led-g {
> +			color = <LED_COLOR_ID_GREEN>;
> +			default-state = "off";
> +			function = LED_FUNCTION_INDICATOR;
> +		};
> +
> +		led_b: led-b {
> +			color = <LED_COLOR_ID_BLUE>;
> +			default-state = "off";
> +			function = LED_FUNCTION_INDICATOR;
> +		};
> +	};
> +};
> +
> +&bam_dmux {
> +	status = "okay";
> +};
> +
> +&bam_dmux_dma {
> +	status = "okay";
> +};
> +
> +&blsp1_uart2 {
> +	status = "okay";
> +};
> +
> +/* Remove &dsi_phy0 from clocks to make sure that gcc probes with display disabled */
That sounds like a bad regression and shouldn't be an issue..

> +&gcc {
> +	clocks = <&xo_board>, <&sleep_clk>, <0>, <0>, <0>, <0>, <0>;
> +};
> +
> +&mpss {
> +	pinctrl-0 = <&sim_ctrl_default>;
> +	pinctrl-names = "default";
> +
> +	status = "okay";
> +};
> +
> +&pm8916_usbin {
> +	status = "okay";
> +};
> +
> +&pronto {
> +	status = "okay";
> +};
> +
> +&sdhc_1 {
> +	pinctrl-0 = <&sdc1_clk_on &sdc1_cmd_on &sdc1_data_on>;
> +	pinctrl-1 = <&sdc1_clk_off &sdc1_cmd_off &sdc1_data_off>;
> +	pinctrl-names = "default", "sleep";
> +
> +	status = "okay";
> +};
> +
> +&usb {
> +	extcon = <&pm8916_usbin>;
> +	dr_mode = "peripheral";
> +
> +	status = "okay";
> +};
> +
> +&usb_hs_phy {
> +	extcon = <&pm8916_usbin>;
> +};
> +
> +&smd_rpm_regulators {
Please sort this reference alphabetically.

Konrad
> +	vdd_l1_l2_l3-supply = <&pm8916_s3>;
> +	vdd_l4_l5_l6-supply = <&pm8916_s4>;
> +	vdd_l7-supply = <&pm8916_s4>;
> +
> +	s3 {
> +		regulator-min-microvolt = <1200000>;
> +		regulator-max-microvolt = <1300000>;
> +	};
> +
> +	s4 {
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <2100000>;
> +	};
> +
> +	l1 {
> +		regulator-min-microvolt = <1225000>;
> +		regulator-max-microvolt = <1225000>;
> +	};
> +
> +	l2 {
> +		regulator-min-microvolt = <1200000>;
> +		regulator-max-microvolt = <1200000>;
> +	};
> +
> +	l4 {
> +		regulator-min-microvolt = <2050000>;
> +		regulator-max-microvolt = <2050000>;
> +	};
> +
> +	l5 {
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +	};
> +
> +	l6 {
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +	};
> +
> +	l7 {
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +	};
> +
> +	l8 {
> +		regulator-min-microvolt = <2850000>;
> +		regulator-max-microvolt = <2900000>;
> +	};
> +
> +	l9 {
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +	};
> +
> +	l10 {
> +		regulator-min-microvolt = <2700000>;
> +		regulator-max-microvolt = <2800000>;
> +	};
> +
> +	l11 {
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <2950000>;
> +		regulator-system-load = <200000>;
> +		regulator-allow-set-load;
> +	};
> +
> +	l12 {
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <2950000>;
> +	};
> +
> +	l13 {
> +		regulator-min-microvolt = <3075000>;
> +		regulator-max-microvolt = <3075000>;
> +	};
> +
> +	l14 {
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <3300000>;
> +	};
> +
> +	l15 {
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <3300000>;
> +	};
> +
> +	l16 {
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <3300000>;
> +	};
> +
> +	l17 {
> +		regulator-min-microvolt = <2850000>;
> +		regulator-max-microvolt = <2850000>;
> +	};
> +
> +	l18 {
> +		regulator-min-microvolt = <2700000>;
> +		regulator-max-microvolt = <2700000>;
> +	};
> +};
> +
> +&msmgpio {
> +	/* pins are board-specific */
> +	button_default: button-default-state {
> +		function = "gpio";
> +		drive-strength = <2>;
> +	};
> +
> +	gpio_leds_default: gpio-leds-default-state {
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-disable;
> +	};
> +
> +	sim_ctrl_default: sim-ctrl-default-state {
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-disable;
> +		output-low;
> +	};
> +};

Re: [PATCH v2 3/3] arm64: dts: qcom: msm8916-thwc: Add initial device tree for Tong Heng Wei Chuang Wifi/LTE dongle UFI-001C and uf896

[Nikita Travkin]

Konrad Dybcio писал(а) 12.01.2023 21:16:
> On 11.01.2023 15:13, Yang Xiwen wrote:
>> This commit adds support for the ufi-001C and uf896 WiFi/LTE dongle made by
>> Tong Heng Wei Chuang based on MSM8916.
>> uf896 is another variant for the usb stick. The board design
>> differs by using different gpios for the keys and leds.
>>
>> Note: The original firmware does not support 64-bit OS. It is necessary
>> to flash 64-bit TZ firmware to boot arm64.
>>
>> Currently supported:
>> - All CPU cores
>> - Buttons
>> - LEDs
>> - Modem
>> - SDHC
>> - USB Device Mode
>> - UART
>>
>> Co-developed-by: Jaime Breva <jbreva@nayarsystems.com>
>> Signed-off-by: Jaime Breva <jbreva@nayarsystems.com>
>> Co-developed-by: Nikita Travkin <nikita@trvn.ru>
>> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
>> Signed-off-by: Yang Xiwen <forbidden405@foxmail.com>
>> ---
>>  arch/arm64/boot/dts/qcom/Makefile             |   2 +
>>  .../boot/dts/qcom/msm8916-thwc-uf896.dts      |  41 +++
>>  .../boot/dts/qcom/msm8916-thwc-ufi001c.dts    |  39 +++
>>  arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi     | 246 ++++++++++++++++++
>>  4 files changed, 328 insertions(+)
>>  create mode 100644 arch/arm64/boot/dts/qcom/msm8916-thwc-uf896.dts
>>  create mode 100644 arch/arm64/boot/dts/qcom/msm8916-thwc-ufi001c.dts
>>  create mode 100644 arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi
>>
>> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
>> index b42362c7be1bf..523dbe6d06be8 100644
>> --- a/arch/arm64/boot/dts/qcom/Makefile
>> +++ b/arch/arm64/boot/dts/qcom/Makefile
>> @@ -21,6 +21,8 @@ dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-samsung-e7.dtb
>>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-samsung-grandmax.dtb
>>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-samsung-j5.dtb
>>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-samsung-serranove.dtb
>> +dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-thwc-uf896.dtb
>> +dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-thwc-ufi001c.dtb
>>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-wingtech-wt88047.dtb
>>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8953-motorola-potter.dtb
>>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8953-xiaomi-daisy.dtb
>> diff --git a/arch/arm64/boot/dts/qcom/msm8916-thwc-uf896.dts b/arch/arm64/boot/dts/qcom/msm8916-thwc-uf896.dts
>> new file mode 100644
>> index 0000000000000..79447ca48dd3a
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/msm8916-thwc-uf896.dts
>> @@ -0,0 +1,41 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +
>> +/dts-v1/;
>> +
>> +#include "msm8916-ufi.dtsi"
>> +
>> +/ {
>> +	model = "uf896 4G Modem Stick";
>> +	compatible = "thwc,uf896", "qcom,msm8916";
>> +};
>> +
>> +&button_restart {
>> +	gpios = <&msmgpio 35 GPIO_ACTIVE_LOW>;
>> +};
>> +
>> +&led_r {
>> +	gpios = <&msmgpio 82 GPIO_ACTIVE_HIGH>;
>> +};
>> +
>> +&led_g {
>> +	gpios = <&msmgpio 83 GPIO_ACTIVE_HIGH>;
>> +	function = LED_FUNCTION_WLAN;
> This is a software property, maybe it would be worth
> commonizing?
> 

The functions are put in-line with the labels on the housing.
The uf896 has green and blue leds labeled with icons but I was told
by Yang that the uf001c doesn't have any labels.

>> +};
>> +
>> +&led_b {
>> +	gpios = <&msmgpio 81 GPIO_ACTIVE_HIGH>;
>> +	function = LED_FUNCTION_WAN;
>> +};
>> +
>> +&button_default {
>> +	pins = "gpio35";
>> +	bias-pull-up;
>> +};
>> +
>> +&gpio_leds_default {
>> +	pins = "gpio81", "gpio82", "gpio83";
>> +};
>> +
>> +&sim_ctrl_default {
>> +	pins = "gpio1", "gpio2";
>> +};
>> diff --git a/arch/arm64/boot/dts/qcom/msm8916-thwc-ufi001c.dts b/arch/arm64/boot/dts/qcom/msm8916-thwc-ufi001c.dts
>> new file mode 100644
>> index 0000000000000..700cf81cbf8c0
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/msm8916-thwc-ufi001c.dts
>> @@ -0,0 +1,39 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +
>> +/dts-v1/;
>> +
>> +#include "msm8916-ufi.dtsi"
>> +
>> +/ {
>> +	model = "ufi-001c/ufi-001b 4G Modem Stick";
>> +	compatible = "thwc,ufi001c", "qcom,msm8916";
>> +};
>> +
>> +&button_restart {
>> +	gpios = <&msmgpio 37 GPIO_ACTIVE_HIGH>;
>> +};
>> +
>> +&led_r {
>> +	gpios = <&msmgpio 22 GPIO_ACTIVE_HIGH>;
>> +};
>> +
>> +&led_g {
>> +	gpios = <&msmgpio 21 GPIO_ACTIVE_HIGH>;
>> +};
>> +
>> +&led_b {
>> +	gpios = <&msmgpio 20 GPIO_ACTIVE_HIGH>;
>> +};
>> +
>> +&button_default {
>> +	pins = "gpio37";
>> +	bias-pull-down;
>> +};
>> +
>> +&gpio_leds_default {
>> +	pins = "gpio20", "gpio21", "gpio22";
>> +};
>> +
>> +&sim_ctrl_default {
>> +	pins = "gpio1", "gpio2";
> Identical between both boards, move to common please.
> 
>> +};
>> diff --git a/arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi b/arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi
>> new file mode 100644
>> index 0000000000000..286768d0fbe70
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi
>> @@ -0,0 +1,246 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +
>> +#include "msm8916-pm8916.dtsi"
>> +
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/leds/common.h>
>> +
>> +/ {
>> +	chassis-type = "embedded";
>> +
>> +	aliases {
>> +		serial0 = &blsp1_uart2;
>> +	};
>> +
>> +	chosen {
>> +		stdout-path = "serial0";
>> +	};
>> +
>> +	reserved-memory {
>> +		mpss_mem: mpss@86800000 {
>> +			reg = <0x0 0x86800000 0x0 0x5500000>;
>> +			no-map;
>> +		};
>> +
>> +		gps_mem: gps@8bd00000 {
>> +			reg = <0x0 0x8bd00000 0x0 0x200000>;
>> +			no-map;
>> +		};
>> +	};
>> +
>> +	gpio-keys {
>> +		compatible = "gpio-keys";
>> +
>> +		pinctrl-0 = <&button_default>;
>> +		pinctrl-names = "default";
>> +
>> +		label = "GPIO Buttons";
>> +
>> +		/* gpio is board-specific */
> GPIO is an acronym, please make it uppercase, as you
> did 10 lines below..
> 
>> +		button_restart: button-restart {
>> +			label = "Restart";
>> +			linux,code = <KEY_RESTART>;
>> +		};
>> +	};
>> +
>> +	leds {
>> +		compatible = "gpio-leds";
>> +
>> +		pinctrl-0 = <&gpio_leds_default>;
>> +		pinctrl-names = "default";
>> +
>> +		/* GPIOs are board-specific */
>> +		led_r: led-r {
>> +			color = <LED_COLOR_ID_RED>;
>> +			default-state = "on";
>> +			function = LED_FUNCTION_INDICATOR;
>> +		};
>> +
>> +		led_g: led-g {
>> +			color = <LED_COLOR_ID_GREEN>;
>> +			default-state = "off";
>> +			function = LED_FUNCTION_INDICATOR;
>> +		};
>> +
>> +		led_b: led-b {
>> +			color = <LED_COLOR_ID_BLUE>;
>> +			default-state = "off";
>> +			function = LED_FUNCTION_INDICATOR;
>> +		};
>> +	};
>> +};
>> +
>> +&bam_dmux {
>> +	status = "okay";
>> +};
>> +
>> +&bam_dmux_dma {
>> +	status = "okay";
>> +};
>> +
>> +&blsp1_uart2 {
>> +	status = "okay";
>> +};
>> +
>> +/* Remove &dsi_phy0 from clocks to make sure that gcc probes with display disabled */
> That sounds like a bad regression and shouldn't be an issue..
> 
>> +&gcc {
>> +	clocks = <&xo_board>, <&sleep_clk>, <0>, <0>, <0>, <0>, <0>;
>> +};
>> +
>> +&mpss {
>> +	pinctrl-0 = <&sim_ctrl_default>;
>> +	pinctrl-names = "default";
>> +
>> +	status = "okay";
>> +};
>> +
>> +&pm8916_usbin {
>> +	status = "okay";
>> +};
>> +
>> +&pronto {
>> +	status = "okay";
>> +};
>> +
>> +&sdhc_1 {
>> +	pinctrl-0 = <&sdc1_clk_on &sdc1_cmd_on &sdc1_data_on>;
>> +	pinctrl-1 = <&sdc1_clk_off &sdc1_cmd_off &sdc1_data_off>;
>> +	pinctrl-names = "default", "sleep";
>> +
>> +	status = "okay";
>> +};
>> +
>> +&usb {
>> +	extcon = <&pm8916_usbin>;
>> +	dr_mode = "peripheral";
>> +
>> +	status = "okay";
>> +};
>> +
>> +&usb_hs_phy {
>> +	extcon = <&pm8916_usbin>;
>> +};
>> +
>> +&smd_rpm_regulators {
> Please sort this reference alphabetically.
> 

The regulators and tlmm nodes are deliberately put last,
in line with all other msm8916 devices. We move them away
so they don't clutter more interesting nodes with these "dry"
and somewhat uninteresting voltage/pin definitions.

Thanks for the review,
Nikita

> Konrad
>> +	vdd_l1_l2_l3-supply = <&pm8916_s3>;
>> +	vdd_l4_l5_l6-supply = <&pm8916_s4>;
>> +	vdd_l7-supply = <&pm8916_s4>;
>> +
>> +	s3 {
>> +		regulator-min-microvolt = <1200000>;
>> +		regulator-max-microvolt = <1300000>;
>> +	};
>> +
>> +	s4 {
>> +		regulator-min-microvolt = <1800000>;
>> +		regulator-max-microvolt = <2100000>;
>> +	};
>> +
>> +	l1 {
>> +		regulator-min-microvolt = <1225000>;
>> +		regulator-max-microvolt = <1225000>;
>> +	};
>> +
>> +	l2 {
>> +		regulator-min-microvolt = <1200000>;
>> +		regulator-max-microvolt = <1200000>;
>> +	};
>> +
>> +	l4 {
>> +		regulator-min-microvolt = <2050000>;
>> +		regulator-max-microvolt = <2050000>;
>> +	};
>> +
>> +	l5 {
>> +		regulator-min-microvolt = <1800000>;
>> +		regulator-max-microvolt = <1800000>;
>> +	};
>> +
>> +	l6 {
>> +		regulator-min-microvolt = <1800000>;
>> +		regulator-max-microvolt = <1800000>;
>> +	};
>> +
>> +	l7 {
>> +		regulator-min-microvolt = <1800000>;
>> +		regulator-max-microvolt = <1800000>;
>> +	};
>> +
>> +	l8 {
>> +		regulator-min-microvolt = <2850000>;
>> +		regulator-max-microvolt = <2900000>;
>> +	};
>> +
>> +	l9 {
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +	};
>> +
>> +	l10 {
>> +		regulator-min-microvolt = <2700000>;
>> +		regulator-max-microvolt = <2800000>;
>> +	};
>> +
>> +	l11 {
>> +		regulator-min-microvolt = <1800000>;
>> +		regulator-max-microvolt = <2950000>;
>> +		regulator-system-load = <200000>;
>> +		regulator-allow-set-load;
>> +	};
>> +
>> +	l12 {
>> +		regulator-min-microvolt = <1800000>;
>> +		regulator-max-microvolt = <2950000>;
>> +	};
>> +
>> +	l13 {
>> +		regulator-min-microvolt = <3075000>;
>> +		regulator-max-microvolt = <3075000>;
>> +	};
>> +
>> +	l14 {
>> +		regulator-min-microvolt = <1800000>;
>> +		regulator-max-microvolt = <3300000>;
>> +	};
>> +
>> +	l15 {
>> +		regulator-min-microvolt = <1800000>;
>> +		regulator-max-microvolt = <3300000>;
>> +	};
>> +
>> +	l16 {
>> +		regulator-min-microvolt = <1800000>;
>> +		regulator-max-microvolt = <3300000>;
>> +	};
>> +
>> +	l17 {
>> +		regulator-min-microvolt = <2850000>;
>> +		regulator-max-microvolt = <2850000>;
>> +	};
>> +
>> +	l18 {
>> +		regulator-min-microvolt = <2700000>;
>> +		regulator-max-microvolt = <2700000>;
>> +	};
>> +};
>> +
>> +&msmgpio {
>> +	/* pins are board-specific */
>> +	button_default: button-default-state {
>> +		function = "gpio";
>> +		drive-strength = <2>;
>> +	};
>> +
>> +	gpio_leds_default: gpio-leds-default-state {
>> +		function = "gpio";
>> +		drive-strength = <2>;
>> +		bias-disable;
>> +	};
>> +
>> +	sim_ctrl_default: sim-ctrl-default-state {
>> +		function = "gpio";
>> +		drive-strength = <2>;
>> +		bias-disable;
>> +		output-low;
>> +	};
>> +};

Re: [PATCH v2 3/3] arm64: dts: qcom: msm8916-thwc: Add initial device tree for Tong Heng Wei Chuang Wifi/LTE dongle UFI-001C and uf896

[Konrad Dybcio]

On 12.01.2023 17:28, Nikita Travkin wrote:
> Konrad Dybcio писал(а) 12.01.2023 21:16:
>> On 11.01.2023 15:13, Yang Xiwen wrote:
>>> This commit adds support for the ufi-001C and uf896 WiFi/LTE dongle made by
>>> Tong Heng Wei Chuang based on MSM8916.
>>> uf896 is another variant for the usb stick. The board design
>>> differs by using different gpios for the keys and leds.
>>>
>>> Note: The original firmware does not support 64-bit OS. It is necessary
>>> to flash 64-bit TZ firmware to boot arm64.
>>>
>>> Currently supported:
>>> - All CPU cores
>>> - Buttons
>>> - LEDs
>>> - Modem
>>> - SDHC
>>> - USB Device Mode
>>> - UART
>>>
>>> Co-developed-by: Jaime Breva <jbreva@nayarsystems.com>
>>> Signed-off-by: Jaime Breva <jbreva@nayarsystems.com>
>>> Co-developed-by: Nikita Travkin <nikita@trvn.ru>
>>> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
>>> Signed-off-by: Yang Xiwen <forbidden405@foxmail.com>
>>> ---
>>>  arch/arm64/boot/dts/qcom/Makefile             |   2 +
>>>  .../boot/dts/qcom/msm8916-thwc-uf896.dts      |  41 +++
>>>  .../boot/dts/qcom/msm8916-thwc-ufi001c.dts    |  39 +++
>>>  arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi     | 246 ++++++++++++++++++
>>>  4 files changed, 328 insertions(+)
>>>  create mode 100644 arch/arm64/boot/dts/qcom/msm8916-thwc-uf896.dts
>>>  create mode 100644 arch/arm64/boot/dts/qcom/msm8916-thwc-ufi001c.dts
>>>  create mode 100644 arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
>>> index b42362c7be1bf..523dbe6d06be8 100644
>>> --- a/arch/arm64/boot/dts/qcom/Makefile
>>> +++ b/arch/arm64/boot/dts/qcom/Makefile
>>> @@ -21,6 +21,8 @@ dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-samsung-e7.dtb
>>>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-samsung-grandmax.dtb
>>>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-samsung-j5.dtb
>>>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-samsung-serranove.dtb
>>> +dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-thwc-uf896.dtb
>>> +dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-thwc-ufi001c.dtb
>>>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-wingtech-wt88047.dtb
>>>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8953-motorola-potter.dtb
>>>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8953-xiaomi-daisy.dtb
>>> diff --git a/arch/arm64/boot/dts/qcom/msm8916-thwc-uf896.dts b/arch/arm64/boot/dts/qcom/msm8916-thwc-uf896.dts
>>> new file mode 100644
>>> index 0000000000000..79447ca48dd3a
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/qcom/msm8916-thwc-uf896.dts
>>> @@ -0,0 +1,41 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +
>>> +/dts-v1/;
>>> +
>>> +#include "msm8916-ufi.dtsi"
>>> +
>>> +/ {
>>> +	model = "uf896 4G Modem Stick";
>>> +	compatible = "thwc,uf896", "qcom,msm8916";
>>> +};
>>> +
>>> +&button_restart {
>>> +	gpios = <&msmgpio 35 GPIO_ACTIVE_LOW>;
>>> +};
>>> +
>>> +&led_r {
>>> +	gpios = <&msmgpio 82 GPIO_ACTIVE_HIGH>;
>>> +};
>>> +
>>> +&led_g {
>>> +	gpios = <&msmgpio 83 GPIO_ACTIVE_HIGH>;
>>> +	function = LED_FUNCTION_WLAN;
>> This is a software property, maybe it would be worth
>> commonizing?
>>
> 
> The functions are put in-line with the labels on the housing.
> The uf896 has green and blue leds labeled with icons but I was told
> by Yang that the uf001c doesn't have any labels.
Okay, that probably deserves a comment.

> 
>>> +};
>>> +
>>> +&led_b {
>>> +	gpios = <&msmgpio 81 GPIO_ACTIVE_HIGH>;
>>> +	function = LED_FUNCTION_WAN;
>>> +};
>>> +
>>> +&button_default {
>>> +	pins = "gpio35";
>>> +	bias-pull-up;
>>> +};
>>> +
>>> +&gpio_leds_default {
>>> +	pins = "gpio81", "gpio82", "gpio83";
>>> +};
>>> +
>>> +&sim_ctrl_default {
>>> +	pins = "gpio1", "gpio2";
>>> +};
>>> diff --git a/arch/arm64/boot/dts/qcom/msm8916-thwc-ufi001c.dts b/arch/arm64/boot/dts/qcom/msm8916-thwc-ufi001c.dts
>>> new file mode 100644
>>> index 0000000000000..700cf81cbf8c0
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/qcom/msm8916-thwc-ufi001c.dts
>>> @@ -0,0 +1,39 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +
>>> +/dts-v1/;
>>> +
>>> +#include "msm8916-ufi.dtsi"
>>> +
>>> +/ {
>>> +	model = "ufi-001c/ufi-001b 4G Modem Stick";
>>> +	compatible = "thwc,ufi001c", "qcom,msm8916";
>>> +};
>>> +
>>> +&button_restart {
>>> +	gpios = <&msmgpio 37 GPIO_ACTIVE_HIGH>;
>>> +};
>>> +
>>> +&led_r {
>>> +	gpios = <&msmgpio 22 GPIO_ACTIVE_HIGH>;
>>> +};
>>> +
>>> +&led_g {
>>> +	gpios = <&msmgpio 21 GPIO_ACTIVE_HIGH>;
>>> +};
>>> +
>>> +&led_b {
>>> +	gpios = <&msmgpio 20 GPIO_ACTIVE_HIGH>;
>>> +};
>>> +
>>> +&button_default {
>>> +	pins = "gpio37";
>>> +	bias-pull-down;
>>> +};
>>> +
>>> +&gpio_leds_default {
>>> +	pins = "gpio20", "gpio21", "gpio22";
>>> +};
>>> +
>>> +&sim_ctrl_default {
>>> +	pins = "gpio1", "gpio2";
>> Identical between both boards, move to common please.
>>
>>> +};
>>> diff --git a/arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi b/arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi
>>> new file mode 100644
>>> index 0000000000000..286768d0fbe70
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi
>>> @@ -0,0 +1,246 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +
>>> +#include "msm8916-pm8916.dtsi"
>>> +
>>> +#include <dt-bindings/gpio/gpio.h>
>>> +#include <dt-bindings/leds/common.h>
>>> +
>>> +/ {
>>> +	chassis-type = "embedded";
>>> +
>>> +	aliases {
>>> +		serial0 = &blsp1_uart2;
>>> +	};
>>> +
>>> +	chosen {
>>> +		stdout-path = "serial0";
>>> +	};
>>> +
>>> +	reserved-memory {
>>> +		mpss_mem: mpss@86800000 {
>>> +			reg = <0x0 0x86800000 0x0 0x5500000>;
>>> +			no-map;
>>> +		};
>>> +
>>> +		gps_mem: gps@8bd00000 {
>>> +			reg = <0x0 0x8bd00000 0x0 0x200000>;
>>> +			no-map;
>>> +		};
>>> +	};
>>> +
>>> +	gpio-keys {
>>> +		compatible = "gpio-keys";
>>> +
>>> +		pinctrl-0 = <&button_default>;
>>> +		pinctrl-names = "default";
>>> +
>>> +		label = "GPIO Buttons";
>>> +
>>> +		/* gpio is board-specific */
>> GPIO is an acronym, please make it uppercase, as you
>> did 10 lines below..
>>
>>> +		button_restart: button-restart {
>>> +			label = "Restart";
>>> +			linux,code = <KEY_RESTART>;
>>> +		};
>>> +	};
>>> +
>>> +	leds {
>>> +		compatible = "gpio-leds";
>>> +
>>> +		pinctrl-0 = <&gpio_leds_default>;
>>> +		pinctrl-names = "default";
>>> +
>>> +		/* GPIOs are board-specific */
>>> +		led_r: led-r {
>>> +			color = <LED_COLOR_ID_RED>;
>>> +			default-state = "on";
>>> +			function = LED_FUNCTION_INDICATOR;
>>> +		};
>>> +
>>> +		led_g: led-g {
>>> +			color = <LED_COLOR_ID_GREEN>;
>>> +			default-state = "off";
>>> +			function = LED_FUNCTION_INDICATOR;
>>> +		};
>>> +
>>> +		led_b: led-b {
>>> +			color = <LED_COLOR_ID_BLUE>;
>>> +			default-state = "off";
>>> +			function = LED_FUNCTION_INDICATOR;
>>> +		};
>>> +	};
>>> +};
>>> +
>>> +&bam_dmux {
>>> +	status = "okay";
>>> +};
>>> +
>>> +&bam_dmux_dma {
>>> +	status = "okay";
>>> +};
>>> +
>>> +&blsp1_uart2 {
>>> +	status = "okay";
>>> +};
>>> +
>>> +/* Remove &dsi_phy0 from clocks to make sure that gcc probes with display disabled */
>> That sounds like a bad regression and shouldn't be an issue..
>>
>>> +&gcc {
>>> +	clocks = <&xo_board>, <&sleep_clk>, <0>, <0>, <0>, <0>, <0>;
>>> +};
>>> +
>>> +&mpss {
>>> +	pinctrl-0 = <&sim_ctrl_default>;
>>> +	pinctrl-names = "default";
>>> +
>>> +	status = "okay";
>>> +};
>>> +
>>> +&pm8916_usbin {
>>> +	status = "okay";
>>> +};
>>> +
>>> +&pronto {
>>> +	status = "okay";
>>> +};
>>> +
>>> +&sdhc_1 {
>>> +	pinctrl-0 = <&sdc1_clk_on &sdc1_cmd_on &sdc1_data_on>;
>>> +	pinctrl-1 = <&sdc1_clk_off &sdc1_cmd_off &sdc1_data_off>;
>>> +	pinctrl-names = "default", "sleep";
>>> +
>>> +	status = "okay";
>>> +};
>>> +
>>> +&usb {
>>> +	extcon = <&pm8916_usbin>;
>>> +	dr_mode = "peripheral";
>>> +
>>> +	status = "okay";
>>> +};
>>> +
>>> +&usb_hs_phy {
>>> +	extcon = <&pm8916_usbin>;
>>> +};
>>> +
>>> +&smd_rpm_regulators {
>> Please sort this reference alphabetically.
>>
> 
> The regulators and tlmm nodes are deliberately put last,
> in line with all other msm8916 devices. We move them away
> so they don't clutter more interesting nodes with these "dry"
> and somewhat uninteresting voltage/pin definitions.
> 
> Thanks for the review,
> Nikita
Ugh, I don't know anymore.. wild west..

Konrad
> 
>> Konrad
>>> +	vdd_l1_l2_l3-supply = <&pm8916_s3>;
>>> +	vdd_l4_l5_l6-supply = <&pm8916_s4>;
>>> +	vdd_l7-supply = <&pm8916_s4>;
>>> +
>>> +	s3 {
>>> +		regulator-min-microvolt = <1200000>;
>>> +		regulator-max-microvolt = <1300000>;
>>> +	};
>>> +
>>> +	s4 {
>>> +		regulator-min-microvolt = <1800000>;
>>> +		regulator-max-microvolt = <2100000>;
>>> +	};
>>> +
>>> +	l1 {
>>> +		regulator-min-microvolt = <1225000>;
>>> +		regulator-max-microvolt = <1225000>;
>>> +	};
>>> +
>>> +	l2 {
>>> +		regulator-min-microvolt = <1200000>;
>>> +		regulator-max-microvolt = <1200000>;
>>> +	};
>>> +
>>> +	l4 {
>>> +		regulator-min-microvolt = <2050000>;
>>> +		regulator-max-microvolt = <2050000>;
>>> +	};
>>> +
>>> +	l5 {
>>> +		regulator-min-microvolt = <1800000>;
>>> +		regulator-max-microvolt = <1800000>;
>>> +	};
>>> +
>>> +	l6 {
>>> +		regulator-min-microvolt = <1800000>;
>>> +		regulator-max-microvolt = <1800000>;
>>> +	};
>>> +
>>> +	l7 {
>>> +		regulator-min-microvolt = <1800000>;
>>> +		regulator-max-microvolt = <1800000>;
>>> +	};
>>> +
>>> +	l8 {
>>> +		regulator-min-microvolt = <2850000>;
>>> +		regulator-max-microvolt = <2900000>;
>>> +	};
>>> +
>>> +	l9 {
>>> +		regulator-min-microvolt = <3300000>;
>>> +		regulator-max-microvolt = <3300000>;
>>> +	};
>>> +
>>> +	l10 {
>>> +		regulator-min-microvolt = <2700000>;
>>> +		regulator-max-microvolt = <2800000>;
>>> +	};
>>> +
>>> +	l11 {
>>> +		regulator-min-microvolt = <1800000>;
>>> +		regulator-max-microvolt = <2950000>;
>>> +		regulator-system-load = <200000>;
>>> +		regulator-allow-set-load;
>>> +	};
>>> +
>>> +	l12 {
>>> +		regulator-min-microvolt = <1800000>;
>>> +		regulator-max-microvolt = <2950000>;
>>> +	};
>>> +
>>> +	l13 {
>>> +		regulator-min-microvolt = <3075000>;
>>> +		regulator-max-microvolt = <3075000>;
>>> +	};
>>> +
>>> +	l14 {
>>> +		regulator-min-microvolt = <1800000>;
>>> +		regulator-max-microvolt = <3300000>;
>>> +	};
>>> +
>>> +	l15 {
>>> +		regulator-min-microvolt = <1800000>;
>>> +		regulator-max-microvolt = <3300000>;
>>> +	};
>>> +
>>> +	l16 {
>>> +		regulator-min-microvolt = <1800000>;
>>> +		regulator-max-microvolt = <3300000>;
>>> +	};
>>> +
>>> +	l17 {
>>> +		regulator-min-microvolt = <2850000>;
>>> +		regulator-max-microvolt = <2850000>;
>>> +	};
>>> +
>>> +	l18 {
>>> +		regulator-min-microvolt = <2700000>;
>>> +		regulator-max-microvolt = <2700000>;
>>> +	};
>>> +};
>>> +
>>> +&msmgpio {
>>> +	/* pins are board-specific */
>>> +	button_default: button-default-state {
>>> +		function = "gpio";
>>> +		drive-strength = <2>;
>>> +	};
>>> +
>>> +	gpio_leds_default: gpio-leds-default-state {
>>> +		function = "gpio";
>>> +		drive-strength = <2>;
>>> +		bias-disable;
>>> +	};
>>> +
>>> +	sim_ctrl_default: sim-ctrl-default-state {
>>> +		function = "gpio";
>>> +		drive-strength = <2>;
>>> +		bias-disable;
>>> +		output-low;
>>> +	};
>>> +};

RE: [PATCH v2 3/3] arm64: dts: qcom: msm8916-thwc: Add initial device tree for Tong Heng Wei Chuang Wifi/LTE dongle UFI-001C and uf896

[forbidden405]

On 11.01.2023 16:13, Konrad Dybcio wrote:
> > +&sim_ctrl_default {
> > +	pins = "gpio1", "gpio2";
> Identical between both boards, move to common please.

It is just happened to be identical for these two boards, but not true 
For many other devices that will be supported sooner or later.

Best regards,
Yang