mmc: meson-mx-sdio: Refactor internal clock initialization

Use modern common clock framework helpers for simplifying the clock
controller management:
- switch to struct clk_hw for internal clocks and only get the
  "cfg_div_clk" (which has to be managed at runtime) as struct clk
  using devm_clk_hw_get_clk() which is then the only clock used by
  struct meson_mx_mmc_host.
- use CLK_HW_INIT_FW_NAME and CLK_HW_INIT_HW helper macros for simpler
  init data initialization
- keep the clock controller memory allocation separate to prevent a
  potential use-after-free because struct meson_mx_mmc_host_clkc is
  free'd before controller_dev

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

diff --git a/drivers/mmc/host/meson-mx-sdio.c b/drivers/mmc/host/meson-mx-sdio.c
index 1159cc911359de1f3b86776fcc9e2d55f5a23675..ce1828d915da1ad78ee508303560a8278937207c 100644 (file)
--- a/drivers/mmc/host/meson-mx-sdio.c
+++ b/drivers/mmc/host/meson-mx-sdio.c
@@ -99,15 +99,15 @@
 #define MESON_MX_SDIO_RESPONSE_CRC16_BITS                      (16 - 1)
 #define MESON_MX_SDIO_MAX_SLOTS                                        3
 
+struct meson_mx_mmc_host_clkc {
+       struct clk_divider              cfg_div;
+       struct clk_fixed_factor         fixed_div2;
+};
+
 struct meson_mx_mmc_host {
        struct device                   *controller_dev;
 
-       struct clk                      *parent_clk;
-       struct clk_divider              cfg_div;
        struct clk                      *cfg_div_clk;
-       struct clk_fixed_factor         fixed_factor;
-       struct clk                      *fixed_factor_clk;
-
        struct regmap                   *regmap;
        int                             irq;
        spinlock_t                      irq_lock;
@@ -548,8 +548,7 @@ static int meson_mx_mmc_add_host(struct meson_mx_mmc_host *host)
 
        /* Get the min and max supported clock rates */
        mmc->f_min = clk_round_rate(host->cfg_div_clk, 1);
-       mmc->f_max = clk_round_rate(host->cfg_div_clk,
-                                   clk_get_rate(host->parent_clk));
+       mmc->f_max = clk_round_rate(host->cfg_div_clk, ULONG_MAX);
 
        mmc->caps |= MMC_CAP_CMD23 | MMC_CAP_WAIT_WHILE_BUSY;
        mmc->ops = &meson_mx_mmc_ops;
@@ -565,54 +564,62 @@ static int meson_mx_mmc_add_host(struct meson_mx_mmc_host *host)
        return 0;
 }
 
-static int meson_mx_mmc_register_clks(struct meson_mx_mmc_host *host,
-                                     void __iomem *base)
+static struct clk *meson_mx_mmc_register_clk(struct device *dev,
+                                            void __iomem *base)
 {
-       struct clk_init_data init;
-       const char *clk_div_parent, *clk_fixed_factor_parent;
-
-       clk_fixed_factor_parent = __clk_get_name(host->parent_clk);
-       init.name = devm_kasprintf(host->controller_dev, GFP_KERNEL,
-                                  "%s#fixed_factor",
-                                  dev_name(host->controller_dev));
-       if (!init.name)
-               return -ENOMEM;
+       const char *fixed_div2_name, *cfg_div_name;
+       struct meson_mx_mmc_host_clkc *host_clkc;
+       struct clk *clk;
+       int ret;
 
-       init.ops = &clk_fixed_factor_ops;
-       init.flags = 0;
-       init.parent_names = &clk_fixed_factor_parent;
-       init.num_parents = 1;
-       host->fixed_factor.div = 2;
-       host->fixed_factor.mult = 1;
-       host->fixed_factor.hw.init = &init;
-
-       host->fixed_factor_clk = devm_clk_register(host->controller_dev,
-                                                &host->fixed_factor.hw);
-       if (WARN_ON(IS_ERR(host->fixed_factor_clk)))
-               return PTR_ERR(host->fixed_factor_clk);
-
-       clk_div_parent = __clk_get_name(host->fixed_factor_clk);
-       init.name = devm_kasprintf(host->controller_dev, GFP_KERNEL,
-                                  "%s#div", dev_name(host->controller_dev));
-       if (!init.name)
-               return -ENOMEM;
+       /* use a dedicated memory allocation for the clock controller to
+        * prevent use-after-free as meson_mx_mmc_host is free'd before
+        * dev (controller dev, not mmc_host->dev) is free'd.
+        */
+       host_clkc = devm_kzalloc(dev, sizeof(*host_clkc), GFP_KERNEL);
+       if (!host_clkc)
+               return ERR_PTR(-ENOMEM);
+
+       fixed_div2_name = devm_kasprintf(dev, GFP_KERNEL, "%s#fixed_div2",
+                                        dev_name(dev));
+       if (!fixed_div2_name)
+               return ERR_PTR(-ENOMEM);
+
+       host_clkc->fixed_div2.div = 2;
+       host_clkc->fixed_div2.mult = 1;
+       host_clkc->fixed_div2.hw.init = CLK_HW_INIT_FW_NAME(fixed_div2_name,
+                                                           "clkin",
+                                                           &clk_fixed_factor_ops,
+                                                           0);
+       ret = devm_clk_hw_register(dev, &host_clkc->fixed_div2.hw);
+       if (ret)
+               return dev_err_ptr_probe(dev, ret,
+                                        "Failed to register %s clock\n",
+                                        fixed_div2_name);
+
+       cfg_div_name = devm_kasprintf(dev, GFP_KERNEL, "%s#div", dev_name(dev));
+       if (!cfg_div_name)
+               return ERR_PTR(-ENOMEM);
+
+       host_clkc->cfg_div.reg = base + MESON_MX_SDIO_CONF;
+       host_clkc->cfg_div.shift = MESON_MX_SDIO_CONF_CMD_CLK_DIV_SHIFT;
+       host_clkc->cfg_div.width = MESON_MX_SDIO_CONF_CMD_CLK_DIV_WIDTH;
+       host_clkc->cfg_div.hw.init = CLK_HW_INIT_HW(cfg_div_name,
+                                                   &host_clkc->fixed_div2.hw,
+                                                   &clk_divider_ops,
+                                                   CLK_DIVIDER_ALLOW_ZERO);
+       ret = devm_clk_hw_register(dev, &host_clkc->cfg_div.hw);
+       if (ret)
+               return dev_err_ptr_probe(dev, ret,
+                                        "Failed to register %s clock\n",
+                                        cfg_div_name);
 
-       init.ops = &clk_divider_ops;
-       init.flags = CLK_SET_RATE_PARENT;
-       init.parent_names = &clk_div_parent;
-       init.num_parents = 1;
-       host->cfg_div.reg = base + MESON_MX_SDIO_CONF;
-       host->cfg_div.shift = MESON_MX_SDIO_CONF_CMD_CLK_DIV_SHIFT;
-       host->cfg_div.width = MESON_MX_SDIO_CONF_CMD_CLK_DIV_WIDTH;
-       host->cfg_div.hw.init = &init;
-       host->cfg_div.flags = CLK_DIVIDER_ALLOW_ZERO;
-
-       host->cfg_div_clk = devm_clk_register(host->controller_dev,
-                                             &host->cfg_div.hw);
-       if (WARN_ON(IS_ERR(host->cfg_div_clk)))
-               return PTR_ERR(host->cfg_div_clk);
+       clk = devm_clk_hw_get_clk(dev, &host_clkc->cfg_div.hw, "cfg_div_clk");
+       if (IS_ERR(clk))
+               return dev_err_ptr_probe(dev, PTR_ERR(clk),
+                                        "Failed to get the cfg_div clock\n");
 
-       return 0;
+       return clk;
 }
 
 static int meson_mx_mmc_probe(struct platform_device *pdev)
@@ -682,16 +689,12 @@ static int meson_mx_mmc_probe(struct platform_device *pdev)
                goto error_free_mmc;
        }
 
-       host->parent_clk = devm_clk_get(host->controller_dev, "clkin");
-       if (IS_ERR(host->parent_clk)) {
-               ret = PTR_ERR(host->parent_clk);
+       host->cfg_div_clk = meson_mx_mmc_register_clk(&pdev->dev, base);
+       if (IS_ERR(host->cfg_div_clk)) {
+               ret = PTR_ERR(host->cfg_div_clk);
                goto error_free_mmc;
        }
 
-       ret = meson_mx_mmc_register_clks(host, base);
-       if (ret)
-               goto error_free_mmc;
-
        ret = clk_prepare_enable(host->cfg_div_clk);
        if (ret) {
                dev_err(host->controller_dev, "Failed to enable MMC clock\n");