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>
---
drivers/mmc/host/meson-mx-sdio.c | 119 ++++++++++++++++---------------
1 file changed, 61 insertions(+), 58 deletions(-)
diff --git a/drivers/mmc/host/meson-mx-sdio.c b/drivers/mmc/host/meson-mx-sdio.c
index 1159cc911359..ce1828d915da 100644
--- 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");
--
2.51.2On 11/9/25 00:12, Martin Blumenstingl wrote:
> 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>
> ---
> drivers/mmc/host/meson-mx-sdio.c | 119 ++++++++++++++++---------------
> 1 file changed, 61 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-mx-sdio.c b/drivers/mmc/host/meson-mx-sdio.c
> index 1159cc911359..ce1828d915da 100644
> --- 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");
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
© 2016 - 2025 Red Hat, Inc.