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

bsp: cvitek: fix cv18xx_risc-v and c906_little IRQ_MAX_NR error num #9261

Merged
merged 2 commits into from
Aug 7, 2024

Conversation

Z8MAN8
Copy link
Contributor

@Z8MAN8 Z8MAN8 commented Jul 29, 2024

注意,这个 PR 包含了多个 commit,merge 时请保留,不要 squash,因为这是提交上的设计使然,不是中间过程。具体原因参考 #9259

拉取/合并请求描述:(PR description)

[

为什么提交这份PR (why to submit this PR)

Fixed #9259

你的解决方案是什么 (what is your solution)

参照datasheet将cv18xx_risc-v和c906_little的IRQ_MAX_NR值分别改为101和61

请提供验证的bsp和config (provide the config and bsp)

  • BSP: bsp/cvitek
  • .config:
  • action:

]

当前拉取/合并请求的状态 Intent for your PR

必须选择一项 Choose one (Mandatory):

  • 本拉取/合并请求是一个草稿版本 This PR is for a code-review and is intended to get feedback
  • 本拉取/合并请求是一个成熟版本 This PR is mature, and ready to be integrated into the repo

代码质量 Code Quality:

我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:

  • 已经仔细查看过代码改动的对比 Already check the difference between PR and old code
  • 代码风格正确,包括缩进空格,命名及其他风格 Style guide is adhered to, including spacing, naming and other styles
  • 没有垃圾代码,代码尽量精简,不包含#if 0代码,不包含已经被注释了的代码 All redundant code is removed and cleaned up
  • 所有变更均有原因及合理的,并且不会影响到其他软件组件代码或BSP All modifications are justified and not affect other components or BSP
  • 对难懂代码均提供对应的注释 I've commented appropriately where code is tricky
  • 代码是高质量的 Code in this PR is of high quality
  • 已经使用formatting 等源码格式化工具确保格式符合RT-Thread代码规范 This PR complies with RT-Thread code specification

Analysis: The IRQ_MAX_NR value of cv18xx_risc-v is wrong.
The wrong IRQ_MAX_NR will cause the install of an interrupt
number larger than its value to fail.

Solution: Change IRQ_MAX_NR to the correct value 101 in
the datasheet.

Signed-off-by: Shicheng Chu <[email protected]>
Reviewed-by: Chen Wang <[email protected]>
@unicornx unicornx self-requested a review July 29, 2024 06:39
@unicornx unicornx added BSP: Cvitek BSP related with cvitek Arch: RISC-V BSP related with risc-v labels Jul 29, 2024
@@ -15,7 +15,7 @@

#define IRQ_OFFSET 16
#ifndef IRQ_MAX_NR
#define IRQ_MAX_NR 207
#define IRQ_MAX_NR 61
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

既然是作为 Kconfig 配置了,为啥还要在头文件中定义?请继续研究。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里只有在没有定义 IRQ_MAX_NR 的情况下才会启用。且该文件没有统一使用 libcpu 中的文件,继续推进可能涉及到 #8977

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里只有在没有定义 IRQ_MAX_NR 的情况下才会启用。且该文件没有统一使用 libcpu 中的文件,继续推进可能涉及到 #8977

我没深入看 #8977

但是这里一个工程里出现两个 IRQ_MAX_NR 肯定不合适的。

是否可以去掉 bsp/cvitek/c906_little/board/interrupt.h 里的 IRQ_MAX_NR,然后在 bsp/cvitek/c906_little/board/interrupt.h#include <rtconfig.h>。也就是说我们以 rtconfig.h 也就是我们 Kconfig 配置的值为准。

Copy link
Contributor

@unicornx unicornx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

见上面 comments。

@unicornx unicornx self-requested a review August 6, 2024 00:38
Copy link
Contributor

@unicornx unicornx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

需要继续修改

Analysis: The IRQ_MAX_NR value of c906_little is wrong.
interrupt.h relies on IRQ_MAX_NR defined in rtconfig.h but
does not explicitly include this header file.

Solution: Change IRQ_MAX_NR to the correct value 61 in
the datasheet. Explicitly include rtconfig.h in interrupt.h.

Signed-off-by: Shicheng Chu <[email protected]>
Reviewed-by: Chen Wang <[email protected]>
@Z8MAN8 Z8MAN8 force-pushed the cvitek_riscv_irq branch from e671783 to f35f06d Compare August 6, 2024 14:40
@Z8MAN8 Z8MAN8 requested a review from unicornx August 7, 2024 01:18
Copy link
Contributor

@unicornx unicornx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

请将 commit 压缩成一个后再提交 review。

review request 前请仔细检查。

@Z8MAN8
Copy link
Contributor Author

Z8MAN8 commented Aug 7, 2024

请将 commit 压缩成一个后再提交 review。

review request 前请仔细检查。

是因为 #9259 建议大小核分成两个 commit

@unicornx
Copy link
Contributor

unicornx commented Aug 7, 2024

请将 commit 压缩成一个后再提交 review。
review request 前请仔细检查。

是因为 #9259 建议大小核分成两个 commit

sorry, 我忘记了,我收回我的话。

Copy link
Contributor

@unicornx unicornx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已 review。

@Rbb666 Rbb666 merged commit b6c26d4 into RT-Thread:master Aug 7, 2024
40 of 43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: RISC-V BSP related with risc-v BSP: Cvitek BSP related with cvitek
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] bsp/cvitek cv18xx_risc-v IRQ_MAX_NR num error
3 participants