Re: [PATCH 07/10] scsi: ufs: Add NXP S32N79 UFS host controller driver

[email protected]
Newsgroups org.kernel.vger.linux-scsi,dev.linux.lists.imx,org.kernel.vger.linux-devicetree
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The error handling for an unrecognized `nxp,mphy-boot-mode` property aborts the probe instead of skipping MPHY initialization, contradicting its own error message.
--

commit cc6f571e7075af648ae4e8cc7125fa859f589eaf
Author: Larisa Grigore <[email protected]>

scsi: ufs: Add NXP S32N79 UFS host controller driver

This commit adds support for the UFS host controller on the NXP S32N79 SoC
using the DWC architecture. It handles specific initialization steps prior
to UIC_LINKSTARTUP and introduces the 'nxp,mphy-boot-mode' device tree
property.

> diff --git a/drivers/ufs/host/ufs-nxp-s32n7.c b/drivers/ufs/host/ufs-nxp-s32n7.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/ufs/host/ufs-nxp-s32n7.c

[ ... ]

> +static int ufs_s32n_init(struct ufs_hba *hba)
> +{

[ ... ]

> +	ret = of_property_read_string(np, "nxp,mphy-boot-mode", &mphy_boot_mode);
> +	if (ret || !mphy_boot_mode) {
> +		dev_dbg(dev,
> +			"nxp,mphy-boot-mode property not found. UFS MPHY init will be skipped.\n");
> +		goto init_out;
> +	}
> +
> +	if (!strcmp(mphy_boot_mode, "rom")) {
> +		ufs->mphy_boot_mode = MPHY_BOOT_ROM;
> +	} else {
> +		return dev_err_probe(dev, -EINVAL,
> +				     "Unrecognized nxp,mphy-boot-mode property. UFS MPHY init will be skipped.\n");
> +	}

[Severity: Medium]
Does this code inadvertently abort the entire UFS host controller probe?

The error message explicitly states "UFS MPHY init will be skipped", but
returning the result of dev_err_probe() propagates the -EINVAL error up the
call stack and completely fails the probe.

If the intent is to truly skip the MPHY initialization as indicated by the log
message, should this perhaps log a warning and use `goto init_out` instead of
returning a fatal error code?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.