[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <01de9fc2-f60c-4bc0-af61-c070213df29b@oss.qualcomm.com>
Date: Thu, 22 Jan 2026 10:15:04 +0100
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Neil Armstrong <neil.armstrong@...aro.org>, Rob Herring
<robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>,
Rob Clark <robin.clark@....qualcomm.com>,
Dmitry Baryshkov
<lumag@...nel.org>,
Abhinav Kumar <abhinav.kumar@...ux.dev>,
Jessica Zhang <jesszhan0024@...il.com>, Sean Paul <sean@...rly.run>,
Marijn Suijten <marijn.suijten@...ainline.org>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
freedreno@...ts.freedesktop.org, KancyJoe <kancy2333@...look.com>
Subject: Re: [PATCH 3/3] arm64: dts: qcom: add basic devicetree for Ayaneo
Pocket S2 gaming console
On 1/21/26 5:40 PM, Neil Armstrong wrote:
> From: KancyJoe <kancy2333@...look.com>
>
> Add initial Device Tree for the Ayaneo Pocket S2 gaming console based
> on the Qualcomm Snapdragon 8 Gen 3 platform.
>
> The design is similar to a phone wihout the modem, the game control
> is handled via a standalone controller connected to a PCIe USB
> controller.
>
> Display support will be added in a second time.
>
> Signed-off-by: KancyJoe <kancy2333@...look.com>
> Signed-off-by: Neil Armstrong <neil.armstrong@...aro.org>
> ---
[...]
> + fan: pwm-fan {
I'd call it fan {} but gray/grey
> + status = "okay";
You can drop this line (nothing disables it)
> + compatible = "pwm-fan";
> +
> + interrupt-parent = <&tlmm>;
> + interrupts = <14 IRQ_TYPE_EDGE_FALLING>;
interrupts-extended looks neater
> +
> + pinctrl-0 = <&fan_pwr_active>,
> + <&pwm_fan_ctrl_default>,
> + <&fan_int_active>;
> + pinctrl-1 = <&fan_pwr_sleep>;
fan-pwr looks like an EN pin of a GPIO-controlled regulator
> + pinctrl-names = "default",
> + "sleep";
> +
> + pwms = <&pm8550_pwm 3 50000>;
> +
> + #cooling-cells = <2>;
> + cooling-levels = <0 16 32 45 60 80 105 130 155 180 205 230 255>;
Does this come from a preexisting map?
> + };
> +
> + gpio-keys {
> + compatible = "gpio-keys";
> +
> + pinctrl-0 = <&volume_up_n>;
> + pinctrl-names = "default";
> +
> + key-volume-up {
> + label = "Volume Up";
> + linux,code = <KEY_VOLUMEUP>;
> + gpios = <&pm8550_gpios 6 GPIO_ACTIVE_LOW>;
> + debounce-interval = <15>;
> + linux,can-disable;
> + wakeup-source;
> + };
> + };
> +
> + pmic-glink {
> + compatible = "qcom,sm8650-pmic-glink",
> + "qcom,sm8550-pmic-glink",
> + "qcom,pmic-glink";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + orientation-gpios = <&tlmm 29 GPIO_ACTIVE_HIGH>;
> +
> + connector@0 {
> + compatible = "usb-c-connector";
> + reg = <0>;
> +
> + power-role = "dual";
> + data-role = "dual";
> + self-powered;
Is this property interpreted at all by our setup?
[...]
> + sound {
> + compatible = "qcom,sm8650-sndcard", "qcom,sm8450-sndcard";
> + model = "SM8650-APS2";
> + audio-routing = "SpkrLeft IN", "WSA_SPK1 OUT",
> + "SpkrRight IN", "WSA_SPK2 OUT",
> + "IN1_HPHL", "HPHL_OUT",
> + "IN2_HPHR", "HPHR_OUT",
> + "DMIC1", "MIC BIAS1",
> + "DMIC2", "MIC BIAS2",
> + "AMIC2", "MIC BIAS2",
> + "TX SWR_INPUT1", "ADC2_OUTPUT",
> + "TX SWR_INPUT7", "DMIC1_OUTPUT",
> + "TX SWR_INPUT8", "DMIC2_OUTPUT";
> +
> + wcd-playback-dai-link {
> + link-name = "WCD Playback";
> +
> + cpu {
> + sound-dai = <&q6apmbedai RX_CODEC_DMA_RX_0>;
> + };
> +
> + codec {
> + sound-dai = <&wcd939x 0>,
> + <&swr1 0>,
> + <&lpass_rxmacro 0>;
> + };
'co'dec < 'cp'u
[...]
> + wcd939x: audio-codec {
'a'udio-codec should be way higher
[...]
> + thermal-zones {
> + cpu2-top-thermal {
> + trips {
> + cpu2_active: cpu2-active {
> + temperature = <38000>;
> + hysteresis = <2000>;
> + type = "active";
This is shaky.. let's perhaps reference each thermal zone that you want
to extend with a label.. Or maybe a pair of labels for trips/cooling-maps
per zone?
[...]
> +&pcieport1 {
> + pinctrl-0 = <&upd720201_active>;
Is this a regulator?
> + pinctrl-names = "default";
> +
> + /* Renesas μPD720201 PCIe USB3.0 HOST CONTROLLER */
DON'T SCREAM! :P
> + usb-controller@0 {
> + compatible = "pci1912,0014";
> + reg = <0x10000 0x0 0x0 0x0 0x0>;
> +
> + pinctrl-0 = <&gamepad_pwr_en>;
> + pinctrl-names = "default";
Is there a hub connected to it? Or does it go directly to the
aforementioned (game) controller?
[...]
> +&pm8550_pwm {
> + status = "okay";
> +
> + multi-led {
> + color = <LED_COLOR_ID_RGB>;
> + function = LED_FUNCTION_STATUS;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
Would a label="xyz" be useful here?
[...]
> +&tlmm {
> + /* Reserved I/Os for NFC */
> + gpio-reserved-ranges = <32 4>, <36 1>, <38 6>, <74 1>;
double space
Are they all for NFC, are they all required?
[...]
> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> index 07ae74851621..fcd5a1a45803 100644
> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> @@ -3917,7 +3917,7 @@ opp-32000000-4 {
> };
> };
>
> - pcie@0 {
> + pcieport1: pcie@0 {
pcie1_port0, please
Konrad
Powered by blists - more mailing lists