Re: [PATCH v3 01/88] scsi: qla2xxx: Add 29xx series PCI device ID support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incomplete hardware initialization and capability detection for QLA29XX adapters.
- [Medium] Missing speed-capability logic modifications promised in the commit message.
--

--- Patch [1]: [PATCH v3 01/88] scsi: qla2xxx: Add 29xx series PCI device ID support ---
commit bb93cf91545563a12abad49c8f50d47644a2f103
Author: Manish Rangankar <mrangankar@marvell.com>
Date:   Tue Jul 7 05:53:08 2026 +0000

    scsi: qla2xxx: Add 29xx series PCI device ID support
    
    The QLA29xx is a new generation FC HBA that shares much of its
    architecture with the 27xx/28xx family.  Register the new PCI
    device IDs, wire up IS_QLA29XX() capability checks in the probe
    and ISP-flags paths,

>     and extend speed-capability logic so the driver correctly recognises and initialises 29xx adapters.

[Severity: Medium]
This isn't a bug, but the commit message states it extends speed-capability
logic. Are there changes missing from this patch? I don't see any
modifications related to speed capability or link rates in the diff.

>     
>     Signed-off-by: Manish Rangankar <mrangankar@marvell.com>
>     Signed-off-by: Nilesh Javali <njavali@marvell.com>
>     Reviewed-by: Hannes Reinecke <hare@kernel.org>

> diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
> index 5593ad7fad274..6337a056b1497 100644
> --- a/drivers/scsi/qla2xxx/qla_def.h
> +++ b/drivers/scsi/qla2xxx/qla_def.h

[ ... ]

> @@ -4372,9 +4386,10 @@ struct qla_hw_data {
>  				IS_QLA25XX(ha) || IS_QLA81XX(ha) || \
>  				IS_QLA82XX(ha) || IS_QLA83XX(ha) || \
>  				IS_QLA8044(ha) || IS_QLA27XX(ha) || \
> -				IS_QLA28XX(ha))
> +				IS_QLA28XX(ha) || IS_QLA29XX(ha))
>  #define IS_MSIX_NACK_CAPABLE(ha) (IS_QLA81XX(ha) || IS_QLA83XX(ha) || \
> -				IS_QLA27XX(ha) || IS_QLA28XX(ha))
> +				IS_QLA27XX(ha) || IS_QLA28XX(ha) || \
> +				IS_QLA29XX(ha))
>  #define IS_NOPOLLING_TYPE(ha)	(IS_QLA81XX(ha) && (ha)->flags.msix_enabled)
>  #define IS_FAC_REQUIRED(ha)	(IS_QLA81XX(ha) || IS_QLA83XX(ha) || \
>  				IS_QLA27XX(ha) || IS_QLA28XX(ha))

[Severity: High]
Should IS_FAC_REQUIRED also be updated to include IS_QLA29XX(ha)? It looks
like it was missed here, which could cause missing hardware capability
detection.

[ ... ]

> @@ -4408,12 +4423,12 @@ struct qla_hw_data {
>  #define IS_PI_SPLIT_DET_CAPABLE(ha)	(IS_PI_SPLIT_DET_CAPABLE_HBA(ha) && \
>      (((ha)->fw_attributes_h << 16 | (ha)->fw_attributes) & BIT_22))
>  #define IS_ATIO_MSIX_CAPABLE(ha) (IS_QLA83XX(ha) || IS_QLA27XX(ha) || \

[Severity: High]
Should IS_ATIO_MSIX_CAPABLE and IS_SHADOW_REG_CAPABLE be updated to include
IS_QLA29XX(ha) as well? The macro definitions appear to omit the new adapters,
which might break ATIO interrupt setup and shadow register support.

> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> index 72b1c28e4dae0..c91d2b8bd08e7 100644
> --- a/drivers/scsi/qla2xxx/qla_os.c
> +++ b/drivers/scsi/qla2xxx/qla_os.c

[ ... ]

> @@ -3195,6 +3221,22 @@ qla2x00_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
>  		ha->flash_data_off = FARX_ACCESS_FLASH_DATA_28XX;
>  		ha->nvram_conf_off = ~0;
>  		ha->nvram_data_off = ~0;
> +	} else if (IS_QLA29XX(ha)) {
> +		ha->portnum = PCI_FUNC(ha->pdev->devfn);
> +		ha->max_fibre_devices = MAX_FIBRE_DEVICES_2400;
> +		ha->mbx_count = MAILBOX_REGISTER_COUNT;
> +		req_length = REQUEST_ENTRY_CNT_83XX;
> +		rsp_length = RESPONSE_ENTRY_CNT_83XX;

[Severity: High]
Does this initialization block miss the ha->tgt.atio_q_length setup?
Looking at qla2x00_probe_one(), the older 27xx and 28xx families initialize
this field:

	} else if (IS_QLA28XX(ha)) {
		...
		req_length = REQUEST_ENTRY_CNT_83XX;
		rsp_length = RESPONSE_ENTRY_CNT_83XX;
		ha->tgt.atio_q_length = ATIO_ENTRY_CNT_24XX;

Would omitting it for IS_QLA29XX(ha) break Target mode (ATIO)? Also, should
qlt_probe_one_stage1() in qla_target.c and hardware mailbox functions in
qla_mbx.c (such as qla25xx_init_req_que()) be updated to include the
new IS_QLA29XX(ha) check?

> +		ha->max_loop_id = SNS_LAST_LOOP_ID_2300;
> +		ha->init_cb_size = sizeof(struct mid_init_cb_81xx);
> +		ha->gid_list_info_size = 8;
> +		ha->optrom_size = OPTROM_SIZE_28XX;
> +		ha->nvram_npiv_size = QLA_MAX_VPORTS_QLA25XX;
> +		ha->isp_ops = &qla27xx_isp_ops;
> +		ha->flash_conf_off = ~0;
> +		ha->flash_data_off = ~0;
> +		ha->nvram_conf_off = ~0;
> +		ha->nvram_data_off = ~0;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707055435.2680300-1-njavali@marvell.com?part=1