Quoting Jacky Bai (2019-01-08 01:01:04)
> +
> +static int imx8mm_clocks_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + void __iomem *base;
> + int ret = 0;
Please don't assign ret here. Just let it be assigned later on.
> +
> + clks[IMX8MM_CLK_DUMMY] = imx_clk_fixed("dummy", 0);
> + clks[IMX8MM_CLK_24M] = of_clk_get_by_name(np, "osc_24m");
> + clks[IMX8MM_CLK_32K] = of_clk_get_by_name(np, "osc_32k"); /* Check more */
> + clks[IMX8MM_CLK_EXT1] = of_clk_get_by_name(np, "clk_ext1");
> + clks[IMX8MM_CLK_EXT2] = of_clk_get_by_name(np, "clk_ext2");
> + clks[IMX8MM_CLK_EXT3] = of_clk_get_by_name(np, "clk_ext3");
> + clks[IMX8MM_CLK_EXT4] = of_clk_get_by_name(np, "clk_ext4");
> +
> + np = of_find_compatible_node(NULL, NULL, "fsl,imx8mm-anatop");
> + base = of_iomap(np, 0);
> + if (WARN_ON(!base))
> + return -ENOMEM;
Why do we need to reach into some other node to get the memory region to
map?
> +
> + clks[IMX8MM_AUDIO_PLL1_REF_SEL] = imx_clk_mux("audio_pll1_ref_sel", base + 0x0, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels));
> + clks[IMX8MM_AUDIO_PLL2_REF_SEL] = imx_clk_mux("audio_pll2_ref_sel", base + 0x14, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels));
> + clks[IMX8MM_VIDEO_PLL1_REF_SEL] = imx_clk_mux("video_pll1_ref_sel", base + 0x28, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels));
> + clks[IMX8MM_DRAM_PLL_REF_SEL] = imx_clk_mux("dram_pll_ref_sel", base + 0x50, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels));
[...]
> + clks[IMX8MM_SYS_PLL2_333M] = imx_clk_fixed_factor("sys_pll2_333m", "sys_pll2_out", 1, 3);
> + clks[IMX8MM_SYS_PLL2_500M] = imx_clk_fixed_factor("sys_pll2_500m", "sys_pll2_out", 1, 2);
> + clks[IMX8MM_SYS_PLL2_1000M] = imx_clk_fixed_factor("sys_pll2_1000m", "sys_pll2_out", 1, 1);
> +
> + np = dev->of_node;
> + base = of_iomap(np, 0);
> + if (WARN_ON(!base))
> + return -ENOMEM;
This can surely use the platform device APIs to map and retrieve memory
regions.
> +
[...]
> +
> + imx_check_clocks(clks, ARRAY_SIZE(clks));
> +
> + clk_data.clks = clks;
> + clk_data.clk_num = ARRAY_SIZE(clks);
> + ret = of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
> + if (ret < 0) {
> + dev_err(dev, "failed to register clks for i.MX8MM\n");
> + return -EINVAL;
> + }
> +
> + imx_register_uart_clocks(uart_clks);
> +
> + pr_info("i.MX8MM clock driver init done\n");
Please no "I"m alive" messages.
> +
> + return 0;
> +}
> +
