Re: Bugs in State Machine of PCI Local Bus Spec

[Dong Wang <dongw@cs.cmu.edu>]

For the OE_IRDY# problem, I looked at the spec, pay attention the 
equation is OE_IRDY# == [M_DATA+ADDR], so this means when
the last state is M_DATA or ADDR, which is almost the same
as your implementation.

For your changes to FRAME#, master time out and the deassertion 
of Grant should be both present to make FRAME# deasserted(p38).

I agree with you on your suggestions about the state machine,
which will fix the first bug I proposed.

Regards,

Dong

dsteven@lexmark.com wrote:
> 
> Dong;
> 
> I ran across a similar error.
> Attached is a note I sent to PCI SPEC with a negative response two years ago.
> I personally believe there are errors in the state machines.
> I have been using the following modifications in proprietary design
> so use at your own risk.  And let me know your own results.
> 
> ==========================================================================
> In PCI Spec Rev 2.1 I have found several errors in the state machines in
> appendix B.
> I realize that there is a disclaimer on the first line.  But I implemented these
> state
> machines in an ASIC and have got a part back with a bug in it.  I would
> appreciate
> any updates to these  state machines to make sure I have no further bugs.
> 
> The bug that I got caught with is on page 241:
> OE{IRDY#}  = [ m_data + addr ]
> 
> Previous to my ASIC release I had noticed one problem with this equation.
> Section 3.2.4
> say that there is no OE of irdy during the Address phase,  and I noticed that
> during turn_ar
> we needed to drive high so I released the chip with the correction as:
> OE{IRDY#}  = [ m_data + turn_ar]
> 
> The problem that I missed is that following a target disconnect w/o data I was
> driving
> IRDY active low.  Following the STOP the state machine exits via s_tar.  This
> leaves
> IRDY floating at an active low level instead of being driven inactive high
> during s_tar and then
> released. This causes the PCI target at the other end to lockup
> 
> I believe the correct equation should be:
> OE{IRDY#}  = [ m_data + turnar + s_tar ]
> This will drive Irdy to an inactive level before putting it into 3s mode.
> 
> =================================================================
> 
> OTHER Problems found previous to ASIC release.
> 
> B). In master state machine I think you need to add master_time_out to the
> Frame_out equations;
> 
> Spec
>     frame_out   <=  (addr_s2  OR (m_data_s2  AND !dev_to  AND
>                  (( !lastdata  AND (!mst_to  OR gnt_sync )  AND !stop_sync )
>                    OR !master_ready ) ));
> 
> Correction
>     frame_out   <=  (addr_s2  OR (m_data_s2  AND !dev_to  AND  !mst_to AND
>                  (( !lastdata  AND (!mst_to  OR gnt_sync )  AND !stop_sync )
>                    OR !master_ready ) ));
> =================================================================
> C). In master state machine REQ# page 241:
> 
> WAS
> REQ# =  ![Request * ! Lock_a  + Request * Lock_a * Free) * !(s_tar * Last state
> s_tar )
> 
> There should be an OR not an AND between s_tar and Last state s_tar)
> 
> REQ# =  ![Request * ! Lock_a  + Request * Lock_a * Free) * !(s_tar  + Last state
> s_tar )
> 
> =================================================================
> D). In target state machine  page 237:
> 
> I believe that the order of the next states in the b_busy transition to next
> state are incorrect.
> Go to Idle should be the lowest priority transition, not the 2nd highest
> priority.
> 
>       WHEN b_busy =>
>      IF ((frame_in = '1' OR dcd_done= '0') AND hit = '0' ) THEN
>        state_next <= b_busy;
>      ELSIF ((frame_in = '1' OR irdy_in = '1') AND hit = '1' AND
>             ( term = '0' OR (term = '1' AND target_ready = '1'))) THEN
>        state_next <= s_data;
>      ELSIF ((frame_in = '1' OR irdy_in = '1') AND hit = '1' AND
>             ( term = '1' AND target_ready = '0')) THEN
>        state_next <= backoff;
>      ELSIF ((frame_in = '0' AND dcd_done = '1') OR
>          ( frame_in = '0' AND dcd_done = '0' AND devsel_in= '1')) THEN
>        state_next <= idle;
>      ELSE
>        state_next <= b_busy;
>      END IF;
> 
> dongw@cs.cmu.edu on 02/16/99 09:40:22 PM
> 
> To:   pci-sig-request@znyx.com
> cc:    (bcc: Craig Stevenson/Lex/Lexmark)
> Subject:  Bugs in State Machine of PCI Local Bus Spec
> 
> Any comments on the two possible bugs we found in the state
> machine of PCI Local Bus Spec Rev 2.2 and 2.1 are appreciated.
> 
> 1. In the target state machine(Page 243 in Rev2.2 and Page 237
> in Rev 2.1), there is a transition from B_BUSY to IDLE, the
> condition is (FRAME#*D_done+FRAME#*!D_done*!DEVSEL#).
> 
> What we found is if the master want to do a single data phase
> transaction, then it will assert IRDY# and deassert FRMAE#,
> and if the target finish decode and determine this is a hit
> on itself, so the target will make a transition from B_BUSY
> to S_DATA. But according to the above condition, the target
> will go to IDLE because the FRAME#*D_done is true.
> 
> 2. We believe there is an inconsistence between the spec and
> the state machine implementation.
> 
> P40 in Rev2.1 and P51 in Rev2.2
> Once a master has asserted IRDY#, it can not change IRDY#
> or FRMAE# until the current data phase completes.
> 
> In the state machine, the equation for FRAME# is
> FRAME#=!(ADDR+M_DATA*!Dev_to*{[!Comp*(!To+!GNT#)*STOP#]+!Ready})
> 
> But if Master asserts IRDY#, then because the internal timer
> expires and GNT# is deasserted, according to the state machine
> Master will deassert FRAME# before the next data phase begins.
> 
> Thanks.
> 
> Dong Wang