soc: wch: Deduplicate common files across ch32v socs #90007
Conversation
| @@ -10,7 +10,7 @@ | |||
| /** | |||
| * @brief Type to hold a pin's pinctrl configuration. | |||
| */ | |||
| struct ch32v208_pinctrl_soc_pin { | |||
| struct qingke_v4_pinctrl_soc_pin { | |||
There was a problem hiding this comment.
I don't think we should do this. The pinmux is a feature of the SOC, not the microarchitecture.
For example, both the CH32V003 and CH32V006 are QingKe V2, but they have different pinmux.
I'm concerned this PR will have the same issues as the previous one where there's lots of rebasing due to differing opinions. Should we work this out in a bug or chat first?
| @@ -10,7 +10,7 @@ | |||
| /** | |||
| * @brief Type to hold a pin's pinctrl configuration. | |||
| */ | |||
| struct ch32v208_pinctrl_soc_pin { | |||
| struct qingke_v4_pinctrl_soc_pin { | |||
There was a problem hiding this comment.
I don't think we should do this. The pinmux is a feature of the SOC, not the microarchitecture.
For example, both the CH32V003 and CH32V006 are QingKe V2, but they have different pinmux.
I'm concerned this PR will have the same issues as the previous one where there's lots of rebasing due to differing opinions. Should we work this out in a bug or chat first?
There was a problem hiding this comment.
I'm concerned this PR will have the same issues as the previous one where there's lots of rebasing due to differing opinions. Should we work this out in a bug or chat first?
Maybe, I figured having a starting point could help us discuss it but if you prefer we can talk about it on another place, I have no preference.
There was a problem hiding this comment.
I don't think we should do this. The pinmux is a feature of the SOC, not the microarchitecture.
For example, both the CH32V003 and CH32V006 are QingKe V2, but they have different pinmux.
I understand this, that's why I didn't move them. For now all qingke_v4-based soc are part of the ch32v20x/30x series and they all have the same pinmux, but if there is another SoC with a qingke_v4 it would indeed be a problem.
But it would still be good to deduplicate this file no ?
There was a problem hiding this comment.
CH32V006
the 003 is a V2A and the 006 is a V2C. So different Cores, they also have different instruction sets. (Infos you can find in datasheet on capital 1.4.1)
With this background it makes sence to move from SOC to core-naming.
| @@ -3,8 +3,8 @@ | |||
| # SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| zephyr_sources( | |||
| soc_irq.S | |||
| vector.S | |||
| ../common/qingke_v2x/soc_irq.S | |||
There was a problem hiding this comment.
I don't think a directory should refer to a different directory. I can't find any examples of this in the other socs.
As soc_irq.S is fairly trivial, shall we just denormalise it across all SOCs?
There was a problem hiding this comment.
I don't think a directory should refer to a different directory. I can't find any examples of this in the other socs.
There is some exemples, like a some espressif SoCs, one nxp, ... I don't see how to deduplicate files without doing that.
As soc_irq.S is fairly trivial, shall we just denormalise it across all SOCs?
True, I will do it.
|
I rebased to include the ch32v303. For the pinctrl file, I moved it back to the folder for each SoC series, but renamed the one in the qingke_v4f folder to 30x from 303, that way it can be re-used by other 30x SoC. Does it seems like a good solution ? |
|
I can't find your message about vector.S, what was the problem ? |
with the 2 lines |
|
The goal of these lines is to go from aliased space (adress at 0x00....) to non-aliased space (0x80...), to use the full flash. So |
Why not in the same way like all the other implementation with just the j __start? For me it is also strange |
|
Because the processor start in aliased space, with only limited amout of memory available. So we need to jump to non aliased space if we want the full memory space. |
Moves shared files from multiple ch32v SoCs into a new `common/` directory to eliminate duplication. Signed-off-by: Miguel Gazquez <miguel.gazquez@bootlin.com>
Rename the struct ch32v303_pinctrl_soc_pin to ch32v30x_pinctrl_soc_pin to allow the same struct to be used for other SoC of the series. Signed-off-by: Miguel Gazquez <miguel.gazquez@bootlin.com>
|
Please retry analysis of this Pull-Request directly on SonarQube Cloud |
|
@nzmichaelh Could you revisit this ? |
Moves shared files from multiple ch32v SoCs into a new
common/directory to eliminate duplication.I only de-duplicated
pinctrl_soc.hfor the qingke_v4x socs, because the pin controller is not the same between the qingke_v2a (ch32v003) and the qingke_v2c (other ch32v00x).