On 02/06/2022 14:07, Stefan Hansson wrote: > From: Anton Bambura <jenneron@xxxxxxxxxxxxxx> > > Adds initial support for the LG G7 (judyln) and > LG V35 (judyp) phones. Thank you for your patch. There is something to discuss/improve. > > Currently supported features: > > - Display via simplefb (panel driver is WIP) > - Keys > - Micro SD card > - Modem (not tested much, but initialises) > - UFS (crashes during intensive workloads, may need quirks) > - USB in peripheral mode > > Signed-off-by: Anton Bambura <jenneron@xxxxxxxxxxxxxx> > Signed-off-by: Stefan Hansson <newbie13xd@xxxxxxxxx> > Tested-by: Gregari Ivanov <llamashere@xxxxxxxxx> > --- > arch/arm64/boot/dts/qcom/Makefile | 2 + > .../arm64/boot/dts/qcom/sdm845-lg-common.dtsi | 621 ++++++++++++++++++ > arch/arm64/boot/dts/qcom/sdm845-lg-judyln.dts | 64 ++ > arch/arm64/boot/dts/qcom/sdm845-lg-judyp.dts | 40 ++ > 4 files changed, 727 insertions(+) > create mode 100644 arch/arm64/boot/dts/qcom/sdm845-lg-common.dtsi > create mode 100644 arch/arm64/boot/dts/qcom/sdm845-lg-judyln.dts > create mode 100644 arch/arm64/boot/dts/qcom/sdm845-lg-judyp.dts > > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile > index f9e6343acd03..2f31e62f550c 100644 > --- a/arch/arm64/boot/dts/qcom/Makefile > +++ b/arch/arm64/boot/dts/qcom/Makefile > @@ -99,6 +99,8 @@ dtb-$(CONFIG_ARCH_QCOM) += sdm845-cheza-r1.dtb > dtb-$(CONFIG_ARCH_QCOM) += sdm845-cheza-r2.dtb > dtb-$(CONFIG_ARCH_QCOM) += sdm845-cheza-r3.dtb > dtb-$(CONFIG_ARCH_QCOM) += sdm845-db845c.dtb > +dtb-$(CONFIG_ARCH_QCOM) += sdm845-lg-judyln.dtb > +dtb-$(CONFIG_ARCH_QCOM) += sdm845-lg-judyp.dtb > dtb-$(CONFIG_ARCH_QCOM) += sdm845-mtp.dtb > dtb-$(CONFIG_ARCH_QCOM) += sdm845-oneplus-enchilada.dtb > dtb-$(CONFIG_ARCH_QCOM) += sdm845-oneplus-fajita.dtb > diff --git a/arch/arm64/boot/dts/qcom/sdm845-lg-common.dtsi b/arch/arm64/boot/dts/qcom/sdm845-lg-common.dtsi > new file mode 100644 > index 000000000000..2a961454d248 > --- /dev/null > +++ b/arch/arm64/boot/dts/qcom/sdm845-lg-common.dtsi > @@ -0,0 +1,621 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * SDM845 LG G7 / V35 (judyln / judyp) common device tree > + * > + * Copyright (c) 2022, The Linux Foundation. All rights reserved. > + */ > + > +#include <dt-bindings/gpio/gpio.h> > +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h> > +#include <dt-bindings/regulator/qcom,rpmh-regulator.h> > + > +#include "sdm845.dtsi" > +#include "pm8998.dtsi" > +#include "pmi8998.dtsi" > + > +/delete-node/ &adsp_mem; > +/delete-node/ &cdsp_mem; > +/delete-node/ &gpu_mem; > +/delete-node/ &ipa_fw_mem; > +/delete-node/ &mba_region; > +/delete-node/ &mpss_region; > +/delete-node/ &qseecom_mem; > +/delete-node/ &rmtfs_mem; > +/delete-node/ &slpi_mem; > +/delete-node/ &spss_mem; > +/delete-node/ &venus_mem; > +/delete-node/ &wlan_msa_mem; > + > +/ { > + chosen { > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + framebuffer@9d400000 { > + compatible = "simple-framebuffer"; > + width = <1440>; > + stride = <(1440 * 4)>; > + format = "a8r8g8b8"; > + }; > + }; > + > + reserved-memory { > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + qseecom_mem: memory@b2000000 { > + reg = <0 0xb2000000 0 0x1800000>; > + no-map; > + }; > + > + gpu_mem: memory@8c415000 { > + reg = <0 0x8c415000 0 0x2000>; > + no-map; > + }; > + > + ipa_fw_mem: memory@8c400000 { > + reg = <0 0x8c400000 0 0x10000>; > + no-map; > + }; > + > + adsp_mem: memory@8c500000 { > + reg = <0 0x8c500000 0 0x1e00000>; > + no-map; > + }; > + > + wlan_msa_mem: memory@8e300000 { > + reg = <0 0x8e300000 0 0x100000>; > + no-map; > + }; > + > + mpss_region: memory@8e400000 { > + reg = <0 0x8e400000 0 0x8900000>; > + no-map; > + }; > + > + venus_mem: memory@96d00000 { > + reg = <0 0x96d00000 0 0x500000>; > + no-map; > + }; > + > + cdsp_mem: memory@97200000 { > + reg = <0 0x97200000 0 0x800000>; > + no-map; > + }; > + > + mba_region: memory@97a00000 { > + reg = <0 0x97a00000 0 0x200000>; > + no-map; > + }; > + > + slpi_mem: memory@97c00000 { > + reg = <0 0x97c00000 0 0x1400000>; > + no-map; > + }; > + > + spss_mem: memory@99000000 { > + reg = <0 0x99000000 0 0x100000>; > + no-map; > + }; > + > + /* Framebuffer region */ > + memory@9d400000 { > + reg = <0x0 0x9d400000 0x0 0x2400000>; > + no-map; > + }; > + > + /* rmtfs lower guard */ > + memory@f0800000 { > + reg = <0 0xf0800000 0 0x1000>; > + no-map; > + }; > + > + rmtfs_mem: memory@f0801000 { > + compatible = "qcom,rmtfs-mem"; > + reg = <0 0xf0801000 0 0x200000>; > + no-map; > + > + qcom,client-id = <1>; > + qcom,vmid = <15>; > + }; > + > + /* rmtfs upper guard */ > + memory@f0a01000 { > + reg = <0 0xf0a01000 0 0x1000>; > + no-map; > + }; > + }; > + > + gpio-keys { > + compatible = "gpio-keys"; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&vol_up_pin_a>; > + > + label = "GPIO Buttons"; Is "label" really a property of gpio-keys node? > + > + vol-up { Generic node names, please, so "key-0" or "key-vol-up". > + label = "Volume up"; > + linux,code = <KEY_VOLUMEUP>; > + gpios = <&pm8998_gpio 6 GPIO_ACTIVE_LOW>; > + }; > + }; > + > + 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 { Generic node names, so also please suffix it with regulator, e.g. pm8998-smps4-regulator > + 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>; > + }; > +}; > + > +&adsp_pas { > + status = "okay"; > +}; > + (...) > + > +&gpu { > + status = "disabled"; Why is it being disabled? > + > + zap-shader { > + memory-region = <&gpu_mem>; > + }; > +}; > + > +&ipa { > + status = "okay"; > + modem-init; > +}; > + > +&mss_pil { > + status = "okay"; > +}; > + > +&pm8998_pon { > + resin { > + compatible = "qcom,pm8941-resin"; > + interrupts = <0x0 0x8 1 IRQ_TYPE_EDGE_BOTH>; > + debounce = <15625>; > + bias-pull-up; > + linux,code = <KEY_VOLUMEDOWN>; > + }; > +}; > + > +&sdhc_2 { > + status = "okay"; > + > + cd-gpios = <&tlmm 126 GPIO_ACTIVE_LOW>; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&sdc2_clk &sdc2_cmd &sdc2_data &sd_card_det_n>; > + > + vmmc-supply = <&vreg_l21a_2p95>; > + vqmmc-supply = <&vddpx_2>; > +}; > + > +/* > + * UFS works partially and only with clk_ignore_unused. > + * Sometimes it crashes with I/O errors. > + */ > +&ufs_mem_hc { > + status = "okay"; > + > + reset-gpios = <&tlmm 150 GPIO_ACTIVE_LOW>; > + > + vcc-supply = <&vreg_l20a_2p95>; > + vcc-max-microamp = <600000>; > +}; > + > +&ufs_mem_phy { > + status = "okay"; > + > + vdda-phy-supply = <&vdda_ufs1_core>; > + vdda-pll-supply = <&vdda_ufs1_1p2>; > +}; > + > +&usb_1 { > + status = "okay"; > +}; > + > +&usb_1_dwc3 { > + /* TODO: these devices have usb id pin */ > + dr_mode = "peripheral"; > +}; > + > +&usb_1_hsphy { > + status = "okay"; > + > + vdd-supply = <&vdda_usb1_ss_core>; > + vdda-pll-supply = <&vdda_qusb_hs0_1p8>; > + vdda-phy-dpdm-supply = <&vdda_qusb_hs0_3p1>; > + > + qcom,imp-res-offset-value = <8>; > + qcom,hstx-trim-value = <QUSB2_V2_HSTX_TRIM_21_6_MA>; > + qcom,preemphasis-level = <QUSB2_V2_PREEMPHASIS_5_PERCENT>; > + qcom,preemphasis-width = <QUSB2_V2_PREEMPHASIS_WIDTH_HALF_BIT>; > +}; > + > +&usb_1_qmpphy { > + status = "okay"; > + > + vdda-phy-supply = <&vdda_usb1_ss_1p2>; > + vdda-pll-supply = <&vdda_usb1_ss_core>; > +}; > + > +/* PINCTRL - additions to nodes defined in sdm845.dtsi */ > + > +&tlmm { > + gpio-reserved-ranges = <28 4>, <81 4>; > + > + sdc2_clk: sdc2-clk { > + pinconf { > + pins = "sdc2_clk"; > + bias-disable; > + > + /* > + * It seems that mmc_test reports errors if drive > + * strength is not 16 on clk, cmd, and data pins. > + * > + * TODO: copy-pasted from mtp, try other values > + * on these devices. > + */ > + drive-strength = <16>; > + }; > + }; > + > + sdc2_cmd: sdc2-cmd { > + pinconf { > + pins = "sdc2_cmd"; > + bias-pull-up; > + drive-strength = <16>; > + }; > + }; > + > + sdc2_data: sdc2-data { > + pinconf { > + pins = "sdc2_data"; > + bias-pull-up; > + drive-strength = <16>; > + }; > + }; > + > + sd_card_det_n: sd-card-det-n { > + pinmux { > + pins = "gpio126"; > + function = "gpio"; > + }; > + > + pinconf { > + pins = "gpio126"; > + bias-pull-up; > + }; > + }; > +}; > + > +&pm8998_gpio { > + vol_up_pin_a: vol-up-active { You need -pins suffix https://lore.kernel.org/all/20220507194913.261121-3-krzysztof.kozlowski@xxxxxxxxxx/ > + pins = "gpio6"; > + function = "normal"; > + input-enable; > + bias-pull-up; > + qcom,drive-strength = <PMIC_GPIO_STRENGTH_NO>; > + }; > +}; > diff --git a/arch/arm64/boot/dts/qcom/sdm845-lg-judyln.dts b/arch/arm64/boot/dts/qcom/sdm845-lg-judyln.dts > new file mode 100644 > index 000000000000..a1a0110e7af7 > --- /dev/null > +++ b/arch/arm64/boot/dts/qcom/sdm845-lg-judyln.dts > @@ -0,0 +1,64 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * SDM845 LG G7 (judyln) device tree. > + * > + * Copyright (c) 2022, The Linux Foundation. All rights reserved. > + */ > + > +/dts-v1/; > + > +#include "sdm845-lg-common.dtsi" > + > +/ { > + model = "LG G7 ThinQ"; > + compatible = "lg,judyln", "qcom,sdm845"; You need to document the compatibles in arm/qcom.yaml > + > + chosen { > + framebuffer@9d400000 { > + reg = <0x0 0x9d400000 0x0 (1440 * 3120 * 4)>; > + height = <3120>; > + lab-supply = <&lab>; > + ibb-supply = <&ibb>; You already have it in your DTSI, so either override by label or move everything to each DTS. I think the second seems more readable, because it is weird to see device node with unit address but without reg (as it is in DTSI). > + }; > + }; > + > + /* Additional ThinQ key */ > + gpio-keys { > + pinctrl-0 = <&vol_up_pin_a &thinq_key_default>; > + > + thinq { Generic node names, so "key-0" or "key-thinq". Same comments for second DTS. > + label = "ThinQ"; > + linux,code = <KEY_ASSISTANT>; > + interrupt-parent = <&tlmm>; > + interrupts = <89 IRQ_TYPE_LEVEL_LOW>; > + }; > + }; Best regards, Krzysztof