Re: [PATCH 5/20] libata: implement several LLD init helpers

[Brian King <brking@us.ibm.com>]

Tejun Heo wrote:
> Hello, Brian.
> 
> Brian King wrote:
>> Tejun Heo wrote:
>>
>>> +static void __ata_host_init_pinfo(struct ata_host *host,
>>> +				  const struct ata_port_info **pinfo,
>>> +				  int n_ports, int pi_is_ar)
>>> +{
>>> +	int i;
>>> +
>>> +	if (host->private_data == NULL)
>>> +		host->private_data = pinfo[0]->private_data;
>>> +	host->ops = pinfo[0]->port_ops;
>>> +
>>> +	for (i = 0; i < host->n_ports; i++) {
>>> +		struct ata_port *ap = host->ports[i];
>>> +		const struct ata_port_info *pi;
>>> +
>>> +		if (pi_is_ar)
>>> +			pi = pinfo[i];
>>> +		else
>>> +			pi = pinfo[0];
>>> +
>>
>>> +		ap->pio_mask = pi->pio_mask;
>>> +		ap->mwdma_mask = pi->mwdma_mask;
>>> +		ap->udma_mask = pi->udma_mask;
>>> +		ap->flags |= pi->flags;
>>> +		ap->ops = pi->port_ops;
>> Could this bit be moved into ata_port_init and possibly change the
>> parameter list of ata_port_init to something like:
>>
>> void ata_port_init(struct ata_port *ap, struct ata_host *host,
>> 		   const struct ata_port_info *pinfo, unsigned int port_no)
>>
>> Then we could get rid of the struct ata_probe_ent stuff completely.
> 
> Hmmm... ata_port_init() is still exported only to support ata_sas_* 
> helpers.  Once the SAS helpers are converted to use new model, this 
> function will be made static or merged into ata_host_alloc().  In the 
> new init model, initialization is done by...
> 
> 1. allocate the host and associated ports with ata_host_alloc().  If 
> port number cannot be determined at this point.  ports can be added 
> later.  The returned ata_host has only the basic fields, which assume 
> the same initial values regardless of the user, initialized.
> 
> 2. initialize ata_host and associated ports appropriately.  During this 
> ata_host_add_ports() can be used to add ports if host has no ports yet.
> 
> 3. attach the initialized ata_host using ata_host_attach().

Are you suggesting SAS would follow this host initialization model as well?
ata_host_attach currently does not work for SAS since it calls scsi_add_host
for each ata port, but that is easily fixed. I think the broader question is
whether or not SAS should be calling ata_host_attach to probe all "ports" on
the SAS adapter. Since for SAS, the concept of a SATA "port" is really a
virtual and very dynamic thing, I'm inclined to think SAS users calling
ata_host_attach to scan all "ports" might not be the best approach.

Brian

-- 
Brian King
eServer Storage I/O
IBM Linux Technology Center