Re: [PATCH 5/6] firewire: SBP-2 highlevel driver

Stefan Richter
2007-05-09T21:58:29Z

Kristian Høgsberg wrote:
> Christoph Hellwig wrote:
>>> +static int add_scsi_devices(struct fw_unit *unit)
>>> +{
>>> + struct sbp2_device *sd = unit->device.driver_data;
>>> + int retval, lun;
>>> +
>>> + if (sd->scsi_host != NULL)
>>> + return 0;
>>> +
>>> + sd->scsi_host = scsi_host_alloc(&scsi_driver_template,
>>> + sizeof(unsigned long));
>>> + if (sd->scsi_host == NULL) {
>>> + fw_error("failed to register scsi host\n");
>>> + return -1;
>>> + }
>>> +
>>> + sd->scsi_host->hostdata[0] = (unsigned long)unit;
>>
>> Please take a look ar ther other scsi drivers how this is supposed
>> to be used.
>
> I was trying to be clever and only allocate the host once the device had
> been discovered and initialized. I have now changed the code to just
> allocate the host up front and use the hostdata mechanism for the
> sbp2_device struct, which also addresses the host life cycle comments
> below.

I have doubts. IMO the previous code is 100% correct as long as 1 SBP-2
target LU maps to 1 Scsi_Host.

The lifetime of the Scsi_Host would only be longer than that of the LU
if all LUs (or all LUs at the same initiator port) would be added beneath
the same instance of Scsi_Host. Then the lifetime of the Scsi_Host would
be that of the fw-sbp2 driver, or that of fw-sbp2's representation of a
FireWire bus.

However, I'll see how it turns out in your update patch.
--
Stefan Richter
-=====-=-=== -=-= -=--=