Re: [PATCH v6 2/2] scsi: ufs: spacemit: k3: Add UFS Host Controller driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] scsi: ufs: spacemit: missing M-PHY TX EQ configuration for lane 1
- [Medium] scsi: ufs: spacemit: unchecked return value from quirk configuration
--

commit 16f84e82c7f6101530a90dc4cc7509a53d6ac1b4
Author: Yixun Lan <dlan@kernel.org>

scsi: ufs: spacemit: k3: Add UFS Host Controller driver

This commit introduces the UFS host controller driver for the SpacemiT K3
SoC. It implements the vendor-specific configurations and initialization
sequences required to interact with the M-PHY and UniPro hardware.

> diff --git a/drivers/ufs/host/ufs-spacemit.c b/drivers/ufs/host/ufs-spacemit.c
> new file mode 100644
> index 0000000000000..941506cc5ab68
> --- /dev/null
> +++ b/drivers/ufs/host/ufs-spacemit.c

[ ... ]

> @@ -211,6 +211,9 @@ static int ufs_spacemit_uniprov1p6_init(struct ufs_hba *hba)
> +		/* RX HB8_TIME CAP */
> +		{ UIC_ARG_MIB_SEL(0x0092, UIC_ARG_MPHY_RX_GEN_SEL_INDEX(0)), 0x64 },
> +		{ UIC_ARG_MIB_SEL(0x0092, UIC_ARG_MPHY_RX_GEN_SEL_INDEX(1)), 0x64 },
> +		/* TX EQ 3DB */
> +		{ UIC_ARG_MIB_SEL(0x00CD, UIC_ARG_MPHY_TX_GEN_SEL_INDEX(0)),  0x5 },

[Severity: Medium]
The TX EQ 3DB attribute is explicitly configured for lane 0, but the
corresponding configuration for lane 1 (UIC_ARG_MPHY_TX_GEN_SEL_INDEX(1))
is missing in ufs_spacemit_uniprov1p6_init(), unlike other dual-lane
attributes in this array.

Can this missing equalization on lane 1 cause signal integrity issues and a
high error rate on the second TX lane, potentially degrading performance or
causing transmission failures unconditionally during the UniPro initialization
sequence when the device operates in 2-lane mode?

> +		/* RX garbage cnt = 32 SI */
> +		{ UIC_ARG_MIB_SEL(0x00F2, UIC_ARG_MPHY_RX_GEN_SEL_INDEX(0)), 0x9F },
> +		{ UIC_ARG_MIB_SEL(0x00F2, UIC_ARG_MPHY_RX_GEN_SEL_INDEX(1)), 0x9F },
> +	};

[ ... ]

> @@ -401,6 +401,9 @@ static int ufs_spacemit_apply_dev_quirks(struct ufs_hba *hba)
> +	int err;
> +
> +	if (hba->dev_quirks & UFS_DEVICE_QUIRK_HOST_PA_SAVECONFIGTIME)
> +		ufs_spacemit_quirk_host_pa_saveconfigtime(hba);

[Severity: Medium]
The return value of ufs_spacemit_quirk_host_pa_saveconfigtime() is neither
assigned nor checked in ufs_spacemit_apply_dev_quirks(), ignoring potential
DME access errors.

If the PA_SaveConfigTime quirk configuration fails, could the error be
silently ignored, causing the device to operate with an invalid timing
configuration and leading to link instability or failure when a device
requiring UFS_DEVICE_QUIRK_HOST_PA_SAVECONFIGTIME is initialized?

> +
> +	if (hba->dev_info.wmanufacturerid == UFS_VENDOR_WDC)
> +		hba->dev_quirks |= UFS_DEVICE_QUIRK_HOST_PA_TACTIVATE;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921-08-k3-ufs-support-v6-0-3479768a2299@kernel.org?part=2