Re: [PATCH 2/2] arm64: dts: qcom: sm8650: Add initial Valve Deckard devicetree

[Neil Armstrong]

Hi,

On 9/22/26 13:39, Konrad Dybcio wrote:
> On 9/22/26 1:28 PM, Neil Armstrong wrote:
> > Add an initial DeviceTree for the Valve "Steam Frame"
> > codename Deckard. The Steam Frame is a virtual reality headset
> > based on the Qualcomm Snapdragon 8 Gen 3 SoC (SM8650).
> >
> > Providing initial support for:
> > - System reserved memory, regulators, gpio keys, PMIC functions..
> > - PMIC ADC Channels
> > - UFS Storage
> > - USB Gadget
> > - WCN7850 Wifi7 + Bluetooth
> > - I2C + basic peripherals
> > - SDCard slot
> >
> > The bootloader starts Linux in EL2, thus a bunch of EL1 & DSP related
> > nodes needs to be removed and the reserved-memory mapping must
> > be defined.
> >
> > Signed-off-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
> > ---
>
> [...]
>
> > +/* Remove nodes */
> > +/delete-node/ &ipa;
> > +/delete-node/ &gpu_zap_shader;
> > +/delete-node/ &lpass_wsa2macro;
> > +/delete-node/ &lpass_rxmacro;
> > +/delete-node/ &lpass_txmacro;
> > +/delete-node/ &lpass_wsamacro;
> > +/delete-node/ &lpass_vamacro;
> > +/delete-node/ &lpass_tlmm;
> > +/delete-node/ &remoteproc_adsp;
> > +/delete-node/ &remoteproc_cdsp;
> > +/delete-node/ &remoteproc_mpss;
> > +/delete-node/ &reserved_memory;
> > +/delete-node/ &swr0;
> > +/delete-node/ &swr1;
> > +/delete-node/ &swr2;
> > +/delete-node/ &swr3;
>
> Hm, maybe we should think about sm8650-audioreach (simialrly to
> how we have a split for the kodiak variants)

I will probably go in this direction when adding support for audio.

> > +
> > +/ {
> > +	model = "Valve Deckard";
> > +	compatible = "valve,deckard", "qcom,sm8650";
> > +
> > +	aliases {
> > +		serial0 = &uart15;
> > +		serial1 = &uart14;
> > +	};
> > +
> > +	chosen {
> > +		stdout-path = "serial0:115200n8";
> > +
> > +		framebuffer {
> > +			compatible = "simple-framebuffer";
> > +			width = <4320>;
> > +			height = <2160>;
> > +			stride = <(4320 * 4)>;
> > +			format = "a8r8g8b8";
> > +			memory-region = <&framebuffer_mem>;
> > +
> > +			/* Bootloader will enable the node */
> > +			status = "disabled";
>
> Is there a reason not to enable it? Does the bootloader prefer to
> only enable it sometiems?

Exact

> [...]
>
> > +	reserved-memory {
> > +		ranges;
> > +
> > +		#address-cells = <2>;
> > +		#size-cells = <2>;
>
> it's usually:
>
> addr
> size
> ranges
> <newline>

Ack

> [...]
>
> > +
> > +		aop_mem: aop-region@81c00000 {
> > +			no-map;
> > +			reg = <0x0 0x81c00000 0x0 0x60000>;
>
> 0.dtsi

Good catch

> > +		};
> > +
> > +		ramoops: ramoops-region@84a00000 {
> > +			compatible = "ramoops";
> > +			reg = <0 0x84a00000 0 0x100000>;
> > +			record-size = <0x40000>;
> > +			console-size = <0x40000>;
> > +			pmsg-size = <0x1000>;
> > +			ecc-size = <16>;
> > +		};
> > +
> > +		aop_cmd_db_mem: aop-cmd-db-region@81c60000 {
> > +			compatible = "qcom,cmd-db";
> > +			reg = <0x0 0x81c60000 0x0 0x20000>;
> > +			no-map;
> > +		};
>
> Likewise many of these reservations also exist there

Not sure slitting this would be worth it.

> [...]
>
> > +	/delete-node/ smp2p-adsp;
> > +	/delete-node/ smp2p-cdsp;
> > +	/delete-node/ smp2p-modem;
>
> I think these shouldn't hurt even if your usecase is different

Right, but those describes a pure firmware interface.

> > +
> > +	wcn7850-pmu {
> > +		compatible = "qcom,wcn7850-pmu";
> > +
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&wlan_en>, <&bt_default>;
>
> nit:
>
> property-n
> property-names

Will fix

> please
>
> otherwise looks pretty good
>
> Konrad

Thanks,
Neil