Re: [PATCH v3 5/5] scsi: ufs: rockchip: initial support for UFS

From: Christophe JAILLET
Date: Sun Nov 03 2024 - 08:09:40 EST


Le 08/10/2024 à 08:15, Shawn Lin a écrit :
RK3576 SoC contains a UFS controller, add initial support fot it.

s/fot/for/

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: enbale controller->enbale intr->dme_reset->dme_enable

s/enbale/enable/ x 2

(2) disable unipro timeout values before power mode change

Signed-off-by: Shawn Lin <shawn.lin@xxxxxxxxxxxxxx>


No need for an extra empty line

---

...

+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;
+
+ 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))
+ return dev_err_probe(dev, PTR_ERR(host->ufs_sys_ctrl),
+ "cannot ioremap for hci system control register\n");
+
+ /* system control register for mphy */
+ host->ufs_phy_ctrl = devm_platform_ioremap_resource_byname(pdev, "mphy_grf");
+ if (IS_ERR(host->ufs_phy_ctrl))
+ return dev_err_probe(dev, PTR_ERR(host->ufs_phy_ctrl),
+ "cannot ioremap for mphy system control register\n");
+
+ /* mphy base register */
+ host->mphy_base = devm_platform_ioremap_resource_byname(pdev, "mphy");
+ if (IS_ERR(host->mphy_base))
+ return dev_err_probe(dev, PTR_ERR(host->mphy_base),
+ "cannot ioremap for mphy base register\n");
+
+ host->rst = devm_reset_control_array_get_exclusive(dev);
+ if (IS_ERR(host->rst))
+ return dev_err_probe(dev, PTR_ERR(host->rst),
+ "failed to get reset control\n");
+
+ reset_control_assert(host->rst);
+ usleep_range(1, 2);
+ reset_control_deassert(host->rst);
+
+ host->ref_out_clk = devm_clk_get_enabled(dev, "ref_out");
+ if (IS_ERR(host->ref_out_clk))
+ return dev_err_probe(dev, PTR_ERR(host->ref_out_clk),
+ "ref_out unavailable\n");
+
+ host->rst_gpio = devm_gpiod_get(&pdev->dev, "reset", GPIOD_OUT_LOW);
+ if (IS_ERR(host->rst_gpio))
+ return dev_err_probe(&pdev->dev, PTR_ERR(host->rst_gpio),
+ "invalid reset-gpios property in node\n");
+
+ 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)
+ return dev_err_probe(dev, err, "failed to get clocks\n");
+
+ err = clk_bulk_prepare_enable(UFS_MAX_CLKS, host->clks);

This has to be undone somewhere, likely at the end of ufs_rockchip_remove().

+ if (err)
+ return dev_err_probe(dev, err, "failed to enable clocks\n");
+
+ host->hba = hba;
+
+ ufshcd_set_variant(hba, host);
+
+ 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-ufshc", .data = &ufs_hba_rk3576_vops},

Missing space before ending }

+ {},

No need for an extra , here.

+};
+MODULE_DEVICE_TABLE(of, ufs_rockchip_of_match);

...

+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);

No need for clk_disable_unprepare(), ref_out_clk comes from devm_clk_get_enabled(), so the framework will already call clk_disable_unprepare().

+}

...

CJ