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)

Reply via email to