Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix PLL_MULTIPLICATION definition for ch32v303 #555

Merged
merged 1 commit into from
Mar 10, 2025

Conversation

miggazElquez
Copy link
Contributor

The problem I noticed in #498 was only partially solved by 5ec73e1: there is still a condition checking only for ch32v20x before setting PLL_MULTIPLICATION.

As this file is only included when using a ch32v30x I just removed the check.

@cnlohr
Copy link
Owner

cnlohr commented Mar 10, 2025

Err realized I can ping you here, @AlexanderMandera can you read over this and make sure it matches your intent?

@AlexanderMandera
Copy link
Contributor

@cnlohr
The change is fine to me, might be something I have overlooked initially.
It might be good to strip out the other if macros as well that check for unrelated chip families.

For example:

#if defined(CH32V10x) || defined(CH32V20x) || defined(CH32V30x_D8)

could have the check for CH32V10x and CH32V20x removed.
Maybe we should do this for the other header files too.

@cnlohr
Copy link
Owner

cnlohr commented Mar 10, 2025

I would love to have another PR with that fix! If anyone else is up for testing it. I just am not set up at the moment to test it. Will merge this now.

@cnlohr cnlohr merged commit f29b93c into cnlohr:master Mar 10, 2025
83 checks passed
@miggazElquez
Copy link
Contributor Author

Yeah, there is still quite a lot of checks for ch32v20x and ch32v10x in this file (and the same problem reversed in other files).
I can try deleting these for the 30x, but I'm testing on Zephyr, which doesn't support a lot of devices currently so I can only test if it build and can run Zephyr basic samples (like "Hello world").
I can also only test the ch32v303, but it seems to be the one the most affected by this problem (#if defined(CH32V20x) || defined(CH32V30x_D8) || defined(CH32V10x). There is also some #if defined(CH32V20x_D8) || defined(CH32V20x_D8W) that I suppose could be removed ?

@cnlohr
Copy link
Owner

cnlohr commented Mar 11, 2025

IIRC there are different CH32V20x's so they could be removed from the v303 headers, but cannot be removed from the v20x headers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants