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

Edge triggered IRQ does not work as expected #29

Open
wwwweb opened this issue Apr 27, 2015 · 4 comments
Open

Edge triggered IRQ does not work as expected #29

wwwweb opened this issue Apr 27, 2015 · 4 comments
Assignees
Labels

Comments

@wwwweb
Copy link

wwwweb commented Apr 27, 2015

When using edge triggered IRQs, the (rising) edge seems to be detected using the PICSR itself and the incoming IRQLINE (!spr_picsr[irqline] & irq_unmasked[irqline] in mor1kx-pic.v). When resetting the according IRQ bit in PICSR with the IRQLINE still high (no new edge but still high level), the IRQ is triggered in the next cycle again, so this just works like a level triggered IRQ.

From my opinion there needs to be an additional register stage for edge detection which only generates a 1 cycle set. Using this, I think the logic that "CLEAR" is served before "SET" is not sufficient, as I would expect that when an edge occurs in the instant I clear the IRQ (usually within the ISR), the IRQ bit stays active and another IRQ is triggered.

Best regards,
wwwweb

@skristiansson skristiansson self-assigned this Apr 27, 2015
@skristiansson
Copy link
Member

I agree, it doesn't look right. I'll look into it closer.

skristiansson added a commit that referenced this issue May 14, 2015
As pointed out in github issue #29, clearing an edge triggered
interrupt while the interrupt source is still high would
re-trigger the interrupt even though no new edge has
occurred.

This cures this by adding an additional registering
of the irq_unmasked signals and only set the picsr
spr register when an edge occurs.

Also, the priority between setting and clearing the
bits in picsr have been reversed.
@skristiansson
Copy link
Member

This should be fixed by:
6caa7db

Please test and confirm.

@wwwweb
Copy link
Author

wwwweb commented Jun 8, 2015

Hi skristiansson,

I had a look at the bugfix. I think it is not fully fixed yet. Assuming, that the according irqline is high when enabling the irq in picmr, the check on the rising edge is true immediately, although this rising edge may have occurred when the irqline was masked or may not have occurred at all if irqline is pulled up by default. I think, irq_unmasked_r <= irq_i may work better in the registered process.

Best regards,
wwwweb

@skristiansson
Copy link
Member

yes, agreed, the registering need to take place before the unmasking.

@skristiansson skristiansson reopened this Jun 10, 2015
bandvig pushed a commit to bandvig/mor1kx that referenced this issue Dec 3, 2016
As pointed out in github issue openrisc#29, clearing an edge triggered
interrupt while the interrupt source is still high would
re-trigger the interrupt even though no new edge has
occurred.

This cures this by adding an additional registering
of the irq_unmasked signals and only set the picsr
spr register when an edge occurs.

Also, the priority between setting and clearing the
bits in picsr have been reversed.
bandvig added a commit to bandvig/mor1kx that referenced this issue Dec 3, 2016
Squashed commit:

[4db0d97] Open old RM_CTRL, add new for small modifications

[a2ea2a8] at last the working variant of LATTE

[ab27b85] intermediate

[443b9d7] (1) some improvement on SPR access from MT(F)SPR. (2) Prevent instruction issue if a FETCH exception occurs.

[67b1e0b] re-factoring for decode_bubble & cosmetics

[29bd160] RM_CTRL

[3f469a1] A step toward moving LSU and MF(T)SPR on execute stage

[e1cd327] Re-factoring exceptions (mostly for LSU).

[4d6c80b] Open NEW_FLUSH. Block lsu_valid if an exception ocures in LSU.

[1ab2865] Cosmetics

[6caa7db] pic: fix OPTION_PIC_TRIGGER="EDGE"

As pointed out in github issue openrisc#29, clearing an edge triggered
interrupt while the interrupt source is still high would
re-trigger the interrupt even though no new edge has
occurred.

This cures this by adding an additional registering
of the irq_unmasked signals and only set the picsr
spr register when an edge occurs.

Also, the priority between setting and clearing the
bits in picsr have been reversed.

[28674ca] cappuccino/ctrl: add missing '?' in exception cause decoding

[1296337] (1) Priority flush over advance. (2) Start re-factoring for LSU

[374ba33] Start re-factoring for ctrl module

[c08c176] remove decode_valid from execute

[c81711c] Complete RF re-factoring

[ed2be82] (1) Restore 3-stage multiplier as it more suitable for future pipeline marocchinno. (2) Re-factoring RF module.

[620b9c3] complete renaming

[f453ad0] Create own version of RF. Continue renaming

[fa992f9] move WB=related code into wb_mux

[85e78a5] massive wire renaming and cosmetics

[a51a50d] move pipelined multiplier out of execute module

[7e35ae9] combine cappuccino's decode and decode_execute into latte's decode

[e50460c] start merging decode and decode_execute into decode_latte

[f4c4038] combine "execute_alu" and "execute_ctrl" into one module: "execute_latte"

[2064bcb] execute for latte

[54df168] 1st step of latte pipe implementation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants