On Wed, Jun 22, 2022 at 02:39:43PM +0200, Konrad Dybcio wrote: > On 22.06.2022 06:12, Bjorn Andersson wrote: > > From: Johan Hovold <johan+linaro@xxxxxxxxxx> > > > > Add an initial Lenovo Thinkpad X13s devicetree. > > > > Signed-off-by: Johan Hovold <johan+linaro@xxxxxxxxxx> > > Signed-off-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx> > > --- > > > > Changs since v1: > > - New patch > > > > arch/arm64/boot/dts/qcom/Makefile | 1 + > > .../qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 393 ++++++++++++++++++ > > 2 files changed, 394 insertions(+) > > create mode 100644 arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts > > > > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile > > index 2f416b84b71c..43053f1f6be9 100644 > > --- a/arch/arm64/boot/dts/qcom/Makefile > > +++ b/arch/arm64/boot/dts/qcom/Makefile > > @@ -91,6 +91,7 @@ dtb-$(CONFIG_ARCH_QCOM) += sc7280-idp.dtb > > dtb-$(CONFIG_ARCH_QCOM) += sc7280-idp2.dtb > > dtb-$(CONFIG_ARCH_QCOM) += sc7280-crd-r3.dtb > > dtb-$(CONFIG_ARCH_QCOM) += sc8280xp-crd.dtb > > +dtb-$(CONFIG_ARCH_QCOM) += sc8280xp-lenovo-thinkpad-x13s.dtb > > dtb-$(CONFIG_ARCH_QCOM) += sdm630-sony-xperia-ganges-kirin.dtb > > dtb-$(CONFIG_ARCH_QCOM) += sdm630-sony-xperia-nile-discovery.dtb > > dtb-$(CONFIG_ARCH_QCOM) += sdm630-sony-xperia-nile-pioneer.dtb > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts > > new file mode 100644 > > index 000000000000..58d3d8035751 > > --- /dev/null > > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts > > @@ -0,0 +1,393 @@ > > +// SPDX-License-Identifier: BSD-3-Clause > > +/* > > + * Copyright (c) 2021, The Linux Foundation. All rights reserved. > > + * Copyright (c) 2022, Linaro Limited > > + */ > > + > > +/dts-v1/; > > + > > +#include <dt-bindings/gpio/gpio.h> > Can be moved to SoC dtsi. Again, no, the SoC dtsi does not need that header and we include files where they are used. > > +&qup0_i2c4 { > > + clock-frequency = <400000>; > > + > > + pinctrl-names = "default"; > > + pinctrl-0 = <&qup0_i2c4_default>, <&ts0_default>; > > + > > + status = "okay"; > Again, the ordering of status= question from one of the previous patches. This is the right order, but the ADP dts hasn't been updated yet. > > > + > > + /* FIXME: verify, missing on some x13s */ > Won't this fail gracefully-ish if it's absent? Sure, but that's not the point. I don't access to a X13s with a touchscreen yet so the i2c address and hid descriptor address has not been verified yet. > > + touchscreen@10 { > > + compatible = "hid-over-i2c"; > > + reg = <0x10>; > > + hid-descr-addr = <0x1>; > > + interrupts-extended = <&tlmm 175 IRQ_TYPE_LEVEL_LOW>; > > + vdd-supply = <&vreg_misc_3p3>; > > + }; > > +}; > > +/* PINCTRL - additions to nodes defined in sc8280xp.dtsi */ > Seems redundant. Again, no. But I've dropped the reference to sc8280xp.dtsi. > > + tpad_default: tpad-default-state { > > + int-n { > Same comment as in the reference device dt. I think it's better to keep the child node as documentation. Johan