Re: [PATCH] SAS transport class

Christoph Hellwig
2005-09-06T14:01:07Z

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.