linux-kernel - Re: [PATCH v1 6/6] arm64: dts: qcom: Add support for X1-based Asus Zenbook A14

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ef783edb-c966-4ef7-b496-564420c1b852@oss.qualcomm.com>
Date: Tue, 1 Apr 2025 17:59:44 +0200
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Aleksandrs Vinarskis <alex.vinarskis@...il.com>,
        Bjorn Andersson <andersson@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Dmitry Baryshkov <lumag@...nel.org>,
        Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Abel Vesa <abel.vesa@...aro.org>,
        Johan Hovold <johan+linaro@...nel.org>, linux-arm-msm@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-usb@...r.kernel.org
Cc: Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley
 <conor+dt@...nel.org>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Subject: Re: [PATCH v1 6/6] arm64: dts: qcom: Add support for X1-based Asus
 Zenbook A14

On 3/31/25 11:53 PM, Aleksandrs Vinarskis wrote:
> Initial support for Asus Zenbook A14. Particular moddel exists
> in X1-26-100, X1P-42-100 (UX3407QA) and X1E-78-100 (UX3407RA).
> 
> Mostly similar to other X1-based laptops. Notable differences are:
> * Wifi/Bluetooth combo being Qualcomm FastConnect 6900 on UX3407QA
>   and Qualcomm FastConnect 7800 on UX3407RA
> * USB Type-C retimers are Parade PS8833, appear to behave identical
>   to Parade PS8830
> * gpio90 is TZ protected

[...]

> +	leds {
> +		compatible = "gpio-leds";
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&cam_indicator_en>;

property-n
property-names

please, we're trying to unify such small things even though we know
it's "wrong" in a lot of places

> +
> +&i2c0 {
> +	clock-frequency = <400000>;
> +	status = "okay";
> +
> +	/* ELAN, 04F3:3315 */
> +	touchpad@15 {
> +		compatible = "hid-over-i2c";
> +		reg = <0x15>;
> +
> +		hid-descr-addr = <0x1>;
> +		interrupts-extended = <&tlmm 3 IRQ_TYPE_LEVEL_LOW>;
> +
> +		pinctrl-0 = <&tpad_default>;
> +		pinctrl-names = "default";
> +
> +		wakeup-source;
> +	};
> +};
> +
> +&i2c3 {
> +	clock-frequency = <400000>;
> +	status = "okay";

It's also customary to leave a newline before 'status'

> +&pm8550_gpios {
> +	rtmr0_default: rtmr0-reset-n-active-state {
> +		pins = "gpio10";
> +		function = "normal";
> +		power-source = <1>; /* 1.8V */

Drop the 1.8v comments please

[...]

> +&spi10 {
> +	status = "disabled";
> +
> +	/* Unknown device */
> +};

Does the device crash if you enable this bus? Keeping it 'okay' would
make it easier for folks to poke at it

> +
> +&tlmm {
> +	gpio-reserved-ranges = <44 4>,  /* SPI11, TZ Protected */
> +			       <90 1>;	/* Unknown, TZ Protected */
> +
> +	bt_en_default: bt-en-sleep {
> +		pins = "gpio116";
> +		function = "gpio";
> +		output-low;
> +		bias-disable;
> +		qcom,drive-strength = <16>;

drop "qcom," and please keep the order of:

pins
function
drive-strength
bias
output/input

as you did below

> +
> +/ {
> +	model = "ASUS Zenbook A14 UX3407RA";

There's no strict policy, but variants usually go in braces

> +	compatible = "asus,x1e80100-zenbook-a14", "qcom,x1e80100";
> +};
> +
> +&gpu_zap_shader {
> +	firmware-name = "qcom/x1e80100/ASUSTeK/zenbook-a14/qcdxkmsuc8380.mbn";
> +};
> +
> +&remoteproc_adsp {
> +	firmware-name = "qcom/x1e80100/ASUSTeK/zenbook-a14/qcadsp8380.mbn",
> +			"qcom/x1e80100/ASUSTeK/zenbook-a14/adsp_dtbs.elf";
> +
> +	status = "okay";
> +};
> +
> +&remoteproc_cdsp {
> +	firmware-name = "qcom/x1e80100/ASUSTeK/zenbook-a14/qccdsp8380.mbn",
> +			"qcom/x1e80100/ASUSTeK/zenbook-a14/cdsp_dtbs.elf";
> +
> +	status = "okay";
> +};
> +
> +&uart14 {
> +	status = "okay";
> +
> +	bluetooth {
> +		compatible = "qcom,wcn7850-bt";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&bt_en_default>;
> +		enable-gpios = <&tlmm 116 GPIO_ACTIVE_HIGH>;
> +		max-speed = <3000000>;
> +	};
> +};
> diff --git a/arch/arm64/boot/dts/qcom/x1p42100-asus-zenbook-a14.dts b/arch/arm64/boot/dts/qcom/x1p42100-asus-zenbook-a14.dts
> new file mode 100644
> index 000000000000..b6c9a707090f
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/x1p42100-asus-zenbook-a14.dts
> @@ -0,0 +1,48 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2025 Aleksandrs Vinarskis <alex.vinarskis@...il.com>
> + */
> +
> +/dts-v1/;
> +
> +#include "x1p42100.dtsi"
> +#include "x1-zenbook-a14.dtsi"
> +
> +/delete-node/ &pmc8380_6;
> +/delete-node/ &pmc8380_6_thermal;
> +
> +/ {
> +	model = "ASUS Zenbook A14 UX3407QA";
> +	compatible = "asus,x1p42100-zenbook-a14", "qcom,x1p42100";
> +};
> +
> +&gpu_zap_shader {
> +	firmware-name = "qcom/x1p42100/ASUSTeK/zenbook-a14/qcdxkmsuc8380.mbn";
> +};

This file is not going to work on this SoC, you can drop it

> +
> +&remoteproc_adsp {
> +	firmware-name = "qcom/x1p42100/ASUSTeK/zenbook-a14/qcadsp8380.mbn",
> +			"qcom/x1p42100/ASUSTeK/zenbook-a14/adsp_dtbs.elf";
> +
> +	status = "okay";
> +};
> +
> +&remoteproc_cdsp {
> +	firmware-name = "qcom/x1p42100/ASUSTeK/zenbook-a14/qccdsp8380.mbn",
> +			"qcom/x1p42100/ASUSTeK/zenbook-a14/cdsp_dtbs.elf";
> +
> +	status = "okay";

Are the DSP firmware files actually different between the two?

Konrad