On Tue, Sep 06, 2005 at 09:40:43AM -0400, James.Smart@xxxxxxxxxx wrote: > > + parent = sas_port_lookup(grandparent, port); > > + if (!parent) > > + return NULL; > > So this is odd... grandparent ? why pass in port # (you should > know the object already)? why is channel skipping over / not > associated with the local port ? I didn't really want to need the drivers to keep around arrays or lists of sas_port objects. > What I would have expected is : > > - sas_port_alloc() to either: > a) also have a "channel" argument, which gets saved in the local > port. > -or- > b) The port # is actually used as the channel #. I'm probably going to be with b). It's on my TODO list for the same time when I'm switching over to not pass in a target ID anymore in favour of allocating one internal to the transport class. > - sas_rport_alloc() becomes: > sas_rport_alloc(struct device *parent, uint id) > Where parent is the local port. You can still obtain the shost > via dev_to_shost(parent), and you can kill all that port searching > logic. Actually the id will also go away. > I'll point out, but not delve into - that you've avoided the target > id bindings again, thus forcing the lldd's/adapters to maintain tables > of port#/SASaddr to target id (if they care, and I believe enterprise > storage will care!). I can understand if you're going to eventually > insert a generic abstraction layer for scsi's b/c/t/l addressing > scheme, but since it's not here today, I expect you to hold SAS to the > same "code sharing" level as FC. Expect some updates in that area with the next code drop. > Also, to deal with cable pulls, SAS reconfiguration and rescanning, > I expect that you will need to add in the block/unblock functionality > that the FC transport has. [Note: we've seen an issue in the current > FC design, so expect a transport update]. yes, this will be added. Primary goal for this first stage of work is to support a basic topology representation. Stage two will be support for all kinds of runtime-events, aka addition of new devices, remove of devices including the unplug timer bits, and stage3 will support for a full managment interface. I hope parallel to that or shortly after Adaptec or someone else will provide support for full host-base SAS discovery aswell. > > +void sas_rport_add(struct sas_rport *rport) > > +{ > > .... > > + if (identify->device_type == SAS_END_DEVICE && > > + (identify->target_port_protocols & > > + (SAS_PROTOCOL_SSP|SAS_PROTOCOL_STP|SAS_PROTOCOL_SATA))) > > + scsi_scan_target(&rport->dev, rport->channel, > > + rport->id, ~0, 0); > > I highly recommend the use of workqueues for the scan function as we > did in the fc transport. This will happen in stage two, as part of the support for run-time addition of devices. - : send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html