dts: wch: Enable using whole flash with CH32V208 #87345

VynDragon · 2025-03-19T12:40:49Z

Enables using the whole flash on CH32V208
~~This needs #87141~~ This fixes it differently.
and #85395

soc/wch/ch32v/qingke_v4c/vector.S

fabiobaltieri · 2025-03-21T21:47:50Z

cc @kholia (need to open a issue to add you to the collaborators)

dts/riscv/wch/ch32v208/ch32v208.dtsi

paulwedeck · 2025-05-05T19:26:56Z

soc/wch/ch32v/qingke_v4/vector.S

 	j       __start
 	.rept   CONFIG_VECTOR_TABLE_SIZE
 	.word	_isr_wrapper
 	.endr

 SECTION_FUNC(vectors, __start)
-	li 	a0, 3
+	li 	a0, 0xf


At first, I wanted to ask what this change is about but I now have some idea:
You need this because lui and jr from above are now at 0x0 and then the table starts at 0x8, right?
I think this warrants a comment in code for the next person that tries to understand this line.

2 first bits are settings, 2 after that are 0xC start of the table (0x4+0x4+0x4 3 32-bits instructions)

oh, my bad.
But then vector table is one entry to small (check VECTOR_TABLE_SIZE and Table 9-2 in RM).
Could also verify using DMA2_CH11 interrupt

You are right, the number should be 104, I'll make a PR for fixing this later.

I have applied it there since I had to spend another hour updating this PR again since other things were merged before it was.

I have applied it there since I had to spend another hour updating this PR again since other things were merged before it was.

Feel free to mark other PRs as DNM if there are dependencies or preferences w.r.t merge order

Not only do i expect people to be pissed if I do that, but I also expected y'all take the normal time to merge that PR, eg 2 to 3 months counted in buisness days so i could have the time to actually take a look at it.

Not only do i expect people to be pissed if I do that, but I also expected y'all take the normal time to merge that PR, eg 2 to 3 months counted in buisness days so i could have the time to actually take a look at it.

I don't know what this is supposed to mean. PRs get merged when they're... mergeable?

What it means is being unpredictable is worse than being slow, you cant just have trivial PRs taking months to merge because no one bothers taking a look or assigned person disappeared, while having big PRs being reviewed very partially (100 % certain nordicjm's review was only relevant to the board part, dont know about yours but i'm also sure there wasnt a overall architectural review because it didn't match the current architecture of the wch soc folders that my PRs had to match being merged) and merged near-instantly.

like ffs we still cant use west flash on ch32v208 because we have to make sure <random person 458485> is ok with fixing a obvious bug but apparently breaking other PRs and making 4 contributors wait for months on end because no one can be bothered to actually review something agreed upon by 3/4th of the contributors of a platform is perfectly ok.

I want to apologize to you two, it's really not your fault, it's only the normal, insanely frustrating (especially in areas of low mainstream interest), process...

Enables using the whole flash on CH32V208 This also involves limiting frequency of the CPU to 120Mhz from 144Mhz to meet recommendations. Signed-off-by: Camille BAUD <mail@massdriver.space>

nzmichaelh

In general, the ch32v00x series (like the ch32v006) is different from the ch32v003 - including having a different microarchitecture (v2c) and different pinmux.

Happy to discuss.

nzmichaelh · 2025-05-10T13:27:33Z

drivers/clock_control/clock_control_wch_rcc.c

@@ -90,7 +90,7 @@ static void clock_control_wch_rcc_setup_flash(void)
 	} else {
 		latency = FLASH_ACTLR_LATENCY_1;
 	}
-#elif defined(CONFIG_SOC_SERIES_CH32V00X)
+#elif defined(CONFIG_SOC_SERIES_QINGKE_V2A)


The original is correct.

To add detail: the flash latency is a SOC feature, not a core feature, and it's different on the CH32V003 to the others in the CH32V00x series.

But this is the correct filter right? This will be CONFIG_SOC_SERIES_QINGKE_V2 soon.

nzmichaelh · 2025-05-10T13:28:34Z

soc/wch/ch32v/qingke_v2a/Kconfig

@@ -6,3 +6,4 @@ config SOC_SERIES_QINGKE_V2A
 	select RISCV_ISA_EXT_ZICSR
 	select RISCV_ISA_EXT_ZIFENCEI
 	select RISCV_ISA_EXT_C
+	select RISCV_ISA_EXT_ZMMUL


The V2A does not have ZMMUL

nzmichaelh · 2025-05-10T13:29:10Z

soc/wch/ch32v/qingke_v2a/Kconfig.defconfig

 # SPDX-License-Identifier: Apache-2.0

 if SOC_SERIES_QINGKE_V2A

 config SYS_CLOCK_HW_CYCLES_PER_SEC
 	default $(dt_node_int_prop_int,/cpus/cpu@0,clock-frequency)

-config MAIN_STACK_SIZE


These are needed on the CH32V003, which is the only SOC using the V2A.

Please look at the new version while i run functionaility tests.

nzmichaelh · 2025-05-10T13:29:33Z

soc/wch/ch32v/qingke_v2a/Kconfig.defconfig.ch32v006

@@ -1,12 +1,12 @@
 # Copyright (c) 2025 Michael Hope <michaelh@juju.nz>
 # SPDX-License-Identifier: Apache-2.0

-if SOC_SERIES_CH32V00X
+if SOC_CH32V006



Good catch! No action requested.

nzmichaelh · 2025-05-10T13:29:58Z

soc/wch/ch32v/qingke_v2a/Kconfig.soc.ch32v006

@@ -3,7 +3,7 @@

 config SOC_CH32V006
 	bool
-	select SOC_SERIES_CH32V00X
+	select SOC_SERIES_QINGKE_V2A


The original is correct. Also, the CH32V00x series is a QINGKE_V2C.

soc/wch/ch32v/qingke_v4/Kconfig.soc

nzmichaelh · 2025-05-10T13:31:25Z

soc/wch/ch32v/qingke_v4/pinctrl_soc.h

@@ -10,7 +10,7 @@
 /**
 * @brief Type to hold a pin's pinctrl configuration.
 */
-struct ch32v00x_pinctrl_soc_pin {
+struct qingke_v4_pinctrl_soc_pin {


Why was this file moved?

deduplication.

nzmichaelh · 2025-05-10T13:32:32Z

soc/wch/ch32v/soc.yml

@@ -7,9 +7,7 @@ family:
  - name: qingke-v2a
    socs:
    - name: ch32v003
-  - name: ch32v00x


The ch32v006 is not a qingke-v2a but a v2c

If it's not clear i'm quite a bit annoyed CH32V006 was merged, because it messes up this PR and means it requires ANOTHER round of reviews and hours of update for me, and obviously I have made errors on adding ch32v006 to it, but I'm really not inclined in spending more time on it while everyone has been dragging their feets about this PR.

And you know, the fact that it's probably going to need another round of reorganization as discussed there #87345 (comment) and so it's basically doubly wasted time.

I hear you, and I'm sorry for your wasted time. When I reviewed it, this PR breaks both the ch32v003 and ch32v00x, so I can't LGTM it as is.

Could I suggest breaking this up into separate PRs covering separate concerns? For example, the CH32V208 PLL, flash, and startup changes are independent of the rename.

I am working on fixing it so it doesnt break them and they are organized as they are supposed to be in this version.

It should be good now.

nzmichaelh · 2025-05-10T13:46:01Z

re: naming in general, is #87345 (comment) the plan of record? I used that for the ch32v00x structure, but this PR reverses much of that.

VynDragon · 2025-05-10T16:23:06Z

Someone else can take care of the reorganization reviewers have asked for: #87125 (comment)
patch.txt

sonarqubecloud · 2025-05-10T16:49:08Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

nzmichaelh

LGTM

zephyrbot added the area: RISCV RISCV Architecture (32-bit & 64-bit) label Mar 19, 2025

zephyrbot requested review from carlocaione, edersondisouza, fkokosinski, katsuster, kgugala, mgielda, npitre, tgorochowik and ycsin March 19, 2025 12:41

zephyrbot assigned fkokosinski, kgugala and tgorochowik Mar 19, 2025

VynDragon force-pushed the ch32v208_fix_flash_addr branch from 1d88336 to d4880f1 Compare March 19, 2025 12:57

VynDragon mentioned this pull request Mar 19, 2025

Add support for WCH CH32V303 #87125

Merged

fkokosinski reviewed Mar 20, 2025

View reviewed changes

soc/wch/ch32v/qingke_v4c/vector.S Outdated Show resolved Hide resolved

VynDragon mentioned this pull request Mar 20, 2025

boards: linkw: fix west flash with minichlink #87141

Closed

VynDragon force-pushed the ch32v208_fix_flash_addr branch from d4880f1 to cbb1ae2 Compare March 20, 2025 13:34

fkokosinski previously approved these changes Mar 21, 2025

View reviewed changes

kartben requested a review from fabiobaltieri March 21, 2025 20:40

fabiobaltieri requested a review from nzmichaelh March 21, 2025 21:47

fabiobaltieri assigned nzmichaelh and unassigned kgugala, tgorochowik and fkokosinski Mar 21, 2025

VynDragon dismissed fkokosinski’s stale review via 9b716f6 March 24, 2025 19:35

zephyrbot added the area: Timer Timer label Mar 24, 2025

zephyrbot requested review from andyross and teburd March 24, 2025 19:35

kartben reviewed May 5, 2025

View reviewed changes

dts/riscv/wch/ch32v208/ch32v208.dtsi Show resolved Hide resolved

VynDragon force-pushed the ch32v208_fix_flash_addr branch from 145559b to f6d4d2e Compare May 5, 2025 15:08

paulwedeck reviewed May 5, 2025

View reviewed changes

VynDragon requested a review from kartben May 8, 2025 13:55

VynDragon dismissed nzmichaelh’s stale review via f6d4d2e May 8, 2025 18:34

VynDragon dismissed fabiobaltieri’s stale review via c6bf867 May 9, 2025 13:20

VynDragon force-pushed the ch32v208_fix_flash_addr branch 3 times, most recently from 1e6a8b8 to b8cca14 Compare May 9, 2025 13:29

github-actions bot added the area: Clock Control label May 9, 2025

dts: wch: Enable using whole flash with CH32V208

8ae6dfa

Enables using the whole flash on CH32V208 This also involves limiting frequency of the CPU to 120Mhz from 144Mhz to meet recommendations. Signed-off-by: Camille BAUD <mail@massdriver.space>

VynDragon force-pushed the ch32v208_fix_flash_addr branch from b8cca14 to 964f09e Compare May 9, 2025 13:34

VynDragon requested review from fabiobaltieri, miggazElquez, paulwedeck and nzmichaelh May 9, 2025 13:35

VynDragon mentioned this pull request May 10, 2025

soc: wch: drop the stack size overrides for the CH32V208 #89763

Closed

nzmichaelh requested changes May 10, 2025

View reviewed changes

VynDragon force-pushed the ch32v208_fix_flash_addr branch 3 times, most recently from bdc2db6 to 8ae6dfa Compare May 10, 2025 16:19

VynDragon changed the title ~~dts: wch: Enable using whole flash with CH32V208 And reorganize soc folder for future deduplication~~ dts: wch: Enable using whole flash with CH32V208 May 10, 2025

VynDragon requested a review from nzmichaelh May 10, 2025 16:25

nzmichaelh approved these changes May 10, 2025

View reviewed changes

fabiobaltieri approved these changes May 12, 2025

View reviewed changes

kartben merged commit bab50a5 into zephyrproject-rtos:main May 12, 2025
29 checks passed

VynDragon deleted the ch32v208_fix_flash_addr branch May 14, 2025 15:09

dts: wch: Enable using whole flash with CH32V208 #87345

dts: wch: Enable using whole flash with CH32V208 #87345

Uh oh!

Conversation

VynDragon commented Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

fabiobaltieri commented Mar 21, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VynDragon May 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VynDragon May 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VynDragon May 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nzmichaelh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

VynDragon commented Mar 19, 2025 •

edited

Loading

VynDragon May 10, 2025 •

edited

Loading

VynDragon May 10, 2025 •

edited

Loading

VynDragon May 10, 2025 •

edited

Loading

VynDragon commented May 10, 2025 •

edited

Loading