[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f42baf44-3143-8895-5270-e1af6e0d66ba@linaro.org>
Date: Fri, 4 Nov 2022 15:01:39 -0400
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>, agross@...nel.org
Cc: andersson@...nel.org, konrad.dybcio@...ainline.org,
robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
lee@...nel.org, ulf.hansson@...aro.org,
srinivas.kandagatla@...aro.org, jic23@...nel.org, lars@...afoo.de,
keescook@...omium.org, tony.luck@...el.com, gpiccoli@...lia.com,
bhupesh.sharma@...aro.org, linux-arm-msm@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-mmc@...r.kernel.org, linux-iio@...r.kernel.org,
linux-hardening@...r.kernel.org, marijn.suijten@...ainline.org,
kernel@...labora.com, luca@...tu.xyz, a39.skl@...il.com,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@...ainline.org>
Subject: Re: [PATCH 8/9] arm64: dts: qcom: Add DTS for MSM8976 and MSM8956
SoCs
On 04/11/2022 13:21, AngeloGioacchino Del Regno wrote:
> From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...ainline.org>
>
> This commit adds device trees for MSM8956 and MSM8976 SoCs.
> They are *almost* identical, with minor differences, such as
> MSM8956 having two A72 cores less.
>
Thank you for your patch. There is something to discuss/improve.
> +
> + sdhc_1: mmc@...4000 {
> + compatible = "qcom,msm8976-sdhci", "qcom,sdhci-msm-v4";
> + reg = <0x07824900 0x500>, <0x07824000 0x800>;
> + reg-names = "hc", "core";
> +
> + interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "hc_irq", "pwr_irq";
> +
> + clocks = <&gcc GCC_SDCC1_AHB_CLK>,
> + <&gcc GCC_SDCC1_APPS_CLK>,
> + <&rpmcc RPM_SMD_XO_CLK_SRC>;
> + clock-names = "iface", "core", "xo";
> +
> + bus-width = <8>;
> + non-removable;
Aren't these two depend where is eMMC and where SD? Similarly to the
node below. I wouold expect board DTS define them.
> + status = "disabled";
> + };
> +
> + sdhc_2: mmc@...4000 {
> + compatible = "qcom,msm8976-sdhci", "qcom,sdhci-msm-v4";
> + reg = <0x07864900 0x11c>, <0x07864000 0x800>;
> + reg-names = "hc", "core";
> +
> + interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "hc_irq", "pwr_irq";
> +
> + clocks = <&gcc GCC_SDCC2_AHB_CLK>,
> + <&gcc GCC_SDCC2_APPS_CLK>,
> + <&rpmcc RPM_SMD_XO_CLK_SRC>;
> + clock-names = "iface", "core", "xo";
> +
> + bus-width = <4>;
> + status = "disabled";
> + };
> +
> + blsp1_dma: dma-controller@...4000 {
> + compatible = "qcom,bam-v1.7.0";
> + reg = <0x07884000 0x1f000>;
> + interrupts = <GIC_SPI 238 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&gcc GCC_BLSP1_AHB_CLK>;
> + clock-names = "bam_clk";
> + #dma-cells = <1>;
> + qcom,ee = <0>;
> + };
> +
> + blsp1_uart1: serial@...f000 {
> + compatible = "qcom,msm-uartdm-v1.4", "qcom,msm-uartdm";
> + reg = <0x078af000 0x200>;
> + interrupts = <GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&gcc GCC_BLSP1_UART1_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>;
> + clock-names = "core", "iface";
> + dmas = <&blsp1_dma 0>, <&blsp1_dma 1>;
> + dma-names = "tx", "rx";
> + status = "disabled";
> + };
> +
> + blsp1_uart2: serial@...0000 {
> + compatible = "qcom,msm-uartdm-v1.4", "qcom,msm-uartdm";
> + reg = <0x078b0000 0x200>;
> + interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&gcc GCC_BLSP1_UART2_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>;
> + clock-names = "core", "iface";
> + dmas = <&blsp1_dma 2>, <&blsp1_dma 3>;
> + dma-names = "tx", "rx";
> + status = "disabled";
> + };
> +
> + blsp1_spi1: spi@...5000 {
> + compatible = "qcom,spi-qup-v2.2.1";
> + reg = <0x078b5000 0x500>;
> + interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&gcc GCC_BLSP1_QUP1_SPI_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>;
> + clock-names = "core", "iface";
> + dmas = <&blsp1_dma 4>, <&blsp1_dma 5>;
> + dma-names = "tx", "rx";
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <&spi1_default>;
> + pinctrl-1 = <&spi1_sleep>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> + };
> +
> + blsp1_i2c2: i2c@...6000 {
> + compatible = "qcom,i2c-qup-v2.2.1";
> + reg = <0x078b6000 0x500>;
> + interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&gcc GCC_BLSP1_QUP2_I2C_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>;
> + clock-names = "core", "iface";
> + clock-frequency = <400000>;
> + dmas = <&blsp1_dma 6>, <&blsp1_dma 7>;
> + dma-names = "tx", "rx";
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <&blsp1_i2c2_default>;
> + pinctrl-1 = <&blsp1_i2c2_default>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> + };
> +
> + blsp1_i2c4: i2c@...8000 {
> + compatible = "qcom,i2c-qup-v2.2.1";
> + reg = <0x078b8000 0x500>;
> + interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&gcc GCC_BLSP1_QUP4_I2C_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>;
> + clock-names = "core", "iface";
> + clock-frequency = <400000>;
> + dmas = <&blsp1_dma 10>, <&blsp1_dma 11>;
> + dma-names = "tx", "rx";
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <&blsp1_i2c4_default>;
> + pinctrl-1 = <&blsp1_i2c4_sleep>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> + };
> +
> + otg: usb@...b000 {
> + compatible = "qcom,ci-hdrc";
> + reg = <0x078db000 0x200>,
> + <0x078db200 0x200>;
> + interrupts = <GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 140 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&gcc GCC_USB_HS_AHB_CLK>, <&gcc GCC_USB_HS_SYSTEM_CLK>;
> + clock-names = "iface", "core";
> + assigned-clocks = <&gcc GCC_USB_HS_SYSTEM_CLK>;
> + assigned-clock-rates = <80000000>;
> + resets = <&gcc RST_USB_HS_BCR>;
> + reset-names = "core";
> + ahb-burst-config = <0>;
> + dr_mode = "peripheral";
> + phy_type = "ulpi";
> + phy-names = "usb-phy";
> + phys = <&usb_hs_phy>;
> + status = "disabled";
> + #reset-cells = <1>;
> + };
> +
> + sdhc_3: mmc@...4000 {
> + compatible = "qcom,msm8976-sdhci", "qcom,sdhci-msm-v4";
> + reg = <0x07a24900 0x11c>, <0x07a24000 0x800>;
> + reg-names = "hc", "core";
> +
> + interrupts = <GIC_SPI 295 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 297 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "hc_irq", "pwr_irq";
> +
> + clocks = <&gcc GCC_SDCC3_AHB_CLK>,
> + <&gcc GCC_SDCC3_APPS_CLK>,
> + <&rpmcc RPM_SMD_XO_CLK_SRC>;
> + clock-names = "iface", "core", "xo";
> + bus-width = <4>;
> + #address-cells = <1>;
> + #size-cells = <0>;
These two are needed also for the board if SDIO is attached here.
> +
> + status = "disabled";
> + };
> +
> + blsp2_dma: dma-controller@...4000 {
> + compatible = "qcom,bam-v1.7.0";
> + reg = <0x07ac4000 0x1f000>;
> + interrupts = <GIC_SPI 239 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&gcc GCC_BLSP2_AHB_CLK>;
> + clock-names = "bam_clk";
> + #dma-cells = <1>;
> + qcom,ee = <0>;
> + };
> +
> + blsp2_uart2: serial@...0000 {
> + compatible = "qcom,msm-uartdm-v1.4", "qcom,msm-uartdm";
> + reg = <0x07af0000 0x200>;
> + interrupts = <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&gcc GCC_BLSP2_UART2_APPS_CLK>, <&gcc GCC_BLSP2_AHB_CLK>;
> + clock-names = "core", "iface";
> + dmas = <&blsp2_dma 0>, <&blsp2_dma 1>;
> + dma-names = "tx", "rx";
> + status = "disabled";
> + };
> +
> + blsp2_i2c2: i2c@...6000 {
> + compatible = "qcom,i2c-qup-v2.2.1";
> + reg = <0x07af6000 0x600>;
> + interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&gcc GCC_BLSP2_QUP2_I2C_APPS_CLK>, <&gcc GCC_BLSP2_AHB_CLK>;
> + clock-names = "core", "iface";
> + clock-frequency = <400000>;
> + dmas = <&blsp2_dma 6>, <&blsp2_dma 7>;
> + dma-names = "tx", "rx";
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <&blsp2_i2c2_default>;
> + pinctrl-1 = <&blsp2_i2c2_sleep>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> + };
> +
> + blsp2_i2c4: i2c@...8000 {
> + compatible = "qcom,i2c-qup-v2.2.1";
> + reg = <0x07af8000 0x600>;
> + interrupts = <GIC_SPI 302 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&gcc GCC_BLSP2_QUP4_I2C_APPS_CLK>, <&gcc GCC_BLSP2_AHB_CLK>;
> + clock-names = "core", "iface";
> + clock-frequency = <400000>;
> + dmas = <&blsp2_dma 10>, <&blsp2_dma 11>;
> + dma-names = "tx", "rx";
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <&blsp2_i2c4_default>;
> + pinctrl-1 = <&blsp2_i2c4_sleep>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> + };
> +
> + intc: interrupt-controller@...0000 {
> + compatible = "qcom,msm-qgic2";
> + reg = <0x0b000000 0x1000>, <0x0b002000 0x1000>;
> + interrupt-controller;
> + #interrupt-cells = <3>;
> + };
> +
> + apcs: syscon@...1000 {
> + compatible = "syscon";
This cannot be alone, you need specific compatible.
> + reg = <0x0b011000 0x1000>;
> + };
> +
> + timer@...0000 {
> + compatible = "arm,armv7-timer-mem";
> + reg = <0x0b120000 0x1000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + clock-frequency = <19200000>;
> +
> + frame@...1000 {
> + reg = <0x0b121000 0x1000>, <0x0b122000 0x1000>;
> + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> + frame-number = <0>;
> + };
> +
> + frame@...3000 {
> + reg = <0x0b123000 0x1000>;
> + interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>;
> + frame-number = <1>;
> + status = "disabled";
> + };
> +
> + frame@...4000 {
> + reg = <0x0b124000 0x1000>;
> + interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
> + frame-number = <2>;
> + status = "disabled";
> + };
> +
> + frame@...5000 {
> + reg = <0x0b125000 0x1000>;
> + interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
> + frame-number = <3>;
> + status = "disabled";
> + };
> +
> + frame@...6000 {
> + reg = <0x0b126000 0x1000>;
> + interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
> + frame-number = <4>;
> + status = "disabled";
> + };
> +
> + frame@...7000 {
> + reg = <0x0b127000 0x1000>;
> + interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
> + frame-number = <5>;
> + status = "disabled";
> + };
> +
> + frame@...8000 {
> + reg = <0x0b128000 0x1000>;
> + interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;
> + frame-number = <6>;
> + status = "disabled";
> + };
> + };
> +
> + imem: imem@...0000 {
> + compatible = "simple-mfd";
You need specific compatible. This is imem. I was already fixing other
boards but I think new ones appeared... so I will re-fix these.
> + reg = <0x08600000 0x1000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + ranges = <0 0x08600000 0x1000>;
> +
> + pil-reloc@94c {
> + compatible = "qcom,pil-reloc-info";
> + reg = <0x94c 0xc8>;
> + };
> + };
> + };
> +
Best regards,
Krzysztof
Powered by blists - more mailing lists