[...]
> +
> +static int ufs_rockchip_runtime_suspend(struct device *dev)
> +{
> + struct ufs_hba *hba = dev_get_drvdata(dev);
> + struct ufs_rockchip_host *host = ufshcd_get_variant(hba);
> + struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
pd_to_genpd() isn't safe to use like this. It's solely to be used by
genpd provider drivers.
> +
> + clk_disable_unprepare(host->ref_out_clk);
> +
> + /*
> + * Shouldn't power down if rpm_lvl is less than level 5.
Can you elaborate on why we must not power-off the power-domain when
level is less than 5?
What happens if we power-off anyway when the level is less than 5?
> + * This flag will be passed down to platform power-domain driver
> + * which has the final decision.
> + */
> + if (hba->rpm_lvl < UFS_PM_LVL_5)
> + genpd->flags |= GENPD_FLAG_RPM_ALWAYS_ON;
> + else
> + genpd->flags &= ~GENPD_FLAG_RPM_ALWAYS_ON;
The genpd->flags is not supposed to be changed like this - and
especially not from a genpd consumer driver.
I am trying to understand a bit more of the use case here. Let's see
if that helps me to potentially suggest an alternative approach.
> +
> + return ufshcd_runtime_suspend(dev);
> +}
> +
> +static int ufs_rockchip_runtime_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;
> + }
> +
> + reset_control_assert(host->rst);
> + usleep_range(1, 2);
> + reset_control_deassert(host->rst);
> +
> + return ufshcd_runtime_resume(dev);
> +}
> +
> +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);
> +
> + /* Pass down desired spm_lvl to Firmware */
> + arm_smccc_smc(ROCKCHIP_SIP_SUSPEND_MODE, ROCKCHIP_SLEEP_PD_CONFIG,
> + host->pd_id, hba->spm_lvl < 5 ? 1 : 0, 0, 0, 0, 0, NULL);
Can you please elaborate on what goes on here? Is this turning off the
power-domain that the dev is attached to - or what is actually
happening?
> +
> + return ufshcd_system_suspend(dev);
> +}
> +
> +static const struct dev_pm_ops ufs_rockchip_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(ufs_rockchip_system_suspend, ufshcd_system_resume)
> + SET_RUNTIME_PM_OPS(ufs_rockchip_runtime_suspend, ufs_rockchip_runtime_resume, NULL)
> + .prepare = ufshcd_suspend_prepare,
> + .complete = ufshcd_resume_complete,
> +};
> +
> +static struct platform_driver ufs_rockchip_pltform = {
> + .probe = ufs_rockchip_probe,
> + .remove = ufs_rockchip_remove,
> + .driver = {
> + .name = "ufshcd-rockchip",
> + .pm = &ufs_rockchip_pm_ops,
> + .of_match_table = ufs_rockchip_of_match,
> + },
> +};
> +module_platform_driver(ufs_rockchip_pltform);
> +
[...]
Kind regards
Uffe
