diff mbox series

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

Message ID 20240426-th1520-clk-v2-v2-4-96b829e6fcee@tenstorrent.com (mailing list archive)
State Superseded
Headers show
Series clk: thead: Add support for TH1520 AP_SUBSYS clock controller | expand

Checks

Context Check Description
conchuod/vmtest-for-next-PR fail PR summary
conchuod/patch-4-test-1 fail .github/scripts/patches/tests/build_rv32_defconfig.sh
conchuod/patch-4-test-2 success .github/scripts/patches/tests/build_rv64_clang_allmodconfig.sh
conchuod/patch-4-test-3 success .github/scripts/patches/tests/build_rv64_gcc_allmodconfig.sh
conchuod/patch-4-test-4 success .github/scripts/patches/tests/build_rv64_nommu_k210_defconfig.sh
conchuod/patch-4-test-5 success .github/scripts/patches/tests/build_rv64_nommu_virt_defconfig.sh
conchuod/patch-4-test-6 success .github/scripts/patches/tests/checkpatch.sh
conchuod/patch-4-test-7 success .github/scripts/patches/tests/dtb_warn_rv64.sh
conchuod/patch-4-test-8 success .github/scripts/patches/tests/header_inline.sh
conchuod/patch-4-test-9 success .github/scripts/patches/tests/kdoc.sh
conchuod/patch-4-test-10 success .github/scripts/patches/tests/module_param.sh
conchuod/patch-4-test-11 success .github/scripts/patches/tests/verify_fixes.sh
conchuod/patch-4-test-12 success .github/scripts/patches/tests/verify_signedoff.sh

Commit Message

Drew Fustini April 27, 2024, 12:10 a.m. UTC
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@tenstorrent.com>
---
 arch/riscv/boot/dts/thead/th1520.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Thomas Bonnefille May 2, 2024, 7:47 a.m. UTC | #1
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@tenstorrent.com>

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.

Thomas
Drew Fustini May 2, 2024, 3:53 p.m. UTC | #2
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@tenstorrent.com>
> 
> 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
Thomas Bonnefille May 3, 2024, 6:58 a.m. UTC | #3
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@tenstorrent.com>
>>
>> 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
diff mbox series

Patch

diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
index 6285cdf91bd6..a6f51ec9fb55 100644
--- a/arch/riscv/boot/dts/thead/th1520.dtsi
+++ b/arch/riscv/boot/dts/thead/th1520.dtsi
@@ -324,7 +324,7 @@  emmc: mmc@ffe7080000 {
 			compatible = "thead,th1520-dwcmshc";
 			reg = <0xff 0xe7080000 0x0 0x10000>;
 			interrupts = <62 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&sdhci_clk>;
+			clocks = <&clk CLK_EMMC_SDIO>;
 			clock-names = "core";
 			status = "disabled";
 		};
@@ -333,7 +333,7 @@  sdio0: mmc@ffe7090000 {
 			compatible = "thead,th1520-dwcmshc";
 			reg = <0xff 0xe7090000 0x0 0x10000>;
 			interrupts = <64 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&sdhci_clk>;
+			clocks = <&clk CLK_EMMC_SDIO>;
 			clock-names = "core";
 			status = "disabled";
 		};
@@ -342,7 +342,7 @@  sdio1: mmc@ffe70a0000 {
 			compatible = "thead,th1520-dwcmshc";
 			reg = <0xff 0xe70a0000 0x0 0x10000>;
 			interrupts = <71 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&sdhci_clk>;
+			clocks = <&clk CLK_EMMC_SDIO>;
 			clock-names = "core";
 			status = "disabled";
 		};