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

[Caleb Connolly]

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