Bug fix for reset problem

cgracey · 2014-08-09 09:21

In the cog.v file, there are problems in the assignments to the outa and dira registers, apparently.

I'm not at my computer right now, but from what pik33 posted, the dira and outa assignments need to be changed to:

always @(posedge clk_cog or negedge ena)
if (!ena)
    outa <= 32'b0;
else if (setouta)
    outa <= alu_r;

always @(posedge clk_cog or negedge ena)
if (!ena)
    dira <= 32'b0;
else if (setdira)
    dira <= alu_r;

The ena signal must be used to reset outa and dira. I don't know how I missed this on the AHDL-to-Verilog translation. I'll update the main files when I get back from DEFCON.

pik33 · 2014-08-09 09:25

Looks better than my quick and dirty patch

Time to try it.

Peter Jakacki · 2014-08-09 09:26

cgracey wrote: »
In the cog.v file, there are problems in the assignments to the outa and dira registers, apparently.

I'm not at my computer right now, but from what pik posted, the dira and outa assignments need to be changed to:
always @(posedge clk or negedge ena)
if (!ena)
    outa <= 32'b0;
else if (setouta)
    outa <= alu_r;

always @(posedge clk or negedge ena)
if (!ena)
    dira <= 32'b0;
else if (setdira)
    dira <= alu_r;
The ena signal must be used to reset outa and dira. I don't know how I missed this on the AHDL-to-Verilog translation. I'll update the main files when I get back from DEFCON.

Great, thanks Chip, I'm just getting stuck into the P1 emulation now and just hooked up an EEPROM (actually I bridged another Prop board) so I will give this a go.

pik33 · 2014-08-09 09:33

This - (posedge clk...) - shouldn't it be posedge clk_cog?

Edit: compiled and tested ok with clk_cog as below:

always @(posedge clk_cog or negedge ena)
if (!ena)
    outa <= 32'b0;
else if (setouta)
    outa <= alu_r;

always @(posedge clk_cog or negedge ena)
if (!ena)
    dira <= 32'b0;
else if (setdira)
    dira <= alu_r;

cgracey · 2014-08-09 09:48

pik33 wrote: »
This - (posedge clk...) - shouldn't it be posedge clk_cog?

Edit: compiled and tested ok with clk_cog as below:
always @(posedge clk_cog or negedge ena)
if (!ena)
    outa <= 32'b0;
else if (setouta)
    outa <= alu_r;

always @(posedge clk_cog or negedge ena)
if (!ena)
    dira <= 32'b0;
else if (setdira)
    dira <= alu_r;

That's right!

Thanks for pointing this out. I'm going to correct my first post.

Cluso99 · 2014-08-09 16:02

Enjoy DEFCON Chip - you have earned a break... eaarh...distraction

Cluso99 · 2014-08-10 19:19

Chip has said only this fix is required...
http://forums.parallax.com/showthread.php/156860-Here-are-the-files-needed-for-running-P8X32A-on-the-BeMicroCV?p=1285152&viewfull=1#post1285152

always @(posedge clk_cog or negedge ena)
if (!ena)     
    dira <= 32'b0; 
else if (setdira)     
    dira <= alu_r;

mindrobots · 2014-08-11 08:34

When I apply this fix and build for the Nano it does not work. The Nano is essentially dead - it does not respond to F7 or F10 in Propeller Tool and does not boot from the attached EEPROM.

// 2014_08_10 Official fix from CGRACEY for reset problem taken from BE Micro thread post #1
always @(posedge clk_cog or negedge ena)
if (!ena)
    dira <= 32'b0;
else if (setdira)
    dira <= alu_r;

When I revert to this fix:

// Initial fix - 2014_08_09 CGracey & PIK33
  
always @(posedge clk_cog or negedge ena)
if (!ena)
    outa <= 32'b0;
else if (setouta)
    outa <= alu_r;

always @(posedge clk_cog or negedge ena)
if (!ena)
    dira <= 32'b0;
else if (setdira)
    dira <= alu_r;

With this fix, the Nano works as expected (F7 and F10 work repeatedly)

I don't really want to commit the "unofficial" fix even though it works if there will be a fixed "official" fix at some future point. Doing that would just lead to two slightly different sources floating around.

Can anyone else confirm or deny the working of the "official" fix?

cgracey · 2014-08-11 08:42

mindrobots wrote: »
When I apply this fix and build for the Nano it does not work. The Nano is essentially dead - it does not respond to F7 or F10 in Propeller Tool and does not boot from the attached EEPROM.
// 2014_08_10 Official fix from CGRACEY for reset problem taken from BE Micro thread post #1
always @(posedge clk_cog or negedge ena)
if (!ena)
    dira <= 32'b0;
else if (setdira)
    dira <= alu_r;
When I revert to this fix:
// Initial fix - 2014_08_09 CGracey & PIK33
  
always @(posedge clk_cog or negedge ena)
if (!ena)
    outa <= 32'b0;
else if (setouta)
    outa <= alu_r;

always @(posedge clk_cog or negedge ena)
if (!ena)
    dira <= 32'b0;
else if (setdira)
    dira <= alu_r;
With this fix, the Nano works as expected (F7 and F10 work repeatedly)

I don't really want to commit the "unofficial" fix even though it works if there will be a fixed "official" fix at some future point. Doing that would just lead to two slightly different sources floating around.

Can anyone else confirm or deny the working of the "official" fix?

The actual P8X32A chip only hard resets the DIRA register. OUTA gets reset during loading, being written with a zero. It doesn't make sense to my why OUTA should need to be hard reset. It's the DIRA that must be immediately reset on a cog stop or chip reset, to free the pins. Once DIRA is zeroed, it doesn't matter what OUTA holds.

It doesn't hurt anything to reset both DIRA and OUTA, though. It just shouldn't be necessary to do OUTA.

Heater. · 2014-08-11 08:46

Chip,

It doesn't make sense to my why OUTA should need to be hard reset.

But surely if my program wakes up from reset it may well set DIRA to output in the expectation that the OUTA is low until told otherwise. Which could be some time later. Driving a HIGH out when not expected might lead to bad outcomes.

OK, wait, you said OUTA is set during loading which is before my program can set DIRA I presume.

mindrobots · 2014-08-11 08:53

cgracey wrote: »

The actual P8X32A chip only hard resets the DIRA register. OUTA gets reset during loading, being written with a zero. It doesn't make sense to my why OUTA should need to be hard reset. It's the DIRA that must be immediately reset on a cog stop or chip reset, to free the pins. Once DIRA is zeroed, it doesn't matter what OUTA holds.

It doesn't hurt anything to reset both DIRA and OUTA, though. It just shouldn't be necessary to do OUTA.

Thank you for the explanation - we need to collect this wisdom in a book for the P1V architecture class.

My immediate concern was that resetting just DIRA in the FPGA left me with a dead Nano while resetting both left me with a properly functioning Nano. I may have messed up my build process so I was hoping someone could test the DIRA only fix to see if it is me or if it is the fix that broke my version of this .jic file.

I was going to put the "offical" fix into my GitHub repository and roll it up to Heater's but don't want to do that until it is independently tested.

cgracey · 2014-08-11 09:45

Yeah, you should only need to hard reset DIRA. OUTA does get cleared to zero when the cog gets loaded.

I'm going to review the reset sequence this morning. I might have over-simplified something in there when doing the translation.

mindrobots · 2014-08-11 09:47

cgracey wrote: »

Yeah, you should only need to hard reset DIRA. OUTA does get cleared to zero when the cog gets loaded.

I'm going to review the reset sequence this morning. I might have over-simplified something in there when doing the translation.

Thank you!

potatohead · 2014-08-11 09:51

mindrobots, you might consider doing it anyway as an exercise. There is history, so this change could reflect the current understanding. Works, but needs review, because Chip says it shouldn't need to be there to work. And you can add that comment to the repository!

Later, when review happens, say Chip finds out he did oversimplify. It's very highly likely a few files get touched. So then that gets done, and the history rolls forward.

Sometime in the future, people would see the initial repo, heater doing tab cleanup, you pushing the bug fix, then a more complete fix later on.

When it's deemed "Good", then a tag can be added.

And we've not yet discussed revisions!! Which revision or version is this?

I think it should be something like 1.0, initial release from Parallax.

When the big fix is complete, then perhaps that's version 1.1, reset issue fixed up proper.

etc...

Heater. · 2014-08-11 10:05

potatohead,

And we've not yet discussed revisions!! Which revision or version is this?

My 8X32A_Emulation repo has the tag "Version-2014-08-06". To mark the start of all things.

Since then tabs have been removed and README's changes a bit maybe but there is no new version. No reason to be.

One could tag all significant releases or fixes or whatever like "Reset bug fix". Not very meaningful though. There may be a hundred reset bug fixes in the future.

potatohead · 2014-08-11 10:51

Totally.

I guess we don't need to resolve that now either. Use dates, until such time as a better nomenclature makes sense.

Heater. · 2014-08-11 11:19

Ah, version numbering schemes. What a nightmare. Major numbers, minor numbers, da da da...

At the end of the day any user of your stuff does not care if major numbers mean new features were added and minor numbers were bug fixes only, or major numbers mean you broke it by changing the API, or whatever.

What they do care about is that they had a version that they used and it worked and they would like to be able to find it again.

They certainly don't want to go back to older versions than the one they have tried and tested.

They may be willing to try newer versions, as long as they can go back to the old one when you break it in an update.

So, any orderable, monotonically increasing version identifier will do.

I used the date there because who am I to say this is version 0.0.1 or 1.0.0 or whatever? No it was released on that historic day. So that is what I called it.

potatohead · 2014-08-11 11:40

Yes. Absolutely. Roll with what you know. That perspective is very useful Heater.

cgracey · 2014-08-11 12:35

mindrobots wrote: »
When I apply this fix and build for the Nano it does not work. The Nano is essentially dead - it does not respond to F7 or F10 in Propeller Tool and does not boot from the attached EEPROM.
// 2014_08_10 Official fix from CGRACEY for reset problem taken from BE Micro thread post #1
always @(posedge clk_cog or negedge ena)
if (!ena)
    dira <= 32'b0;
else if (setdira)
    dira <= alu_r;
When I revert to this fix:
// Initial fix - 2014_08_09 CGracey & PIK33
  
always @(posedge clk_cog or negedge ena)
if (!ena)
    outa <= 32'b0;
else if (setouta)
    outa <= alu_r;

always @(posedge clk_cog or negedge ena)
if (!ena)
    dira <= 32'b0;
else if (setdira)
    dira <= alu_r;
With this fix, the Nano works as expected (F7 and F10 work repeatedly)

I don't really want to commit the "unofficial" fix even though it works if there will be a fixed "official" fix at some future point. Doing that would just lead to two slightly different sources floating around.

Can anyone else confirm or deny the working of the "official" fix?

I looked through the reset circuitry and it seems to be fine. I don't know why you're having this problem. Could you please mail your cog.v file to my forum account? How about the one that works and the one that doesn't work? I don't know what else to check. Thanks.

cgracey · 2014-08-11 12:43

Is anyone else having any reset issues with any boards?

That section in cog.v should be:

// outa/dira

reg [31:0] outa;
reg [31:0] dira;

always @(posedge clk_cog)
if (setouta)
	outa <= alu_r;

always @(posedge clk_cog or negedge ena)
if (!ena)
	dira <= 32'b0;
else if (setdira)
	dira <= alu_r;

mindrobots · 2014-08-11 13:03

cgracey wrote: »
Is anyone else having any reset issues with any boards?

That section in cog.v should be:
// outa/dira

reg [31:0] outa;
reg [31:0] dira;

always @(posedge clk_cog)
if (setouta)
    outa <= alu_r;

always @(posedge clk_cog or negedge ena)
if (!ena)
    dira <= 32'b0;
else if (setdira)
    dira <= alu_r;

Probably not!

I had lost these lines somewhere along the line:

always @(posedge clk_cog) if (setouta) 	outa <= alu_r;

which led to the confusing discussion about OUTA not needing to be cleared.

I cut and pasted too much when going from the first fix with two always blocks to you final fix which was just a replacement for the second always block.

I will rebuild correctly and report back.

(This is a good case for a problem with forum snippets)

jazzed · 2014-08-11 13:17

Hi Chip.

Works for me on the DE2-115.

mindrobots · 2014-08-11 13:18

Chip,

Your *COMPLETE* patch works fine.

I'll go sit in the corner for a while. Sorry I wasted your time.

Thanks!

Ron Sutcliffe · 2014-08-11 13:24

Chip, your first fix for the reset issue worked fine. I have just tried your fix in your second post and cannot load ram. I have just rebuilt with your first fix again and it's all OK..
I used the same cog.v file but just commented out the outa reset code and rebuilt to test and it failed. UN-commented outa reset code, rebuilt and its its OK

Ron

Ron Sutcliffe · 2014-08-11 13:26

@ Rick can you confirm is worked on the Nano

Ron

mindrobots · 2014-08-11 13:41

Yes, just built and tested. It works fine on the Nano.

It got confusing the first fix - provided by Chip and Pik33 had 2 always blocks; one for OUTA and one for DIRA, the original code also had two blocks so it was an easy cut and paste replacing the two blocks with two new blocks.

The "official code provided by Chip in the BE-micro thread just have one always block for DIRA. So I put the one block in place of the two blocks and that caused OUTA to never be reset.

When Chip posted his entire code section in post #20 of this thread, showing the section comment, the two reg statements and the two always blocks, it dawned on me what I had messed up in cutting and pasting the snippets from the various threads.

If you take the code from post #20 and use it to replace the entire "// outa/dira" section in cog.v, you should be in good shape.

I'm building BE-micro and DE2 now for testing.

Ron Sutcliffe · 2014-08-11 13:50

Thanks,Rick, i will go and sit in the other corner and look at Chip's post again.

Ron

mindrobots · 2014-08-11 13:53

I'm going to make a Propeller dunce cone instead of a beanie.....I'll get more use out of the dunce cone! :frown:

cgracey · 2014-08-11 14:10

Okay, so it looks like everything is fine now. Sorry for the confusion. We will post new files on the original page soon. Thanks for your help, Guys.

Cluso99 · 2014-08-11 14:39

While you are posting, might it be possible to save them with spaces instead of tabs. Its an option in quartus.

cgracey · 2014-08-11 15:15

Cluso99 wrote: »

While you are posting, might it be possible to save them with spaces instead of tabs. Its an option in quartus.

I just tried:

Edit | Replace Tabs With Spaces

And it literally replaced tabs ($09) with spaces ($20), on a one-to-one basis, without preserving the indention levels by using variable numbers of spaces. I don't see any other options in Quartus. Is there another way that you know of?

I've always wished that some character (like $1F) would be an escape function where the following byte would determine the number of spaces to spit out. That would make text files really compact.

Bug fix for reset problem

Comments