[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZrXcbHWXPvVj-lQd@linaro.org>
Date: Fri, 9 Aug 2024 11:07:56 +0200
From: Stephan Gerhold <stephan.gerhold@...aro.org>
To: Konrad Dybcio <konradybcio@...nel.org>
Cc: Bjorn Andersson <andersson@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Marijn Suijten <marijn.suijten@...ainline.org>,
linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org,
Konrad Dybcio <quic_kdybcio@...cinc.com>
Subject: Re: [PATCH 4/4] arm64: dts: qcom: Add support for X1-based Surface
Laptop 7 devices
On Fri, Aug 09, 2024 at 03:43:23AM +0200, Konrad Dybcio wrote:
> From: Konrad Dybcio <quic_kdybcio@...cinc.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@...cinc.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@...0 {
> 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
Powered by blists - more mailing lists