OpenCores
Issue List
NMI int inadvertently masked by int_acc #23
Closed sjackson opened this issue over 10 years ago
sjackson commented over 10 years ago

In the frontend module, if there is a pending irq_acc the inst_nmi can be unconditionally cleared causing the nmi interrupt to hang as it never gets serviced. Depending on application this might be rare but in our application it's highly repeatable though there's only a 1 clock window of failure.

reg inst_nmi; always @(posedge mclk or negedge puc_rstn) if (~puc_rstn) inst_nmi <= 1'b0; else if (nmi_evt) inst_nmi <= 1'b1; else if (i_state==I_IRQ_DONE) inst_nmi <= 1'b0;

We fixed this by creating a signal delayed by one clock based on irq_acc as follows;

eg irq_acc_delay; always @(posedge mclk or negedge puc_rstn) if (~puc_rstn) irq_acc_delay <= 1'b0; else irq_acc_delay <= |(irq_acc);

reg inst_nmi; always @(posedge mclk or negedge puc_rstn) if (~puc_rstn) inst_nmi <= 1'b0; else if (nmi_evt) inst_nmi <= 1'b1; else if (i_state==I_IRQ_DONE && !irq_acc_delay) inst_nmi <= 1'b0;

The fix is working through extensive simulations which shmoo the interrupts across each other.

sjackson commented over 10 years ago

Text got mangled

reg irq_acc_delay; always @(posedge mclk or negedge puc_rstn) if (~puc_rstn) irq_acc_delay <= 1'b0; else irq_acc_delay <= |(irq_acc);

sjackson commented over 10 years ago

Got mangled again

irq_acc_delay <= |(irq_acc);

sjackson commented over 10 years ago

Post appears to have a problem with "<="

sjackson commented over 10 years ago

What a POS

sjackson commented over 10 years ago

Turn off the HTML!

olivier.girard commented over 10 years ago

Hi Steve,

Thanks a lot for your report. I have restructured the NMI handling several years ago and the current code doesn't look as the one you pasted in your report (in particular, the inst_nmi register doesn't exist anymore)...

Would it be possible for you to update to the latest code version and try to run your IRQ shmoo test on it ?

Thanks again, Olivier

olivier.girard was assigned over 10 years ago
sjackson commented almost 10 years ago

Hi Olivier,

Sorry I didn't get back sooner but we had 2 tapeouts since June. I've only updated the core once to get the expanded peripheral space change. Since our original tapeout I've been rather hesitant to update the core as I've made other changes to accommodate RAM(for downloading firmware) and a boot ROM in the pmem space. I'll try to update the core offline or at least give the code a look.

I also want to get a look at some of the other changes especially with ASIC/FPGA clocking changes. A rather big shortfall in the present design is lack of clocking of memory outputs and inputs, this causes the present design pressed to make timing in an FPGA development board even at 32MHz. In simulation this also cause contamination with Xs of the mab if one is not careful with memory models.

Steve

olivier.girard commented almost 10 years ago

Hi Steve,

Regarding the clocking, I agree that there is huge timing path from the data output of the memories to the address computation of the pmem.

I thought about deepening the pipeline at the very beginning when I started this project, and it just boiled down to the following options:

1- keep the pipeline as short as in the original MSP430... which has the advantage of reducing the number of clock cycle required to execute instructions and also keep the core area small... with the drawback that the maximum synthesis frequency is limited.

2- increasing the pipeline depth on the other end would definitely allow faster target frequencies but would deteriorate the power consumption (i.e. more FF toggling), increase the area and lower the performance per MHz ratio.

Most of the users of the core (at least the ones I'm aware of) are using it in mixed-signal ASICs on either:

  • older processes (up to 250nm) where area is a huge concern.
  • newer processes (down to 65nm) where power consumption is a big concern (and apparently not only leakage).

In both cases, the shorter pipeline is usually not really limiting as the openMSP430 is typically used for control and relatively non-compute intensive applications.

Now for FPGA, it is true that it might be more complicated, especially if you use lower end ones... in addition, the timings are probably further degraded by the additional decoding you've added to support your boot ROM and your firmware loader.

Olivier

olivier.girard commented almost 10 years ago

Actually, I found the X contamination quite useful when verifying firmware in simulation (i.e. a ROM bootloader). For example, if your program reads a memory address which was never been written before (i.e. a bug), then you'll immediately notice it :-)

In any case, I hope that the two tapeouts went smoothly and that you already have running silicon :-)

Cheers, Olivier

sjackson commented almost 10 years ago

The core has been taped out 3 times in 28nm HPM TSMC and the last should be back shortly, it is used primarily for control but there is a lot of memory for storing both program and protocol setups for customer communication to the analog. We're only running at 32MHz but the FPGA porting performance was still pushed though makeable.

Looked over the newer implementation of the NMI and other changes to the latest release and it's pretty far removed from what we have now which is pretty much a forked version at this point. RTL is all negedge resets(scan insertion likes to match library), other changes were made for scan that are pretty close to what is in the code now(ifdef'd), watchdog timer has been altered to yield real world delays, memory backbone changed for ROM/RAM operation etc, so at this point you should probably just close this one.

The X-contamination does not only occur in just a bug situation, but also due to the dual access of the memory for smooth transition out of boot mode easily solvable by preloading the SRAM. In a clocked situation though with one pipeline stage this would not occur.

Regards, Steve

olivier.girard closed this over 9 years ago

Assignee
olivier.girard
Labels
Bug