2024-03-05 12:25:51

by John Garry

[permalink] [raw]
Subject: [PATCH 0/6] Add LIBSAS_SHT_BASE for libsas

There is much duplication in the scsi_host_template structure for the
drivers which use libsas.

Similar to how a standard template is used in libata with __ATA_BASE_SHT,
create a standard template in LIBSAS_SHT_BASE.

Based on following:
9f3dbcb5632d (origin/queue, origin/6.9/scsi-queue, libsas-6.8) scsi:
csiostor: Avoid function pointer casts

plus this:
https://lore.kernel.org/linux-scsi/[email protected]/T/#t

John Garry (6):
scsi: libsas: Add LIBSAS_SHT_BASE
scsi: pm8001: Use LIBSAS_SHT_BASE
scsi: hisi_sas: Use LIBSAS_SHT_BASE_NO_SLAVE_INIT
scsi: aic94xx: Use LIBSAS_SHT_BASE
scsi: mvsas: Use LIBSAS_SHT_BASE
scsi: isci: Use LIBSAS_SHT_BASE

drivers/scsi/aic94xx/aic94xx_init.c | 21 ++----------------
drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 18 +---------------
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 18 +---------------
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 18 +---------------
drivers/scsi/isci/init.c | 22 +------------------
drivers/scsi/mvsas/mv_init.c | 19 +---------------
drivers/scsi/pm8001/pm8001_init.c | 20 +----------------
include/scsi/libsas.h | 30 ++++++++++++++++++++++++++
8 files changed, 38 insertions(+), 128 deletions(-)

--
2.31.1



2024-03-05 12:27:14

by John Garry

[permalink] [raw]
Subject: [PATCH 3/6] scsi: hisi_sas: Use LIBSAS_SHT_BASE_NO_SLAVE_INIT

Use standard template for scsi_host_template structure to reduce
duplication.

Signed-off-by: John Garry <[email protected]>
---
drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 18 +-----------------
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 18 +-----------------
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 18 +-----------------
3 files changed, 3 insertions(+), 51 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
index 3c555579f9a1..161feae3acab 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
@@ -1735,28 +1735,12 @@ static struct attribute *host_v1_hw_attrs[] = {
ATTRIBUTE_GROUPS(host_v1_hw);

static const struct scsi_host_template sht_v1_hw = {
- .name = DRV_NAME,
- .proc_name = DRV_NAME,
- .module = THIS_MODULE,
- .queuecommand = sas_queuecommand,
- .dma_need_drain = ata_scsi_dma_need_drain,
- .target_alloc = sas_target_alloc,
+ LIBSAS_SHT_BASE_NO_SLAVE_INIT
.slave_configure = hisi_sas_slave_configure,
.scan_finished = hisi_sas_scan_finished,
.scan_start = hisi_sas_scan_start,
- .change_queue_depth = sas_change_queue_depth,
- .bios_param = sas_bios_param,
- .this_id = -1,
.sg_tablesize = HISI_SAS_SGE_PAGE_CNT,
- .max_sectors = SCSI_DEFAULT_MAX_SECTORS,
- .eh_device_reset_handler = sas_eh_device_reset_handler,
- .eh_target_reset_handler = sas_eh_target_reset_handler,
.slave_alloc = hisi_sas_slave_alloc,
- .target_destroy = sas_target_destroy,
- .ioctl = sas_ioctl,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = sas_ioctl,
-#endif
.shost_groups = host_v1_hw_groups,
.host_reset = hisi_sas_host_reset,
};
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index b5d379ebe05d..d89e97e8f5c2 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -3567,28 +3567,12 @@ static void map_queues_v2_hw(struct Scsi_Host *shost)
}

static const struct scsi_host_template sht_v2_hw = {
- .name = DRV_NAME,
- .proc_name = DRV_NAME,
- .module = THIS_MODULE,
- .queuecommand = sas_queuecommand,
- .dma_need_drain = ata_scsi_dma_need_drain,
- .target_alloc = sas_target_alloc,
+ LIBSAS_SHT_BASE_NO_SLAVE_INIT
.slave_configure = hisi_sas_slave_configure,
.scan_finished = hisi_sas_scan_finished,
.scan_start = hisi_sas_scan_start,
- .change_queue_depth = sas_change_queue_depth,
- .bios_param = sas_bios_param,
- .this_id = -1,
.sg_tablesize = HISI_SAS_SGE_PAGE_CNT,
- .max_sectors = SCSI_DEFAULT_MAX_SECTORS,
- .eh_device_reset_handler = sas_eh_device_reset_handler,
- .eh_target_reset_handler = sas_eh_target_reset_handler,
.slave_alloc = hisi_sas_slave_alloc,
- .target_destroy = sas_target_destroy,
- .ioctl = sas_ioctl,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = sas_ioctl,
-#endif
.shost_groups = host_v2_hw_groups,
.sdev_groups = sdev_groups_v2_hw,
.host_reset = hisi_sas_host_reset,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index ebdfb7e7c88d..756660588a1e 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -3320,30 +3320,14 @@ static void hisi_sas_map_queues(struct Scsi_Host *shost)
}

static const struct scsi_host_template sht_v3_hw = {
- .name = DRV_NAME,
- .proc_name = DRV_NAME,
- .module = THIS_MODULE,
- .queuecommand = sas_queuecommand,
- .dma_need_drain = ata_scsi_dma_need_drain,
- .target_alloc = sas_target_alloc,
+ LIBSAS_SHT_BASE_NO_SLAVE_INIT
.slave_configure = slave_configure_v3_hw,
.scan_finished = hisi_sas_scan_finished,
.scan_start = hisi_sas_scan_start,
.map_queues = hisi_sas_map_queues,
- .change_queue_depth = sas_change_queue_depth,
- .bios_param = sas_bios_param,
- .this_id = -1,
.sg_tablesize = HISI_SAS_SGE_PAGE_CNT,
.sg_prot_tablesize = HISI_SAS_SGE_PAGE_CNT,
- .max_sectors = SCSI_DEFAULT_MAX_SECTORS,
- .eh_device_reset_handler = sas_eh_device_reset_handler,
- .eh_target_reset_handler = sas_eh_target_reset_handler,
.slave_alloc = hisi_sas_slave_alloc,
- .target_destroy = sas_target_destroy,
- .ioctl = sas_ioctl,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = sas_ioctl,
-#endif
.shost_groups = host_v3_hw_groups,
.sdev_groups = sdev_groups_v3_hw,
.tag_alloc_policy = BLK_TAG_ALLOC_RR,
--
2.31.1


2024-03-05 12:27:16

by John Garry

[permalink] [raw]
Subject: [PATCH 5/6] scsi: mvsas: Use LIBSAS_SHT_BASE

Use standard template for scsi_host_template structure to reduce
duplication.

Signed-off-by: John Garry <[email protected]>
---
drivers/scsi/mvsas/mv_init.c | 19 +------------------
1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
index f1090bb5f2c9..c792e4486e54 100644
--- a/drivers/scsi/mvsas/mv_init.c
+++ b/drivers/scsi/mvsas/mv_init.c
@@ -31,28 +31,11 @@ static const struct attribute_group *mvst_sdev_groups[];
#define SOC_SAS_NUM 2

static const struct scsi_host_template mvs_sht = {
- .module = THIS_MODULE,
- .name = DRV_NAME,
- .queuecommand = sas_queuecommand,
- .dma_need_drain = ata_scsi_dma_need_drain,
- .target_alloc = sas_target_alloc,
- .slave_configure = sas_slave_configure,
+ LIBSAS_SHT_BASE
.scan_finished = mvs_scan_finished,
.scan_start = mvs_scan_start,
- .change_queue_depth = sas_change_queue_depth,
- .bios_param = sas_bios_param,
.can_queue = 1,
- .this_id = -1,
.sg_tablesize = SG_ALL,
- .max_sectors = SCSI_DEFAULT_MAX_SECTORS,
- .eh_device_reset_handler = sas_eh_device_reset_handler,
- .eh_target_reset_handler = sas_eh_target_reset_handler,
- .slave_alloc = sas_slave_alloc,
- .target_destroy = sas_target_destroy,
- .ioctl = sas_ioctl,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = sas_ioctl,
-#endif
.shost_groups = mvst_host_groups,
.sdev_groups = mvst_sdev_groups,
.track_queue_depth = 1,
--
2.31.1


2024-03-05 12:27:25

by John Garry

[permalink] [raw]
Subject: [PATCH 6/6] scsi: isci: Use LIBSAS_SHT_BASE

Use standard template for scsi_host_template structure to reduce
duplication.

Signed-off-by: John Garry <[email protected]>
---
drivers/scsi/isci/init.c | 22 +---------------------
1 file changed, 1 insertion(+), 21 deletions(-)

diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
index d0a23ce4afba..49e64232def1 100644
--- a/drivers/scsi/isci/init.c
+++ b/drivers/scsi/isci/init.c
@@ -155,31 +155,11 @@ static const struct attribute_group *isci_sdev_groups[] = {
};

static const struct scsi_host_template isci_sht = {
-
- .module = THIS_MODULE,
- .name = DRV_NAME,
- .proc_name = DRV_NAME,
- .queuecommand = sas_queuecommand,
- .dma_need_drain = ata_scsi_dma_need_drain,
- .target_alloc = sas_target_alloc,
- .slave_configure = sas_slave_configure,
+ LIBSAS_SHT_BASE
.scan_finished = isci_host_scan_finished,
.scan_start = isci_host_start,
- .change_queue_depth = sas_change_queue_depth,
- .bios_param = sas_bios_param,
.can_queue = ISCI_CAN_QUEUE_VAL,
- .this_id = -1,
.sg_tablesize = SG_ALL,
- .max_sectors = SCSI_DEFAULT_MAX_SECTORS,
- .eh_abort_handler = sas_eh_abort_handler,
- .eh_device_reset_handler = sas_eh_device_reset_handler,
- .eh_target_reset_handler = sas_eh_target_reset_handler,
- .slave_alloc = sas_slave_alloc,
- .target_destroy = sas_target_destroy,
- .ioctl = sas_ioctl,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = sas_ioctl,
-#endif
.shost_groups = isci_host_groups,
.sdev_groups = isci_sdev_groups,
.track_queue_depth = 1,
--
2.31.1


2024-03-05 12:37:25

by John Garry

[permalink] [raw]
Subject: [PATCH 2/6] scsi: pm8001: Use LIBSAS_SHT_BASE

Use standard template for scsi_host_template structure to reduce
duplication.

Signed-off-by: John Garry <[email protected]>
---
drivers/scsi/pm8001/pm8001_init.c | 20 +-------------------
1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index e6b1108f6117..1e63cb6cd8e3 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -110,29 +110,11 @@ static void pm8001_map_queues(struct Scsi_Host *shost)
* The main structure which LLDD must register for scsi core.
*/
static const struct scsi_host_template pm8001_sht = {
- .module = THIS_MODULE,
- .name = DRV_NAME,
- .proc_name = DRV_NAME,
- .queuecommand = sas_queuecommand,
- .dma_need_drain = ata_scsi_dma_need_drain,
- .target_alloc = sas_target_alloc,
- .slave_configure = sas_slave_configure,
+ LIBSAS_SHT_BASE
.scan_finished = pm8001_scan_finished,
.scan_start = pm8001_scan_start,
- .change_queue_depth = sas_change_queue_depth,
- .bios_param = sas_bios_param,
.can_queue = 1,
- .this_id = -1,
.sg_tablesize = PM8001_MAX_DMA_SG,
- .max_sectors = SCSI_DEFAULT_MAX_SECTORS,
- .eh_device_reset_handler = sas_eh_device_reset_handler,
- .eh_target_reset_handler = sas_eh_target_reset_handler,
- .slave_alloc = sas_slave_alloc,
- .target_destroy = sas_target_destroy,
- .ioctl = sas_ioctl,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = sas_ioctl,
-#endif
.shost_groups = pm8001_host_groups,
.sdev_groups = pm8001_sdev_groups,
.track_queue_depth = 1,
--
2.31.1


2024-03-05 12:38:16

by John Garry

[permalink] [raw]
Subject: [PATCH 4/6] scsi: aic94xx: Use LIBSAS_SHT_BASE

Use standard template for scsi_host_template structure to reduce
duplication.

Signed-off-by: John Garry <[email protected]>
---
drivers/scsi/aic94xx/aic94xx_init.c | 21 ++-------------------
1 file changed, 2 insertions(+), 19 deletions(-)

diff --git a/drivers/scsi/aic94xx/aic94xx_init.c b/drivers/scsi/aic94xx/aic94xx_init.c
index ccccd0eb6275..538a5867e8ab 100644
--- a/drivers/scsi/aic94xx/aic94xx_init.c
+++ b/drivers/scsi/aic94xx/aic94xx_init.c
@@ -25,6 +25,7 @@

/* The format is "version.release.patchlevel" */
#define ASD_DRIVER_VERSION "1.0.3"
+#define DRV_NAME "aic94xx"

static int use_msi = 0;
module_param_named(use_msi, use_msi, int, S_IRUGO);
@@ -38,29 +39,11 @@ static void asd_scan_start(struct Scsi_Host *);
static const struct attribute_group *asd_sdev_groups[];

static const struct scsi_host_template aic94xx_sht = {
- .module = THIS_MODULE,
- /* .name is initialized */
- .name = "aic94xx",
- .queuecommand = sas_queuecommand,
- .dma_need_drain = ata_scsi_dma_need_drain,
- .target_alloc = sas_target_alloc,
- .slave_configure = sas_slave_configure,
+ LIBSAS_SHT_BASE
.scan_finished = asd_scan_finished,
.scan_start = asd_scan_start,
- .change_queue_depth = sas_change_queue_depth,
- .bios_param = sas_bios_param,
.can_queue = 1,
- .this_id = -1,
.sg_tablesize = SG_ALL,
- .max_sectors = SCSI_DEFAULT_MAX_SECTORS,
- .eh_device_reset_handler = sas_eh_device_reset_handler,
- .eh_target_reset_handler = sas_eh_target_reset_handler,
- .slave_alloc = sas_slave_alloc,
- .target_destroy = sas_target_destroy,
- .ioctl = sas_ioctl,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = sas_ioctl,
-#endif
.track_queue_depth = 1,
.sdev_groups = asd_sdev_groups,
};
--
2.31.1


2024-03-05 12:53:08

by Jason Yan

[permalink] [raw]
Subject: Re: [PATCH 2/6] scsi: pm8001: Use LIBSAS_SHT_BASE

On 2024/3/5 20:24, John Garry wrote:
> Use standard template for scsi_host_template structure to reduce
> duplication.
>
> Signed-off-by: John Garry<[email protected]>
> ---
> drivers/scsi/pm8001/pm8001_init.c | 20 +-------------------
> 1 file changed, 1 insertion(+), 19 deletions(-)

Reviewed-by: Jason Yan <[email protected]>

2024-03-05 12:54:56

by Jason Yan

[permalink] [raw]
Subject: Re: [PATCH 3/6] scsi: hisi_sas: Use LIBSAS_SHT_BASE_NO_SLAVE_INIT

On 2024/3/5 20:24, John Garry wrote:
> Use standard template for scsi_host_template structure to reduce
> duplication.
>
> Signed-off-by: John Garry<[email protected]>
> ---
> drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 18 +-----------------
> drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 18 +-----------------
> drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 18 +-----------------
> 3 files changed, 3 insertions(+), 51 deletions(-)

Reviewed-by: Jason Yan <[email protected]>

2024-03-05 13:09:45

by Jason Yan

[permalink] [raw]
Subject: Re: [PATCH 4/6] scsi: aic94xx: Use LIBSAS_SHT_BASE

On 2024/3/5 20:24, John Garry wrote:
> Use standard template for scsi_host_template structure to reduce
> duplication.
>
> Signed-off-by: John Garry<[email protected]>
> ---
> drivers/scsi/aic94xx/aic94xx_init.c | 21 ++-------------------
> 1 file changed, 2 insertions(+), 19 deletions(-)

Reviewed-by: Jason Yan <[email protected]>

2024-03-05 13:18:05

by Jason Yan

[permalink] [raw]
Subject: Re: [PATCH 2/6] scsi: pm8001: Use LIBSAS_SHT_BASE

On 2024/3/5 20:24, John Garry wrote:
> Use standard template for scsi_host_template structure to reduce
> duplication.
>
> Signed-off-by: John Garry <[email protected]>
> ---
> drivers/scsi/pm8001/pm8001_init.c | 20 +-------------------
> 1 file changed, 1 insertion(+), 19 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
> index e6b1108f6117..1e63cb6cd8e3 100644
> --- a/drivers/scsi/pm8001/pm8001_init.c
> +++ b/drivers/scsi/pm8001/pm8001_init.c
> @@ -110,29 +110,11 @@ static void pm8001_map_queues(struct Scsi_Host *shost)
> * The main structure which LLDD must register for scsi core.
> */
> static const struct scsi_host_template pm8001_sht = {
> - .module = THIS_MODULE,
> - .name = DRV_NAME,
> - .proc_name = DRV_NAME,
> - .queuecommand = sas_queuecommand,
> - .dma_need_drain = ata_scsi_dma_need_drain,
> - .target_alloc = sas_target_alloc,
> - .slave_configure = sas_slave_configure,
> + LIBSAS_SHT_BASE
> .scan_finished = pm8001_scan_finished,
> .scan_start = pm8001_scan_start,
> - .change_queue_depth = sas_change_queue_depth,
> - .bios_param = sas_bios_param,
> .can_queue = 1,
> - .this_id = -1,
> .sg_tablesize = PM8001_MAX_DMA_SG,
> - .max_sectors = SCSI_DEFAULT_MAX_SECTORS,
> - .eh_device_reset_handler = sas_eh_device_reset_handler,
> - .eh_target_reset_handler = sas_eh_target_reset_handler,
> - .slave_alloc = sas_slave_alloc,
> - .target_destroy = sas_target_destroy,
> - .ioctl = sas_ioctl,
> -#ifdef CONFIG_COMPAT
> - .compat_ioctl = sas_ioctl,
> -#endif
> .shost_groups = pm8001_host_groups,
> .sdev_groups = pm8001_sdev_groups,
> .track_queue_depth = 1,
>

Sorry, I missed that this driver doesn't have ->eh_abort_handler. Please
fix it.

Thanks,
Jason

2024-03-05 13:19:24

by Jason Yan

[permalink] [raw]
Subject: Re: [PATCH 3/6] scsi: hisi_sas: Use LIBSAS_SHT_BASE_NO_SLAVE_INIT

On 2024/3/5 20:24, John Garry wrote:
> Use standard template for scsi_host_template structure to reduce
> duplication.
>
> Signed-off-by: John Garry <[email protected]>
> ---
> drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 18 +-----------------
> drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 18 +-----------------
> drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 18 +-----------------
> 3 files changed, 3 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
> index 3c555579f9a1..161feae3acab 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
> @@ -1735,28 +1735,12 @@ static struct attribute *host_v1_hw_attrs[] = {
> ATTRIBUTE_GROUPS(host_v1_hw);
>
> static const struct scsi_host_template sht_v1_hw = {
> - .name = DRV_NAME,
> - .proc_name = DRV_NAME,
> - .module = THIS_MODULE,
> - .queuecommand = sas_queuecommand,
> - .dma_need_drain = ata_scsi_dma_need_drain,
> - .target_alloc = sas_target_alloc,
> + LIBSAS_SHT_BASE_NO_SLAVE_INIT
> .slave_configure = hisi_sas_slave_configure,
> .scan_finished = hisi_sas_scan_finished,
> .scan_start = hisi_sas_scan_start,
> - .change_queue_depth = sas_change_queue_depth,
> - .bios_param = sas_bios_param,
> - .this_id = -1,
> .sg_tablesize = HISI_SAS_SGE_PAGE_CNT,
> - .max_sectors = SCSI_DEFAULT_MAX_SECTORS,
> - .eh_device_reset_handler = sas_eh_device_reset_handler,
> - .eh_target_reset_handler = sas_eh_target_reset_handler,
> .slave_alloc = hisi_sas_slave_alloc,
> - .target_destroy = sas_target_destroy,
> - .ioctl = sas_ioctl,
> -#ifdef CONFIG_COMPAT
> - .compat_ioctl = sas_ioctl,
> -#endif
> .shost_groups = host_v1_hw_groups,
> .host_reset = hisi_sas_host_reset,
> };
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
> index b5d379ebe05d..d89e97e8f5c2 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
> @@ -3567,28 +3567,12 @@ static void map_queues_v2_hw(struct Scsi_Host *shost)
> }
>
> static const struct scsi_host_template sht_v2_hw = {
> - .name = DRV_NAME,
> - .proc_name = DRV_NAME,
> - .module = THIS_MODULE,
> - .queuecommand = sas_queuecommand,
> - .dma_need_drain = ata_scsi_dma_need_drain,
> - .target_alloc = sas_target_alloc,
> + LIBSAS_SHT_BASE_NO_SLAVE_INIT
> .slave_configure = hisi_sas_slave_configure,
> .scan_finished = hisi_sas_scan_finished,
> .scan_start = hisi_sas_scan_start,
> - .change_queue_depth = sas_change_queue_depth,
> - .bios_param = sas_bios_param,
> - .this_id = -1,
> .sg_tablesize = HISI_SAS_SGE_PAGE_CNT,
> - .max_sectors = SCSI_DEFAULT_MAX_SECTORS,
> - .eh_device_reset_handler = sas_eh_device_reset_handler,
> - .eh_target_reset_handler = sas_eh_target_reset_handler,
> .slave_alloc = hisi_sas_slave_alloc,
> - .target_destroy = sas_target_destroy,
> - .ioctl = sas_ioctl,
> -#ifdef CONFIG_COMPAT
> - .compat_ioctl = sas_ioctl,
> -#endif
> .shost_groups = host_v2_hw_groups,
> .sdev_groups = sdev_groups_v2_hw,
> .host_reset = hisi_sas_host_reset,
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> index ebdfb7e7c88d..756660588a1e 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> @@ -3320,30 +3320,14 @@ static void hisi_sas_map_queues(struct Scsi_Host *shost)
> }
>
> static const struct scsi_host_template sht_v3_hw = {
> - .name = DRV_NAME,
> - .proc_name = DRV_NAME,
> - .module = THIS_MODULE,
> - .queuecommand = sas_queuecommand,
> - .dma_need_drain = ata_scsi_dma_need_drain,
> - .target_alloc = sas_target_alloc,
> + LIBSAS_SHT_BASE_NO_SLAVE_INIT
> .slave_configure = slave_configure_v3_hw,
> .scan_finished = hisi_sas_scan_finished,
> .scan_start = hisi_sas_scan_start,
> .map_queues = hisi_sas_map_queues,
> - .change_queue_depth = sas_change_queue_depth,
> - .bios_param = sas_bios_param,
> - .this_id = -1,
> .sg_tablesize = HISI_SAS_SGE_PAGE_CNT,
> .sg_prot_tablesize = HISI_SAS_SGE_PAGE_CNT,
> - .max_sectors = SCSI_DEFAULT_MAX_SECTORS,
> - .eh_device_reset_handler = sas_eh_device_reset_handler,
> - .eh_target_reset_handler = sas_eh_target_reset_handler,
> .slave_alloc = hisi_sas_slave_alloc,
> - .target_destroy = sas_target_destroy,
> - .ioctl = sas_ioctl,
> -#ifdef CONFIG_COMPAT
> - .compat_ioctl = sas_ioctl,
> -#endif
> .shost_groups = host_v3_hw_groups,
> .sdev_groups = sdev_groups_v3_hw,
> .tag_alloc_policy = BLK_TAG_ALLOC_RR,
>

Doesn't hvae ->eh_abort_handler too.

Thanks,
Jason

2024-03-05 13:21:53

by Jason Yan

[permalink] [raw]
Subject: Re: [PATCH 6/6] scsi: isci: Use LIBSAS_SHT_BASE

On 2024/3/5 20:24, John Garry wrote:
> Use standard template for scsi_host_template structure to reduce
> duplication.
>
> Signed-off-by: John Garry <[email protected]>
> ---
> drivers/scsi/isci/init.c | 22 +---------------------
> 1 file changed, 1 insertion(+), 21 deletions(-)
>
> diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
> index d0a23ce4afba..49e64232def1 100644
> --- a/drivers/scsi/isci/init.c
> +++ b/drivers/scsi/isci/init.c
> @@ -155,31 +155,11 @@ static const struct attribute_group *isci_sdev_groups[] = {
> };
>
> static const struct scsi_host_template isci_sht = {
> -
> - .module = THIS_MODULE,
> - .name = DRV_NAME,
> - .proc_name = DRV_NAME,
> - .queuecommand = sas_queuecommand,
> - .dma_need_drain = ata_scsi_dma_need_drain,
> - .target_alloc = sas_target_alloc,
> - .slave_configure = sas_slave_configure,
> + LIBSAS_SHT_BASE
> .scan_finished = isci_host_scan_finished,
> .scan_start = isci_host_start,
> - .change_queue_depth = sas_change_queue_depth,
> - .bios_param = sas_bios_param,
> .can_queue = ISCI_CAN_QUEUE_VAL,
> - .this_id = -1,
> .sg_tablesize = SG_ALL,
> - .max_sectors = SCSI_DEFAULT_MAX_SECTORS,

max_sectors is not defined in LIBSAS_SHT_BASE.

Thanks,
Jaosn

> - .eh_abort_handler = sas_eh_abort_handler,
> - .eh_device_reset_handler = sas_eh_device_reset_handler,
> - .eh_target_reset_handler = sas_eh_target_reset_handler,
> - .slave_alloc = sas_slave_alloc,
> - .target_destroy = sas_target_destroy,
> - .ioctl = sas_ioctl,
> -#ifdef CONFIG_COMPAT
> - .compat_ioctl = sas_ioctl,
> -#endif
> .shost_groups = isci_host_groups,
> .sdev_groups = isci_sdev_groups,
> .track_queue_depth = 1,
>

2024-03-05 13:30:00

by John Garry

[permalink] [raw]
Subject: Re: [PATCH 6/6] scsi: isci: Use LIBSAS_SHT_BASE

On 05/03/2024 13:21, Jason Yan wrote:
>>       .sg_tablesize            = SG_ALL,
>> -    .max_sectors            = SCSI_DEFAULT_MAX_SECTORS,
>
> .max_sectors is not defined in LIBSAS_SHT_BASE.

Sure, but when not set, scsi_host_alloc() will default to
SCSI_DEFAULT_MAX_SECTORS automatically

Thanks,
John

2024-03-05 13:30:21

by John Garry

[permalink] [raw]
Subject: Re: [PATCH 5/6] scsi: mvsas: Use LIBSAS_SHT_BASE

On 05/03/2024 13:19, Jason Yan wrote:
>> -    .ioctl            = sas_ioctl,
>> -#ifdef CONFIG_COMPAT
>> -    .compat_ioctl        = sas_ioctl,
>> -#endif
>>       .shost_groups        = mvst_host_groups,
>>       .sdev_groups        = mvst_sdev_groups,
>>       .track_queue_depth    = 1,
>>
>
> Doesn't hvae ->eh_abort_handler too.

Is setting eh_abort_handler actually really ever required for libsas
drivers? We have sas_eh_abort_handler, so I assume so..

Thanks,
John

2024-03-05 13:37:10

by Jason Yan

[permalink] [raw]
Subject: Re: [PATCH 5/6] scsi: mvsas: Use LIBSAS_SHT_BASE

On 2024/3/5 20:24, John Garry wrote:
> Use standard template for scsi_host_template structure to reduce
> duplication.
>
> Signed-off-by: John Garry <[email protected]>
> ---
> drivers/scsi/mvsas/mv_init.c | 19 +------------------
> 1 file changed, 1 insertion(+), 18 deletions(-)
>
> diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
> index f1090bb5f2c9..c792e4486e54 100644
> --- a/drivers/scsi/mvsas/mv_init.c
> +++ b/drivers/scsi/mvsas/mv_init.c
> @@ -31,28 +31,11 @@ static const struct attribute_group *mvst_sdev_groups[];
> #define SOC_SAS_NUM 2
>
> static const struct scsi_host_template mvs_sht = {
> - .module = THIS_MODULE,
> - .name = DRV_NAME,
> - .queuecommand = sas_queuecommand,
> - .dma_need_drain = ata_scsi_dma_need_drain,
> - .target_alloc = sas_target_alloc,
> - .slave_configure = sas_slave_configure,
> + LIBSAS_SHT_BASE
> .scan_finished = mvs_scan_finished,
> .scan_start = mvs_scan_start,
> - .change_queue_depth = sas_change_queue_depth,
> - .bios_param = sas_bios_param,
> .can_queue = 1,
> - .this_id = -1,
> .sg_tablesize = SG_ALL,
> - .max_sectors = SCSI_DEFAULT_MAX_SECTORS,
> - .eh_device_reset_handler = sas_eh_device_reset_handler,
> - .eh_target_reset_handler = sas_eh_target_reset_handler,
> - .slave_alloc = sas_slave_alloc,
> - .target_destroy = sas_target_destroy,
> - .ioctl = sas_ioctl,
> -#ifdef CONFIG_COMPAT
> - .compat_ioctl = sas_ioctl,
> -#endif
> .shost_groups = mvst_host_groups,
> .sdev_groups = mvst_sdev_groups,
> .track_queue_depth = 1,
>

Doesn't hvae ->eh_abort_handler too.

Thanks,
Jason

2024-03-05 13:48:15

by Jason Yan

[permalink] [raw]
Subject: Re: [PATCH 5/6] scsi: mvsas: Use LIBSAS_SHT_BASE

On 2024/3/5 21:28, John Garry wrote:
> On 05/03/2024 13:19, Jason Yan wrote:
>>> -    .ioctl            = sas_ioctl,
>>> -#ifdef CONFIG_COMPAT
>>> -    .compat_ioctl        = sas_ioctl,
>>> -#endif
>>>       .shost_groups        = mvst_host_groups,
>>>       .sdev_groups        = mvst_sdev_groups,
>>>       .track_queue_depth    = 1,
>>>
>>
>> Doesn't hvae ->eh_abort_handler too.
>
> Is setting eh_abort_handler actually really ever required for libsas
> drivers? We have sas_eh_abort_handler, so I assume so..

For now among libsas drivers only isci has eh_abort_handler. But
LIBSAS_SHT_BASE is setting it by default. I think it's better to keep
the same as before for other four drivers.

Thanks,
Jason

>
> Thanks,
> John
> .

2024-03-05 14:02:40

by Jason Yan

[permalink] [raw]
Subject: Re: [PATCH 6/6] scsi: isci: Use LIBSAS_SHT_BASE

On 2024/3/5 21:27, John Garry wrote:
> On 05/03/2024 13:21, Jason Yan wrote:
>>>       .sg_tablesize            = SG_ALL,
>>> -    .max_sectors            = SCSI_DEFAULT_MAX_SECTORS,
>>
>> .max_sectors is not defined in LIBSAS_SHT_BASE.
>
> Sure, but when not set, scsi_host_alloc() will default to
> SCSI_DEFAULT_MAX_SECTORS automatically

OK, so it's fine to let it uninitialized.

Reviewed-by: Jason Yan <[email protected]>

>
> Thanks,
> John
> .

2024-03-05 15:43:02

by John Garry

[permalink] [raw]
Subject: Re: [PATCH 5/6] scsi: mvsas: Use LIBSAS_SHT_BASE

On 05/03/2024 13:47, Jason Yan wrote:
>>>
>>> Doesn't hvae ->eh_abort_handler too.
>>
>> Is setting eh_abort_handler actually really ever required for libsas
>> drivers? We have sas_eh_abort_handler, so I assume so..
>
> For now among libsas drivers only isci has eh_abort_handler. But
> LIBSAS_SHT_BASE is setting it by default. I think it's better to keep
> the same as before for other four drivers.

Sure, but I do wonder why it is required for isci, if at all - I'll check.

The motivation to not require it is a smaller and consistent API used by
all the libsas drivers.

Thanks,
John