[RFC,v3,16/22] ata: libata-scsi: Allocate sdev early in port probe

  Currently the per-ata device sdev is allocated as part of the scsi target
scan, which is after the ata port probe.

However it is useful to have the sdev available in the port probe. As an
example of an advantage, if the request queue is available in the probe
(which it would be if the sdev is available), then it is possible to use
a SCSI cmnd for ATA internal commands. The benefit of this is then we can
put the ATA qc structure in the SCSI cmnd private data. It will also be
useful if we want to send ATA internal commands as requests.

Export scsi_target_reap() so that it can be used to put the extra
reference we get when allocating the sdev.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/ata/libata-eh.c   |  1 +
 drivers/ata/libata-scsi.c | 23 +++++++++--------------
 drivers/scsi/scsi_scan.c  |  1 +
 3 files changed, 11 insertions(+), 14 deletions(-)
  

On 10/25/22 19:18, John Garry wrote:
> Currently the per-ata device sdev is allocated as part of the scsi target
> scan, which is after the ata port probe.
> 
> However it is useful to have the sdev available in the port probe. As an
> example of an advantage, if the request queue is available in the probe
> (which it would be if the sdev is available), then it is possible to use
> a SCSI cmnd for ATA internal commands. The benefit of this is then we can
> put the ATA qc structure in the SCSI cmnd private data. It will also be
> useful if we want to send ATA internal commands as requests.
> 
> Export scsi_target_reap() so that it can be used to put the extra
> reference we get when allocating the sdev.
> 
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
>  drivers/ata/libata-eh.c   |  1 +
>  drivers/ata/libata-scsi.c | 23 +++++++++--------------
>  drivers/scsi/scsi_scan.c  |  1 +
>  3 files changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
> index 08e11bc312c2..1ed5b1b64792 100644
> --- a/drivers/ata/libata-eh.c
> +++ b/drivers/ata/libata-eh.c
> @@ -3446,6 +3446,7 @@ static int ata_eh_schedule_probe(struct ata_device *dev)
>  
>  	ata_eh_detach_dev(dev);
>  	ata_dev_init(dev);
> +	ata_scsi_setup_sdev(dev);
>  	ehc->did_probe_mask |= (1 << dev->devno);
>  	ehc->i.action |= ATA_EH_RESET;
>  	ehc->saved_xfer_mode[dev->devno] = 0;
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index efdba852e363..476e0ef4bd29 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -1109,7 +1109,12 @@ int ata_scsi_dev_config(struct scsi_device *sdev, struct ata_device *dev)
>  	if (dev->flags & ATA_DFLAG_TRUSTED)
>  		sdev->security_supported = 1;
>  
> -	dev->sdev = sdev;
> +	/*
> +	 * Put extra reference which we get when allocating the starget
> +	 * initially
> +	 */
> +	scsi_target_reap(scsi_target(sdev));
> +
>  	return 0;
>  }
>  
> @@ -4289,26 +4294,16 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
>   repeat:
>  	ata_for_each_link(link, ap, EDGE) {
>  		ata_for_each_dev(dev, link, ENABLED) {
> -			struct scsi_device *sdev;
> +			struct Scsi_Host *shost = ap->scsi_host;
>  			int channel = 0, id = 0;
>  
> -			if (dev->sdev)
> -				continue;
> -
>  			if (ata_is_host_link(link))
>  				id = dev->devno;
>  			else
>  				channel = link->pmp;
>  
> -			sdev = __scsi_add_device(ap->scsi_host, channel, id, 0,
> -						 NULL);
> -			if (!IS_ERR(sdev)) {
> -				dev->sdev = sdev;
> -				ata_scsi_assign_ofnode(dev, ap);

Is there something equivalent to what this function does inside
scsi_scan_target() ? I had a quick look but did not see anything...

> -				scsi_device_put(sdev);
> -			} else {
> -				dev->sdev = NULL;
> -			}
> +			scsi_scan_target(&shost->shost_gendev, channel, id,
> +					 0, SCSI_SCAN_INITIAL);
>  		}
>  	}
>  
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index b795c138f2c1..da7bc14b030c 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -598,6 +598,7 @@ void scsi_target_reap(struct scsi_target *starget)
>  	BUG_ON(starget->state == STARGET_DEL);
>  	scsi_target_reap_ref_put(starget);
>  }
> +EXPORT_SYMBOL_GPL(scsi_target_reap);
>  
>  /**
>   * scsi_sanitize_inquiry_string - remove non-graphical chars from an
  

On 10/27/22 03:34, Damien Le Moal wrote:
> On 10/25/22 19:18, John Garry wrote:
>> Currently the per-ata device sdev is allocated as part of the scsi target
>> scan, which is after the ata port probe.
>>
>> However it is useful to have the sdev available in the port probe. As an
>> example of an advantage, if the request queue is available in the probe
>> (which it would be if the sdev is available), then it is possible to use
>> a SCSI cmnd for ATA internal commands. The benefit of this is then we can
>> put the ATA qc structure in the SCSI cmnd private data. It will also be
>> useful if we want to send ATA internal commands as requests.
>>
>> Export scsi_target_reap() so that it can be used to put the extra
>> reference we get when allocating the sdev.
>>
>> Signed-off-by: John Garry <john.garry@huawei.com>
>> ---
>>   drivers/ata/libata-eh.c   |  1 +
>>   drivers/ata/libata-scsi.c | 23 +++++++++--------------
>>   drivers/scsi/scsi_scan.c  |  1 +
>>   3 files changed, 11 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
>> index 08e11bc312c2..1ed5b1b64792 100644
>> --- a/drivers/ata/libata-eh.c
>> +++ b/drivers/ata/libata-eh.c
>> @@ -3446,6 +3446,7 @@ static int ata_eh_schedule_probe(struct ata_device *dev)
>>   
>>   	ata_eh_detach_dev(dev);
>>   	ata_dev_init(dev);
>> +	ata_scsi_setup_sdev(dev);
>>   	ehc->did_probe_mask |= (1 << dev->devno);
>>   	ehc->i.action |= ATA_EH_RESET;
>>   	ehc->saved_xfer_mode[dev->devno] = 0;
>> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
>> index efdba852e363..476e0ef4bd29 100644
>> --- a/drivers/ata/libata-scsi.c
>> +++ b/drivers/ata/libata-scsi.c
>> @@ -1109,7 +1109,12 @@ int ata_scsi_dev_config(struct scsi_device *sdev, struct ata_device *dev)
>>   	if (dev->flags & ATA_DFLAG_TRUSTED)
>>   		sdev->security_supported = 1;
>>   
>> -	dev->sdev = sdev;
>> +	/*
>> +	 * Put extra reference which we get when allocating the starget
>> +	 * initially
>> +	 */
>> +	scsi_target_reap(scsi_target(sdev));
>> +
>>   	return 0;
>>   }
>>   
>> @@ -4289,26 +4294,16 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
>>    repeat:
>>   	ata_for_each_link(link, ap, EDGE) {
>>   		ata_for_each_dev(dev, link, ENABLED) {
>> -			struct scsi_device *sdev;
>> +			struct Scsi_Host *shost = ap->scsi_host;
>>   			int channel = 0, id = 0;
>>   
>> -			if (dev->sdev)
>> -				continue;
>> -
>>   			if (ata_is_host_link(link))
>>   				id = dev->devno;
>>   			else
>>   				channel = link->pmp;
>>   
>> -			sdev = __scsi_add_device(ap->scsi_host, channel, id, 0,
>> -						 NULL);
>> -			if (!IS_ERR(sdev)) {
>> -				dev->sdev = sdev;
>> -				ata_scsi_assign_ofnode(dev, ap);
> 
> Is there something equivalent to what this function does inside
> scsi_scan_target() ? I had a quick look but did not see anything...
> 
Typically, the SCSI layer has two ways of scanning.
One it the old-style serial scanning (originating in the old SCSI 
parallel model):
The scanning code will blindly scan _all_ devices up to max_luns, and 
attach every device for which the scanning code returns 'OK'.
The other one is to issue REPORT_LUNS and scan all LUNs returned there.

Mapped to libata we would need to figure out a stable SCSI enumeration, 
given that we have PMP and slave devices to content with.
To my knowledge we have ATA ports, each can have either a 'master' and 
'slave' device, _or_ it be a PMP port when it can support up to 16 
devices, right?
Point being, master/slave and PMP are exclusive, right?
So we can make the master as LUN 0, and the slave as LUN 1.
And for PMP we can use each PMP address as LUN <pmp> + 1, and keeping 
the actual device as LUN 0.

I think we can figure out whether it's a master/slave device setup 
upfront, so we should be able to set max_luns to '2' for these devices.
For PMP-capable (or devices which _might_ be PMP capable), we could 
emulate the REPORT LUNS command, mapping on the PMP mechanism to figure 
out which devices are connected.

Would that work?

Cheers,

Hannes
  

On 10/27/22 17:11, Hannes Reinecke wrote:
> On 10/27/22 03:34, Damien Le Moal wrote:
>> On 10/25/22 19:18, John Garry wrote:
>>> Currently the per-ata device sdev is allocated as part of the scsi
>>> target
>>> scan, which is after the ata port probe.
>>>
>>> However it is useful to have the sdev available in the port probe. As an
>>> example of an advantage, if the request queue is available in the probe
>>> (which it would be if the sdev is available), then it is possible to use
>>> a SCSI cmnd for ATA internal commands. The benefit of this is then we
>>> can
>>> put the ATA qc structure in the SCSI cmnd private data. It will also be
>>> useful if we want to send ATA internal commands as requests.
>>>
>>> Export scsi_target_reap() so that it can be used to put the extra
>>> reference we get when allocating the sdev.
>>>
>>> Signed-off-by: John Garry <john.garry@huawei.com>
>>> ---
>>>   drivers/ata/libata-eh.c   |  1 +
>>>   drivers/ata/libata-scsi.c | 23 +++++++++--------------
>>>   drivers/scsi/scsi_scan.c  |  1 +
>>>   3 files changed, 11 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
>>> index 08e11bc312c2..1ed5b1b64792 100644
>>> --- a/drivers/ata/libata-eh.c
>>> +++ b/drivers/ata/libata-eh.c
>>> @@ -3446,6 +3446,7 @@ static int ata_eh_schedule_probe(struct
>>> ata_device *dev)
>>>         ata_eh_detach_dev(dev);
>>>       ata_dev_init(dev);
>>> +    ata_scsi_setup_sdev(dev);
>>>       ehc->did_probe_mask |= (1 << dev->devno);
>>>       ehc->i.action |= ATA_EH_RESET;
>>>       ehc->saved_xfer_mode[dev->devno] = 0;
>>> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
>>> index efdba852e363..476e0ef4bd29 100644
>>> --- a/drivers/ata/libata-scsi.c
>>> +++ b/drivers/ata/libata-scsi.c
>>> @@ -1109,7 +1109,12 @@ int ata_scsi_dev_config(struct scsi_device
>>> *sdev, struct ata_device *dev)
>>>       if (dev->flags & ATA_DFLAG_TRUSTED)
>>>           sdev->security_supported = 1;
>>>   -    dev->sdev = sdev;
>>> +    /*
>>> +     * Put extra reference which we get when allocating the starget
>>> +     * initially
>>> +     */
>>> +    scsi_target_reap(scsi_target(sdev));
>>> +
>>>       return 0;
>>>   }
>>>   @@ -4289,26 +4294,16 @@ void ata_scsi_scan_host(struct ata_port
>>> *ap, int sync)
>>>    repeat:
>>>       ata_for_each_link(link, ap, EDGE) {
>>>           ata_for_each_dev(dev, link, ENABLED) {
>>> -            struct scsi_device *sdev;
>>> +            struct Scsi_Host *shost = ap->scsi_host;
>>>               int channel = 0, id = 0;
>>>   -            if (dev->sdev)
>>> -                continue;
>>> -
>>>               if (ata_is_host_link(link))
>>>                   id = dev->devno;
>>>               else
>>>                   channel = link->pmp;
>>>   -            sdev = __scsi_add_device(ap->scsi_host, channel, id, 0,
>>> -                         NULL);
>>> -            if (!IS_ERR(sdev)) {
>>> -                dev->sdev = sdev;
>>> -                ata_scsi_assign_ofnode(dev, ap);
>>
>> Is there something equivalent to what this function does inside
>> scsi_scan_target() ? I had a quick look but did not see anything...
>>
> Typically, the SCSI layer has two ways of scanning.
> One it the old-style serial scanning (originating in the old SCSI
> parallel model):
> The scanning code will blindly scan _all_ devices up to max_luns, and
> attach every device for which the scanning code returns 'OK'.
> The other one is to issue REPORT_LUNS and scan all LUNs returned there.
> 
> Mapped to libata we would need to figure out a stable SCSI enumeration,
> given that we have PMP and slave devices to content with.
> To my knowledge we have ATA ports, each can have either a 'master' and
> 'slave' device, _or_ it be a PMP port when it can support up to 16
> devices, right?

yes

> Point being, master/slave and PMP are exclusive, right?

Never heard of pmp with ide cable :)

> So we can make the master as LUN 0, and the slave as LUN 1.

Yes, but isn't that a little wrong ? That would assume that the ata port
is the device and the ata devices the luns of that device. But beside
the "link busy" stuff that needs to be dealt with, master and slave are
independent devices, unlike LUNs. No ?

> And for PMP we can use each PMP address as LUN <pmp> + 1, and keeping
> the actual device as LUN 0.
> 
> I think we can figure out whether it's a master/slave device setup
> upfront, so we should be able to set max_luns to '2' for these devices.
> For PMP-capable (or devices which _might_ be PMP capable), we could
> emulate the REPORT LUNS command, mapping on the PMP mechanism to figure
> out which devices are connected.
> 
> Would that work?
> 
> Cheers,
> 
> Hannes
  

On 10/27/22 11:16, Damien Le Moal wrote:
> On 10/27/22 17:11, Hannes Reinecke wrote:
>> On 10/27/22 03:34, Damien Le Moal wrote:
>>> On 10/25/22 19:18, John Garry wrote:
>>>> Currently the per-ata device sdev is allocated as part of the scsi
>>>> target
>>>> scan, which is after the ata port probe.
>>>>
>>>> However it is useful to have the sdev available in the port probe. As an
>>>> example of an advantage, if the request queue is available in the probe
>>>> (which it would be if the sdev is available), then it is possible to use
>>>> a SCSI cmnd for ATA internal commands. The benefit of this is then we
>>>> can
>>>> put the ATA qc structure in the SCSI cmnd private data. It will also be
>>>> useful if we want to send ATA internal commands as requests.
>>>>
>>>> Export scsi_target_reap() so that it can be used to put the extra
>>>> reference we get when allocating the sdev.
>>>>
>>>> Signed-off-by: John Garry <john.garry@huawei.com>
>>>> ---
>>>>    drivers/ata/libata-eh.c   |  1 +
>>>>    drivers/ata/libata-scsi.c | 23 +++++++++--------------
>>>>    drivers/scsi/scsi_scan.c  |  1 +
>>>>    3 files changed, 11 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
>>>> index 08e11bc312c2..1ed5b1b64792 100644
>>>> --- a/drivers/ata/libata-eh.c
>>>> +++ b/drivers/ata/libata-eh.c
>>>> @@ -3446,6 +3446,7 @@ static int ata_eh_schedule_probe(struct
>>>> ata_device *dev)
>>>>          ata_eh_detach_dev(dev);
>>>>        ata_dev_init(dev);
>>>> +    ata_scsi_setup_sdev(dev);
>>>>        ehc->did_probe_mask |= (1 << dev->devno);
>>>>        ehc->i.action |= ATA_EH_RESET;
>>>>        ehc->saved_xfer_mode[dev->devno] = 0;
>>>> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
>>>> index efdba852e363..476e0ef4bd29 100644
>>>> --- a/drivers/ata/libata-scsi.c
>>>> +++ b/drivers/ata/libata-scsi.c
>>>> @@ -1109,7 +1109,12 @@ int ata_scsi_dev_config(struct scsi_device
>>>> *sdev, struct ata_device *dev)
>>>>        if (dev->flags & ATA_DFLAG_TRUSTED)
>>>>            sdev->security_supported = 1;
>>>>    -    dev->sdev = sdev;
>>>> +    /*
>>>> +     * Put extra reference which we get when allocating the starget
>>>> +     * initially
>>>> +     */
>>>> +    scsi_target_reap(scsi_target(sdev));
>>>> +
>>>>        return 0;
>>>>    }
>>>>    @@ -4289,26 +4294,16 @@ void ata_scsi_scan_host(struct ata_port
>>>> *ap, int sync)
>>>>     repeat:
>>>>        ata_for_each_link(link, ap, EDGE) {
>>>>            ata_for_each_dev(dev, link, ENABLED) {
>>>> -            struct scsi_device *sdev;
>>>> +            struct Scsi_Host *shost = ap->scsi_host;
>>>>                int channel = 0, id = 0;
>>>>    -            if (dev->sdev)
>>>> -                continue;
>>>> -
>>>>                if (ata_is_host_link(link))
>>>>                    id = dev->devno;
>>>>                else
>>>>                    channel = link->pmp;
>>>>    -            sdev = __scsi_add_device(ap->scsi_host, channel, id, 0,
>>>> -                         NULL);
>>>> -            if (!IS_ERR(sdev)) {
>>>> -                dev->sdev = sdev;
>>>> -                ata_scsi_assign_ofnode(dev, ap);
>>>
>>> Is there something equivalent to what this function does inside
>>> scsi_scan_target() ? I had a quick look but did not see anything...
>>>
>> Typically, the SCSI layer has two ways of scanning.
>> One it the old-style serial scanning (originating in the old SCSI
>> parallel model):
>> The scanning code will blindly scan _all_ devices up to max_luns, and
>> attach every device for which the scanning code returns 'OK'.
>> The other one is to issue REPORT_LUNS and scan all LUNs returned there.
>>
>> Mapped to libata we would need to figure out a stable SCSI enumeration,
>> given that we have PMP and slave devices to content with.
>> To my knowledge we have ATA ports, each can have either a 'master' and
>> 'slave' device, _or_ it be a PMP port when it can support up to 16
>> devices, right?
> 
> yes
> 
>> Point being, master/slave and PMP are exclusive, right?
> 
> Never heard of pmp with ide cable :)
> 
See?

>> So we can make the master as LUN 0, and the slave as LUN 1.
> 
> Yes, but isn't that a little wrong ? That would assume that the ata port
> is the device and the ata devices the luns of that device. But beside
> the "link busy" stuff that needs to be dealt with, master and slave are
> independent devices, unlike LUNs. No ?
> Well; technically, yes.

But we already enumerate the ata ports (which is typically done by the 
hardware/PCI layer etc), and if we were try to model slave devices as 
independent ports we would either have to insert a numbering (awkward) 
or add a number at the en (even more awkward).

And the one key takeaway from the 'multiple actuators' discussion is 
that LUNs _are_ independent (cf all the hoops they had to jump through 
to define a command spanning several LUNs ...)(which, incidentally, we 
could leverage here, too ...), and the target port really only serves as 
an enumeration thingie to address the LUNs.

So we _could_ map the master device on LUN 0 and the slave device on LUN 
1 with no loss of functionality, _but_ enable a consistent SCSI enumeration.

Cheers,

Hannes
  

>>>>>    @@ -4289,26 +4294,16 @@ void ata_scsi_scan_host(struct ata_port
>>>>> *ap, int sync)
>>>>>     repeat:

I've been trying to follow this thread, below, but got a bit lost ....

>>>>>        ata_for_each_link(link, ap, EDGE) {
>>>>>            ata_for_each_dev(dev, link, ENABLED) {
>>>>> -            struct scsi_device *sdev;
>>>>> +            struct Scsi_Host *shost = ap->scsi_host;
>>>>>                int channel = 0, id = 0;
>>>>>    -            if (dev->sdev)
>>>>> -                continue;
>>>>> -
>>>>>                if (ata_is_host_link(link))
>>>>>                    id = dev->devno;
>>>>>                else
>>>>>                    channel = link->pmp;
>>>>>    -            sdev = __scsi_add_device(ap->scsi_host, channel, 
>>>>> id, 0,
>>>>> -                         NULL);
>>>>> -            if (!IS_ERR(sdev)) {
>>>>> -                dev->sdev = sdev;
>>>>> -                ata_scsi_assign_ofnode(dev, ap);
>>>>
>>>> Is there something equivalent to what this function does inside
>>>> scsi_scan_target() ? I had a quick look but did not see anything...
>>>>

So are we discussing below whether we can have fixed channel, id, lun 
per ATA sdev per shost? If so, I don't think it would work as libsas 
uses dynamic target ids per host.

>>> Typically, the SCSI layer has two ways of scanning.
>>> One it the old-style serial scanning (originating in the old SCSI
>>> parallel model):
>>> The scanning code will blindly scan _all_ devices up to max_luns, and
>>> attach every device for which the scanning code returns 'OK'.
>>> The other one is to issue REPORT_LUNS and scan all LUNs returned there.
>>>
>>> Mapped to libata we would need to figure out a stable SCSI enumeration,
>>> given that we have PMP and slave devices to content with.
>>> To my knowledge we have ATA ports, each can have either a 'master' and
>>> 'slave' device, _or_ it be a PMP port when it can support up to 16
>>> devices, right?
>>
>> yes
>>
>>> Point being, master/slave and PMP are exclusive, right?
>>
>> Never heard of pmp with ide cable :)
>>
> See?
> 
>>> So we can make the master as LUN 0, and the slave as LUN 1.
>>
>> Yes, but isn't that a little wrong ? That would assume that the ata port
>> is the device and the ata devices the luns of that device. But beside
>> the "link busy" stuff that needs to be dealt with, master and slave are
>> independent devices, unlike LUNs. No ?
>> Well; technically, yes.
> 
> But we already enumerate the ata ports (which is typically done by the 
> hardware/PCI layer etc), and if we were try to model slave devices as 
> independent ports we would either have to insert a numbering (awkward) 
> or add a number at the en (even more awkward).
> 
> And the one key takeaway from the 'multiple actuators' discussion is 
> that LUNs _are_ independent (cf all the hoops they had to jump through 
> to define a command spanning several LUNs ...)(which, incidentally, we 
> could leverage here, too ...), and the target port really only serves as 
> an enumeration thingie to address the LUNs.
> 
> So we _could_ map the master device on LUN 0 and the slave device on LUN 
> 1 with no loss of functionality, _but_ enable a consistent SCSI enumeration

Thanks,
John
  

On 11/7/22 11:09, John Garry wrote:
> 
>>>>>>    @@ -4289,26 +4294,16 @@ void ata_scsi_scan_host(struct ata_port
>>>>>> *ap, int sync)
>>>>>>     repeat:
> 
> I've been trying to follow this thread, below, but got a bit lost ....
> 
>>>>>>        ata_for_each_link(link, ap, EDGE) {
>>>>>>            ata_for_each_dev(dev, link, ENABLED) {
>>>>>> -            struct scsi_device *sdev;
>>>>>> +            struct Scsi_Host *shost = ap->scsi_host;
>>>>>>                int channel = 0, id = 0;
>>>>>>    -            if (dev->sdev)
>>>>>> -                continue;
>>>>>> -
>>>>>>                if (ata_is_host_link(link))
>>>>>>                    id = dev->devno;
>>>>>>                else
>>>>>>                    channel = link->pmp;
>>>>>>    -            sdev = __scsi_add_device(ap->scsi_host, channel, 
>>>>>> id, 0,
>>>>>> -                         NULL);
>>>>>> -            if (!IS_ERR(sdev)) {
>>>>>> -                dev->sdev = sdev;
>>>>>> -                ata_scsi_assign_ofnode(dev, ap);
>>>>>
>>>>> Is there something equivalent to what this function does inside
>>>>> scsi_scan_target() ? I had a quick look but did not see anything...
>>>>>
> 
> So are we discussing below whether we can have fixed channel, id, lun 
> per ATA sdev per shost? If so, I don't think it would work as libsas 
> uses dynamic target ids per host.
> 
Not static. target port enumeration would still be left to the driver / 
transport, so it can do whatever it wants here.
Idea was to match the 'ata_port' structure to the scsi target port; 
well, not exactly 'match' but have a 1:1 relationship between them.
_And_ to have a defined way on how the devices are enumerated; PATA 
drives would always have LUN 0 and LUN 1(for the slave), and everything 
else would use REPORT LUNs. For which we need an emulation for libata, 
but that should be trivially to implement.

With that we would know exactly how many scsi target ports we'll have, 
and can create them based on hardware details before probing starts.
Whether or not a _device_ is attached to that port is being determined 
during scanning via the 'slave_alloc' or 'slave_configure' callbacks, so 
we still would be able to blank out any not implemented or not connected 
ports.
_But_ it would align better with the SCSI layer, such that we can work 
on integrating scanning and resetting, and make things like 
'sd_revalidate' actually work for libata.

And for libsas it might be worthwhile to check the 'scan_start' and 
'scan_finished' callbacks; that seems to be the ticket here.

Cheers,

Hannes