Re: [PATCH 7/7] arm64: dts: qcom: add OnePlus 8T (kebab)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 26/06/2024 06:16, Bjorn Andersson wrote:
On Mon, Jun 24, 2024 at 03:30:31AM GMT, Caleb Connolly wrote:
Initial support for USB, UFS, touchscreen, panel, wifi, and bluetooth.


Nice.

diff --git a/arch/arm64/boot/dts/qcom/sm8250-oneplus-common.dtsi b/arch/arm64/boot/dts/qcom/sm8250-oneplus-common.dtsi
[..]
+	vph_pwr: vph-pwr-regulator {

Please keep nodes sorted by address, then node name, then label (as
applicable). Perhaps making the -regulator suffix a regulator- prefix
instead (to keep them grouped).

+		compatible = "regulator-fixed";
+		regulator-name = "vph_pwr";
+		regulator-min-microvolt = <3700000>;
+		regulator-max-microvolt = <3700000>;
+		regulator-always-on;
+	};
+
+	vreg_s4a_1p8: vreg-s4a-1p8 {
+		compatible = "regulator-fixed";
+		regulator-name = "vreg_s4a_1p8";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		regulator-always-on;
+	};
[..]
+&adsp {
+	status = "okay";

Per Documentation/devicetree/bindings/dts-coding-style.rst please keep
"status" as last property in your nodes.

+	firmware-name = "qcom/sm8250/OnePlus/adsp.mbn";
+};
+
[..]
+&mdss_dsi0 {
+	status = "okay";
+	vdda-supply = <&vreg_l9a_1p2>;
+
+	display_panel: panel@0 {
+		reg = <0>;
+		vddio-supply = <&vreg_l14a_1p8>;
+		vdd-supply = <&vreg_l11c_3p3>;
+		avdd-supply = <&panel_avdd_5p5>;

How do you know that the panel will have these properties, when you
don't give it a compatible here? Not a strong objection, but perhaps
this should be pushed out?

I'll double check, I assumed all the panels on all the variants of this platform used the same regulators (the 8 and 8 pro as well) but i could be mistaken.

+		/* FIXME: There is a bug somewhere in the display stack and it isn't
+		 * possible to get the panel to a working state after toggling reset.
+		 * At best it just shows one or more vertical red lines. So for now
+		 * let's skip the reset GPIO.
+		 */
+		// reset-gpios = <&tlmm 75 GPIO_ACTIVE_LOW>;
+
+		pinctrl-0 = <&panel_reset_pins &panel_vsync_pins &panel_vout_pins>;
+		pinctrl-names = "default";
+
+		status = "disabled";
+
+		port {
+			panel_in_0: endpoint {
+				remote-endpoint = <&mdss_dsi0_out>;
+			};
+		};
+	};
+
+};
[..]
+&pm8150_gpios {
+	gpio-reserved-ranges = <2 1>, <4 1>, <8 1>;

How come?

I'll check this, I forgot to make a note originally, but I do remember that I was only able to figure out which GPIOs were causing the crashdump by squinting at the magic writing in the tz log (one of the values corresponds to to a register address iirc).

+};
+
[..]
+&tlmm {
+	gpio-reserved-ranges = <28 4>, <40 4>;
+
+	bt_en_state: bt-default-state {
+		pins = "gpio21";
+		function = "gpio";
+		drive-strength = <16>;
+		output-low;
+		bias-pull-up;
+	};
+
+	wlan_en_state: wlan-default-state {
+		wlan-en-pins {

Perhaps flatten this? >
+			pins = "gpio20";
+			function = "gpio";
+
+			drive-strength = <16>;
+			output-low;
+			bias-pull-up;
+		};
+	};
+
[..]
diff --git a/arch/arm64/boot/dts/qcom/sm8250-oneplus-kebab.dts b/arch/arm64/boot/dts/qcom/sm8250-oneplus-kebab.dts
[..]
+&i2c13 {
[..]
+};
+
+&display_panel {

'd' < 'i'

Ack, thanks for the review :)

Regards,
Bjorn

+	compatible = "samsung,amb655x";
+	status = "okay";
+};

--
2.45.0


Kind regards,
Caleb (they/them)




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux