| Message ID | 20220607214113.4057684-1-bjorn.andersson@linaro.org |
|---|---|
| Headers | show |
| Series | arm64: dts: qcom: Introduce SC8280XP | expand |
On 07/06/2022 23:41, Bjorn Andersson wrote: > Introduce initial support for the Qualcomm SC8280XP platform, aka 8cx > Gen 3. This initial contribution supports SMP, CPUfreq, CPU cluster > idling, GCC, TLMM, SMMU, RPMh regulators, power-domains and clocks, > interconnects, some QUPs, UFS, remoteprocs, USB, watchdog, LLCC and > tsens. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2195 ++++++++++++++++++++++++ > 1 file changed, 2195 insertions(+) > create mode 100644 arch/arm64/boot/dts/qcom/sc8280xp.dtsi > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > new file mode 100644 > index 000000000000..4143813643ad > --- /dev/null > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > @@ -0,0 +1,2195 @@ > +// SPDX-License-Identifier: BSD-3-Clause > +/* > + * Copyright (c) 2021, The Linux Foundation. All rights reserved. > + * Copyright (c) 2022, Linaro Limited > + */ > + > +#include <dt-bindings/clock/qcom,gcc-sc8280xp.h> > +#include <dt-bindings/clock/qcom,rpmh.h> > +#include <dt-bindings/interrupt-controller/arm-gic.h> > +#include <dt-bindings/interconnect/qcom,sc8280xp.h> > +#include <dt-bindings/mailbox/qcom-ipcc.h> > +#include <dt-bindings/power/qcom-rpmpd.h> > +#include <dt-bindings/soc/qcom,rpmh-rsc.h> > +#include <dt-bindings/thermal/thermal.h> > + > +/ { > + interrupt-parent = <&intc>; > + > + #address-cells = <2>; > + #size-cells = <2>; > + > + clocks { > + xo_board: xo-board { xo-board-clk > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <38400000>; The clock is probably on the board, so the frequency should be rather defined in DTS. > + }; > + > + sleep_clk: sleep-clk { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <32764>; > + }; > + }; (...) > + > + qup1: geniqup@ac0000 { > + compatible = "qcom,geni-se-qup"; > + reg = <0 0x00ac0000 0 0x6000>; > + clocks = <&gcc GCC_QUPV3_WRAP_1_M_AHB_CLK>, > + <&gcc GCC_QUPV3_WRAP_1_S_AHB_CLK>; > + clock-names = "m-ahb", "s-ahb"; > + iommus = <&apps_smmu 0x83 0>; > + > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + status = "disabled"; > + }; > + > + ufs_mem_hc: ufshc@1d84000 { Just "ufs" as node name. > + compatible = "qcom,sc8280xp-ufshc", "qcom,ufshc", > + "jedec,ufs-2.0"; > + reg = <0 0x01d84000 0 0x3000>; > + interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>; > + phys = <&ufs_mem_phy_lanes>; > + phy-names = "ufsphy"; > + lanes-per-direction = <2>; > + #reset-cells = <1>; > + resets = <&gcc GCC_UFS_PHY_BCR>; > + reset-names = "rst"; > + > + power-domains = <&gcc UFS_PHY_GDSC>; > + required-opps = <&rpmhpd_opp_nom>; > + > + iommus = <&apps_smmu 0xe0 0x0>; > + > + clocks = <&gcc GCC_UFS_PHY_AXI_CLK> > + <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>, > + <&gcc GCC_UFS_PHY_AHB_CLK>, > + <&gcc GCC_UFS_PHY_UNIPRO_CORE_CLK>, > + <&rpmhcc RPMH_CXO_CLK>, > + <&gcc GCC_UFS_PHY_TX_SYMBOL_0_CLK>, > + <&gcc GCC_UFS_PHY_RX_SYMBOL_0_CLK>, > + <&gcc GCC_UFS_PHY_RX_SYMBOL_1_CLK>; > + clock-names = "core_clk", > + "bus_aggr_clk", > + "iface_clk", > + "core_clk_unipro", > + "ref_clk", > + "tx_lane0_sync_clk", > + "rx_lane0_sync_clk", > + "rx_lane1_sync_clk"; > + freq-table-hz = <75000000 300000000>, > + <0 0>, > + <0 0>, > + <75000000 300000000>, > + <0 0>, > + <0 0>, > + <0 0>, > + <0 0>; > + status = "disabled"; > + }; > + > + ufs_mem_phy: phy@1d87000 { > + compatible = "qcom,sc8280xp-qmp-ufs-phy"; > + reg = <0 0x01d87000 0 0xe10>; > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + clock-names = "ref", > + "ref_aux"; > + clocks = <&rpmhcc RPMH_CXO_CLK>, > + <&gcc GCC_UFS_PHY_PHY_AUX_CLK>; > + > + resets = <&ufs_mem_hc 0>; > + reset-names = "ufsphy"; > + status = "disabled"; > + > + ufs_mem_phy_lanes: phy@1d87400 { > + reg = <0 0x01d87400 0 0x108>, > + <0 0x01d87600 0 0x1e0>, > + <0 0x01d87c00 0 0x1dc>, > + <0 0x01d87800 0 0x108>, > + <0 0x01d87a00 0 0x1e0>; > + #phy-cells = <0>; > + #clock-cells = <0>; > + }; > + }; > + > + ufs_card_hc: ufshc@1da4000 { node name: ufs > + compatible = "qcom,sc8280xp-ufshc", "qcom,ufshc", > + "jedec,ufs-2.0"; > + reg = <0 0x01da4000 0 0x3000>; > + interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>; > + phys = <&ufs_card_phy_lanes>; > + phy-names = "ufsphy"; > + lanes-per-direction = <2>; > + #reset-cells = <1>; > + resets = <&gcc GCC_UFS_CARD_BCR>; > + reset-names = "rst"; > + > + power-domains = <&gcc UFS_CARD_GDSC>; > + > + iommus = <&apps_smmu 0x4a0 0x0>; > + > + clocks = <&gcc GCC_UFS_CARD_AXI_CLK>, > + <&gcc GCC_AGGRE_UFS_CARD_AXI_CLK>, > + <&gcc GCC_UFS_CARD_AHB_CLK>, > + <&gcc GCC_UFS_CARD_UNIPRO_CORE_CLK>, > + <&rpmhcc RPMH_CXO_CLK>, > + <&gcc GCC_UFS_CARD_TX_SYMBOL_0_CLK>, > + <&gcc GCC_UFS_CARD_RX_SYMBOL_0_CLK>, > + <&gcc GCC_UFS_CARD_RX_SYMBOL_1_CLK>; > + clock-names = "core_clk", > + "bus_aggr_clk", > + "iface_clk", > + "core_clk_unipro", > + "ref_clk", > + "tx_lane0_sync_clk", > + "rx_lane0_sync_clk", > + "rx_lane1_sync_clk"; > + freq-table-hz = <75000000 300000000>, > + <0 0>, > + <0 0>, > + <75000000 300000000>, > + <0 0>, > + <0 0>, > + <0 0>, > + <0 0>; > + status = "disabled"; > + }; > + > + ufs_card_phy: phy@1da7000 { > + compatible = "qcom,sc8280xp-qmp-ufs-phy"; > + reg = <0 0x01da7000 0 0xe10>; > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + clock-names = "ref", > + "ref_aux"; > + clocks = <&gcc GCC_UFS_1_CARD_CLKREF_CLK>, > + <&gcc GCC_UFS_CARD_PHY_AUX_CLK>; > + > + resets = <&ufs_card_hc 0>; > + reset-names = "ufsphy"; > + status = "disabled"; > + > + ufs_card_phy_lanes: phy@1da7400 { > + reg = <0 0x01da7400 0 0x108>, > + <0 0x01da7600 0 0x1e0>, > + <0 0x01da7c00 0 0x1dc>, > + <0 0x01da7800 0 0x108>, > + <0 0x01da7a00 0 0x1e0>; > + #phy-cells = <0>; > + #clock-cells = <0>; > + }; > + }; > + > + tcsr_mutex: hwlock@1f40000 { > + compatible = "qcom,tcsr-mutex"; > + reg = <0x0 0x01f40000 0x0 0x20000>; > + #hwlock-cells = <1>; > + }; > + > + usb_0_hsphy: phy@88e5000 { > + compatible = "qcom,sc8280xp-usb-hs-phy", > + "qcom,usb-snps-hs-5nm-phy"; > + reg = <0 0x088e5000 0 0x400>; > + status = "disabled"; status goes to the end > + #phy-cells = <0>; > + > + clocks = <&gcc GCC_USB2_HS0_CLKREF_CLK>; > + clock-names = "ref"; > + > + resets = <&gcc GCC_QUSB2PHY_PRIM_BCR>; > + }; > + > + usb_2_hsphy0: phy@88e7000 { > + compatible = "qcom,sc8280xp-usb-hs-phy", > + "qcom,usb-snps-hs-5nm-phy"; > + reg = <0 0x088e7000 0 0x400>; > + status = "disabled"; ditto > + #phy-cells = <0>; > + > + clocks = <&gcc GCC_USB2_HS0_CLKREF_CLK>; > + clock-names = "ref"; > + > + resets = <&gcc GCC_QUSB2PHY_HS0_MP_BCR>; > + }; > + > + usb_2_hsphy1: phy@88e8000 { > + compatible = "qcom,sc8280xp-usb-hs-phy", > + "qcom,usb-snps-hs-5nm-phy"; > + reg = <0 0x088e8000 0 0x400>; > + status = "disabled"; ditto > + #phy-cells = <0>; > + > + clocks = <&gcc GCC_USB2_HS1_CLKREF_CLK>; > + clock-names = "ref"; > + > + resets = <&gcc GCC_QUSB2PHY_HS1_MP_BCR>; > + }; > + > + usb_2_hsphy2: phy@88e9000 { > + compatible = "qcom,sc8280xp-usb-hs-phy", > + "qcom,usb-snps-hs-5nm-phy"; > + reg = <0 0x088e9000 0 0x400>; > + status = "disabled"; ditto and so on Best regards, Krzysztof
On 07/06/2022 23:41, Bjorn Andersson wrote: > Add basic support for the SC8280XP reference device, which allows it to > boot to a shell (using EFIFB) with functional storage (UFS), USB, > keyboard, touchpad, touchscreen, backlight and remoteprocs. > > The PMICs are, per socinfo, reused from other platforms. But given that > the address of the PMICs doesn't match other cases and that it's > desirable to label things according to the schematics a new dtsi file is > created to represent the reference combination of PMICs. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > arch/arm64/boot/dts/qcom/Makefile | 1 + > arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 423 +++++++++++++++++++ > arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi | 108 +++++ > 3 files changed, 532 insertions(+) > create mode 100644 arch/arm64/boot/dts/qcom/sc8280xp-crd.dts > create mode 100644 arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi > > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile > index 2f8aec2cc6db..ceeae094a59f 100644 > --- a/arch/arm64/boot/dts/qcom/Makefile > +++ b/arch/arm64/boot/dts/qcom/Makefile > @@ -89,6 +89,7 @@ dtb-$(CONFIG_ARCH_QCOM) += sc7280-herobrine-villager-r0.dtb > 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) += 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-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts > new file mode 100644 > index 000000000000..1031ee039107 > --- /dev/null > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts > @@ -0,0 +1,423 @@ > +// 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> > +#include <dt-bindings/input/gpio-keys.h> > +#include <dt-bindings/input/input.h> > +#include <dt-bindings/regulator/qcom,rpmh-regulator.h> > + > +#include "sc8280xp.dtsi" > +#include "sc8280xp-pmics.dtsi" > + > +/ { > + model = "Qualcomm SC8280XP CRD"; > + compatible = "qcom,sc8280xp-crd", "qcom,sc8280xp"; This needs to be documented in the bindings. > + > + aliases { > + serial0 = &qup2_uart17; > + }; > + > + backlight { > + compatible = "pwm-backlight"; > + pwms = <&pmc8280c_lpg 3 1000000>; > + enable-gpios = <&pmc8280_1_gpios 8 GPIO_ACTIVE_HIGH>; > + power-supply = <&vreg_edp_bl>; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&edp_bl_en>, <&edp_bl_pwm>; > + }; > + > + chosen { > + stdout-path = "serial0:115200n8"; > + }; > + > + vreg_edp_bl: edp-bl-regulator { > + compatible = "regulator-fixed"; > + regulator-name = "VREG_EDP_BL"; > + > + regulator-min-microvolt = <3600000>; > + regulator-max-microvolt = <3600000>; > + > + gpio = <&pmc8280_1_gpios 9 GPIO_ACTIVE_HIGH>; > + enable-active-high; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&edp_bl_reg_en>; > + > + regulator-boot-on; > + }; > + > + vreg_misc_3p3: misc-3p3-regulator { > + compatible = "regulator-fixed"; > + regulator-name = "VREG_MISC_3P3"; > + > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + > + gpio = <&pmc8280_1_gpios 0 GPIO_ACTIVE_HIGH>; > + enable-active-high; > + pinctrl-names = "default"; > + pinctrl-0 = <&misc_3p3_reg_en>; > + > + regulator-boot-on; > + regulator-always-on; > + }; > + > + reserved-memory { > + }; > +}; > + > +&apps_rsc { > + pmc8280-1-rpmh-regulators { > + compatible = "qcom,pm8350-rpmh-regulators"; > + qcom,pmic-id = "b"; > + > + vdd-l3-l5-supply = <&vreg_s11b>; > + > + vreg_s11b: smps11 { > + regulator-name = "vreg_s11b"; > + regulator-min-microvolt = <1272000>; > + regulator-max-microvolt = <1272000>; > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > + }; > + > + vreg_l3b: ldo3 { > + regulator-name = "vreg_l3b"; > + regulator-min-microvolt = <1200000>; > + regulator-max-microvolt = <1200000>; > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > + regulator-allow-set-load; > + regulator-boot-on; > + regulator-always-on; > + }; > + > + vreg_l4b: ldo4 { > + regulator-name = "vreg_l4b"; > + regulator-min-microvolt = <912000>; > + regulator-max-microvolt = <912000>; > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > + regulator-allow-set-load; > + }; > + > + vreg_l6b: ldo6 { > + regulator-name = "vreg_l6b"; > + regulator-min-microvolt = <880000>; > + regulator-max-microvolt = <880000>; > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > + regulator-allow-set-load; > + regulator-boot-on; > + }; > + }; > + > + pmc8280c-rpmh-regulators { > + compatible = "qcom,pm8350c-rpmh-regulators"; > + qcom,pmic-id = "c"; > + > + vreg_l1c: ldo1 { > + regulator-name = "vreg_l1c"; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > + regulator-allow-set-load; > + }; > + > + vreg_l7c: ldo7 { > + regulator-name = "vreg_l7c"; > + regulator-min-microvolt = <2504000>; > + regulator-max-microvolt = <2504000>; > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > + regulator-allow-set-load; > + }; > + > + vreg_l13c: ldo13 { > + regulator-name = "vreg_l13c"; > + regulator-min-microvolt = <3072000>; > + regulator-max-microvolt = <3072000>; > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > + regulator-allow-set-load; > + }; > + }; > + > + pmc8280-2-rpmh-regulators { > + compatible = "qcom,pm8350-rpmh-regulators"; > + qcom,pmic-id = "d"; > + > + vdd-l1-l4-supply = <&vreg_s11b>; > + > + vreg_l3d: ldo3 { > + regulator-name = "vreg_l3d"; > + regulator-min-microvolt = <1200000>; > + regulator-max-microvolt = <1200000>; > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > + regulator-allow-set-load; > + }; > + > + vreg_l4d: ldo4 { > + regulator-name = "vreg_l4d"; > + regulator-min-microvolt = <1200000>; > + regulator-max-microvolt = <1200000>; > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > + regulator-allow-set-load; > + }; > + > + vreg_l6d: ldo6 { > + regulator-name = "vreg_l6d"; > + regulator-min-microvolt = <880000>; > + regulator-max-microvolt = <880000>; > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > + regulator-allow-set-load; > + }; > + > + vreg_l7d: ldo7 { > + regulator-name = "vreg_l7d"; > + regulator-min-microvolt = <3072000>; > + regulator-max-microvolt = <3072000>; > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > + regulator-allow-set-load; > + }; > + > + vreg_l9d: ldo9 { > + regulator-name = "vreg_l9d"; > + regulator-min-microvolt = <912000>; > + regulator-max-microvolt = <912000>; > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > + regulator-allow-set-load; > + }; > + }; > + No need for blank line Best regards, Krzysztof
On 07/06/2022 23:41, Bjorn Andersson wrote: > Introduce the Qualcomm SA8540P automotive platform and the SA8295P ADP > development board. > > The SA8540P and SC8280XP are fairly similar, so the SA8540P is built > ontop of the SC8280XP dtsi to reduce duplication. As more advanced > features are integrated this might be re-evaluated. > > This initial contribution supports SMP, CPUFreq, cluster idle, UFS, RPMh > regulators, debug UART, PMICs, remoteprocs (NSPs crashes shortly after > booting) and USB. > > The SA8295P ADP contains four PM8450 PMICs, which according to their > revid are compatible with PM8150. They are defined within the ADP for > now, to avoid creating additional .dtsi files for PM8150 with just > addresses changed - and to allow using the labels from the schematics. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > arch/arm64/boot/dts/qcom/Makefile | 1 + > arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 434 +++++++++++++++++++++++ > arch/arm64/boot/dts/qcom/sa8540p.dtsi | 133 +++++++ > 3 files changed, 568 insertions(+) > create mode 100644 arch/arm64/boot/dts/qcom/sa8295p-adp.dts > create mode 100644 arch/arm64/boot/dts/qcom/sa8540p.dtsi > > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile > index ceeae094a59f..2f416b84b71c 100644 > --- a/arch/arm64/boot/dts/qcom/Makefile > +++ b/arch/arm64/boot/dts/qcom/Makefile > @@ -52,6 +52,7 @@ dtb-$(CONFIG_ARCH_QCOM) += qcs404-evb-1000.dtb > dtb-$(CONFIG_ARCH_QCOM) += qcs404-evb-4000.dtb > dtb-$(CONFIG_ARCH_QCOM) += qrb5165-rb5.dtb > dtb-$(CONFIG_ARCH_QCOM) += sa8155p-adp.dtb > +dtb-$(CONFIG_ARCH_QCOM) += sa8295p-adp.dtb > dtb-$(CONFIG_ARCH_QCOM) += sc7180-idp.dtb > dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-coachz-r1.dtb > dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-coachz-r1-lte.dtb > diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts > new file mode 100644 > index 000000000000..f78203d7bfd2 > --- /dev/null > +++ b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts > @@ -0,0 +1,434 @@ > +// 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> > +#include <dt-bindings/input/gpio-keys.h> > +#include <dt-bindings/input/input.h> > +#include <dt-bindings/regulator/qcom,rpmh-regulator.h> > +#include <dt-bindings/spmi/spmi.h> > + > +#include "sa8540p.dtsi" > + > +/ { > + model = "Qualcomm SA8295P ADP"; > + compatible = "qcom,sa8295p-adp", "qcom,sa8540p"; Similarly to previous patch - this needs to be documented. Rest looks ok. Best regards, Krzysztof
On Tue, Jun 07, 2022 at 02:41:11PM -0700, Bjorn Andersson wrote: > Introduce initial support for the Qualcomm SC8280XP platform, aka 8cx > Gen 3. This initial contribution supports SMP, CPUfreq, CPU cluster > idling, GCC, TLMM, SMMU, RPMh regulators, power-domains and clocks, > interconnects, some QUPs, UFS, remoteprocs, USB, watchdog, LLCC and > tsens. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2195 ++++++++++++++++++++++++ > 1 file changed, 2195 insertions(+) > create mode 100644 arch/arm64/boot/dts/qcom/sc8280xp.dtsi > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > new file mode 100644 > + aggre1_noc: interconncet-aggre1-noc { typo: interconnect > + compatible = "qcom,sc8280xp-aggre1-noc"; > + #interconnect-cells = <2>; > + qcom,bcm-voters = <&apps_bcm_voter>; > + }; > + usb_2_qmpphy1: phy-wrapper@88f1000 { > + compatible = "qcom,sc8280xp-qmp-usb3-uni-phy"; > + reg = <0 0x088f1000 0 0x1c8>; > + status = "disabled"; I'd also much prefer if you move the status property last throughout, as Krzysztof already mentioned. > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + clocks = <&gcc GCC_USB3_MP_PHY_AUX_CLK>, > + <&rpmhcc RPMH_CXO_CLK>, > + <&gcc GCC_USB3_MP1_CLKREF_CLK>, > + <&gcc GCC_USB3_MP_PHY_COM_AUX_CLK>; > + clock-names = "aux", "ref_clk_src", "ref", "com_aux"; > + > + resets = <&gcc GCC_USB3_UNIPHY_MP1_BCR>, > + <&gcc GCC_USB3UNIPHY_PHY_MP1_BCR>; > + reset-names = "phy", "common"; > + > + power-domains = <&gcc USB30_MP_GDSC>; > + > + usb_2_ssphy1: phy@88efe00 { Wrong unit address, should be phy@88f1e00 > + reg = <0 0x088f1e00 0 0x160>, > + <0 0x088f2000 0 0x1ec>, > + <0 0x088f1200 0 0x1f0>; > + #phy-cells = <0>; > + #clock-cells = <0>; > + clocks = <&gcc GCC_USB3_MP_PHY_PIPE_1_CLK>; > + clock-names = "pipe0"; > + clock-output-names = "usb2_phy1_pipe_clk"; > + }; > + }; > + usb_0_qmpphy: phy-wrapper@88ec000 { > + compatible = "qcom,sc8280xp-qmp-usb43dp-phy"; > + reg = <0 0x088ec000 0 0x1e4>, > + <0 0x088eb000 0 0x40>, > + <0 0x088ed000 0 0x1c8>; > + status = "disabled"; > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>, > + <&rpmhcc RPMH_CXO_CLK>, > + <&gcc GCC_USB4_EUD_CLKREF_CLK>, > + <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>; > + clock-names = "aux", "ref_clk_src", "ref", "com_aux"; > + > + resets = <&gcc GCC_USB3_PHY_PRIM_BCR>, > + <&gcc GCC_USB3_DP_PHY_PRIM_BCR>; > + reset-names = "phy", "common"; > + > + power-domains = <&gcc USB30_PRIM_GDSC>; > + > + usb_0_ssphy: usb3-phy@88e9200 { unit address, should be usb3-phy@88eb400 > + reg = <0 0x088eb400 0 0x100>, > + <0 0x088eb600 0 0x3ec>, > + <0 0x088ec400 0 0x1f0>, > + <0 0x088eba00 0 0x100>, > + <0 0x088ebc00 0 0x3ec>, > + <0 0x088ec700 0 0x64>; I've switched to specifying ranges in the PCIe PHY wrappers to avoid using absolute addresses here. I think we should do that throughout. > + #phy-cells = <0>; > + #clock-cells = <0>; > + clocks = <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>; > + clock-names = "pipe0"; > + clock-output-names = "usb0_phy_pipe_clk_src"; > + }; > + > + usb_0_dpphy: dp-phy@88ed200 { > + reg = <0 0x088ed200 0 0x200>, > + <0 0x088ed400 0 0x200>, > + <0 0x088eda00 0 0x200>, > + <0 0x088ea600 0 0x200>, > + <0 0x088ea800 0 0x200>; > + #clock-cells = <1>; > + #phy-cells = <0>; > + }; > + }; > + usb_1_qmpphy: phy-wrapper@8904000 { > + compatible = "qcom,sc8280xp-qmp-usb43dp-phy"; > + reg = <0 0x08904000 0 0x1e4>, > + <0 0x08903000 0 0x40>, > + <0 0x08905000 0 0x1c8>; > + status = "disabled"; > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + clocks = <&gcc GCC_USB3_SEC_PHY_AUX_CLK>, > + <&rpmhcc RPMH_CXO_CLK>, > + <&gcc GCC_USB4_CLKREF_CLK>, > + <&gcc GCC_USB3_SEC_PHY_COM_AUX_CLK>; > + clock-names = "aux", "ref_clk_src", "ref", "com_aux"; > + > + resets = <&gcc GCC_USB3_PHY_SEC_BCR>, > + <&gcc GCC_USB4_1_DP_PHY_PRIM_BCR>; > + reset-names = "phy", "common"; > + > + power-domains = <&gcc USB30_SEC_GDSC>; > + > + usb_1_ssphy: usb3-phy@88e9200 { Unit address, should be usb3-phy@08903400 > + reg = <0 0x08903400 0 0x100>, > + <0 0x08903c00 0 0x3ec>, > + <0 0x08904400 0 0x1f0>, > + <0 0x08903a00 0 0x100>, > + <0 0x08903c00 0 0x3ec>, > + <0 0x08904200 0 0x18>; > + #phy-cells = <0>; > + #clock-cells = <0>; > + clocks = <&gcc GCC_USB3_SEC_PHY_PIPE_CLK>; > + clock-names = "pipe0"; > + clock-output-names = "usb1_phy_pipe_clk_src"; > + }; > + > + usb_1_dpphy: dp-phy@88ed200 { > + reg = <0 0x08904200 0 0x200>, > + <0 0x08904400 0 0x200>, > + <0 0x08904a00 0 0x200>, > + <0 0x08904600 0 0x200>, > + <0 0x08904800 0 0x200>; > + #clock-cells = <1>; > + #phy-cells = <0>; > + }; > + }; > + > + usb_2: usb@a4f8800 { > + compatible = "qcom,sc8280xp-dwc3", "qcom,dwc3"; This compatible isn't described in any binding yet. > + reg = <0 0x0a4f8800 0 0x400>; > + status = "disabled"; > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + clocks = <&gcc GCC_USB30_MP_MASTER_CLK>, > + <&gcc GCC_CFG_NOC_USB3_MP_AXI_CLK>, > + <&gcc GCC_AGGRE_USB3_MP_AXI_CLK>, > + <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>, > + <&gcc GCC_USB30_MP_SLEEP_CLK>, > + <&gcc GCC_AGGRE_USB_NOC_AXI_CLK>, > + <&gcc GCC_AGGRE_USB_NOC_NORTH_AXI_CLK>, > + <&gcc GCC_AGGRE_USB_NOC_SOUTH_AXI_CLK>, > + <&gcc GCC_SYS_NOC_USB_AXI_CLK>; > + clock-names = "core", "iface", "bus_aggr", "utmi", "sleep", > + "noc_aggr", "noc_aggr_north", "noc_aggr_south", "noc_sys"; And as I mentioned off-list, neither are any of these interconnect clocks. Ideally, they'd be handled by an interconnect driver, but they'd at least need to be described in the binding, I guess. Same for the other controllers. > + > + assigned-clocks = <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>, > + <&gcc GCC_USB30_MP_MASTER_CLK>; > + assigned-clock-rates = <19200000>, <200000000>; > + > + interrupts-extended = <&pdc 127 IRQ_TYPE_EDGE_RISING>, > + <&pdc 126 IRQ_TYPE_EDGE_RISING>, > + <&pdc 129 IRQ_TYPE_EDGE_RISING>, > + <&pdc 128 IRQ_TYPE_EDGE_RISING>; > + interrupt-names = "hs_phy_irq", "dp_hs_phy_irq", > + "dm_hs_phy_irq", "ss_phy_irq"; These interrupts are all wrong (at least for ADP). The vendor dtsi does not specify "hs_phy_irq" at all and 127 is really "dp_hs_phy_irq", 126 is "dm_hs_phy_irq", while 129 and 128 are interrupts for the second HS phy. Since the driver does not yet support multiple ports and the binding hasn't been updated either, perhaps you should just leave out the multi-port usb_2 controller for now. > + > + power-domains = <&gcc USB30_MP_GDSC>; > + > + resets = <&gcc GCC_USB30_MP_BCR>; > + > + interconnects = <&aggre1_noc MASTER_USB3_MP 0 &mc_virt SLAVE_EBI1 0>, > + <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_USB3_MP 0>; > + interconnect-names = "usb-ddr", "apps-usb"; > + > + usb_2_dwc3: usb@a800000 { usb@a400000 > + compatible = "snps,dwc3"; > + reg = <0 0x0a400000 0 0xd93c>; > + interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>; > + iommus = <&apps_smmu 0x800 0x0>; > + phys = <&usb_2_hsphy0>, <&usb_2_ssphy0>; > + phy-names = "usb2-phy", "usb3-phy"; > + /* > + * TODO: Link controller to all 6 phys. > + * phys = <&usb_2_hsphy0>, <&usb_2_ssphy0>, > + * <&usb_2_hsphy1>, <&usb_2_ssphy1>, > + * <&usb_2_hsphy2>, > + * <&usb_2_hsphy3>; > + * phy-names = "usb2-phy", "usb3-phy"; > + */ Same argument with respect to the phy-names here, the binding should probably be updated first. > + }; > + }; > + usb_1: usb@a8f8800 { > + compatible = "qcom,sc8280xp-dwc3", "qcom,dwc3"; > + reg = <0 0x0a8f8800 0 0x400>; > + status = "disabled"; > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + clocks = <&gcc GCC_USB30_SEC_MASTER_CLK>, > + <&gcc GCC_CFG_NOC_USB3_SEC_AXI_CLK>, > + <&gcc GCC_AGGRE_USB3_SEC_AXI_CLK>, > + <&gcc GCC_USB30_SEC_MOCK_UTMI_CLK>, > + <&gcc GCC_USB30_SEC_SLEEP_CLK>, > + <&gcc GCC_AGGRE_USB_NOC_AXI_CLK>, > + <&gcc GCC_AGGRE_USB_NOC_NORTH_AXI_CLK>, > + <&gcc GCC_AGGRE_USB_NOC_SOUTH_AXI_CLK>, > + <&gcc GCC_SYS_NOC_USB_AXI_CLK>; > + clock-names = "core", "iface", "bus_aggr", "utmi", "sleep", > + "noc_aggr", "noc_aggr_north", "noc_aggr_south", "noc_sys"; > + > + assigned-clocks = <&gcc GCC_USB30_SEC_MOCK_UTMI_CLK>, > + <&gcc GCC_USB30_SEC_MASTER_CLK>; > + assigned-clock-rates = <19200000>, <200000000>; > + > + interrupts-extended = <&intc GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>, > + <&pdc 12 IRQ_TYPE_EDGE_BOTH>, > + <&pdc 13 IRQ_TYPE_EDGE_BOTH>, > + <&pdc 16 IRQ_TYPE_LEVEL_HIGH>; The vendor dtsi has 136 instead of 16 here as ss_phy_irq (16 is a usb_2 PHY IRQ). > + interrupt-names = "hs_phy_irq", "dp_hs_phy_irq", > + "dm_hs_phy_irq", "ss_phy_irq"; > + > + power-domains = <&gcc USB30_SEC_GDSC>; > + > + resets = <&gcc GCC_USB30_SEC_BCR>; > + > + interconnects = <&aggre1_noc MASTER_USB3_1 0 &mc_virt SLAVE_EBI1 0>, > + <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_USB3_1 0>; > + interconnect-names = "usb-ddr", "apps-usb"; > + > + usb_1_dwc3: usb@a800000 { > + compatible = "snps,dwc3"; > + reg = <0 0x0a800000 0 0xcd00>; > + interrupts = <GIC_SPI 810 IRQ_TYPE_LEVEL_HIGH>; > + iommus = <&apps_smmu 0x860 0x0>; > + phys = <&usb_1_hsphy>, <&usb_1_ssphy>; > + phy-names = "usb2-phy", "usb3-phy"; > + }; > + }; > + > + system-cache-controller@9200000 { > + compatible = "qcom,sc8280xp-llcc"; > + reg = <0 0x09200000 0 0x58000>, <0 0x09600000 0 0x58000>; > + reg-names = "llcc_base", "llcc_broadcast_base"; > + interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>; > + }; The system-cache-controller node is placed out of order. Johan
On Tue, Jun 07, 2022 at 02:41:12PM -0700, Bjorn Andersson wrote: > Add basic support for the SC8280XP reference device, which allows it to > boot to a shell (using EFIFB) with functional storage (UFS), USB, > keyboard, touchpad, touchscreen, backlight and remoteprocs. > > The PMICs are, per socinfo, reused from other platforms. But given that > the address of the PMICs doesn't match other cases and that it's > desirable to label things according to the schematics a new dtsi file is > created to represent the reference combination of PMICs. nit: missing p in "sc8280xp" in Subject. > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > arch/arm64/boot/dts/qcom/Makefile | 1 + > arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 423 +++++++++++++++++++ > arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi | 108 +++++ > 3 files changed, 532 insertions(+) > create mode 100644 arch/arm64/boot/dts/qcom/sc8280xp-crd.dts > create mode 100644 arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi > + vreg_misc_3p3: misc-3p3-regulator { > + compatible = "regulator-fixed"; > + regulator-name = "VREG_MISC_3P3"; > + > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + > + gpio = <&pmc8280_1_gpios 0 GPIO_ACTIVE_HIGH>; The PMIC gpios are 1-based, so this should be gpio = <&pmc8280_1_gpios 1 GPIO_ACTIVE_HIGH>; or the regulator fails to probe. > + enable-active-high; > + pinctrl-names = "default"; > + pinctrl-0 = <&misc_3p3_reg_en>; > + > + regulator-boot-on; > + regulator-always-on; > + }; > + > + reserved-memory { > + }; > +}; > +&qup0_i2c4 { > + status = "okay"; Please move the status property last throughout here too. > + clock-frequency = <400000>; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&qup0_i2c4_default>, <&ts0_default>; > + > + hid@10 { I've changed this to use the more descriptive name "touchscreen". > + compatible = "hid-over-i2c"; > + reg = <0x10>; > + hid-descr-addr = <0x1>; > + > + interrupts-extended = <&tlmm 175 IRQ_TYPE_LEVEL_LOW>; > + }; > +}; > +&qup2_i2c5 { > + status = "okay"; > + clock-frequency = <400000>; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&qup2_i2c5_default>, <&kybd_default>, <&tpad_default>; > + > + hid@15 { And this to "touchpad@15" > + compatible = "hid-over-i2c"; > + reg = <0x15>; > + hid-descr-addr = <0x1>; > + > + interrupts-extended = <&tlmm 182 IRQ_TYPE_LEVEL_LOW>; > + }; > + > + hid@68 { And keyboard@68 Sure these are multifunction devices, but this is the primary function. > + compatible = "hid-over-i2c"; > + reg = <0x68>; > + hid-descr-addr = <0x1>; > + > + interrupts-extended = <&tlmm 104 IRQ_TYPE_LEVEL_LOW>; > + }; > +}; > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi > new file mode 100644 > index 000000000000..36ed7d808ab8 > --- /dev/null > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi > + pmc8280c: pmic@2 { > + compatible = "qcom,pm8350c", "qcom,spmi-pmic"; > + reg = <0x2 SPMI_USID>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + pmc8280c_gpios: gpio@8800 { > + compatible = "qcom,pm8350c-gpio", "qcom,spmi-gpio"; > + reg = <0x8800>; > + gpio-controller; > + gpio-ranges = <&pmc8280c_gpios 0 0 9>; > + #gpio-cells = <2>; > + interrupt-controller; > + #interrupt-cells = <2>; > + }; > + > + pmc8280c_lpg: lpg@e800 { I renamed the node (and label suffix) "pwm" when I noticed that the binding had changed in mainline. Since this device is used as a PWM provider I guess that's a better name? > + compatible = "qcom,pm8350c-pwm"; > + reg = <0xe800>; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + #pwm-cells = <2>; > + > + status = "disabled"; > + }; > + }; Johan
On Wed 08 Jun 03:18 CDT 2022, Krzysztof Kozlowski wrote: > On 07/06/2022 23:41, Bjorn Andersson wrote: > > Introduce initial support for the Qualcomm SC8280XP platform, aka 8cx > > Gen 3. This initial contribution supports SMP, CPUfreq, CPU cluster > > idling, GCC, TLMM, SMMU, RPMh regulators, power-domains and clocks, > > interconnects, some QUPs, UFS, remoteprocs, USB, watchdog, LLCC and > > tsens. > > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > --- > > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2195 ++++++++++++++++++++++++ > > 1 file changed, 2195 insertions(+) > > create mode 100644 arch/arm64/boot/dts/qcom/sc8280xp.dtsi > > > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > > new file mode 100644 > > index 000000000000..4143813643ad > > --- /dev/null > > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > > @@ -0,0 +1,2195 @@ > > +// SPDX-License-Identifier: BSD-3-Clause > > +/* > > + * Copyright (c) 2021, The Linux Foundation. All rights reserved. > > + * Copyright (c) 2022, Linaro Limited > > + */ > > + > > +#include <dt-bindings/clock/qcom,gcc-sc8280xp.h> > > +#include <dt-bindings/clock/qcom,rpmh.h> > > +#include <dt-bindings/interrupt-controller/arm-gic.h> > > +#include <dt-bindings/interconnect/qcom,sc8280xp.h> > > +#include <dt-bindings/mailbox/qcom-ipcc.h> > > +#include <dt-bindings/power/qcom-rpmpd.h> > > +#include <dt-bindings/soc/qcom,rpmh-rsc.h> > > +#include <dt-bindings/thermal/thermal.h> > > + > > +/ { > > + interrupt-parent = <&intc>; > > + > > + #address-cells = <2>; > > + #size-cells = <2>; > > + > > + clocks { > > + xo_board: xo-board { > > xo-board-clk > > > + compatible = "fixed-clock"; > > + #clock-cells = <0>; > > + clock-frequency = <38400000>; > > The clock is probably on the board, so the frequency should be rather > defined in DTS. > It's an interesting question, but I don't think it's possible to change the rate of this clock from one board to another. So I think it's best to keep this in the .dtsi, to avoid unnecessary duplication. Thanks for the feedback, will update the remaining things. Regards, Bjorn > > + }; > > + > > + sleep_clk: sleep-clk { > > + compatible = "fixed-clock"; > > + #clock-cells = <0>; > > + clock-frequency = <32764>; > > + }; > > + }; > > (...) > > > + > > + qup1: geniqup@ac0000 { > > + compatible = "qcom,geni-se-qup"; > > + reg = <0 0x00ac0000 0 0x6000>; > > + clocks = <&gcc GCC_QUPV3_WRAP_1_M_AHB_CLK>, > > + <&gcc GCC_QUPV3_WRAP_1_S_AHB_CLK>; > > + clock-names = "m-ahb", "s-ahb"; > > + iommus = <&apps_smmu 0x83 0>; > > + > > + #address-cells = <2>; > > + #size-cells = <2>; > > + ranges; > > + > > + status = "disabled"; > > + }; > > + > > + ufs_mem_hc: ufshc@1d84000 { > > Just "ufs" as node name. > > > + compatible = "qcom,sc8280xp-ufshc", "qcom,ufshc", > > + "jedec,ufs-2.0"; > > + reg = <0 0x01d84000 0 0x3000>; > > + interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>; > > + phys = <&ufs_mem_phy_lanes>; > > + phy-names = "ufsphy"; > > + lanes-per-direction = <2>; > > + #reset-cells = <1>; > > + resets = <&gcc GCC_UFS_PHY_BCR>; > > + reset-names = "rst"; > > + > > + power-domains = <&gcc UFS_PHY_GDSC>; > > + required-opps = <&rpmhpd_opp_nom>; > > + > > + iommus = <&apps_smmu 0xe0 0x0>; > > + > > + clocks = <&gcc GCC_UFS_PHY_AXI_CLK> > > + <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>, > > + <&gcc GCC_UFS_PHY_AHB_CLK>, > > + <&gcc GCC_UFS_PHY_UNIPRO_CORE_CLK>, > > + <&rpmhcc RPMH_CXO_CLK>, > > + <&gcc GCC_UFS_PHY_TX_SYMBOL_0_CLK>, > > + <&gcc GCC_UFS_PHY_RX_SYMBOL_0_CLK>, > > + <&gcc GCC_UFS_PHY_RX_SYMBOL_1_CLK>; > > + clock-names = "core_clk", > > + "bus_aggr_clk", > > + "iface_clk", > > + "core_clk_unipro", > > + "ref_clk", > > + "tx_lane0_sync_clk", > > + "rx_lane0_sync_clk", > > + "rx_lane1_sync_clk"; > > + freq-table-hz = <75000000 300000000>, > > + <0 0>, > > + <0 0>, > > + <75000000 300000000>, > > + <0 0>, > > + <0 0>, > > + <0 0>, > > + <0 0>; > > + status = "disabled"; > > + }; > > + > > + ufs_mem_phy: phy@1d87000 { > > + compatible = "qcom,sc8280xp-qmp-ufs-phy"; > > + reg = <0 0x01d87000 0 0xe10>; > > + #address-cells = <2>; > > + #size-cells = <2>; > > + ranges; > > + clock-names = "ref", > > + "ref_aux"; > > + clocks = <&rpmhcc RPMH_CXO_CLK>, > > + <&gcc GCC_UFS_PHY_PHY_AUX_CLK>; > > + > > + resets = <&ufs_mem_hc 0>; > > + reset-names = "ufsphy"; > > + status = "disabled"; > > + > > + ufs_mem_phy_lanes: phy@1d87400 { > > + reg = <0 0x01d87400 0 0x108>, > > + <0 0x01d87600 0 0x1e0>, > > + <0 0x01d87c00 0 0x1dc>, > > + <0 0x01d87800 0 0x108>, > > + <0 0x01d87a00 0 0x1e0>; > > + #phy-cells = <0>; > > + #clock-cells = <0>; > > + }; > > + }; > > + > > + ufs_card_hc: ufshc@1da4000 { > > node name: ufs > > > + compatible = "qcom,sc8280xp-ufshc", "qcom,ufshc", > > + "jedec,ufs-2.0"; > > + reg = <0 0x01da4000 0 0x3000>; > > + interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>; > > + phys = <&ufs_card_phy_lanes>; > > + phy-names = "ufsphy"; > > + lanes-per-direction = <2>; > > + #reset-cells = <1>; > > + resets = <&gcc GCC_UFS_CARD_BCR>; > > + reset-names = "rst"; > > + > > + power-domains = <&gcc UFS_CARD_GDSC>; > > + > > + iommus = <&apps_smmu 0x4a0 0x0>; > > + > > + clocks = <&gcc GCC_UFS_CARD_AXI_CLK>, > > + <&gcc GCC_AGGRE_UFS_CARD_AXI_CLK>, > > + <&gcc GCC_UFS_CARD_AHB_CLK>, > > + <&gcc GCC_UFS_CARD_UNIPRO_CORE_CLK>, > > + <&rpmhcc RPMH_CXO_CLK>, > > + <&gcc GCC_UFS_CARD_TX_SYMBOL_0_CLK>, > > + <&gcc GCC_UFS_CARD_RX_SYMBOL_0_CLK>, > > + <&gcc GCC_UFS_CARD_RX_SYMBOL_1_CLK>; > > + clock-names = "core_clk", > > + "bus_aggr_clk", > > + "iface_clk", > > + "core_clk_unipro", > > + "ref_clk", > > + "tx_lane0_sync_clk", > > + "rx_lane0_sync_clk", > > + "rx_lane1_sync_clk"; > > + freq-table-hz = <75000000 300000000>, > > + <0 0>, > > + <0 0>, > > + <75000000 300000000>, > > + <0 0>, > > + <0 0>, > > + <0 0>, > > + <0 0>; > > + status = "disabled"; > > + }; > > + > > + ufs_card_phy: phy@1da7000 { > > + compatible = "qcom,sc8280xp-qmp-ufs-phy"; > > + reg = <0 0x01da7000 0 0xe10>; > > + #address-cells = <2>; > > + #size-cells = <2>; > > + ranges; > > + clock-names = "ref", > > + "ref_aux"; > > + clocks = <&gcc GCC_UFS_1_CARD_CLKREF_CLK>, > > + <&gcc GCC_UFS_CARD_PHY_AUX_CLK>; > > + > > + resets = <&ufs_card_hc 0>; > > + reset-names = "ufsphy"; > > + status = "disabled"; > > + > > + ufs_card_phy_lanes: phy@1da7400 { > > + reg = <0 0x01da7400 0 0x108>, > > + <0 0x01da7600 0 0x1e0>, > > + <0 0x01da7c00 0 0x1dc>, > > + <0 0x01da7800 0 0x108>, > > + <0 0x01da7a00 0 0x1e0>; > > + #phy-cells = <0>; > > + #clock-cells = <0>; > > + }; > > + }; > > + > > + tcsr_mutex: hwlock@1f40000 { > > + compatible = "qcom,tcsr-mutex"; > > + reg = <0x0 0x01f40000 0x0 0x20000>; > > + #hwlock-cells = <1>; > > + }; > > + > > + usb_0_hsphy: phy@88e5000 { > > + compatible = "qcom,sc8280xp-usb-hs-phy", > > + "qcom,usb-snps-hs-5nm-phy"; > > + reg = <0 0x088e5000 0 0x400>; > > + status = "disabled"; > > status goes to the end > > > + #phy-cells = <0>; > > + > > + clocks = <&gcc GCC_USB2_HS0_CLKREF_CLK>; > > + clock-names = "ref"; > > + > > + resets = <&gcc GCC_QUSB2PHY_PRIM_BCR>; > > + }; > > + > > + usb_2_hsphy0: phy@88e7000 { > > + compatible = "qcom,sc8280xp-usb-hs-phy", > > + "qcom,usb-snps-hs-5nm-phy"; > > + reg = <0 0x088e7000 0 0x400>; > > + status = "disabled"; > > ditto > > > + #phy-cells = <0>; > > + > > + clocks = <&gcc GCC_USB2_HS0_CLKREF_CLK>; > > + clock-names = "ref"; > > + > > + resets = <&gcc GCC_QUSB2PHY_HS0_MP_BCR>; > > + }; > > + > > + usb_2_hsphy1: phy@88e8000 { > > + compatible = "qcom,sc8280xp-usb-hs-phy", > > + "qcom,usb-snps-hs-5nm-phy"; > > + reg = <0 0x088e8000 0 0x400>; > > + status = "disabled"; > > ditto > > > + #phy-cells = <0>; > > + > > + clocks = <&gcc GCC_USB2_HS1_CLKREF_CLK>; > > + clock-names = "ref"; > > + > > + resets = <&gcc GCC_QUSB2PHY_HS1_MP_BCR>; > > + }; > > + > > + usb_2_hsphy2: phy@88e9000 { > > + compatible = "qcom,sc8280xp-usb-hs-phy", > > + "qcom,usb-snps-hs-5nm-phy"; > > + reg = <0 0x088e9000 0 0x400>; > > + status = "disabled"; > > ditto and so on > > Best regards, > Krzysztof
On Wed 08 Jun 11:17 CDT 2022, Johan Hovold wrote: > On Tue, Jun 07, 2022 at 02:41:12PM -0700, Bjorn Andersson wrote: > > Add basic support for the SC8280XP reference device, which allows it to > > boot to a shell (using EFIFB) with functional storage (UFS), USB, > > keyboard, touchpad, touchscreen, backlight and remoteprocs. > > > > The PMICs are, per socinfo, reused from other platforms. But given that > > the address of the PMICs doesn't match other cases and that it's > > desirable to label things according to the schematics a new dtsi file is > > created to represent the reference combination of PMICs. > > nit: missing p in "sc8280xp" in Subject. > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > --- > > arch/arm64/boot/dts/qcom/Makefile | 1 + > > arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 423 +++++++++++++++++++ > > arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi | 108 +++++ > > 3 files changed, 532 insertions(+) > > create mode 100644 arch/arm64/boot/dts/qcom/sc8280xp-crd.dts > > create mode 100644 arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi > > > + vreg_misc_3p3: misc-3p3-regulator { > > + compatible = "regulator-fixed"; > > + regulator-name = "VREG_MISC_3P3"; > > + > > + regulator-min-microvolt = <3300000>; > > + regulator-max-microvolt = <3300000>; > > + > > + gpio = <&pmc8280_1_gpios 0 GPIO_ACTIVE_HIGH>; > > The PMIC gpios are 1-based, so this should be > > gpio = <&pmc8280_1_gpios 1 GPIO_ACTIVE_HIGH>; > > or the regulator fails to probe. > > > + enable-active-high; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&misc_3p3_reg_en>; > > + > > + regulator-boot-on; > > + regulator-always-on; > > + }; > > + > > + reserved-memory { > > + }; > > +}; > > > +&qup0_i2c4 { > > + status = "okay"; > > Please move the status property last throughout here too. > > > + clock-frequency = <400000>; > > + > > + pinctrl-names = "default"; > > + pinctrl-0 = <&qup0_i2c4_default>, <&ts0_default>; > > + > > + hid@10 { > > I've changed this to use the more descriptive name "touchscreen". > > > + compatible = "hid-over-i2c"; > > + reg = <0x10>; > > + hid-descr-addr = <0x1>; > > + > > + interrupts-extended = <&tlmm 175 IRQ_TYPE_LEVEL_LOW>; > > + }; > > +}; > > > +&qup2_i2c5 { > > + status = "okay"; > > + clock-frequency = <400000>; > > + > > + pinctrl-names = "default"; > > + pinctrl-0 = <&qup2_i2c5_default>, <&kybd_default>, <&tpad_default>; > > + > > + hid@15 { > > And this to "touchpad@15" > > > + compatible = "hid-over-i2c"; > > + reg = <0x15>; > > + hid-descr-addr = <0x1>; > > + > > + interrupts-extended = <&tlmm 182 IRQ_TYPE_LEVEL_LOW>; > > + }; > > + > > + hid@68 { > > And keyboard@68 > > Sure these are multifunction devices, but this is the primary function. > > > + compatible = "hid-over-i2c"; > > + reg = <0x68>; > > + hid-descr-addr = <0x1>; > > + > > + interrupts-extended = <&tlmm 104 IRQ_TYPE_LEVEL_LOW>; > > + }; > > +}; > > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi > > new file mode 100644 > > index 000000000000..36ed7d808ab8 > > --- /dev/null > > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi > > > + pmc8280c: pmic@2 { > > + compatible = "qcom,pm8350c", "qcom,spmi-pmic"; > > + reg = <0x2 SPMI_USID>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + pmc8280c_gpios: gpio@8800 { > > + compatible = "qcom,pm8350c-gpio", "qcom,spmi-gpio"; > > + reg = <0x8800>; > > + gpio-controller; > > + gpio-ranges = <&pmc8280c_gpios 0 0 9>; > > + #gpio-cells = <2>; > > + interrupt-controller; > > + #interrupt-cells = <2>; > > + }; > > + > > + pmc8280c_lpg: lpg@e800 { > > I renamed the node (and label suffix) "pwm" when I noticed that the > binding had changed in mainline. > > Since this device is used as a PWM provider I guess that's a better > name? > The pm8350c seems to include a number of PWM channels, the RGB current sink (triled) and the LUT block - together making up the "Light Pulse Generator". So with that in mind, the compatible seems to have come from the fact that the author only intended to use one of the PWM sub-blocks... Thanks for the feedback on the series, will updated and resubmit accordingly. Regards, Bjorn > > + compatible = "qcom,pm8350c-pwm"; > > + reg = <0xe800>; > > + > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + #pwm-cells = <2>; > > + > > + status = "disabled"; > > + }; > > + }; > > Johan
On 21/06/2022 05:37, Bjorn Andersson wrote: > On Wed 08 Jun 03:18 CDT 2022, Krzysztof Kozlowski wrote: > >> On 07/06/2022 23:41, Bjorn Andersson wrote: >>> Introduce initial support for the Qualcomm SC8280XP platform, aka 8cx >>> Gen 3. This initial contribution supports SMP, CPUfreq, CPU cluster >>> idling, GCC, TLMM, SMMU, RPMh regulators, power-domains and clocks, >>> interconnects, some QUPs, UFS, remoteprocs, USB, watchdog, LLCC and >>> tsens. >>> >>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> >>> --- >>> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2195 ++++++++++++++++++++++++ >>> 1 file changed, 2195 insertions(+) >>> create mode 100644 arch/arm64/boot/dts/qcom/sc8280xp.dtsi >>> >>> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >>> new file mode 100644 >>> index 000000000000..4143813643ad >>> --- /dev/null >>> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >>> @@ -0,0 +1,2195 @@ >>> +// SPDX-License-Identifier: BSD-3-Clause >>> +/* >>> + * Copyright (c) 2021, The Linux Foundation. All rights reserved. >>> + * Copyright (c) 2022, Linaro Limited >>> + */ >>> + >>> +#include <dt-bindings/clock/qcom,gcc-sc8280xp.h> >>> +#include <dt-bindings/clock/qcom,rpmh.h> >>> +#include <dt-bindings/interrupt-controller/arm-gic.h> >>> +#include <dt-bindings/interconnect/qcom,sc8280xp.h> >>> +#include <dt-bindings/mailbox/qcom-ipcc.h> >>> +#include <dt-bindings/power/qcom-rpmpd.h> >>> +#include <dt-bindings/soc/qcom,rpmh-rsc.h> >>> +#include <dt-bindings/thermal/thermal.h> >>> + >>> +/ { >>> + interrupt-parent = <&intc>; >>> + >>> + #address-cells = <2>; >>> + #size-cells = <2>; >>> + >>> + clocks { >>> + xo_board: xo-board { >> >> xo-board-clk >> >>> + compatible = "fixed-clock"; >>> + #clock-cells = <0>; >>> + clock-frequency = <38400000>; >> >> The clock is probably on the board, so the frequency should be rather >> defined in DTS. >> > > It's an interesting question, but I don't think it's possible to change > the rate of this clock from one board to another. > > So I think it's best to keep this in the .dtsi, to avoid unnecessary > duplication. It does not matter whether the frequency can be changed or not. This is the same on almost every SoC and the same comments appear every time - the clock is a property of the board, not of the SoC, so it should be in the board DTSI. To avoid the duplication you can indeed keep here most of the clock properties, but the frequency must be in board DTS. Best regards, Krzysztof
On Mon 20 Jun 23:53 PDT 2022, Krzysztof Kozlowski wrote: > On 21/06/2022 05:37, Bjorn Andersson wrote: > > On Wed 08 Jun 03:18 CDT 2022, Krzysztof Kozlowski wrote: > > > >> On 07/06/2022 23:41, Bjorn Andersson wrote: > >>> Introduce initial support for the Qualcomm SC8280XP platform, aka 8cx > >>> Gen 3. This initial contribution supports SMP, CPUfreq, CPU cluster > >>> idling, GCC, TLMM, SMMU, RPMh regulators, power-domains and clocks, > >>> interconnects, some QUPs, UFS, remoteprocs, USB, watchdog, LLCC and > >>> tsens. > >>> > >>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > >>> --- > >>> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2195 ++++++++++++++++++++++++ > >>> 1 file changed, 2195 insertions(+) > >>> create mode 100644 arch/arm64/boot/dts/qcom/sc8280xp.dtsi > >>> > >>> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > >>> new file mode 100644 > >>> index 000000000000..4143813643ad > >>> --- /dev/null > >>> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > >>> @@ -0,0 +1,2195 @@ > >>> +// SPDX-License-Identifier: BSD-3-Clause > >>> +/* > >>> + * Copyright (c) 2021, The Linux Foundation. All rights reserved. > >>> + * Copyright (c) 2022, Linaro Limited > >>> + */ > >>> + > >>> +#include <dt-bindings/clock/qcom,gcc-sc8280xp.h> > >>> +#include <dt-bindings/clock/qcom,rpmh.h> > >>> +#include <dt-bindings/interrupt-controller/arm-gic.h> > >>> +#include <dt-bindings/interconnect/qcom,sc8280xp.h> > >>> +#include <dt-bindings/mailbox/qcom-ipcc.h> > >>> +#include <dt-bindings/power/qcom-rpmpd.h> > >>> +#include <dt-bindings/soc/qcom,rpmh-rsc.h> > >>> +#include <dt-bindings/thermal/thermal.h> > >>> + > >>> +/ { > >>> + interrupt-parent = <&intc>; > >>> + > >>> + #address-cells = <2>; > >>> + #size-cells = <2>; > >>> + > >>> + clocks { > >>> + xo_board: xo-board { > >> > >> xo-board-clk > >> > >>> + compatible = "fixed-clock"; > >>> + #clock-cells = <0>; > >>> + clock-frequency = <38400000>; > >> > >> The clock is probably on the board, so the frequency should be rather > >> defined in DTS. > >> > > > > It's an interesting question, but I don't think it's possible to change > > the rate of this clock from one board to another. > > > > So I think it's best to keep this in the .dtsi, to avoid unnecessary > > duplication. > > It does not matter whether the frequency can be changed or not. This is > the same on almost every SoC and the same comments appear every time - > the clock is a property of the board, not of the SoC, so it should be in > the board DTSI. To avoid the duplication you can indeed keep here most > of the clock properties, but the frequency must be in board DTS. > I find this to be a rather strict interpretation of "board specific", but I'm okay with it. Regards, Bjorn
On 22/06/2022 05:32, Bjorn Andersson wrote: >>>> >>> >>> It's an interesting question, but I don't think it's possible to change >>> the rate of this clock from one board to another. >>> >>> So I think it's best to keep this in the .dtsi, to avoid unnecessary >>> duplication. >> >> It does not matter whether the frequency can be changed or not. This is >> the same on almost every SoC and the same comments appear every time - >> the clock is a property of the board, not of the SoC, so it should be in >> the board DTSI. To avoid the duplication you can indeed keep here most >> of the clock properties, but the frequency must be in board DTS. >> > > I find this to be a rather strict interpretation of "board specific", > but I'm okay with it. Yes, it is quite strict, but in the long term helps - people explicitly need to enable/fill properties in new board DTSes, which hopefully will trigger some thinking - "do I really have 385 MHz XO clock"? Although here it might not really important, but that approach is useful for all other cases, including aliases and buses. And one day, such clear design will help new guys doing new hardware bring-up and they will say: "I will start my new DTS board on some examples from mainline". :) Best regards, Krzysztof
On 07-06-22, 14:41, Bjorn Andersson wrote: > + cpus { > + #address-cells = <2>; > + #size-cells = <0>; > + > + CPU0: cpu@0 { > + device_type = "cpu"; > + compatible = "qcom,kryo"; Generic kryo and not kryoxxx? > + firmware { > + scm: scm { > + compatible = "qcom,scm-sc8280xp", "qcom,scm"; I dont see patch documenting this compatible, was it added earlier, if not pls add.. > + }; > + }; > + > + aggre1_noc: interconncet-aggre1-noc { s/interconncet/interconnect Hmmm I thought it was required that node name should be interconnect@x > + qup1: geniqup@ac0000 { > + compatible = "qcom,geni-se-qup"; > + reg = <0 0x00ac0000 0 0x6000>; > + clocks = <&gcc GCC_QUPV3_WRAP_1_M_AHB_CLK>, > + <&gcc GCC_QUPV3_WRAP_1_S_AHB_CLK>; > + clock-names = "m-ahb", "s-ahb"; > + iommus = <&apps_smmu 0x83 0>; > + > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + status = "disabled"; > + }; why add this, if no i2c/spi/uarts are defined under this..? > + usb_0_hsphy: phy@88e5000 { > + compatible = "qcom,sc8280xp-usb-hs-phy", > + "qcom,usb-snps-hs-5nm-phy"; > + reg = <0 0x088e5000 0 0x400>; this doesn't match with node address above (I think W=1 would warn you of such mismatches, useful to run on new dts > + > + spmi_bus: spmi@c440000 { Is the new v7 spmi or older one?
On Thu, Jun 23, 2022 at 10:52:50AM +0530, Vinod Koul wrote: > On 07-06-22, 14:41, Bjorn Andersson wrote: > > + aggre1_noc: interconncet-aggre1-noc { > s/interconncet/interconnect Note that you're reviewing v1 and that this has since been fixed. > Hmmm I thought it was required that node name should be interconnect@x > > + usb_0_hsphy: phy@88e5000 { > > + compatible = "qcom,sc8280xp-usb-hs-phy", > > + "qcom,usb-snps-hs-5nm-phy"; > > + reg = <0 0x088e5000 0 0x400>; > > this doesn't match with node address above (I think W=1 would warn you > of such mismatches, useful to run on new dts This unit address is actually correct, but there were some other mismatches that were also fixed in v2. Johan