RE: [PATCH V3 3/4] clk: imx: Add support for i.MX8MN clock driver

Quoting Anson Huang (2019-06-10 19:06:22)

quoted

quoted

Sorry, I am still a little confused, all the clock
register(clk_register()) are via each different clock types like
imx_clk_gate4/imx_clk_pll14xx, if using clk_hw_register, means we need
to re-write the clock driver using different clk register method, that
will make the driver completely different from i.mx8mq/i.mx8mm, they
are actually same series of SoC as i.mx8mn, it will introduce many

confusion, is my understanding correct? And is it OK to just keep what it is
and make them all aligned?

quoted

Ok, the problem I'm trying to point out is that clk registrations need to be
undone, i.e. clk_unregister() needs to be called, when the driver fails to
probe. devm_*() is one way to do this, but if you have other ways of
removing all the registered clks then that works too. Makes sense?

Yes, it makes sense. Do you think it is OK to add an imx_unregister_clocks() API, then
call it in every place of returning failure in .probe function? If yes, I will add it and also
fix it in i.MX8MQ driver which uses platform driver model but does NOT handle this case. 

        base = devm_platform_ioremap_resource(pdev, 0);
-       if (WARN_ON(IS_ERR(base)))
-               return PTR_ERR(base);
+       if (WARN_ON(IS_ERR(base))) {
+               ret = PTR_ERR(base);
+               goto unregister_clks;
+       }

                pr_err("failed to register clks for i.MX8MN\n");
-               return -EINVAL;
+               goto unregister_clks;
        }

        return 0;
+
+unregister_clks:
+       imx_unregister_clocks(clks, ARRAY_SIZE(clks));
+
+       return ret;

+void imx_unregister_clocks(struct clk *clks[], unsigned int count)
+{
+       unsigned i;
+
+       for (i = 0; i < count; i++)
+               clk_unregister(clks[i]);
+}
+

Yes, looks better.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel