Re: [PATCH 1/3] scsi: hisi_sas: Fix incorrect delay values from magic-number cleanup

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Hardware callback executes a 50ms busy-wait delay while holding an IRQ-disabled spinlock, severely degrading system latency and risking watchdog timeouts.
--

commit e8ddd9bef8daf2db411c3a4e51016ed28809f393
Author: Xingui Yang <yangxingui@huawei.com>

scsi: hisi_sas: Fix incorrect delay values from magic-number cleanup

This commit fixes incorrect delay values in the hisi_sas v3 hardware driver
that were introduced during a magic-number cleanup. It adds separate macros
for each delay site to restore their original timings of 50us, 50ms, and
10ms respectively.

> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> index 8a2500993e19d..9c363e2373538 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c

[ ... ]

> @@ -1072,7 +1075,7 @@ static void disable_phy_v3_hw(struct hisi_hba *hisi_hba, int phy_no)
>  	cfg &= ~PHY_CFG_ENA_MSK;
>  	hisi_sas_phy_write32(hisi_hba, phy_no, PHY_CFG, cfg);
>  
> -	mdelay(HISI_SAS_DELAY_FOR_PHY_DISABLE);
> +	mdelay(DISABLE_PHY_DELAY_MS);

[Severity: High]
This is a pre-existing issue, but is it safe to use a 50ms busy-wait delay
here while holding an IRQ-disabled spinlock?

Looking at hisi_sas_phy_enable() in drivers/scsi/hisi_sas/hisi_sas_main.c,
the hardware callback is invoked with interrupts disabled:

hisi_sas_phy_enable() {
        ...
        spin_lock_irqsave(&phy->lock, flags);
        if (enable) {
                ...
        } else {
                phy->negotiated_linkrate = SAS_PHY_DISABLED;
                hisi_hba->hw->phy_disable(hisi_hba, phy_no);
        }
        ...
}

Since hisi_hba->hw->phy_disable resolves to disable_phy_v3_hw(), this will
block the CPU with interrupts disabled for 50ms. When looping over multiple
PHYs in hisi_sas_stop_phys(), this delay can multiply, potentially causing
severe scheduling latency, dropped interrupts, or triggering softlockup
panics.

Could this delay be moved outside the spinlock, or could we use a sleepable
context here instead?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260829032858.1661019-1-yangxingui@huawei.com?part=1