Re: [PATCH] arm64: dts: qcom: sm8550: add Xiaomi Pad 6S Pro (sheng)

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 7/21/26 3:32 PM, code_0002@163.com wrote:
> From: map220v <map220v300@gmail.com>
> 
> Add device tree for Xiaomi Pad 6S Pro (sheng) based on SM8550.
> 
> Signed-off-by: code002-2 <hiez832670@gmail.com>
> ---

[...]

> +/*
> + * Free unused regions, these regions don't exist in downstream
> + * dts, and they aren't removed from DDR Memory Map by UEFI.
> + */
> +/delete-node/ &hyp_tags_reserved_mem;
> +/delete-node/ &cdsp_secure_heap_mem;
> +/delete-node/ &trust_ui_vm_mem;
> +/delete-node/ &trust_ui_vm_dump;
> +/delete-node/ &trust_ui_vm_qrtr;
> +/delete-node/ &trust_ui_vm_vblk0_ring;
> +/delete-node/ &trust_ui_vm_vblk1_ring;
> +/delete-node/ &trust_ui_vm_swiotlb;
> +/delete-node/ &oem_vm_vblk0_ring;
> +/delete-node/ &oem_vm_swiotlb;
> +/delete-node/ &oem_vm_mem;
> +/delete-node/ &hyp_ext_tags_mem;
> +/delete-node/ &hyp_ext_reserved_mem;

I think all of them can be removed from the SoC DTSI

> +/* Xiaomi uses alternative gpio's for i2c8 */
> +/delete-node/ &qup_i2c8_data_clk;
> +
> +/* Xiaomi change for Nanosic WN803x */
> +/delete-node/ &qup_i2c9_data_clk;

The change here seems to just be a difference in pull-up

> +
> +/* SM8550P doesn't have Modem DSP */
> +/delete-node/ &mpss_dsm_mem;
> +/delete-node/ &mpss_mem;
> +/delete-node/ &q6_mpss_dtb_mem;
> +/delete-node/ &remoteproc_mpss;

I'm not sure that's true. Have you tried booting it up?


> +/ {
> +	model = "Xiaomi Pad 6S Pro 12.4";
> +	compatible = "xiaomi,sheng", "qcom,sm8550";
> +	chassis-type = "tablet";
> +	qcom,msm-id = <536 0x20000>; /* SM8550P v2.0 */

Please define it in include/dt-bindings/arm/qcom,ids.h and use the define here
But you shouldn't need msm-id on non-ancient hardware

> +	qcom,board-id = <0x10008 0>;
> +
> +	aliases {
> +		serial0 = &uart7;
> +		serial1 = &uart14;
> +	};
> +
> +	chosen {
> +		/* Required for libcamera to allocate buffers when udmabuf is used. */
> +		bootargs = "udmabuf.size_limit_mb=256";
> +	};

Is this actually necessary?

> +
> +	fingerprint_fpc {

Node names must not contain underscores and should be generic.
fingerprint-reader {} sounds good


> +		compatible = "fpc,fpc1020";

This compatible doesn't exist in the tree.

> +		interrupt-parent = <&tlmm>;
> +		interrupts = <40 0>;

interrupts-extended = <&tlmm 40 IRQ_TYPE_PLEASEFIX>;

[...]

> +	pmic-glink {
> +		compatible = "xiaomi,sm8550-pmic-glink", "qcom,sm8550-pmic-glink", "qcom,pmic-glink";

1 per line, please

[...]

> +		secondary-i2s-dai-link {
> +			link-name = "Secondary MI2S Playback";
> +
> +			cpu {
> +				sound-dai = <&q6apmbedai SECONDARY_MI2S_RX>;
> +			};
> +
> +			codec {

'co'dec < 'cp'u


> +				sound-dai = <&cs35l43_tll>, <&cs35l43_tlh>, <&cs35l43_trl>,
> +							<&cs35l43_bll>, <&cs35l43_blh>, <&cs35l43_brl>;

Please align (set your tab width to 8)

[...]

> +	wcd938x: audio-codec {

Nodes without a unit address should be sorted by name

https://docs.kernel.org/devicetree/bindings/dts-coding-style.html

> +		compatible = "qcom,wcd9380-codec";
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&wcd_default>;

property-n
property-names

in this order, file-wide please

[...]

> +		/* Xiaomi change for Camera Modules */

I don't think these comments are valuable.

[...]

> +&cci0_i2c1 {
> +	clock-frequency = <1000000>;
> +
> +	camera@10 {
> +		compatible = "samsung,s5kjn1";
> +		reg = <0x10>;
> +
> +		reset-gpios = <&tlmm 118 GPIO_ACTIVE_LOW>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&cam2_default>, <&cam_rear_reset_default>;
> +
> +		clocks = <&camcc CAM_CC_MCLK2_CLK>;
> +
> +		orientation = <1>;

Please use a define from:

https://lore.kernel.org/linux-arm-msm/20260628-kbingham-orientation-v3-0-4ed92968aff8@ideasonboard.com/

> +		rotation = <0>;
> +
> +		lens-focus = <&gt9764>;
> +
> +		vddio-supply = <&vreg_l4b_1p8>;
> +		vddd-supply = <&vreg_l2m_1p08>;
> +		vdda-supply = <&vreg_l4m_2p8>;
> +
> +		port {
> +			s5kjn1_ep: endpoint {
> +				data-lanes = <0 1 2 3>;
> +				link-frequencies = /bits/ 64 <857600000>;
> +				remote-endpoint = <&csiphy3_ep>;
> +			};
> +		};
> +	};
> +
> +	gt9764: camera-lens@c {
> +		compatible = "giantec,gt9764";

This compatible doesn't exist in the tree

[...]

> +&ipa {
> +	qcom,gsi-loader = "self";
> +	memory-region = <&ipa_fw_mem>;

Please rebase atop

https://lore.kernel.org/linux-arm-msm/20260517-ipa-loader-v1-0-3c3764c1b4a3@oss.qualcomm.com/

This is in -next already, so I'm worried you're not working on
linux-next/master.

> +	firmware-name = "qcom/sm8550/sheng/ipa_fws.mbn";
> +	status = "okay";

Please keep a \n above 'status', uniformly

[...]

> +&gpu_zap_shader {
> +	firmware-name = "qcom/sm8550/sheng/a740_zap.mbn";

Paths should contain the name of the vendor

> +};
> +
> +&i2c0 {
> +	clock-frequency = <400000>;
> +	status = "okay";
> +
> +	typec-retimer@28 {
> +		compatible = "parade,ps5169";

This compatible doesn't exist in the tree.

[...]

> +	backlight: backlight@11 {
> +		compatible = "kinetic,ktz8866a";

This compatible doesn't exist in the tree.

> +		reg = <0x11>;
> +		vin-supply = <&vph_pwr>;
> +		enable-gpios = <&tlmm 128 GPIO_ACTIVE_HIGH>;
> +		current-num-sinks = <5>;
> +		kinetic,current-ramp-delay-ms = <256>;
> +		kinetic,led-enable-ramp-delay-ms = <8>;
> +		kinetic,enable-lcd-bias;
> +	};
> +};
> +
> +&i2c5 {
> +	clock-frequency = <1000000>;
> +	status = "okay";
> +
> +	cs35l43_trl: cs35l43@40 {
> +		compatible = "cirrus,cs35l43";

This compatible doesn't exist in the tree.

[...]

> +&i2c9 {
> +	status = "okay";
> +
> +	nanosic_mcu@4c {
> +		compatible = "nanosic,wn8030-sheng";

This compatible doesn't exist in the tree.

[...]

> +	backlight@11 {
> +		compatible = "kinetic,ktz8866b";

This compatible doesn't exist in the tree.

[...]

> +	display_panel: panel@0 {
> +		compatible = "xiaomi,sheng-nt36532e", "novatek,nt36532e";

This compatible doesn't exist in the tree.

[...]

> +&spi4 {
> +	status = "okay";
> +
> +	touchscreen@0 {
> +		compatible = "novatek,NVT-ts-spi";

This compatible definitely doesn't exist in the tree.

[...]

> +	wcd_default: wcd-reset-n-active-state {
> +		pins = "gpio108";
> +		function = "gpio";
> +		drive-strength = <16>;
> +		bias-disable;
> +		output-low;

Remove output- properties from TLMM pins. The consumer drivers will
call the appropriate GPIOD APIs

Konrad