On 04/05/2011 04:00 PM, Kevin Wolf wrote:
I have a hard time each time I try to understand this SCSI stuff without
reading a lot of code and specs. What I would have expected is this:
if (len == 0) {
scsi_command_complete(r, 0);
} else {
r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, len);
}
This would fix the problem that both functions are called, but still use
SCSI_REASON_DONE if no data is transferred. This is similar to what
scsi-disk seems to be doing. However, if you can explain to me why your
version is more correct, I'll gladly take it.
I agree with Kevin that his version seems more correct. The purpose of
the code is to trigger a phase mismatch for the lsi controller, and that
would probably not happen with your version. s->current->dma_len would
never go down to 0 if it were really a short transfer:
s->current->dma_len -= count;
if (s->current->dma_len == 0) {
s->current->dma_buf = NULL;
if (out) {
/* Write the data. */
dev->info->write_data(dev, s->current->tag);
} else {
/* Request any remaining data. */
dev->info->read_data(dev, s->current->tag);
}
} else {
s->current->dma_buf += count;
lsi_resume_script(s);
}
so the SCRIPTS would deadlock, waiting for the full transfer to happen,
and the SCSI_REASON_DONE callback would never be sent.
Even with spapr_vscsi, however, what would happen is that the
SCSI_REASON_DATA does nothing except calling sdev->info->read_data (i.e.
scsi_read_data) again. This would then call scsi_command_complete and
send the SCSI_REASON_DONE callback. So for spapr_vscsi there should be
no difference between the two cases.
I'll include Kevin's patch into my upcoming SCSI series.
Paolo
