[PATCH v2 0/2] arm64: dts: qcom: msm8939-samsung-a7: Add initial dts

[PATCH v2 0/2] arm64: dts: qcom: msm8939-samsung-a7: Add initial dts

[Lin, Meng-Bo]

v2: Squash patches

This dts adds support for Samsung Galaxy A7 smartphone released in 2015.

Add a device tree for A7 with initial support for:

- GPIO keys
- Hall Sensor
- SDHCI (internal and external storage)
- USB Device Mode
- UART (on USB connector via the SM5502 MUIC)
- WCNSS (WiFi/BT)
- Regulators
- Touch key
- Accelerometer/Magnetometer
- Fuelgauge
- NFC
- Vibrator
- Touchscreen

[PATCH v2 1/2] dt-bindings: qcom: Document msm8939,a7

[Lin, Meng-Bo]

Document samsung,a7 bindings used in its device tree.

Signed-off-by: Lin, Meng-Bo <linmengbo0689@protonmail.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 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 450f616774e0..a6f7ef4f0830 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -186,6 +186,7 @@ properties:
 
       - items:
           - enum:
+              - samsung,a7
               - sony,kanuti-tulip
               - square,apq8039-t2
           - const: qcom,msm8939
-- 
2.39.2

[PATCH v2 2/2] arm64: dts: qcom: msm8939-samsung-a7: Add initial dts

[Lin, Meng-Bo]

This dts adds support for Samsung Galaxy A7 smartphone released in 2015.

Add a device tree for A7 with initial support for:

- GPIO keys
- Hall Sensor
- SDHCI (internal and external storage)
- USB Device Mode
- UART (on USB connector via the SM5502 MUIC)
- WCNSS (WiFi/BT)
- Regulators
- Touch key
- Accelerometer/Magnetometer
- Fuelgauge
- NFC
- Vibrator
- Touchscreen

Signed-off-by: Lin, Meng-Bo <linmengbo0689@protonmail.com>
---
 arch/arm64/boot/dts/qcom/Makefile             |   1 +
 .../boot/dts/qcom/msm8939-samsung-a7.dts      | 495 ++++++++++++++++++
 2 files changed, 496 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/msm8939-samsung-a7.dts

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 337abc4ceb17..23fd31d4bf5a 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -39,6 +39,7 @@ 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)	+= msm8916-yiming-uz801v3.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= msm8939-samsung-a7.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8939-sony-xperia-kanuti-tulip.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/msm8939-samsung-a7.dts b/arch/arm64/boot/dts/qcom/msm8939-samsung-a7.dts
new file mode 100644
index 000000000000..66e56ac59998
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8939-samsung-a7.dts
@@ -0,0 +1,495 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/dts-v1/;
+
+#include "msm8939-pm8916.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+
+/ {
+	model = "Samsung Galaxy A7 (2015)";
+	compatible = "samsung,a7", "qcom,msm8939";
+	chassis-type = "handset";
+
+	aliases {
+		mmc0 = &sdhc_1; /* SDC1 eMMC slot */
+		mmc1 = &sdhc_2; /* SDC2 SD card slot */
+		serial0 = &blsp_uart2;
+	};
+
+	chosen {
+		stdout-path = "serial0";
+	};
+
+	reserved-memory {
+		/* Additional memory used by Samsung firmware modifications */
+		tz-apps@85500000 {
+			reg = <0x0 0x85500000 0x0 0xb00000>;
+			no-map;
+		};
+	};
+
+	gpio-hall-sensor {
+		compatible = "gpio-keys";
+
+		pinctrl-0 = <&gpio_hall_sensor_default>;
+		pinctrl-names = "default";
+
+		label = "GPIO Hall Effect Sensor";
+
+		event-hall-sensor {
+			label = "Hall Effect Sensor";
+			gpios = <&tlmm 52 GPIO_ACTIVE_LOW>;
+			linux,input-type = <EV_SW>;
+			linux,code = <SW_LID>;
+			linux,can-disable;
+		};
+	};
+
+	gpio-keys {
+		compatible = "gpio-keys";
+
+		pinctrl-0 = <&gpio_keys_default>;
+		pinctrl-names = "default";
+
+		label = "GPIO Buttons";
+
+		button-volume-up {
+			label = "Volume Up";
+			gpios = <&tlmm 107 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_VOLUMEUP>;
+		};
+
+		button-home {
+			label = "Home";
+			gpios = <&tlmm 109 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_HOMEPAGE>;
+		};
+	};
+
+	i2c-fg {
+		compatible = "i2c-gpio";
+		sda-gpios = <&tlmm 106 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
+		scl-gpios = <&tlmm 105 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
+
+		pinctrl-0 = <&fg_i2c_default>;
+		pinctrl-names = "default";
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		battery@35 {
+			compatible = "richtek,rt5033-battery";
+			reg = <0x35>;
+
+			interrupt-parent = <&tlmm>;
+			interrupts = <121 IRQ_TYPE_EDGE_BOTH>;
+
+			pinctrl-0 = <&fg_alert_default>;
+			pinctrl-names = "default";
+		};
+	};
+
+	i2c-nfc {
+		compatible = "i2c-gpio";
+		sda-gpios = <&tlmm 0 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
+		scl-gpios = <&tlmm 1 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
+
+		pinctrl-0 = <&nfc_i2c_default>;
+		pinctrl-names = "default";
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		nfc@2b {
+			compatible = "nxp,pn547", "nxp,nxp-nci-i2c";
+			reg = <0x2b>;
+
+			interrupt-parent = <&tlmm>;
+			interrupts = <21 IRQ_TYPE_EDGE_RISING>;
+
+			enable-gpios = <&tlmm 116 GPIO_ACTIVE_HIGH>;
+			firmware-gpios = <&tlmm 49 GPIO_ACTIVE_HIGH>;
+
+			pinctrl-0 = <&nfc_default>;
+			pinctrl-names = "default";
+		};
+	};
+
+	i2c-sensor {
+		compatible = "i2c-gpio";
+		sda-gpios = <&tlmm 84 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
+		scl-gpios = <&tlmm 85 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
+
+		pinctrl-0 = <&sensor_i2c_default>;
+		pinctrl-names = "default";
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		accelerometer: accelerometer@10 {
+			compatible = "bosch,bmc150_accel";
+			reg = <0x10>;
+			interrupt-parent = <&tlmm>;
+			interrupts = <115 IRQ_TYPE_EDGE_RISING>;
+
+			vdd-supply = <&pm8916_l17>;
+			vddio-supply = <&pm8916_l5>;
+
+			pinctrl-0 = <&accel_int_default>;
+			pinctrl-names = "default";
+
+			mount-matrix = "-1", "0", "0",
+					"0", "-1", "0",
+					"0", "0", "1";
+		};
+
+		magnetometer@12 {
+			compatible = "bosch,bmc150_magn";
+			reg = <0x12>;
+
+			vdd-supply = <&pm8916_l17>;
+			vddio-supply = <&pm8916_l5>;
+		};
+	};
+
+	i2c-tkey {
+		compatible = "i2c-gpio";
+		sda-gpios = <&tlmm 16 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
+		scl-gpios = <&tlmm 17 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
+
+		pinctrl-0 = <&tkey_i2c_default>;
+		pinctrl-names = "default";
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		touchkey@20 {
+			/* Note: Actually an ABOV MCU that implements same interface */
+			compatible = "coreriver,tc360-touchkey";
+			reg = <0x20>;
+
+			interrupt-parent = <&tlmm>;
+			interrupts = <20 IRQ_TYPE_EDGE_FALLING>;
+
+			vcc-supply = <&reg_touch_key>;
+			vdd-supply = <&reg_keyled>;
+			vddio-supply = <&pm8916_l6>;
+
+			linux,keycodes = <KEY_APPSELECT KEY_BACK>;
+
+			pinctrl-0 = <&tkey_default>;
+			pinctrl-names = "default";
+		};
+	};
+
+	pwm_vibrator: pwm-vibrator {
+		compatible = "clk-pwm";
+		#pwm-cells = <2>;
+
+		clocks = <&gcc GCC_GP2_CLK>;
+
+		pinctrl-0 = <&motor_pwm_default>;
+		pinctrl-names = "default";
+	};
+
+	reg_keyled: regulator-keyled {
+		compatible = "regulator-fixed";
+		regulator-name = "keyled";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		/* NOTE: On some variants e.g. SM-A700FD it's GPIO 91 */
+		gpio = <&tlmm 100 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-0 = <&tkey_led_en_default>;
+		pinctrl-names = "default";
+	};
+
+	reg_touch_key: regulator-touch-key {
+		compatible = "regulator-fixed";
+		regulator-name = "touch_key";
+		regulator-min-microvolt = <2800000>;
+		regulator-max-microvolt = <2800000>;
+
+		gpio = <&tlmm 56 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-0 = <&tkey_en_default>;
+		pinctrl-names = "default";
+	};
+
+	reg_tsp_vdd: regulator-tsp-vdd {
+		compatible = "regulator-fixed";
+		regulator-name = "tsp_vdd";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		vin-supply = <&pm8916_s4>;
+
+		gpio = <&tlmm 8 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-0 = <&reg_tsp_io_en_default>;
+		pinctrl-names = "default";
+	};
+
+	reg_vdd_tsp: regulator-vdd-tsp {
+		compatible = "regulator-fixed";
+		regulator-name = "vdd_tsp";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpio = <&tlmm 73 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-0 = <&reg_tsp_en_default>;
+		pinctrl-names = "default";
+	};
+
+	reg_vibrator: regulator-vibrator {
+		compatible = "regulator-fixed";
+		regulator-name = "motor_en";
+		regulator-min-microvolt = <3000000>;
+		regulator-max-microvolt = <3000000>;
+
+		gpio = <&tlmm 86 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-0 = <&motor_en_default>;
+		pinctrl-names = "default";
+	};
+
+	vibrator {
+		compatible = "pwm-vibrator";
+
+		pwms = <&pwm_vibrator 0 100000>;
+		pwm-names = "enable";
+
+		vcc-supply = <&reg_vibrator>;
+	};
+};
+
+&blsp_i2c1 {
+	status = "okay";
+
+	muic: extcon@25 {
+		compatible = "siliconmitus,sm5502-muic";
+		reg = <0x25>;
+
+		interrupt-parent = <&tlmm>;
+		interrupts = <12 IRQ_TYPE_EDGE_FALLING>;
+
+		pinctrl-0 = <&muic_int_default>;
+		pinctrl-names = "default";
+	};
+};
+
+&blsp_i2c5 {
+	status = "okay";
+
+	touchscreen@24 {
+		compatible = "cypress,tt21000";
+
+		reg = <0x24>;
+		interrupt-parent = <&tlmm>;
+		interrupts = <13 IRQ_TYPE_EDGE_FALLING>;
+
+		vdd-supply = <&reg_vdd_tsp>;
+		vddio-supply = <&reg_tsp_vdd>;
+
+		pinctrl-0 = <&tsp_int_default>;
+		pinctrl-names = "default";
+	};
+};
+
+&blsp_uart2 {
+	status = "okay";
+};
+
+&pm8916_resin {
+	linux,code = <KEY_VOLUMEDOWN>;
+	status = "okay";
+};
+
+&pm8916_rpm_regulators {
+	pm8916_l17: l17 {
+		regulator-min-microvolt = <2850000>;
+		regulator-max-microvolt = <2850000>;
+	};
+};
+
+&sdhc_1 {
+	status = "okay";
+};
+
+&sdhc_2 {
+	pinctrl-0 = <&sdc2_default &sdc2_cd_default>;
+	pinctrl-1 = <&sdc2_sleep &sdc2_cd_default>;
+	pinctrl-names = "default", "sleep";
+
+	cd-gpios = <&tlmm 38 GPIO_ACTIVE_LOW>;
+
+	status = "okay";
+};
+
+&usb {
+	extcon = <&muic>, <&muic>;
+	status = "okay";
+};
+
+&usb_hs_phy {
+	extcon = <&muic>;
+};
+
+&wcnss {
+	status = "okay";
+};
+
+&wcnss_iris {
+	compatible = "qcom,wcn3660b";
+};
+
+&tlmm {
+	accel_int_default: accel-int-default-state {
+		pins = "gpio115";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	fg_alert_default: fg-alert-default-state {
+		pins = "gpio121";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	fg_i2c_default: fg-i2c-default-state {
+		pins = "gpio105", "gpio106";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	gpio_hall_sensor_default: gpio-hall-sensor-default-state {
+		pins = "gpio52";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	gpio_keys_default: gpio-keys-default-state {
+		pins = "gpio107", "gpio109";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-pull-up;
+	};
+
+	motor_en_default: motor-en-default-state {
+		pins = "gpio86";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	motor_pwm_default: motor-pwm-default-state {
+		pins = "gpio50";
+		function = "gcc_gp2_clk_a";
+	};
+
+	muic_int_default: muic-int-default-state {
+		pins = "gpio12";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	nfc_default: nfc-default-state {
+		irq-pins {
+			pins = "gpio21";
+			function = "gpio";
+			drive-strength = <2>;
+			bias-pull-down;
+		};
+
+		nfc-pins {
+			pins = "gpio49", "gpio116";
+			function = "gpio";
+			drive-strength = <2>;
+			bias-disable;
+		};
+	};
+
+	nfc_i2c_default: nfc-i2c-default-state {
+		pins = "gpio0", "gpio1";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	reg_tsp_en_default: reg-tsp-en-default-state {
+		pins = "gpio73";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	reg_tsp_io_en_default: reg-tsp-io-en-default-state {
+		pins = "gpio8";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	sdc2_cd_default: sdc2-cd-default-state {
+		pins = "gpio38";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-pull-up;
+	};
+
+	sensor_i2c_default: sensor-i2c-default-state {
+		pins = "gpio84", "gpio85";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	tkey_default: tkey-default-state {
+		pins = "gpio20";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	tkey_en_default: tkey-en-default-state {
+		pins = "gpio56";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	tkey_i2c_default: tkey-i2c-default-state {
+		pins = "gpio16", "gpio17";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	tkey_led_en_default: tkey-led-en-default-state {
+		pins = "gpio100";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	tsp_int_default: tsp-int-default-state {
+		pins = "gpio13";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+};
-- 
2.39.2

Re: [PATCH v2 2/2] arm64: dts: qcom: msm8939-samsung-a7: Add initial dts

[Bryan O'Donoghue]

On 23/06/2023 11:02, Lin, Meng-Bo wrote:
> This dts adds support for Samsung Galaxy A7 smartphone released in 2015.
> 
> Add a device tree for A7 with initial support for:
> 
> - GPIO keys
> - Hall Sensor
> - SDHCI (internal and external storage)
> - USB Device Mode
> - UART (on USB connector via the SM5502 MUIC)
> - WCNSS (WiFi/BT)
> - Regulators
> - Touch key
> - Accelerometer/Magnetometer
> - Fuelgauge
> - NFC
> - Vibrator
> - Touchscreen
> 
> Signed-off-by: Lin, Meng-Bo <linmengbo0689@protonmail.com>
> ---
>   arch/arm64/boot/dts/qcom/Makefile             |   1 +
>   .../boot/dts/qcom/msm8939-samsung-a7.dts      | 495 ++++++++++++++++++
>   2 files changed, 496 insertions(+)
>   create mode 100644 arch/arm64/boot/dts/qcom/msm8939-samsung-a7.dts
> 
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index 337abc4ceb17..23fd31d4bf5a 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -39,6 +39,7 @@ 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)	+= msm8916-yiming-uz801v3.dtb
> +dtb-$(CONFIG_ARCH_QCOM)	+= msm8939-samsung-a7.dtb
>   dtb-$(CONFIG_ARCH_QCOM)	+= msm8939-sony-xperia-kanuti-tulip.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/msm8939-samsung-a7.dts b/arch/arm64/boot/dts/qcom/msm8939-samsung-a7.dts
> new file mode 100644
> index 000000000000..66e56ac59998
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8939-samsung-a7.dts
> @@ -0,0 +1,495 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/dts-v1/;
> +
> +#include "msm8939-pm8916.dtsi"
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +
> +/ {
> +	model = "Samsung Galaxy A7 (2015)";
> +	compatible = "samsung,a7", "qcom,msm8939";
> +	chassis-type = "handset";

Will the downstream bootloader accept this dts without

         // This is used by the bootloader to find the correct DTB
         qcom,msm-id = <239 0>;
         qcom,board-id = <0xEF08FF1 1>;

?

https://github.com/msm8916-mainline/lk2nd/blob/master/dts/msm8916/msm8939-samsung-r01.dts#L10

---
bod

Re: [PATCH v2 2/2] arm64: dts: qcom: msm8939-samsung-a7: Add initial dts

[Lin, Meng-Bo]

Hi Bryan,

On Friday, June 23rd, 2023 at 10:27 AM, Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:

> > +++ b/arch/arm64/boot/dts/qcom/msm8939-samsung-a7.dts
> > @@ -0,0 +1,495 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +
> > +/dts-v1/;
> > +
> > +#include "msm8939-pm8916.dtsi"
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/input/input.h>
> > +#include <dt-bindings/interrupt-controller/irq.h>
> > +
> > +/ {
> > + model = "Samsung Galaxy A7 (2015)";
> > + compatible = "samsung,a7", "qcom,msm8939";
> > + chassis-type = "handset";
> 
> 
> Will the downstream bootloader accept this dts without
> 
> // This is used by the bootloader to find the correct DTB
> qcom,msm-id = <239 0>;
> 
> qcom,board-id = <0xEF08FF1 1>;
> 
> 
> ?
> 
> https://github.com/msm8916-mainline/lk2nd/blob/master/dts/msm8916/msm8939-samsung-r01.dts#L10
> 

Similar to A3 and A5, and the other msm8916 devices, with lk2nd,
"qcom,msm-id" or "qcom,board-id" are not required in mainline to boot
this dts.
If I understand correctly, lk2nd will attempt to boot an image with any
qcdt appended.

https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/qcom/msm8916-samsung-a3u-eur.dts
https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/qcom/msm8916-samsung-a5u-eur.dts

Regards,
Lin

Re: [PATCH v2 2/2] arm64: dts: qcom: msm8939-samsung-a7: Add initial dts

[Bryan O'Donoghue]

On 23/06/2023 11:47, Lin, Meng-Bo wrote:
> Hi Bryan,
> 
> On Friday, June 23rd, 2023 at 10:27 AM, Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:
> 
>>> +++ b/arch/arm64/boot/dts/qcom/msm8939-samsung-a7.dts
>>> @@ -0,0 +1,495 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +
>>> +/dts-v1/;
>>> +
>>> +#include "msm8939-pm8916.dtsi"
>>> +
>>> +#include <dt-bindings/gpio/gpio.h>
>>> +#include <dt-bindings/input/input.h>
>>> +#include <dt-bindings/interrupt-controller/irq.h>
>>> +
>>> +/ {
>>> + model = "Samsung Galaxy A7 (2015)";
>>> + compatible = "samsung,a7", "qcom,msm8939";
>>> + chassis-type = "handset";
>>
>>
>> Will the downstream bootloader accept this dts without
>>
>> // This is used by the bootloader to find the correct DTB
>> qcom,msm-id = <239 0>;
>>
>> qcom,board-id = <0xEF08FF1 1>;
>>
>>
>> ?
>>
>> https://github.com/msm8916-mainline/lk2nd/blob/master/dts/msm8916/msm8939-samsung-r01.dts#L10
>>
> 
> Similar to A3 and A5, and the other msm8916 devices, with lk2nd,
> "qcom,msm-id" or "qcom,board-id" are not required in mainline to boot
> this dts.
> If I understand correctly, lk2nd will attempt to boot an image with any
> qcdt appended.
> 
> https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/qcom/msm8916-samsung-a3u-eur.dts
> https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/qcom/msm8916-samsung-a5u-eur.dts

I understand.

IMO the upstream DTS should work without depending on lk2nd.

I'd add the board and msm id to the DTS for that reason.

If not then I'd put a comment into the DTS explaining the dependency.

For preference the upstream dts should *just work* as much as possible 
without requiring churning of bootloader.

---
bod

Re: [PATCH v2 2/2] arm64: dts: qcom: msm8939-samsung-a7: Add initial dts

[Stephan Gerhold]

On Fri, Jun 23, 2023 at 11:52:25AM +0100, Bryan O'Donoghue wrote:
> On 23/06/2023 11:47, Lin, Meng-Bo wrote:
> > On Friday, June 23rd, 2023 at 10:27 AM, Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:
> > 
> > > > +++ b/arch/arm64/boot/dts/qcom/msm8939-samsung-a7.dts
> > > > @@ -0,0 +1,495 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > +
> > > > +/dts-v1/;
> > > > +
> > > > +#include "msm8939-pm8916.dtsi"
> > > > +
> > > > +#include <dt-bindings/gpio/gpio.h>
> > > > +#include <dt-bindings/input/input.h>
> > > > +#include <dt-bindings/interrupt-controller/irq.h>
> > > > +
> > > > +/ {
> > > > + model = "Samsung Galaxy A7 (2015)";
> > > > + compatible = "samsung,a7", "qcom,msm8939";
> > > > + chassis-type = "handset";
> > > 
> > > 
> > > Will the downstream bootloader accept this dts without
> > > 
> > > // This is used by the bootloader to find the correct DTB
> > > qcom,msm-id = <239 0>;
> > > 
> > > qcom,board-id = <0xEF08FF1 1>;
> > > 
> > > 
> > > ?
> > > 
> > > https://github.com/msm8916-mainline/lk2nd/blob/master/dts/msm8916/msm8939-samsung-r01.dts#L10
> > > 
> > 
> > Similar to A3 and A5, and the other msm8916 devices, with lk2nd,
> > "qcom,msm-id" or "qcom,board-id" are not required in mainline to boot
> > this dts.
> > If I understand correctly, lk2nd will attempt to boot an image with any
> > qcdt appended.
> > 
> > https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/qcom/msm8916-samsung-a3u-eur.dts
> > https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/qcom/msm8916-samsung-a5u-eur.dts
> 
> I understand.
> 
> IMO the upstream DTS should work without depending on lk2nd.
> 
> I'd add the board and msm id to the DTS for that reason.
> 
> If not then I'd put a comment into the DTS explaining the dependency.
> 
> For preference the upstream dts should *just work* as much as possible
> without requiring churning of bootloader.
> 

Is it really worth it to support a half-working bootloader though?

No one will ever be able to use this properly without fixing the
bootloader. SMP doesn't work with the stock bootloader, many devices
need display panel selection in the bootloader and on some Samsung
devices there is not even USB and UART without special fixes in the
bootloader.

In most cases it's easy to just add the qcom,msm-id/qcom,board-id but
there are also some cases with particularly weird bootloaders that are
more complicated.

I would just like to avoid forcing everyone to waste time testing with
the half-working or entirely broken stock bootloaders. We could add the
(guessed) properties without testing but I'm not sure if that's useful.

Thanks,
Stephan

Re: [PATCH v2 2/2] arm64: dts: qcom: msm8939-samsung-a7: Add initial dts

[Bryan O'Donoghue]

On 25/06/2023 20:43, Stephan Gerhold wrote:
> Is it really worth it to support a half-working bootloader though?

> No one will ever be able to use this properly without fixing the
> bootloader. SMP doesn't work with the stock bootloader, many devices
> need display panel selection in the bootloader and on some Samsung
> devices there is not even USB and UART without special fixes in the
> bootloader.

Why set the bar higher than necessary to boot a kernel though ?

Its two lines in a dts.

---
bod

Re: [PATCH v2 2/2] arm64: dts: qcom: msm8939-samsung-a7: Add initial dts

[Stephan Gerhold]

On Mon, Jun 26, 2023 at 01:05:22AM +0100, Bryan O'Donoghue wrote:
> On 25/06/2023 20:43, Stephan Gerhold wrote:
> > Is it really worth it to support a half-working bootloader though?
> 
> > No one will ever be able to use this properly without fixing the
> > bootloader. SMP doesn't work with the stock bootloader, many devices
> > need display panel selection in the bootloader and on some Samsung
> > devices there is not even USB and UART without special fixes in the
> > bootloader.
> 
> Why set the bar higher than necessary to boot a kernel though ?
> 
> Its two lines in a dts.
> 

I see your point but after adding some really weird workarounds for
various devices in lk2nd I can't help seeing it more complicated.

Adding "just two lines in a dts" is the general case, but I can also
give plenty other examples where one has to:

  - Duplicate the entire downstream MDSS/MDP/display DT nodes because
    the bootloader uses them to initialize the display (older Sony
    MSM8974 devices) [1]

  - Add random DT nodes because the stock bootloader insists on updating
    random DT properties in them [2]

  - Use a custom bootloader anyway to boot 64-bit Linux, because the
    stock bootloader doesn't implement the 32-bit -> 64-bit switch
    (most of these devices used ARM32 on the stock Android)
    [I don't have a full overview which devices need this because
     we have never tested booting mainline Linux with the stock
     bootloader on most devices]

You also need to be really careful when building the Android boot
images. qcom,msm-id/board-ids are not unique, so when you use the
typical "dtbTool" building process which bundles all DTBs in
arch/arm64/boot/dts you risk device damage. Chances are good that the
bootloader will pick the wrong DTB, potentially with wrong regulator
voltages or anything like that.

When qcom,msm-id/board-id are missing such mistakes cannot happen
because the stock bootloader will just refuse to boot it. Personally I
actually consider that to be a good thing because booting with the stock
bootloader is almost always a naive mistake made by end users that have
no experience with the whole complicated DTB selection story.

I won't object to adding the properties but I also don't really see a
use case for them. People porting devices can easily add them when
building a custom kernel. End users want the full functionality
(including SMP) so if they boot through the stock bootloader it's most
likely accidental.

Thanks,
Stephan

[1]: https://github.com/msm8916-mainline/lk2nd/tree/master/dts/msm8974/sony
[2]: https://github.com/msm8916-mainline/lk2nd/blob/dd850aeb0c348cea085db8013f578615715cdd7f/dts/msm8916/msm8916-motorola-surnia.dts#L21-L32

Re: [PATCH v2 0/2] arm64: dts: qcom: msm8939-samsung-a7: Add initial dts

[Bjorn Andersson]

On Fri, 23 Jun 2023 10:01:48 +0000, Lin, Meng-Bo wrote:
> v2: Squash patches
> 
> This dts adds support for Samsung Galaxy A7 smartphone released in 2015.
> 
> Add a device tree for A7 with initial support for:
> 
> - GPIO keys
> - Hall Sensor
> - SDHCI (internal and external storage)
> - USB Device Mode
> - UART (on USB connector via the SM5502 MUIC)
> - WCNSS (WiFi/BT)
> - Regulators
> - Touch key
> - Accelerometer/Magnetometer
> - Fuelgauge
> - NFC
> - Vibrator
> - Touchscreen
> 
> [...]

Applied, thanks!

[1/2] dt-bindings: qcom: Document msm8939,a7
      commit: 0391cb15439625c12f0630dc1f8b00ea8ee71113
[2/2] arm64: dts: qcom: msm8939-samsung-a7: Add initial dts
      commit: 507f9db1e9decaec1b71d320a15c7b1cb95c0848

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>