* Re: sym53c8xx_2 fix
2002-12-19 20:14 sym53c8xx_2 fix Bjorn Helgaas
@ 2002-12-23 11:41 ` Gérard Roudier
0 siblings, 0 replies; 2+ messages in thread
From: Gérard Roudier @ 2002-12-23 11:41 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: linux-scsi, gibbs
On Thu, 19 Dec 2002, Bjorn Helgaas wrote:
> We run the sym53c8xx_2 driver on a simulator. Because the
> simulator's disk model is somewhat simplified compared to real
> hardware, it exercises some code paths that are not exercised
> on the hardware.
>
> Our simulator guru found what appears to be an error in
> one such path. Here's his description and patch (against
> 2.4.20). Let me know if you need more information.
>
> Justin, I copied you because the comment immediately preceding
> this code says it's part of an optimization you suggested
> (negotiating for SYNC immediately after a WIDE response).
>
> > ... There is also a bug in the driver that keeps us from
> > booting. This code path is not executed on hardware but is on the sim with
> > our simple disk model. The current driver assembles an SDTR request but
> > then calls the SDTR response script. Here is the fix:
>
> > --- orig/linux-ia64-2.4/drivers/scsi/sym53c8xx_2/sym_hipd.c Fri Sep 27 17:27:41 2002
> > +++ fixed/linux-ia64-2.4/drivers/scsi/sym53c8xx_2/sym_hipd.c Wed Dec 18 21:26:25 2002
> > @@ -4363,7 +4363,7 @@
> >
> > cp->nego_status = NS_SYNC;
> > OUTB (HS_PRT, HS_NEGOTIATE);
> > - OUTL_DSP (SCRIPTB_BA (np, sdtr_resp));
> > + OUTL_DSP (SCRIPTB_BA (np, send_sdtr));
> > return;
> > }
May-be the driver doesn't behave correctly here, but your patch is not
correct. The reason is that the <send_ptr> script label assumes that the
target is already in MESSAGE OUT phase. This is the case after a selection
with ATN, for example.
But, in our situation, the target just sent a WDTR response (MESSAGE IN
phase) and ATN wasn't asserted. Then the initiator must assert ATN prior
to deasserting ACK for the last message in byte in order to ask the target
for a MESSAGE OUT phase, otherwise the target can switch to any phase it
desires.
Here is the SCSI SCRIPTS (same for both firmwares):
}/*-------------------------< SDTR_RESP >------------------------*/,{
/*
* let the target fetch our answer.
*/
SCR_SET (SCR_ATN),
0,
SCR_CLR (SCR_ACK),
0,
SCR_JUMP ^ IFFALSE (WHEN (SCR_MSG_OUT)),
PADDR_B (nego_bad_phase),
}/*-------------------------< SEND_SDTR >------------------------*/,{
/*
* Send the M_X_SYNC_REQ
*/
SCR_MOVE_ABS (5) ^ SCR_MSG_OUT,
HADDR_1 (msgout),
SCR_JUMP,
PADDR_B (msg_out_done),
}/*-------------------------< PPR_RESP >-------------------------*/,{
/*
* let the target fetch our answer.
Here is a brief translation (the SCRIPTS above look understable, anyway):
SDTR_RESP:
Assert ATN
Clear ACK
Wait for MESSAGE OUT phase
SEND_SDTR:
Send the MESSAGE data
May-be I should have renamed the <SDTR_RESP> label when I implemented
Justin's suggestion in the sym driver, but the needed SCRIPTS was already
there and I just had to make the SCRIPTS processor jump to the appropriate
label. Indeed, the comments are misleading or at least not complete, and I
have to add some.
Anyway, technically, the initial driver code seems correct to me here and
I am surprised that your change allows to fix something. In my opinion, it
should break something, unless the target magically turns to MESSAGE OUT
phase for some reason.
The magic that makes a target switch to MESSAGE OUT phase is the assertion
of the ATN signal. This signal is automatically deasserted by the Symbios
chip SCSI core after a MESSAGE has been sent. As a result, the ATN signal
is not asserted in our situation, since it was cleared after our WDTR was
sent.
The SDTR_RESP stuff asserts ATN prior to deasserting ACK for the last
message byte (as specified by SCSI) in order for the target to enter
MESSAGE OUT phase immediately, then sends the MESSAGE. This behaviour
applies to a normal SDTR response or to a SDTR negotiation after WDTR
negotiation. This was the reason of re-using SDTR_RESP SCRIPTS code.
Regards,
Gérard.
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 2+ messages in thread