On Thu 12 Nov 10:21 CST 2020, Caleb Connolly wrote: [..] > diff --git a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi > new file mode 100644 > index 000000000000..4e6477f1e574 > --- /dev/null > +++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi > @@ -0,0 +1,822 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * SDM845 OnePlus 6(T) (enchilada / fajita) common device tree source > + * > + * Copyright (c) 2020, The Linux Foundation. All rights reserved. > + */ > + > +/dts-v1/; > + > +#include <dt-bindings/gpio/gpio.h> > +#include <dt-bindings/regulator/qcom,rpmh-regulator.h> > +#include <dt-bindings/input/linux-event-codes.h> Please keep these sorted alphabetically. > +#include "sdm845.dtsi" > + > +// Needed for some GPIO (like the volume buttons) This is or is going to be needed for more things, so feel free to skip this comment. > +#include "pm8998.dtsi" > +#include "pmi8998.dtsi" > + > +/ { > + > + aliases { > + hsuart0 = &uart6; > + }; > + > + vph_pwr: vph-pwr-regulator { > + compatible = "regulator-fixed"; > + regulator-name = "vph_pwr"; > + regulator-min-microvolt = <3700000>; > + regulator-max-microvolt = <3700000>; > + }; > + > + /* > + * Apparently RPMh does not provide support for PM8998 S4 because it > + * is always-on; model it as a fixed regulator. > + */ > + vreg_s4a_1p8: pm8998-smps4 { > + compatible = "regulator-fixed"; > + regulator-name = "vreg_s4a_1p8"; > + > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + > + regulator-always-on; > + regulator-boot-on; > + > + vin-supply = <&vph_pwr>; > + }; > + > + /* > + * The touchscreen regulator seems to be controlled somehow by a gpio. > + */ > + ts_1p8_supply: ts_1v8_regulator { Please don't use _ in the node name. > + compatible = "regulator-fixed"; > + regulator-name = "ts_1p8_supply"; > + > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + > + gpio = <&tlmm 88 0>; > + enable-active-high; > + regulator-boot-on; > + }; > + > + gpio_tristate_key: gpio-keys { > + compatible = "gpio-keys"; > + label = "Tri-state keys"; What kind of button is this? > + > + pinctrl-names = "default"; > + pinctrl-0 = <&tri_state_key_default>; > + > + state-top { > + label = "Tri-state key top"; > + linux,code = <KEY_MACRO1>; > + interrupt-parent = <&tlmm>; > + interrupts = <24 IRQ_TYPE_EDGE_FALLING>; > + debounce-interval = <500>; > + linux,can-disable; > + }; > + > + state-middle { > + label = "Tri-state key middle"; > + linux,code = <KEY_MACRO2>; > + interrupt-parent = <&tlmm>; > + interrupts = <52 IRQ_TYPE_EDGE_FALLING>; > + debounce-interval = <500>; > + linux,can-disable; > + }; > + > + state-bottom { > + label = "Tri-state key bottom"; > + linux,code = <KEY_MACRO3>; > + interrupt-parent = <&tlmm>; > + interrupts = <126 IRQ_TYPE_EDGE_FALLING>; > + debounce-interval = <500>; > + linux,can-disable; > + }; > + }; [..] > +/* Reserved memory changes */ > +/delete-node/ &rmtfs_mem; > + > +/ { You already have one top-level section higher up, please group this in there as well. > + reserved-memory { [..] > +&mdss { To avoid trouble finding your way around this file in the future I would prefer if you sorted the nodes alphabetically. > + status = "okay"; > +}; > + [..] > +&i2c12 { > + status = "okay"; > + > + touchscreen: synaptics-rmi4-i2c@20 { You don't reference &touchscreen, so please omit this.. Regards, Bjorn