| Message ID | 20230720140845.33734-1-ruc_gongyuanjun@163.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [1/1] soundwire: use devm_clk_get_enabled() in qcom_swrm_probe() | expand |
On Thu, Jul 20, 2023 at 10:08:45PM +0800, Yuanjun Gong wrote: > in qcom_swrm_probe(), the return value of function > clk_prepare_enable() should be checked, since it may fail. > using devm_clk_get_enabled() instead of devm_clk_get() and > clk_prepare_enable() can avoid this problem. > > Signed-off-by: Yuanjun Gong <ruc_gongyuanjun@163.com> > --- > drivers/soundwire/qcom.c | 12 ++++-------- > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c > index 7970fdb27ba0..04bc917b7a70 100644 > --- a/drivers/soundwire/qcom.c > +++ b/drivers/soundwire/qcom.c > @@ -1549,14 +1549,12 @@ static int qcom_swrm_probe(struct platform_device *pdev) > goto err_init; > } > > - ctrl->hclk = devm_clk_get(dev, "iface"); > + ctrl->hclk = devm_clk_get_enabled(dev, "iface"); This gives the impression that hclk is to be enabled from probe() until cleanup, but hclk is toggled during suspend/resume. So I'm not entirely sure this is good practice. If we choose to use devm_clk_get_enabled() then you need to remove the clk_disable_unprepare() from qcom_swrm_remove(). If you choose to solve your problem by checking the return value of clk_prepare_enable(), note that you have one in swrm_runtime_resume() as well. Regards, Bjorn
diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c index 7970fdb27ba0..04bc917b7a70 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -1549,14 +1549,12 @@ static int qcom_swrm_probe(struct platform_device *pdev) goto err_init; } - ctrl->hclk = devm_clk_get(dev, "iface"); + ctrl->hclk = devm_clk_get_enabled(dev, "iface"); if (IS_ERR(ctrl->hclk)) { ret = PTR_ERR(ctrl->hclk); goto err_init; } - clk_prepare_enable(ctrl->hclk); - ctrl->dev = dev; dev_set_drvdata(&pdev->dev, ctrl); mutex_init(&ctrl->port_lock); @@ -1570,7 +1568,7 @@ static int qcom_swrm_probe(struct platform_device *pdev) ret = qcom_swrm_get_port_config(ctrl); if (ret) - goto err_clk; + goto err_init; params = &ctrl->bus.params; params->max_dr_freq = DEFAULT_CLK_FREQ; @@ -1598,7 +1596,7 @@ static int qcom_swrm_probe(struct platform_device *pdev) "soundwire", ctrl); if (ret) { dev_err(dev, "Failed to request soundwire irq\n"); - goto err_clk; + goto err_init; } ctrl->wake_irq = of_irq_get(dev->of_node, 1); @@ -1617,7 +1615,7 @@ static int qcom_swrm_probe(struct platform_device *pdev) if (ret) { dev_err(dev, "Failed to register Soundwire controller (%d)\n", ret); - goto err_clk; + goto err_init; } qcom_swrm_init(ctrl); @@ -1647,8 +1645,6 @@ static int qcom_swrm_probe(struct platform_device *pdev) err_master_add: sdw_bus_master_delete(&ctrl->bus); -err_clk: - clk_disable_unprepare(ctrl->hclk); err_init: return ret; }
in qcom_swrm_probe(), the return value of function clk_prepare_enable() should be checked, since it may fail. using devm_clk_get_enabled() instead of devm_clk_get() and clk_prepare_enable() can avoid this problem. Signed-off-by: Yuanjun Gong <ruc_gongyuanjun@163.com> --- drivers/soundwire/qcom.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)