On Mon, May 10, 2021 at 06:02:05PM +0100, Jonathan Cameron wrote:
> On Mon, 10 May 2021 08:17:19 +0200
> Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> wrote:
>
> > Allow to add an exit hook to devm managed clocks. Also use
> > clk_get_optional() in devm_clk_get_optional instead of open coding it.
> > The generalisation will be used in the next commit to add some more
> > devm_clk helpers.
> >
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
>
> If feels marginally odd to register cleanup that we know won't do anything
> for the optional case, but it works as far as I can tell and it would be
> a little fiddly to special case it.
It took a moment for me to understand your concern. Yes, I register a
cleanup even if (*init) happens to be clk_get_optional() and it returned
NULL. It's not hard to optimize that:
diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c
index b54f7f0f2a35..2420d6ae7b33 100644
--- a/drivers/clk/clk-devres.c
+++ b/drivers/clk/clk-devres.c
@@ -38,14 +38,17 @@ static struct clk *__devm_clk_get(struct device *dev, const char *id,
goto err_clk_get;
}
- if (init) {
- ret = init(clk);
- if (ret)
- goto err_clk_init;
+ /* short-cut the case where clk_get_optional returned NULL */
+ if (clk) {
+ if (init) {
+ ret = init(clk);
+ if (ret)
+ goto err_clk_init;
+ }
+ state->exit = exit;
}
state->clk = clk;
- state->exit = exit;
devres_add(dev, state);
But I'd value simplicity over micro-optimizing and keep the code as is.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
