On Wed, 5 Feb 2025 at 07:18, Shawn Lin <shawn.lin@xxxxxxxxxxxxxx> wrote:
>
> RK3576 SoC contains a UFS controller, add initial support for it.
> The features are:
> (1) support UFS 2.0 features
> (2) High speed up to HS-G3
> (3) 2RX-2TX lanes
> (4) auto H8 entry and exit
>
> Software limitation:
> (1) HCE procedure: enable controller->enable intr->dme_reset->dme_enable
> (2) disable unipro timeout values before power mode change
>
> Signed-off-by: Shawn Lin <shawn.lin@xxxxxxxxxxxxxx>
One minor comment, see below. Nevertheless, feel free to add:
Reviewed-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
[...]
> +#ifdef CONFIG_PM_SLEEP
> +static int ufs_rockchip_system_suspend(struct device *dev)
> +{
> + struct ufs_hba *hba = dev_get_drvdata(dev);
> + struct ufs_rockchip_host *host = ufshcd_get_variant(hba);
> + int err;
> +
The below code seems to require that the device is runtime resumed.
Maybe that is guaranteed by the upper UFS layer, I don't know. If not,
we need a pm_runtime_get_sync() here - and thus a corresponding
pm_runtime_put_noidle() in ufs_rockchip_system_resume().
> + /*
> + * If spm_lvl is less than level 5, it means we need to keep the host
> + * controller in powered-on state. So device_set_awake_path() is
> + * calling pm core to notify the genpd provider to meet this requirement
> + */
> + if (hba->spm_lvl < UFS_PM_LVL_5)
> + device_set_awake_path(dev);
> +
> + err = ufshcd_system_suspend(dev);
> + if (err) {
> + dev_err(hba->dev, "UFSHCD system susped failed %d\n", err);
> + return err;
> + }
> +
> + clk_disable_unprepare(host->ref_out_clk);
> +
> + return 0;
> +}
> +
> +static int ufs_rockchip_system_resume(struct device *dev)
> +{
> + struct ufs_hba *hba = dev_get_drvdata(dev);
> + struct ufs_rockchip_host *host = ufshcd_get_variant(hba);
> + int err;
> +
> + err = clk_prepare_enable(host->ref_out_clk);
> + if (err) {
> + dev_err(hba->dev, "failed to enable ref_out clock %d\n", err);
> + return err;
> + }
> +
> + return ufshcd_system_resume(dev);
> +}
> +#endif
> +
> +static const struct dev_pm_ops ufs_rockchip_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(ufs_rockchip_system_suspend, ufs_rockchip_system_resume)
> + SET_RUNTIME_PM_OPS(ufs_rockchip_runtime_suspend, ufs_rockchip_runtime_resume, NULL)
> + .prepare = ufshcd_suspend_prepare,
> + .complete = ufshcd_resume_complete,
> +};
> +
[...]
Kind regards
Uffe
