Re: [PATCH v6 17/56] scsi: qla2xxx: Enable init_firmware mailbox for 29xx

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing IS_QLA29XX() generation checks in firmware state and data rate mailbox commands, leading to incomplete hardware support.
--

--- Patch [17]: [PATCH v6 17/56] scsi: qla2xxx: Enable init_firmware mailbox for 29xx ---
commit 7e60ed82a282887839ef4256a5c2633b284fa01f
Author: Nilesh Javali <njavali@marvell.com>

scsi: qla2xxx: Enable init_firmware mailbox for 29xx
    
The init_firmware mailbox command needs 29xx adapter support for reading
back SFP information via mb3 and for validating SFP status on successful
firmware initialization. Add IS_QLA29XX() checks alongside the existing
27xx/28xx checks.

> diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
> index 3fc08120fdf11..9c78aa66e12b3 100644
> --- a/drivers/scsi/qla2xxx/qla_mbx.c
> +++ b/drivers/scsi/qla2xxx/qla_mbx.c
> @@ -1968,7 +1968,7 @@ qla2x00_init_firmware(scsi_qla_host_t *vha, uint16_t size)
>  
>  	/* 1 and 2 should normally be captured. */
>  	mcp->in_mb = MBX_2|MBX_1|MBX_0;
> -	if (IS_QLA83XX(ha) || IS_QLA27XX(ha) || IS_QLA28XX(ha))
> +	if (IS_QLA83XX(ha) || IS_QLA27XX(ha) || IS_QLA28XX(ha) || IS_QLA29XX(ha))
>  		/* mb3 is additional info about the installed SFP. */
>  		mcp->in_mb  |= MBX_3;
>  	mcp->buf_size = size;

[ ... ]

> @@ -1992,7 +1992,7 @@ qla2x00_init_firmware(scsi_qla_host_t *vha, uint16_t size)
>  			    0x0104d, ha->ex_init_cb, sizeof(*ha->ex_init_cb));
>  		}
>  	} else {
> -		if (IS_QLA27XX(ha) || IS_QLA28XX(ha)) {
> +		if (IS_QLA27XX(ha) || IS_QLA28XX(ha) || IS_QLA29XX(ha)) {

[Severity: Medium]
Since IS_QLA29XX(ha) is added here for SFP validation and mailbox commands,
should it also be added to other mailbox commands like
qla2x00_get_firmware_state()?

For example, without it, MPI state reporting fails to request mcp->mb[12]
on 29xx adapters:

drivers/scsi/qla2xxx/qla_mbx.c:qla2x00_get_firmware_state() {
	...
	if (IS_QLA27XX(ha) || IS_QLA28XX(ha)) {
		mcp->mb[12] = 0;
		mcp->out_mb |= MBX_12;
		mcp->in_mb |= MBX_12;
	}
	...
}

This also causes sysfs MPI state reads to fail for 29xx adapters:

drivers/scsi/qla2xxx/qla_attr.c:qla2x00_mpi_fw_state_show() {
	...
	if (!(IS_QLA27XX(ha) || IS_QLA28XX(ha)))
		return scnprintf(buf, PAGE_SIZE,
				"MPI state reporting is not supported for this HBA.\n");
	...
}

Does IS_QLA29XX(ha) also need to be included in data rate commands to report
FEC link status?

drivers/scsi/qla2xxx/qla_mbx.c:qla2x00_set_data_rate() {
	...
	if (IS_QLA83XX(ha) || IS_QLA27XX(ha) || IS_QLA28XX(ha))
		mcp->in_mb |= MBX_4|MBX_3;
	...
}

drivers/scsi/qla2xxx/qla_mbx.c:qla2x00_get_data_rate() {
	...
	if (IS_QLA83XX(ha) || IS_QLA27XX(ha) || IS_QLA28XX(ha))
		mcp->in_mb |= MBX_4|MBX_3;
	...
}

>  			if (mcp->mb[2] == 6 || mcp->mb[3] == 2)
>  				ql_dbg(ql_dbg_mbx, vha, 0x119d,
>  				    "Invalid SFP/Validation Failed\n");

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