Re: [PATCH] SAS transport class

[Christoph Hellwig <hch@lst.de>]

On Tue, Sep 06, 2005 at 09:40:43AM -0400, James.Smart@Emulex.Com 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.