Re: [PATCH 3/3] Import fw-sbp2 driver.

Stefan Richter
2006-12-14T21:40:55Z

Kristian Høgsberg wrote:
> Jeff Garzik wrote:
>> doesn't allowing the stack to issue REPORT LUNS take care of this?
>
> Possibly, I don't have firewire multi-LUN devices to test with here.
> The LUNs are also discoverable from the firewire config rom, which is
> why I put the comment there. This doesn't mean that the SCSI commands
> for discovering LUNs doesn't also work.

I expect REPORT LUNS won't work for many SBP-2 devices. It is not included
in RBC.

We discover LUs properly from the information in the ISO 13213 ROM. We just
don't map multiple LUs of the same target to scsi_device's beneath a single
scsi_target. (We instantiate one Scsi_Host for each LU. I might implement
a respective mapping some day, but there is no bigger benefit of doing so.)

>>> +static int sbp2_scsi_queuecommand(struct scsi_cmnd *cmd,
>>> scsi_done_fn_t done)
>>> +{
>>> + if (cmd->cmnd[0] == REQUEST_SENSE) {
>>> + fw_notify("request_sense");
>>> + memcpy(cmd->request_buffer, cmd->sense_buffer,
>>> cmd->request_bufflen);
>>> + memset(cmd->sense_buffer, 0, sizeof(cmd->sense_buffer));
>>> + cmd->result = DID_OK << 16;
>>> + done(cmd);
>>> + return 0;
>>> + }
>>
>> this is a broken emulation. this command is specified to not
>> repeatedly return the same sense data.
>
> I copied it over from the old stack under the assumption that it fixed
> something for some device.

Yes, it's in the old driver. I haven't checked yet when and why it was
written that way. Will do so eventually.

Usually, the SBP-2 status block is used to communicate autosense data.
Targets which do so may not support REQUEST SENSE. Targets which don't
do so have to support REQUEST SENSE; I suppose sbp2's curious REQUEST
SENSE handling is badly broken for such devices as far as any exist.

> I took it out and tested with the 10 or so
> storage devices I have here and it makes no difference. I've never seen
> the fw_notify() that I put in there trigger. I'm taking out this
> workaround for now,

OK.

> unless someone can tell me why it should stay there.
>
> Kristian

--
Stefan Richter
-=====-=-==- ==-- -===-