Re: [PATCH v3 03/23] ata: libata-scsi: link ata port and scsi device

[Damien Le Moal <dlemoal@kernel.org>]

On 2023/09/19 6:21, Niklas Cassel wrote:
>> diff --git a/drivers/ata/pata_macio.c b/drivers/ata/pata_macio.c
>> index 17f6ccee53c7..32968b4cf8e4 100644
>> --- a/drivers/ata/pata_macio.c
>> +++ b/drivers/ata/pata_macio.c
>> @@ -918,6 +918,7 @@ static const struct scsi_host_template pata_macio_sht = {
>>  	 * use 64K minus 256
>>  	 */
>>  	.max_segment_size	= MAX_DBDMA_SEG,
>> +	.slave_alloc		= ata_scsi_slave_alloc,
>>  	.slave_configure	= pata_macio_slave_config,
>>  	.sdev_groups		= ata_common_sdev_groups,
>>  	.can_queue		= ATA_DEF_QUEUE,
>> diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
>> index d105db5c7d81..353ac7b2f14a 100644
>> --- a/drivers/ata/sata_mv.c
>> +++ b/drivers/ata/sata_mv.c
>> @@ -673,6 +673,7 @@ static const struct scsi_host_template mv6_sht = {
>>  	.sdev_groups		= ata_ncq_sdev_groups,
>>  	.change_queue_depth	= ata_scsi_change_queue_depth,
>>  	.tag_alloc_policy	= BLK_TAG_ALLOC_RR,
>> +	.slave_alloc		= ata_scsi_slave_alloc,
> 
> It seems wrong to add .slave_alloc to all different ata_port_operations structs.
> The .slave_configure is added on all different ata_port_operations structs,
> because the callback can be different for different drivers.
> 
> However, adding the device link is done in .ata_scsi_slave_alloc,
> removing the device link is done in .ata_scsi_slave_destroy.
> 
> Thus, I suggest that we only add the
> .slave_alloc = ata_scsi_slave_alloc,
> 
> to __ATA_BASE_SHT() in libata.h, which is currently the only
> place which has .slave_destroy defined.

Good point. I will make this change.


-- 
Damien Le Moal
Western Digital Research