| Message ID | 20240803065231.342695-1-cuigaosheng1@huawei.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [-next] bus: bt1-apb: Add missing clk_disable_unprepare in bt1_apb_request_clk | expand |
Hi Gaosheng On Sat, Aug 03, 2024 at 02:52:31PM +0800, Gaosheng Cui wrote: > Add the missing clk_disable_unprepare() before return in > bt1_apb_request_clk(). > > Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com> > --- > drivers/bus/bt1-apb.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/bus/bt1-apb.c b/drivers/bus/bt1-apb.c > index 595fb22b73e0..244f03988545 100644 > --- a/drivers/bus/bt1-apb.c > +++ b/drivers/bus/bt1-apb.c > @@ -210,12 +210,14 @@ static int bt1_apb_request_clk(struct bt1_apb *apb) Thanks for the patch, but it's fully redundant. > ret = devm_add_action_or_reset(apb->dev, bt1_apb_disable_clk, apb); Please see the __devm_add_action_or_reset() method semantics. It executes the passed "action" in case if the add-action procedure fails. > if (ret) { > dev_err(apb->dev, "Can't add APB EHB clocks disable action\n"); > + clk_disable_unprepare(apb->pclk); > return ret; > } > > apb->rate = clk_get_rate(apb->pclk); > if (!apb->rate) { > dev_err(apb->dev, "Invalid clock rate\n"); > + clk_disable_unprepare(apb->pclk); If the rate getting fails for some reason, then the action registered above will be called in the framework of all the device-managed cleanups executed for the probe() method. So to speak there is no need in the change suggested by you here. But what could be done is the devm_clk_get(), clk_prepare_enable() and devm_add_action_or_reset() calls replacement with a single devm_clk_get_enabled() invocation. That simplification will also cause the bt1_apb_disable_clk() method removal. A similar change can be applied to the bt1_axi_request_clk() method in the drivers/bus/bt1-axi.c driver. -Serge(y) > return -EINVAL; > } > > -- > 2.25.1 >
diff --git a/drivers/bus/bt1-apb.c b/drivers/bus/bt1-apb.c index 595fb22b73e0..244f03988545 100644 --- a/drivers/bus/bt1-apb.c +++ b/drivers/bus/bt1-apb.c @@ -210,12 +210,14 @@ static int bt1_apb_request_clk(struct bt1_apb *apb) ret = devm_add_action_or_reset(apb->dev, bt1_apb_disable_clk, apb); if (ret) { dev_err(apb->dev, "Can't add APB EHB clocks disable action\n"); + clk_disable_unprepare(apb->pclk); return ret; } apb->rate = clk_get_rate(apb->pclk); if (!apb->rate) { dev_err(apb->dev, "Invalid clock rate\n"); + clk_disable_unprepare(apb->pclk); return -EINVAL; }
Add the missing clk_disable_unprepare() before return in bt1_apb_request_clk(). Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com> --- drivers/bus/bt1-apb.c | 2 ++ 1 file changed, 2 insertions(+)