Re: [PATCH v7 09/12] scsi: ufs: Add NXP S32N79 UFS host controller driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ufs: core_clk remains permanently enabled during PM suspend states
--

commit aea859e7e22ef8d785ddf1572d56548e903ae544
Author: Larisa Grigore <larisa.grigore@oss.nxp.com>
scsi: ufs: Add NXP S32N79 UFS host controller driver

This commit adds support for the UFS host controller on the NXP S32N79 SoC,
built on the Synopsys DesignWare architecture. It introduces platform glue
and specific initialization routines for clock scaling, M-PHY calibration,
and device hibernation transitions.

> diff --git a/drivers/ufs/host/ufs-nxp-s32n7.c b/drivers/ufs/host/ufs-nxp-s32n7.c
> new file mode 100644
> index 0000000000000..5374662763b6e
> --- /dev/null
> +++ b/drivers/ufs/host/ufs-nxp-s32n7.c
[ ... ]
> +static int ufs_s32n7_init(struct ufs_hba *hba)
> +{
[ ... ]
> +	/*
> +	 * The core reference clock is not managed by the UFS core (the DT node
> +	 * has no clock-names/freq-table-hz, so it is never added to
> +	 * hba->clk_list_head).
> +	 */
> +	ufs->core_clk = devm_clk_get_enabled(dev, NULL);
> +	if (IS_ERR(ufs->core_clk))
> +		return dev_err_probe(dev, PTR_ERR(ufs->core_clk),
> +				     "Failed to get and enable core clock.\n");
> +
> +	ufshcd_set_variant(hba, ufs);
> +
> +	return 0;
> +}
[ ... ]
> +static const struct dev_pm_ops ufs_s32n7_pm_ops = {
> +	SYSTEM_SLEEP_PM_OPS(ufshcd_system_suspend, ufshcd_system_resume)
> +	RUNTIME_PM_OPS(ufshcd_runtime_suspend, ufshcd_runtime_resume, NULL)
> +	.prepare	= pm_sleep_ptr(ufshcd_suspend_prepare),
> +	.complete	= pm_sleep_ptr(ufshcd_resume_complete),
> +};

[Severity: Medium]
Because the core clock is unconditionally enabled via devm_clk_get_enabled()
in ufs_s32n7_init() and is explicitly excluded from the UFS core's PM logic,
will this clock remain permanently enabled across all low-power states?

Since these power management ops map directly to the generic UFS core hooks
and the UFS variant ops don't implement suspend or resume callbacks, there
does not appear to be a mechanism to disable ufs->core_clk during system
suspend or runtime idle. Could this lead to unnecessary power consumption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929100946.693962-1-larisa.grigore@oss.nxp.com?part=9