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