Re: [PATCH 4/4] arm64: dts: qcom: Add support for X1-based Surface Laptop 7 devices

[Stephan Gerhold <stephan.gerhold@linaro.org>]

On Fri, Aug 09, 2024 at 03:43:23AM +0200, Konrad Dybcio wrote:
> From: Konrad Dybcio <quic_kdybcio@quicinc.com>
> 
> Add support for Surface Laptop 7 machines, based on X1E80100.
> 
> The feature status is mostly on par with other X Elite machines,
> notably lacking:
> 
> - USB-A and probably USB-over-Surface-connector
> - SD card reader (Realtek RTS5261 connected over PCIe)
> - Touchscreen and touchpad support (hid-over-SPI [1])
> - Keyboard support (low-hanging fruit, works with pending Surface EC
>   changes)
> - Audio (a quick look suggests the setup is very close to the one in
>   X1E CRD)
> 
> The two Surface Laptop 7 SKUs (13.8" and 15") only have very minor
> differences, amounting close to none on the software side. Even the
> MBN firmware files and ACPI tables are shared between the two machines.
> 
> With that in mind, support is added for both, although only the larger
> one was physically tested. Display differences will be taken care of
> through fused-in EDID and other matters should be solved within the
> EC and boot firmware.
> 
> [1] https://www.microsoft.com/en-us/download/details.aspx?id=103325
> 
> Signed-off-by: Konrad Dybcio <quic_kdybcio@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/Makefile                  |   2 +
>  .../boot/dts/qcom/x1e80100-microsoft-romulus.dtsi  | 818 +++++++++++++++++++++
>  .../boot/dts/qcom/x1e80100-microsoft-romulus13.dts |  13 +
>  .../boot/dts/qcom/x1e80100-microsoft-romulus15.dts |  13 +
>  arch/arm64/boot/dts/qcom/x1e80100-pmics.dtsi       |   8 +
>  5 files changed, 854 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index e534442620a1..820b768cdb71 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -270,4 +270,6 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sm8650-qrd.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= x1e80100-asus-vivobook-s15.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= x1e80100-crd.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= x1e80100-lenovo-yoga-slim7x.dtb
> +dtb-$(CONFIG_ARCH_QCOM)	+= x1e80100-microsoft-romulus13.dtb
> +dtb-$(CONFIG_ARCH_QCOM)	+= x1e80100-microsoft-romulus15.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= x1e80100-qcp.dtb
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100-microsoft-romulus.dtsi b/arch/arm64/boot/dts/qcom/x1e80100-microsoft-romulus.dtsi
> new file mode 100644
> index 000000000000..3f6d4b93db50
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/x1e80100-microsoft-romulus.dtsi
> @@ -0,0 +1,818 @@
> [...]
> +	backlight: backlight {
> +		compatible = "pwm-backlight";
> +		pwms = <&pmk8550_pwm 0 5000000>;
> +		enable-gpios = <&pmc8380_3_gpios 4 GPIO_ACTIVE_HIGH>;
> +		/* TODO: power-supply? */

There seems to be something at <&pmc8380_3_gpios 10>, any idea what?

> [...]
> +&pmk8550_gpios {
> +	edp_bl_pwm: edp-bl-pwm-state {
> +		pins = "gpio5";
> +		function = "func3";

Can you add the power-source here to make this more complete?

> +	};
> +
> [...]
> +
> +&uart2 {
> +	status = "okay";
> +};

Any idea what this UART is used for?

> [...]
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100-pmics.dtsi b/arch/arm64/boot/dts/qcom/x1e80100-pmics.dtsi
> index a5ca0fa4e5ae..5b54ee79f048 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100-pmics.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1e80100-pmics.dtsi
> @@ -249,6 +249,14 @@ pmk8550_gpios: gpio@8800 {
>  			interrupt-controller;
>  			#interrupt-cells = <2>;
>  		};
> +
> +		pmk8550_pwm: pwm {
> +			compatible = "qcom,pmk8550-pwm";
> +
> +			#pwm-cells = <2>;
> +
> +			status = "disabled";
> +		};

I don't mind personally but usually we would have this non-device
addition in a separate patch. :-)

Thanks,
Stephan