Re: [PATCH 2/3] arm64: dts: qcom: glymur: Add Asus Zenbook A16 (UX3607OA)

[Abel Vesa]

On 26-07-21 19:11:27, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>
> 
> UX3607OA is a variant of the Asus Zenbook A16 built on the Qualcomm
> Glymur platform. It comes with an 18-core X2 Elite Extreme SoC, 48GiB
> of RAM, NVMe storage, a 2880 x 1800 OLED screen, and the other typical
> Glymur platform capabilities.
> 
> The hardware wiring is very different versus the 14" SKU - the larger
> one is based on the System-In-Package reference design, unlike the
> smaller one, which derives from a more common Chip-On-Board base.
> 
> Currently working:
> - Audio playback (4 speakers)
> - GPU, Display
> - HDMI port
> - Keyboard, touchpad, lid switch
> - Privacy LEDs (camera-use, camera-block, mic-mute)
> - Charging, Type-C, Type-A port
> - Onboard SD Card slot (RTS5329)
> - Wi-Fi, Bluetooth (QCC2072)
> - Touchscreen should work on the SKUs that have it, but it remains
>   untested due to lack of access to such a device
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>
> ---
>  arch/arm64/boot/dts/qcom/Makefile                  |    1 +
>  .../dts/qcom/glymur-asus-zenbook-a16-ux3607oa.dts  | 1271 ++++++++++++++++++++
>  arch/arm64/boot/dts/qcom/pmh0104-glymur.dtsi       |    2 +-
>  arch/arm64/boot/dts/qcom/pmh0110-glymur.dtsi       |    2 +-
>  4 files changed, 1274 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index e05414290d8e..1c86e7e98f55 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -16,6 +16,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= apq8096sg-db820c.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= apq8096-ifc6640.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= eliza-cqs-evk.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= eliza-mtp.dtb
> +dtb-$(CONFIG_ARCH_QCOM)	+= glymur-asus-zenbook-a16-ux3607oa.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= glymur-crd.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= hamoa-iot-evk.dtb
>  
> diff --git a/arch/arm64/boot/dts/qcom/glymur-asus-zenbook-a16-ux3607oa.dts b/arch/arm64/boot/dts/qcom/glymur-asus-zenbook-a16-ux3607oa.dts
> new file mode 100644
> index 000000000000..be1a217c5bee
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/glymur-asus-zenbook-a16-ux3607oa.dts
> @@ -0,0 +1,1271 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +/dts-v1/;
> +
> +#include "glymur.dtsi"
> +
> +#include "pmcx0102.dtsi"
> +#include "pmh0101.dtsi"
> +#include "pmh0110-glymur.dtsi"
> +#include "pmh0104-glymur.dtsi"
> +#include "pmk8850.dtsi"
> +#include "smb2370.dtsi"
> +
> +#include <dt-bindings/input/gpio-keys.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
> +
> +/delete-node/ &pmh0104_l_e1;
> +/delete-node/ &pmh0104_l1_thermal;
> +/delete-node/ &pmh0110_h_e0;
> +/delete-node/ &pmh0110_h0_thermal;
> +

[...]

> +&remoteproc_adsp {
> +	firmware-name = "qcom/glymur/ASUSTeK/UX3607OA/qcadsp8480.mbn",
> +			"qcom/glymur/ASUSTeK/UX3607OA/adsp_dtbs.elf";
> +
> +

Nit: Drop the extra blank line above.

> +	status = "okay";
> +};
> +
> +&remoteproc_cdsp {
> +	firmware-name = "qcom/glymur/ASUSTeK/UX3607OA/qccdsp8480.mbn",
> +			"qcom/glymur/ASUSTeK/UX3607OA/cdsp_dtbs.elf";
> +
> +

Ditto.

> +	status = "okay";
> +};


Otherwise, it looks good to me:

Reviewed-by: Abel Vesa <abel.vesa@xxxxxxxxxxxxxxxx>