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

drivers:intc_plic: fix plic PLIC_TRIG_LEVEL define mistake #67301

Merged
merged 1 commit into from
Jan 10, 2024

Conversation

Cherish-Gww
Copy link
Contributor

@Cherish-Gww Cherish-Gww commented Jan 8, 2024

Describe the bug
RISCV interrupt-controller PLIC driver each interrupt can entry once,

because in drivers/interrupt-controller/intc_plic.c Due to PLIC_TRIG_LEVEL define mistake case that in function plic_irq_handler no clean the claim register, the interrupt always in pending.

Reason
when compile c file, the BIT(n) is #define BIT(n) (1UL << (n)

the PLIC_TRIG_LEVEL is defined to ((uint32_t)~BIT(0)) it equal to 0xfffffffe, not 0

	if (trig_val == PLIC_TRIG_LEVEL) {
		sys_write32(local_irq, claim_complete_addr);
	}

it will not clean the claim register, and I have printf the PLIC_TRIG_LEVEL, it equal 0xfffffffe.

Additional context
In nceplic100 pdf:
http://www.andestech.com/wp-content/uploads/AX45MP-1C-Rev.-5.0.0-Datasheet.pdf
Page 277 say:
These registers are read-only and indicate the configured interrupt trigger type of interrupt sources.
Every interrupt source occupies 1 bit. There are a total of 32 registers, each 32-bit wide, for 1023
interrupt sources. The location of the interrupt trigger type bit for interrupt source n (1 ≤ n ≤ 1023)
can be determined by the following equations

Value Meaning
0 Level-triggered interrupt
1 Edge-triggered interrupt

@zephyrbot zephyrbot added area: RISCV RISCV Architecture (32-bit & 64-bit) area: Interrupt Controller Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc. labels Jan 8, 2024
@Cherish-Gww Cherish-Gww force-pushed the fix_riscv_plic_bug branch 2 times, most recently from 8e33124 to 65069eb Compare January 8, 2024 11:02
fkokosinski
fkokosinski previously approved these changes Jan 9, 2024
@fkokosinski
Copy link
Member

Could you take a look at this as well @ycsin?

ycsin
ycsin previously approved these changes Jan 9, 2024
Copy link
Collaborator

@ycsin ycsin left a comment

Choose a reason for hiding this comment

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

LGTM

drivers/interrupt_controller/intc_plic.c Outdated Show resolved Hide resolved
Interrupt trigger type register each bit indicate the configured interrupt
type. bit value is 0 indicate level trigger interrupt, 1 indicate edge
trigger interrupt.

The level trigger defined to ~BIT(0) equal 0xfffffffe not equal 0.

Signed-off-by: Weiwei Guo <guoweiwei@syriusrobotics.com>
@ycsin ycsin requested review from cfriedt and jgl-meta January 9, 2024 10:11
@ycsin ycsin added the bug The issue is a bug, or the PR is fixing a bug label Jan 10, 2024
@carlescufi carlescufi merged commit eac50e3 into zephyrproject-rtos:main Jan 10, 2024
19 checks passed
@Cherish-Gww Cherish-Gww deleted the fix_riscv_plic_bug branch January 11, 2024 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Interrupt Controller area: RISCV RISCV Architecture (32-bit & 64-bit) bug The issue is a bug, or the PR is fixing a bug Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants