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

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

 



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?

> +		/* 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?

> +};
> +
[..]
> +&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'

Regards,
Bjorn

> +	compatible = "samsung,amb655x";
> +	status = "okay";
> +};
> 
> -- 
> 2.45.0
> 



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux