Re: [PATCH v1 3/3] scsi: ufs: rockchip: init support for UFS

[Krzysztof Kozlowski <krzk@kernel.org>]

On 06/08/2024 09:20, Shawn Lin wrote:
> RK3576 contains a UFS controller, add init support fot it.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> 
> ---



> +
> +static int ufs_rockchip_common_init(struct ufs_hba *hba)
> +{
> +	struct device *dev = hba->dev;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct ufs_rockchip_host *host;
> +	int err = 0;
> +
> +	host = devm_kzalloc(dev, sizeof(*host), GFP_KERNEL);
> +	if (!host)
> +		return -ENOMEM;
> +
> +	/* system control register for hci */
> +	host->ufs_sys_ctrl = devm_platform_ioremap_resource_byname(pdev, "hci_grf");
> +	if (IS_ERR(host->ufs_sys_ctrl)) {
> +		dev_err(dev, "cannot ioremap for hci system control register\n");
> +		return PTR_ERR(host->ufs_sys_ctrl);
> +	}
> +
> +	/* system control register for mphy */
> +	host->ufs_phy_ctrl = devm_platform_ioremap_resource_byname(pdev, "mphy_grf");
> +	if (IS_ERR(host->ufs_phy_ctrl)) {
> +		dev_err(dev, "cannot ioremap for mphy system control register\n");
> +		return PTR_ERR(host->ufs_phy_ctrl);
> +	}
> +
> +	/* mphy base register */
> +	host->mphy_base = devm_platform_ioremap_resource_byname(pdev, "mphy");
> +	if (IS_ERR(host->mphy_base)) {
> +		dev_err(dev, "cannot ioremap for mphy base register\n");
> +		return PTR_ERR(host->mphy_base);
> +	}
> +
> +	host->rst = devm_reset_control_array_get_exclusive(dev);
> +	if (IS_ERR(host->rst)) {
> +		dev_err(dev, "failed to get reset control\n");
> +		return PTR_ERR(host->rst);

return dev_err_probe, assuming this is a probe path?

> +	}
> +
> +	reset_control_assert(host->rst);
> +	udelay(1);
> +	reset_control_deassert(host->rst);
> +
> +	host->ref_out_clk = devm_clk_get(dev, "ref_out");
> +	if (IS_ERR(host->ref_out_clk)) {
> +		dev_err(dev, "ciu-drive not available\n");
> +		return PTR_ERR(host->ref_out_clk);

Ditto

> +	}
> +	err = clk_prepare_enable(host->ref_out_clk);
> +	if (err) {
> +		dev_err(dev, "failed to enable ref out clock %d\n", err);
> +		return err;
> +	}
> +
> +	host->rst_gpio = devm_gpiod_get(&pdev->dev, "reset", GPIOD_OUT_LOW);

Hm? Test your DTS and bindings.

> +	if (IS_ERR(host->rst_gpio)) {
> +		dev_err(&pdev->dev, "invalid reset-gpios property in node\n");

Post your DTS so we can review it. Post also results of dtbs_check...

> +		err = PTR_ERR(host->rst_gpio);

dev_err_probe

> +		goto out;
> +	}
> +	udelay(20);
> +	gpiod_set_value_cansleep(host->rst_gpio, 1);
> +
> +	host->clks[0].id = "core";
> +	host->clks[1].id = "pclk";
> +	host->clks[2].id = "pclk_mphy";
> +	err = devm_clk_bulk_get_optional(dev, UFS_MAX_CLKS, host->clks);
> +	if (err) {
> +		dev_err(dev, "failed to get clocks %d\n", err);
> +		goto out;
> +	}
> +
> +	err = clk_bulk_prepare_enable(UFS_MAX_CLKS, host->clks);
> +	if (err) {
> +		dev_err(dev, "failed to enable clocks %d\n", err);
> +		goto out;
> +	}
> +
> +	if (device_property_read_u32(dev, "ufs-phy-config-mode",

No, stop adding undocumented properties.

> +				     &host->phy_config_mode))
> +		host->phy_config_mode = 0;
> +
> +	pm_runtime_set_active(&pdev->dev);
> +
> +	host->hba = hba;
> +	ufs_rockchip_set_pm_lvl(hba);
> +
> +	ufshcd_set_variant(hba, host);
> +
> +	return 0;
> +out:
> +	clk_disable_unprepare(host->ref_out_clk);
> +	return err;
> +}
> +
> +static int ufs_rockchip_rk3576_init(struct ufs_hba *hba)
> +{
> +	int ret = 0;
> +	struct device *dev = hba->dev;
> +
> +	hba->quirks = UFSHCI_QUIRK_BROKEN_HCE | UFSHCD_QUIRK_SKIP_DEF_UNIPRO_TIMEOUT_SETTING;
> +
> +	/* Enable BKOPS when suspend */
> +	hba->caps |= UFSHCD_CAP_AUTO_BKOPS_SUSPEND;
> +	/* Enable putting device into deep sleep */
> +	hba->caps |= UFSHCD_CAP_DEEPSLEEP;
> +	/* Enable devfreq of UFS */
> +	hba->caps |= UFSHCD_CAP_CLK_SCALING;
> +	/* Enable WriteBooster */
> +	hba->caps |= UFSHCD_CAP_WB_EN;
> +
> +	ret = ufs_rockchip_common_init(hba);
> +	if (ret) {
> +		dev_err(dev, "ufs common init fail\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ufs_rockchip_device_reset(struct ufs_hba *hba)
> +{
> +	struct ufs_rockchip_host *host = ufshcd_get_variant(hba);
> +
> +	if (!host->rst_gpio)
> +		return -EOPNOTSUPP;
> +
> +	gpiod_set_value_cansleep(host->rst_gpio, 0);
> +	udelay(20);
> +
> +	gpiod_set_value_cansleep(host->rst_gpio, 1);
> +	udelay(20);
> +
> +	return 0;
> +}
> +
> +static const struct ufs_hba_variant_ops ufs_hba_rk3576_vops = {
> +	.name = "rk3576",
> +	.init = ufs_rockchip_rk3576_init,
> +	.device_reset = ufs_rockchip_device_reset,
> +	.hce_enable_notify = ufs_rockchip_hce_enable_notify,
> +	.phy_initialization = ufs_rockchip_rk3576_phy_init,
> +};
> +
> +static const struct of_device_id ufs_rockchip_of_match[] = {
> +	{ .compatible = "rockchip,rk3576-ufs", .data = &ufs_hba_rk3576_vops},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, ufs_rockchip_of_match);
> +
> +static int ufs_rockchip_probe(struct platform_device *pdev)
> +{
> +	int err;
> +	struct device *dev = &pdev->dev;
> +	const struct ufs_hba_variant_ops *vops;
> +
> +	vops = device_get_match_data(dev);
> +	err = ufshcd_pltfrm_init(pdev, vops);
> +	if (err)
> +		dev_err(dev, "ufshcd_pltfrm_init() failed %d\n", err);
> +
> +	return err;
> +}
> +
> +static void ufs_rockchip_remove(struct platform_device *pdev)
> +{
> +	struct ufs_hba *hba = platform_get_drvdata(pdev);
> +	struct ufs_rockchip_host *host = ufshcd_get_variant(hba);
> +
> +	pm_runtime_forbid(&pdev->dev);
> +	pm_runtime_get_noresume(&pdev->dev);
> +	ufshcd_remove(hba);
> +	ufshcd_dealloc_host(hba);
> +	clk_disable_unprepare(host->ref_out_clk);
> +}
> +
> +static int ufs_rockchip_restore_link(struct ufs_hba *hba, bool is_store)
> +{
> +	struct ufs_rockchip_host *host = ufshcd_get_variant(hba);
> +	int err, retry = 3;
> +
> +	if (is_store) {
> +		host->ie = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
> +		host->ahit = ufshcd_readl(hba, REG_AUTO_HIBERNATE_IDLE_TIMER);
> +		return 0;
> +	}
> +
> +	/* Enable controller */
> +	err = ufshcd_hba_enable(hba);
> +	if (err)
> +		return err;
> +
> +	/* Link startup and wait for DP */
> +	do {
> +		err = ufshcd_dme_link_startup(hba);
> +		if (!err && ufshcd_is_device_present(hba)) {
> +			dev_dbg_ratelimited(hba->dev, "rockchip link startup successfully.\n");
> +			break;
> +		}
> +	} while (retry--);
> +
> +	if (retry < 0) {
> +		dev_err(hba->dev, "rockchip link startup failed.\n");
> +		return -ENXIO;
> +	}
> +
> +	/* Restore negotiated power mode */
> +	err = ufshcd_config_pwr_mode(hba, &(hba->pwr_info));
> +	if (err)
> +		dev_err(hba->dev, "Failed to restore power mode, err = %d\n", err);
> +
> +	/* Restore task and transfer list */
> +	ufshcd_writel(hba, 0xffffffff, REG_INTERRUPT_STATUS);
> +	ufshcd_make_hba_operational(hba);
> +
> +	/* Restore lost regs */
> +	ufshcd_writel(hba, host->ie, REG_INTERRUPT_ENABLE);
> +	ufshcd_writel(hba, host->ahit, REG_AUTO_HIBERNATE_IDLE_TIMER);
> +	ufshcd_writel(hba, 0x1, REG_UTP_TRANSFER_REQ_LIST_RUN_STOP);
> +	ufshcd_writel(hba, 0x1, REG_UTP_TASK_REQ_LIST_RUN_STOP);
> +
> +	return err;
> +}
> +
> +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);
> +
> +	clk_disable_unprepare(host->ref_out_clk);
> +	return ufs_rockchip_restore_link(hba, true);
> +}
> +
> +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);
> +	udelay(1);
> +	reset_control_deassert(host->rst);
> +
> +	return ufs_rockchip_restore_link(hba, false);
> +}
> +
> +static int ufs_rockchip_suspend(struct device *dev)
> +{
> +	struct ufs_hba *hba = dev_get_drvdata(dev);
> +
> +	if (pm_runtime_suspended(hba->dev))
> +		return 0;
> +
> +	ufs_rockchip_restore_link(hba, true);
> +
> +	return 0;
> +}
> +
> +static int ufs_rockchip_resume(struct device *dev)
> +{
> +	struct ufs_hba *hba = dev_get_drvdata(dev);
> +
> +	if (pm_runtime_suspended(hba->dev))
> +		return 0;
> +
> +	/* Reset device if possible */
> +	ufs_rockchip_device_reset(hba);
> +	ufs_rockchip_restore_link(hba, false);
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops ufs_rockchip_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(ufs_rockchip_suspend, ufs_rockchip_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 = of_match_ptr(ufs_rockchip_of_match),

Drop of_match_ptr, you have here warnings.

Best regards,
Krzysztof


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip