Re: [PATCH RFC v2 4/4] riscv: dts: thead: Add clock to TH1520 mmc controllers

From: Thomas Bonnefille
Date: Fri May 03 2024 - 02:58:52 EST




On 5/2/24 5:53 PM, Drew Fustini wrote:
On Thu, May 02, 2024 at 09:47:43AM +0200, Thomas Bonnefille wrote:


On 4/27/24 2:10 AM, Drew Fustini wrote:
Change the clock property in the T-Head TH1520 mmc controller nodes to a
real clock provided by the AP_SUBSYS clock driver.

Signed-off-by: Drew Fustini <dfustini@xxxxxxxxxxxxxxx>

I experienced that, when the I2C clocks were correctly configured, the UART
stopped working, likely due to their dependence on FOUTPOSTDIV.
Setting up the UART correctly, for instance:

uartx: serial@xxxxxxxxxx {
...
clocks = <&clk CLK_UART_SCLK>, <&clk
CLK_UARTX_PCLK>;
clock-names = "baudclk", "apb_pclk";
...
status = "disabled";
};
resolved the issue.
As this would be mandatory in the future, I suggest that you configure all
the nodes currently set to a fixed clock, not just the MMC controller.

Thank you for testing and discovering this issue.

Could you post your device tree so I can be sure I'm testing the same as
what you have?

Drew

Of course, I'll attach the two Device Trees used.
Note that I also use Emil's patch which adds support for pinctrl.

Thomas// SPDX-License-Identifier: (GPL-2.0 OR MIT)
/*
* Copyright (C) 2021 Alibaba Group Holding Limited.
* Copyright (C) 2023 Jisheng Zhang <jszhang@xxxxxxxxxx>
*/

#include <dt-bindings/interrupt-controller/irq.h>
#include <dt-bindings/clock/thead,th1520-clk-ap.h>

/ {
compatible = "thead,th1520";
#address-cells = <2>;
#size-cells = <2>;

cpus: cpus {
#address-cells = <1>;
#size-cells = <0>;
timebase-frequency = <3000000>;

c910_0: cpu@0 {
compatible = "thead,c910", "riscv";
device_type = "cpu";
riscv,isa = "rv64imafdc";
riscv,isa-base = "rv64i";
riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "zicntr", "zicsr",
"zifencei", "zihpm";
reg = <0>;
i-cache-block-size = <64>;
i-cache-size = <65536>;
i-cache-sets = <512>;
d-cache-block-size = <64>;
d-cache-size = <65536>;
d-cache-sets = <512>;
next-level-cache = <&l2_cache>;
mmu-type = "riscv,sv39";

cpu0_intc: interrupt-controller {
compatible = "riscv,cpu-intc";
interrupt-controller;
#interrupt-cells = <1>;
};
};

c910_1: cpu@1 {
compatible = "thead,c910", "riscv";
device_type = "cpu";
riscv,isa = "rv64imafdc";
riscv,isa-base = "rv64i";
riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "zicntr", "zicsr",
"zifencei", "zihpm";
reg = <1>;
i-cache-block-size = <64>;
i-cache-size = <65536>;
i-cache-sets = <512>;
d-cache-block-size = <64>;
d-cache-size = <65536>;
d-cache-sets = <512>;
next-level-cache = <&l2_cache>;
mmu-type = "riscv,sv39";

cpu1_intc: interrupt-controller {
compatible = "riscv,cpu-intc";
interrupt-controller;
#interrupt-cells = <1>;
};
};

c910_2: cpu@2 {
compatible = "thead,c910", "riscv";
device_type = "cpu";
riscv,isa = "rv64imafdc";
riscv,isa-base = "rv64i";
riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "zicntr", "zicsr",
"zifencei", "zihpm";
reg = <2>;
i-cache-block-size = <64>;
i-cache-size = <65536>;
i-cache-sets = <512>;
d-cache-block-size = <64>;
d-cache-size = <65536>;
d-cache-sets = <512>;
next-level-cache = <&l2_cache>;
mmu-type = "riscv,sv39";

cpu2_intc: interrupt-controller {
compatible = "riscv,cpu-intc";
interrupt-controller;
#interrupt-cells = <1>;
};
};

c910_3: cpu@3 {
compatible = "thead,c910", "riscv";
device_type = "cpu";
riscv,isa = "rv64imafdc";
riscv,isa-base = "rv64i";
riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "zicntr", "zicsr",
"zifencei", "zihpm";
reg = <3>;
i-cache-block-size = <64>;
i-cache-size = <65536>;
i-cache-sets = <512>;
d-cache-block-size = <64>;
d-cache-size = <65536>;
d-cache-sets = <512>;
next-level-cache = <&l2_cache>;
mmu-type = "riscv,sv39";

cpu3_intc: interrupt-controller {
compatible = "riscv,cpu-intc";
interrupt-controller;
#interrupt-cells = <1>;
};
};

l2_cache: l2-cache {
compatible = "cache";
cache-block-size = <64>;
cache-level = <2>;
cache-size = <1048576>;
cache-sets = <1024>;
cache-unified;
};
};

osc: oscillator {
compatible = "fixed-clock";
clock-output-names = "osc_24m";
#clock-cells = <0>;
};

osc_32k: 32k-oscillator {
compatible = "fixed-clock";
clock-output-names = "osc_32k";
#clock-cells = <0>;
};

aonsys_clk: aonsys-clk {
compatible = "fixed-clock";
clock-output-names = "aonsys_clk";
#clock-cells = <0>;
};

apb_clk: apb-clk-clock {
compatible = "fixed-clock";
clock-output-names = "apb_clk";
#clock-cells = <0>;
};

sdhci_clk: sdhci-clock {
compatible = "fixed-clock";
clock-frequency = <198000000>;
clock-output-names = "sdhci_clk";
#clock-cells = <0>;
};

soc {
compatible = "simple-bus";
interrupt-parent = <&plic>;
#address-cells = <2>;
#size-cells = <2>;
dma-noncoherent;
ranges;

clk: clock-controller@ffef010000 {
compatible = "thead,th1520-clk-ap";
reg = <0xff 0xef010000 0x0 0x1000>;
clocks = <&osc>;
clock-names = "osc";
#clock-cells = <1>;
};

plic: interrupt-controller@ffd8000000 {
compatible = "thead,th1520-plic", "thead,c900-plic";
reg = <0xff 0xd8000000 0x0 0x01000000>;
interrupts-extended = <&cpu0_intc 11>, <&cpu0_intc 9>,
<&cpu1_intc 11>, <&cpu1_intc 9>,
<&cpu2_intc 11>, <&cpu2_intc 9>,
<&cpu3_intc 11>, <&cpu3_intc 9>;
interrupt-controller;
#address-cells = <0>;
#interrupt-cells = <2>;
riscv,ndev = <240>;
};

clint: timer@ffdc000000 {
compatible = "thead,th1520-clint", "thead,c900-clint";
reg = <0xff 0xdc000000 0x0 0x00010000>;
interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>,
<&cpu1_intc 3>, <&cpu1_intc 7>,
<&cpu2_intc 3>, <&cpu2_intc 7>,
<&cpu3_intc 3>, <&cpu3_intc 7>;
};

uart0: serial@ffe7014000 {
compatible = "snps,dw-apb-uart";
reg = <0xff 0xe7014000 0x0 0x100>;
interrupts = <36 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clk CLK_UART_SCLK>, <&clk CLK_UART0_PCLK>;
clock-names = "baudclk", "apb_pclk";
reg-shift = <2>;
reg-io-width = <4>;
status = "disabled";
};

emmc: mmc@ffe7080000 {
compatible = "thead,th1520-dwcmshc";
reg = <0xff 0xe7080000 0x0 0x10000>;
interrupts = <62 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clk CLK_EMMC_SDIO>;
clock-names = "core";
status = "disabled";
};

sdio0: mmc@ffe7090000 {
compatible = "thead,th1520-dwcmshc";
reg = <0xff 0xe7090000 0x0 0x10000>;
interrupts = <64 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clk CLK_EMMC_SDIO>;
clock-names = "core";
status = "disabled";
};

sdio1: mmc@ffe70a0000 {
compatible = "thead,th1520-dwcmshc";
reg = <0xff 0xe70a0000 0x0 0x10000>;
interrupts = <71 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clk CLK_EMMC_SDIO>;
clock-names = "core";
status = "disabled";
};

uart1: serial@ffe7f00000 {
compatible = "snps,dw-apb-uart";
reg = <0xff 0xe7f00000 0x0 0x100>;
interrupts = <37 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clk CLK_UART_SCLK>, <&clk CLK_UART1_PCLK>;
clock-names = "baudclk", "apb_pclk";
reg-shift = <2>;
reg-io-width = <4>;
status = "disabled";
};

uart3: serial@ffe7f04000 {
compatible = "snps,dw-apb-uart";
reg = <0xff 0xe7f04000 0x0 0x100>;
interrupts = <39 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clk CLK_UART_SCLK>, <&clk CLK_UART3_PCLK>;
clock-names = "baudclk", "apb_pclk";
reg-shift = <2>;
reg-io-width = <4>;
status = "disabled";
};

gpio@ffe7f34000 {
compatible = "snps,dw-apb-gpio";
reg = <0xff 0xe7f34000 0x0 0x1000>;
#address-cells = <1>;
#size-cells = <0>;

gpio2: gpio-controller@0 {
compatible = "snps,dw-apb-gpio-port";
gpio-controller;
#gpio-cells = <2>;
ngpios = <32>;
gpio-ranges = <&padctrl0_apsys 0 0 32>;
reg = <0>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <58 IRQ_TYPE_LEVEL_HIGH>;
};
};

i2c0: i2c@ffe7f20000 {
compatible = "thead,th1520-i2c", "snps,designware-i2c";
reg = <0xff 0xe7f20000 0x0 0x4000>;
interrupts = <44 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clk CLK_I2C0>;
#address-cells = <1>;
#size-cells = <0>;
status = "disabled";
};

i2c1: i2c@ffe7f24000 {
compatible = "thead,th1520-i2c", "snps,designware-i2c";
reg = <0xff 0xe7f24000 0x0 0x4000>;
interrupts = <45 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clk CLK_I2C1>;
#address-cells = <1>;
#size-cells = <0>;
status = "disabled";
};

i2c4: i2c@ffe7f28000 {
compatible = "thead,th1520-i2c", "snps,designware-i2c";
reg = <0xff 0xe7f28000 0x0 0x4000>;
interrupts = <48 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clk CLK_I2C4>;
#address-cells = <1>;
#size-cells = <0>;
status = "disabled";
};

gpio@ffe7f38000 {
compatible = "snps,dw-apb-gpio";
reg = <0xff 0xe7f38000 0x0 0x1000>;
#address-cells = <1>;
#size-cells = <0>;

gpio3: gpio-controller@0 {
compatible = "snps,dw-apb-gpio-port";
gpio-controller;
#gpio-cells = <2>;
ngpios = <23>;
gpio-ranges = <&padctrl0_apsys 0 32 23>;
reg = <0>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <59 IRQ_TYPE_LEVEL_HIGH>;
};
};

padctrl1_apsys: pinctrl@ffe7f3c000 {
compatible = "thead,th1520-group2-pinctrl";
reg = <0xff 0xe7f3c000 0x0 0x1000>;
clocks = <&apb_clk>;
};

gpio@ffec005000 {
compatible = "snps,dw-apb-gpio";
reg = <0xff 0xec005000 0x0 0x1000>;
#address-cells = <1>;
#size-cells = <0>;

gpio0: gpio-controller@0 {
compatible = "snps,dw-apb-gpio-port";
gpio-controller;
#gpio-cells = <2>;
ngpios = <32>;
gpio-ranges = <&padctrl1_apsys 0 0 32>;
reg = <0>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <56 IRQ_TYPE_LEVEL_HIGH>;
};
};

gpio@ffec006000 {
compatible = "snps,dw-apb-gpio";
reg = <0xff 0xec006000 0x0 0x1000>;
#address-cells = <1>;
#size-cells = <0>;

gpio1: gpio-controller@0 {
compatible = "snps,dw-apb-gpio-port";
gpio-controller;
#gpio-cells = <2>;
ngpios = <31>;
gpio-ranges = <&padctrl1_apsys 0 32 31>;
reg = <0>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <57 IRQ_TYPE_LEVEL_HIGH>;
};
};

padctrl0_apsys: pinctrl@ffec007000 {
compatible = "thead,th1520-group3-pinctrl";
reg = <0xff 0xec007000 0x0 0x1000>;
clocks = <&apb_clk>;
};

i2c2: i2c@ffec00c000 {
compatible = "thead,th1520-i2c", "snps,designware-i2c";
reg = <0xff 0xec00c000 0x0 0x4000>;
interrupts = <46 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clk CLK_I2C2>;
#address-cells = <1>;
#size-cells = <0>;
status = "disabled";
};

uart2: serial@ffec010000 {
compatible = "snps,dw-apb-uart";
reg = <0xff 0xec010000 0x0 0x4000>;
interrupts = <38 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clk CLK_UART_SCLK>, <&clk CLK_UART2_PCLK>;
clock-names = "baudclk", "apb_pclk";
reg-shift = <2>;
reg-io-width = <4>;
status = "disabled";
};

i2c3: i2c@ffec014000 {
compatible = "thead,th1520-i2c", "snps,designware-i2c";
reg = <0xff 0xec014000 0x0 0x4000>;
interrupts = <47 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clk CLK_I2C3>;
#address-cells = <1>;
#size-cells = <0>;
status = "disabled";
};

dmac0: dma-controller@ffefc00000 {
compatible = "snps,axi-dma-1.01a";
reg = <0xff 0xefc00000 0x0 0x1000>;
interrupts = <27 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&apb_clk>, <&apb_clk>;
clock-names = "core-clk", "cfgr-clk";
#dma-cells = <1>;
dma-channels = <4>;
snps,block-size = <65536 65536 65536 65536>;
snps,priority = <0 1 2 3>;
snps,dma-masters = <1>;
snps,data-width = <4>;
snps,axi-max-burst-len = <16>;
status = "disabled";
};

timer0: timer@ffefc32000 {
compatible = "snps,dw-apb-timer";
reg = <0xff 0xefc32000 0x0 0x14>;
clocks = <&apb_clk>;
clock-names = "timer";
interrupts = <16 IRQ_TYPE_LEVEL_HIGH>;
status = "disabled";
};

timer1: timer@ffefc32014 {
compatible = "snps,dw-apb-timer";
reg = <0xff 0xefc32014 0x0 0x14>;
clocks = <&apb_clk>;
clock-names = "timer";
interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
status = "disabled";
};

timer2: timer@ffefc32028 {
compatible = "snps,dw-apb-timer";
reg = <0xff 0xefc32028 0x0 0x14>;
clocks = <&apb_clk>;
clock-names = "timer";
interrupts = <18 IRQ_TYPE_LEVEL_HIGH>;
status = "disabled";
};

timer3: timer@ffefc3203c {
compatible = "snps,dw-apb-timer";
reg = <0xff 0xefc3203c 0x0 0x14>;
clocks = <&apb_clk>;
clock-names = "timer";
interrupts = <19 IRQ_TYPE_LEVEL_HIGH>;
status = "disabled";
};

uart4: serial@fff7f08000 {
compatible = "snps,dw-apb-uart";
reg = <0xff 0xf7f08000 0x0 0x4000>;
interrupts = <40 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clk CLK_UART_SCLK>, <&clk CLK_UART4_PCLK>;
clock-names = "baudclk", "apb_pclk";
reg-shift = <2>;
reg-io-width = <4>;
status = "disabled";
};

uart5: serial@fff7f0c000 {
compatible = "snps,dw-apb-uart";
reg = <0xff 0xf7f0c000 0x0 0x4000>;
interrupts = <41 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clk CLK_UART_SCLK>, <&clk CLK_UART5_PCLK>;
clock-names = "baudclk", "apb_pclk";
reg-shift = <2>;
reg-io-width = <4>;
status = "disabled";
};

i2c5: i2c@fff7f2c000 {
compatible = "thead,th1520-i2c", "snps,designware-i2c";
reg = <0xff 0xf7f2c000 0x0 0x4000>;
interrupts = <49 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clk CLK_I2C5>;
#address-cells = <1>;
#size-cells = <0>;
status = "disabled";
};

timer4: timer@ffffc33000 {
compatible = "snps,dw-apb-timer";
reg = <0xff 0xffc33000 0x0 0x14>;
clocks = <&apb_clk>;
clock-names = "timer";
interrupts = <20 IRQ_TYPE_LEVEL_HIGH>;
status = "disabled";
};

timer5: timer@ffffc33014 {
compatible = "snps,dw-apb-timer";
reg = <0xff 0xffc33014 0x0 0x14>;
clocks = <&apb_clk>;
clock-names = "timer";
interrupts = <21 IRQ_TYPE_LEVEL_HIGH>;
status = "disabled";
};

timer6: timer@ffffc33028 {
compatible = "snps,dw-apb-timer";
reg = <0xff 0xffc33028 0x0 0x14>;
clocks = <&apb_clk>;
clock-names = "timer";
interrupts = <22 IRQ_TYPE_LEVEL_HIGH>;
status = "disabled";
};

timer7: timer@ffffc3303c {
compatible = "snps,dw-apb-timer";
reg = <0xff 0xffc3303c 0x0 0x14>;
clocks = <&apb_clk>;
clock-names = "timer";
interrupts = <23 IRQ_TYPE_LEVEL_HIGH>;
status = "disabled";
};

gpio@fffff41000 {
compatible = "snps,dw-apb-gpio";
reg = <0xff 0xfff41000 0x0 0x1000>;
#address-cells = <1>;
#size-cells = <0>;

aogpio: gpio-controller@0 {
compatible = "snps,dw-apb-gpio-port";
gpio-controller;
#gpio-cells = <2>;
ngpios = <16>;
gpio-ranges = <&padctrl_aosys 0 9 16>;
reg = <0>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <76 IRQ_TYPE_LEVEL_HIGH>;
};
};

padctrl_aosys: pinctrl@fffff4a000 {
compatible = "thead,th1520-group1-pinctrl";
reg = <0xff 0xfff4a000 0x0 0x2000>;
clocks = <&aonsys_clk>;
};

gpio@fffff52000 {
compatible = "snps,dw-apb-gpio";
reg = <0xff 0xfff52000 0x0 0x1000>;
#address-cells = <1>;
#size-cells = <0>;

gpio4: gpio-controller@0 {
compatible = "snps,dw-apb-gpio-port";
gpio-controller;
#gpio-cells = <2>;
ngpios = <23>;
gpio-ranges = <&padctrl_aosys 0 25 22>, <&padctrl_aosys 22 7 1>;
reg = <0>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <55 IRQ_TYPE_LEVEL_HIGH>;
};
};
};
};

Attachment: th1520-beaglev-ahead.dts
Description: audio/vnd.dts