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

[Stefan Richter <stefanr@s5r6.in-berlin.de>]

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
-=====-=-==- ==-- -===-
http://arcgraph.de/sr/