On 09/24/2013 08:06 PM, Andrew Bresticker wrote:
+static int exynos_audss_clk_probe(struct platform_device *pdev)
{
[...]
clk_table[EXYNOS_MOUT_AUDSS] = clk_register_mux(NULL, "mout_audss",
mout_audss_p, ARRAY_SIZE(mout_audss_p),
@@ -123,13 +124,83 @@ static void __init exynos_audss_clk_init(struct
device_node *np)
"div_pcm0", CLK_SET_RATE_PARENT,
reg_base + ASS_CLK_GATE, 5, 0,&lock);
+ for (i = 0; i< EXYNOS_AUDSS_MAX_CLKS; i++) {
+ if (IS_ERR(clk_table[i])) {
+ dev_err(&pdev->dev, "failed to regsiter clock %d\n", i);
typo: regsiter -> register
+ ret = PTR_ERR(clk_table[i]);
+ goto unregister;
+ }
+ }
+
+ clk_data.clks = clk_table;
+ clk_data.clk_num = EXYNOS_AUDSS_MAX_CLKS;
+ ret = of_clk_add_provider(pdev->dev.of_node, of_clk_src_onecell_get,
+ &clk_data);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to add clock provider\n");
+ goto unregister;
+ }
+
[...]
+ return 0;
+
+unregister:
+ for (i = 0; i< EXYNOS_AUDSS_MAX_CLKS; i++) {
+ if (!IS_ERR_OR_NULL(clk_table[i]))
+ clk_unregister(clk_table[i]);
+ }
Couldn't this instead be:
while (--i >= 0)
clk_unregister(clk_table[i]);
?
+static int exynos_audss_clk_remove(struct platform_device *pdev)
+{
+ int i;
+
+ of_clk_del_provider(pdev->dev.of_node);
+
+ for (i = 0; i< EXYNOS_AUDSS_MAX_CLKS; i++) {
+ if (!IS_ERR_OR_NULL(clk_table[i]))
+ clk_unregister(clk_table[i]);
+ }
Since we only get here if all the clocks are registered properly and we
always register EXYNOS_AUDSS_MAX_CLKS clocks, couldn't this simply be:
for (i = 0; i < EXYNOS_AUDSS_MAX_CLKS; i++)
clk_unregister(clk_table[i]);
?
+ return 0;
+}
Otherwise the whole series looks good to me. Feel free to add:
Reviewed-by: Sylwester Nawrocki <s.nawro...@samsung.com>
--
Thanks,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
