Re: [PATCH 5/5] arm64: dts: qcom: sm8350-lemonade(p): new devices

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 16/10/2023 14:47, Nia Espera wrote:
> Device tree files for OnePlus 9 and 9 Pro. Details of supported features
> mentioned in the cover letter for this patch series, but for
> accessibility also repeated here:
> 
> - USB OTG
> - UFS
> - Framebuffer display
> - Touchscreen (for lemonade)
> - Power & volume down keys
> - Battery reading
> - Modem, IPA, and remoteproc bringup
> 
> Steps to get booting:
> 

...

> +};
> +
> +&pmk8350_adc_tm {
> +	status = "okay";
> +
> +	pm8350_msm_therm {

No, underscores are not allowed. Do not usptream junky DTS from
downstream. Instead take upstream, good quality DTS and customize it.
Why do we need to point the issue fixed long, long time ago?


> +		reg = <0x144>;
> +		qcom,ratiometric;
> +		qcom,hw-settle-time = <0xc8>;
> +	};
> +
> +	pm8350_cam_flash_therm {
> +		reg = <0x145>;
> +		qcom,ratiometric;
> +		qcom,hw-settle-time = <0xc8>;
> +	};
> +
> +	pm8350_hot_pocket_therm {
> +		reg = <0x146>;
> +		qcom,ratiometric;
> +		qcom,hw-settle-time = <0xc8>;
> +	};
> +
> +	pm8350_wide_rfc_therm {
> +		reg = <0x147>;
> +		qcom,ratiometric;
> +		qcom,hw-settle-time = <0xc8>;
> +	};
> +
> +	pm8350_rear_tof_therm {
> +		reg = <0x148>;
> +		qcom,ratiometric;
> +		qcom,hw-settle-time = <0xc8>;
> +	};
> +
> +	pm8350b_usb_conn_therm {
> +		reg = <0x347>;
> +		qcom,ratiometric;
> +		qcom,hw-settle-time = <0xc8>;
> +	};
> +
> +	pm8350b_wl_chg_therm {
> +		reg = <0x34b>;
> +		qcom,ratiometric;
> +		qcom,hw-settle-time = <0xc8>;
> +	};
> +
> +	pmk8350_xo_therm {
> +		reg = <0x44>;
> +		qcom,ratiometric;
> +		qcom,hw-settle-time = <0xc8>;
> +	};
> +};
> +
> +&pon_pwrkey {
> +	status = "okay";
> +};
> +
> +&pon_resin {
> +	linux,code = <KEY_VOLUMEUP>;
> +	status = "okay";
> +};
> +
> +&qupv3_id_0 {
> +	status = "okay";
> +};
> +
> +&qupv3_id_1 {
> +	status = "okay";
> +};
> +
> +&qupv3_id_2 {
> +	status = "okay";
> +};
> +
> +&gpi_dma0 {
> +	status = "okay";
> +};
> +
> +&gpi_dma1 {
> +	status = "okay";
> +};
> +
> +&gpi_dma2 {
> +	status = "okay";
> +};
> +
> +&removed_mem {

Hm, what is removed_mem?

> +	reg = <0x0 0xd8800000 0x0 0x8e00000>;
> +};
> +
> +&tlmm {
> +	gpio-reserved-ranges = <52 8>;
> +
> +	pcie0_default_state: pcie0-default-state {

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).

> +		perst-pins {
> +			pins = "gpio94";
> +			function = "gpio";
> +			drive-strength = <2>;
> +			bias-pull-down;
> +		};
> +


> diff --git a/arch/arm64/boot/dts/qcom/sm8350-oneplus-lemonade.dts b/arch/arm64/boot/dts/qcom/sm8350-oneplus-lemonade.dts
> new file mode 100644
> index 000000000000..f2c27894f3c4
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sm8350-oneplus-lemonade.dts
> @@ -0,0 +1,82 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2023 Igalia S.L.
> + * Authors:
> + *	Nia Espera <nespera@igalia.com>
> + */
> +
> +/dts-v1/;
> +
> +#include "sm8350-oneplus-common.dtsi"
> +
> +/ {
> +	model = "OnePlus 9";
> +	compatible = "oneplus,lemonade", "qcom,sm8350";
> +};
> +
> +&i2c4 {
> +	touchscreen@48 {
> +		compatible = "samsung,s6sy761";
> +		reg = <0x48>;
> +		interrupts-extended = <&tlmm 23 0x2008>;
> +
> +		vdd-supply = <&pm8350c_l8>;
> +		avdd-supply = <&pm8350c_l13>;
> +
> +		pinctrl-names = "default", "sleep";
> +		pinctrl-0 = <&tp_rst_active &tp_irq_active>;

Multiple phandles <>, not one.

> +		pinctrl-1 = <&tp_rst_suspend &tp_irq_suspend>;
> +	};
> +};
> +
> +&tlmm {
> +	tp_rst_suspend: tp_rst_suspend {

Ehh...
> +		pins = "gpio22";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-pull-down;
> +	};
> +
> +	tp_enable_2v8: tp_enable_2v8 {
> +		pins = "gpio74";
> +		function = "gpio";
> +		drive-strength = <8>;
> +		bias-pull-up;
> +		output-high;
> +	};
> +
> +	/* Modem antenna pins exclusive to lemonade */
> +	rf_cable_ant1_active: rf_cable_ant1_active {
> +		pins = "gpio27";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-pull-up;
> +	};
> +	rf_cable_ant2_active: rf_cable_ant2_active {
> +		pins = "gpio92";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-pull-up;
> +	};
> +	rf_cable_ant3_active: rf_cable_ant3_active {
> +		pins = "gpio44";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-pull-up;
> +	};
> +	rf_cable_ant7_active: rf_cable_ant7_active {
> +		pins = "gpio155";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-pull-up;
> +	};
> +};
> +
> +&mpss {

Wrong order. t is after m

> +	pinctrl-names = "default";
> +	pinctrl-1 = <&rf_cable_ant0_active

Same problem.

> +		     &rf_cable_ant1_active
> +		     &rf_cable_ant2_active
> +		     &rf_cable_ant3_active
> +		     &rf_cable_ant7_active>;
> +};
> diff --git a/arch/arm64/boot/dts/qcom/sm8350-oneplus-lemonadep.dts b/arch/arm64/boot/dts/qcom/sm8350-oneplus-lemonadep.dts
> new file mode 100644
> index 000000000000..de8597d26091
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sm8350-oneplus-lemonadep.dts
> @@ -0,0 +1,37 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2023 Igalia S.L.
> + * Authors:
> + *	Nia Espera <nespera@igalia.com>
> + */
> +
> +/dts-v1/;
> +
> +#include "sm8350-oneplus-common.dtsi"
> +
> +/ {
> +	model = "OnePlus 9 Pro";
> +	compatible = "oneplus,lemonadep", "qcom,sm8350";

Missing bindings.

> +};
> +
> +&tlmm {
> +	tp_rst_suspend: tp_rst_suspend {

No underscores in node names. This wasn't tested.

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).



Best regards,
Krzysztof