Issues :: risc16f84 :: OpenCores

When the op-code contains a litteral and this litteral is 0x03, it occurs that the wire "addr_stat" becomes true. In this particular case, an incorrect flag assignment occurs.

Add the following verilog code to patch this behavior :

wire disable_status_z; wire disable_status_c; wire disable_status_dc;

// disable write access for status flags assign disable_status_z = (inst_addwf || inst_andwf || inst_clrf || inst_clrw || inst_comf || inst_decf || inst_incf || inst_iorwf || inst_movf || inst_subwf || inst_xorwf || inst_addlw || inst_andlw || inst_iorlw || inst_iorlw || inst_sublw || inst_xorlw) ? 1:0;

assign disable_status_c = (inst_addwf || inst_subwf || inst_rlf || inst_rrf || inst_addlw || inst_sublw ) ? 1:0;

assign disable_status_dc = (inst_addwf || inst_subwf || inst_addlw || inst_sublw ) ? 1:0;

...

  // 2-5-2-2. Set data RAM/special registers,
  if (writeram_node)
  begin
    if (addr_stat)
    begin
      status_reg[7:5] <= aluout[7:5];      // write IRP,RP1,RP0
      // status(4),status(3)...unwritable, see below (/PD,/T0 part)

      if( ~disable_status_c )
          status_reg[0] <= aluout[0];      // write C
	  if( ~disable_status_dc )
          status_reg[1] <= aluout[1];      // write DC
    end				    
    if (addr_fsr)        fsr_reg        <= aluout;       // write FSR
    if (addr_pclath)     pclath_reg     <= aluout[4:0];  // write PCLATH
    if (addr_intcon)     intcon_reg     <= aluout;       // write INTCON
    if (addr_aux_adr_lo) aux_adr_lo_reg <= aluout;       // write AUX low
    if (addr_aux_adr_hi) aux_adr_hi_reg <= aluout;       // write AUX high
  end

  // 2-5-2-3. Set/clear Z flag.
  if (addr_stat && ~disable_status_z) status_reg[2] <= aluout[2]; // (dest. is Z flag)
  else if (   inst_addlw || inst_addwf || inst_andlw || inst_andwf
           || inst_clrf  || inst_clrw  || inst_comf  || inst_decf
           || inst_incf  || inst_movf  || inst_sublw || inst_subwf
           || inst_xorlw || inst_xorwf || inst_iorlw || inst_iorwf )
          status_reg[2] <= aluout_zero_node; // Z=1 if result == 0

I think C and DC flags no problem. Probably Z flag is wrong. All these should not effect the function.

I have to confirm, but I got a bug with the carry flag once. A 16-bits addition was wrong (+/- 1). The first time I saw the carry flag bug, I thought it was the C compiler, so I modified few lines in the code and it fixed the problem. The patch I did should be right according to the microntroller specification, but it's possible that the way I have implemented it is wrong. Up to now, I haven't encountered any bugs with this patch...

I looked at the code and I also think that C and DC flags should not be affected in your case. BUT , you are right, the following thing stated in PIC datasheet seems to be not respected : " When the STATUS register is the destination for an instruction that affects the Z, DC or C bits, then the write to these three bits is disabled. The specified bit(s) will be updated according to device logic ". I do not know Verilog a lot, but if it works as VHDL does, the following simple patch should be sufficient: merely put the code block managing write to special registers BEFORE all code blocks managing C,DC and Z flags updates. In this way C,DC and Z flags code blocks will override special register write code block, in case of concurrent changes. This is because, in case of race condition, the latest verified condition will win. What do u think of it? Moreover, with your patch, line // 2-5-2-3 .... , you do not include the writeram_node condition in the if test. I think it is strange (buggy ?)

Stanislav Corboot submitted a new version of "risc16f84_clk2x.v" which may have fixed this problem. It is checked into SVN as revision 12.

Browse

risc16f84