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

Hi, Stephen

-----Original Message-----
From: Stephen Boyd <sboyd@kernel.org>
Sent: Monday, June 10, 2019 11:14 PM
To: bjorn.andersson@linaro.org; catalin.marinas@arm.com;
devicetree@vger.kernel.org; dinguyen@kernel.org;
enric.balletbo@collabora.com; festevam@gmail.com;
horms+renesas@verge.net.au; jagan@amarulasolutions.com;
kernel@pengutronix.de; l.stach@pengutronix.de; linux-arm-
kernel@lists.infradead.org; linux-clk@vger.kernel.org; linux-
kernel@vger.kernel.org; mark.rutland@arm.com;
maxime.ripard@bootlin.com; mturquette@baylibre.com; olof@lixom.net;
robh+dt@kernel.org; s.hauer@pengutronix .de [off-list ref];
shawnguo@kernel.org; will.deacon@arm.com; Abel Vesa
[off-list ref]; Aisheng Dong [off-list ref]; Anson
Huang [off-list ref]; Jacky Bai [off-list ref]; Leonard
Crestez [off-list ref]
Cc: dl-linux-imx <redacted>
Subject: RE: [PATCH V3 3/4] clk: imx: Add support for i.MX8MN clock driver

Quoting Anson Huang (2019-06-08 02:58:18)

quoted

Hi, Stephen

quoted

-----Original Message-----
From: Stephen Boyd <sboyd@kernel.org>
Sent: Saturday, June 8, 2019 2:01 AM
To: bjorn.andersson@linaro.org; catalin.marinas@arm.com;
devicetree@vger.kernel.org; dinguyen@kernel.org;
enric.balletbo@collabora.com; festevam@gmail.com;
horms+renesas@verge.net.au; jagan@amarulasolutions.com;
kernel@pengutronix.de; l.stach@pengutronix.de; linux-arm-
kernel@lists.infradead.org; linux-clk@vger.kernel.org; linux-
kernel@vger.kernel.org; mark.rutland@arm.com;
maxime.ripard@bootlin.com; mturquette@baylibre.com; olof@lixom.net;
robh+dt@kernel.org; s.hauer@pengutronix .de
robh+[off-list ref];
shawnguo@kernel.org; will.deacon@arm.com; Abel Vesa
[off-list ref]; Aisheng Dong [off-list ref]; Anson
Huang [off-list ref]; Jacky Bai [off-list ref];

Leonard

quoted

quoted

Crestez [off-list ref]
Cc: dl-linux-imx <redacted>
Subject: RE: [PATCH V3 3/4] clk: imx: Add support for i.MX8MN clock
driver

Quoting Anson Huang (2019-06-06 17:50:28)

quoted

I will use devm_platform_ioremap_resource() instead of ioremap(),
and can you be more specific about devmified clk registration?

I mean using things like devm_clk_hw_register().

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]);
+}
+

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