Now that the common mtk_clk_simple_{probe,remove}() functions can deal
with divider clocks it is possible to migrate more clock drivers to it:
in this case, it's about topckgen.
While at it, also perform a fast migration for mcucfg.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/clk/mediatek/clk-mt2712.c | 127 +++++-------------------------
 1 file changed, 21 insertions(+), 106 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mt2712.c b/drivers/clk/mediatek/clk-mt2712.c
index 94f8fc2a4f7b..db20c46e088b 100644
--- a/drivers/clk/mediatek/clk-mt2712.c
+++ b/drivers/clk/mediatek/clk-mt2712.c
@@ -36,14 +36,11 @@ static const struct mtk_fixed_clk top_fixed_clks[] = {
 	FIXED_CLK(CLK_TOP_CVBSPLL, "cvbspll", NULL, 108000000),
 };
 
-static const struct mtk_fixed_factor top_early_divs[] = {
+static const struct mtk_fixed_factor top_divs[] = {
 	FACTOR(CLK_TOP_SYS_26M, "sys_26m", "clk26m", 1,
 		1),
 	FACTOR(CLK_TOP_CLK26M_D2, "clk26m_d2", "sys_26m", 1,
 		2),
-};
-
-static const struct mtk_fixed_factor top_divs[] = {
 	FACTOR(CLK_TOP_ARMCA35PLL, "armca35pll_ck", "armca35pll", 1,
 		1),
 	FACTOR(CLK_TOP_ARMCA35PLL_600M, "armca35pll_600m", "armca35pll_ck", 1,
@@ -1295,114 +1292,30 @@ static int clk_mt2712_apmixed_probe(struct platform_device *pdev)
 	return r;
 }
 
-static struct clk_hw_onecell_data *top_clk_data;
-
-static void clk_mt2712_top_init_early(struct device_node *node)
-{
-	int r, i;
-
-	if (!top_clk_data) {
-		top_clk_data = mtk_alloc_clk_data(CLK_TOP_NR_CLK);
-
-		for (i = 0; i < CLK_TOP_NR_CLK; i++)
-			top_clk_data->hws[i] = ERR_PTR(-EPROBE_DEFER);
-	}
-
-	mtk_clk_register_factors(top_early_divs, ARRAY_SIZE(top_early_divs),
-			top_clk_data);
-
-	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, top_clk_data);
-	if (r)
-		pr_err("%s(): could not register clock provider: %d\n",
-			__func__, r);
-}
-
-CLK_OF_DECLARE_DRIVER(mt2712_topckgen, "mediatek,mt2712-topckgen",
-			clk_mt2712_top_init_early);
-
-static int clk_mt2712_top_probe(struct platform_device *pdev)
-{
-	int r, i;
-	struct device_node *node = pdev->dev.of_node;
-	void __iomem *base;
-
-	base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(base)) {
-		pr_err("%s(): ioremap failed\n", __func__);
-		return PTR_ERR(base);
-	}
-
-	if (!top_clk_data) {
-		top_clk_data = mtk_alloc_clk_data(CLK_TOP_NR_CLK);
-	} else {
-		for (i = 0; i < CLK_TOP_NR_CLK; i++) {
-			if (top_clk_data->hws[i] == ERR_PTR(-EPROBE_DEFER))
-				top_clk_data->hws[i] = ERR_PTR(-ENOENT);
-		}
-	}
-
-	mtk_clk_register_fixed_clks(top_fixed_clks, ARRAY_SIZE(top_fixed_clks),
-			top_clk_data);
-	mtk_clk_register_factors(top_early_divs, ARRAY_SIZE(top_early_divs),
-			top_clk_data);
-	mtk_clk_register_factors(top_divs, ARRAY_SIZE(top_divs), top_clk_data);
-	mtk_clk_register_composites(&pdev->dev, top_muxes,
-				    ARRAY_SIZE(top_muxes), base,
-				    &mt2712_clk_lock, top_clk_data);
-	mtk_clk_register_dividers(top_adj_divs, ARRAY_SIZE(top_adj_divs), base,
-			&mt2712_clk_lock, top_clk_data);
-	mtk_clk_register_gates(&pdev->dev, node, top_clks,
-			       ARRAY_SIZE(top_clks), top_clk_data);
-
-	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, top_clk_data);
-
-	if (r != 0)
-		pr_err("%s(): could not register clock provider: %d\n",
-			__func__, r);
-
-	return r;
-}
-
-static int clk_mt2712_mcu_probe(struct platform_device *pdev)
-{
-	struct clk_hw_onecell_data *clk_data;
-	int r;
-	struct device_node *node = pdev->dev.of_node;
-	void __iomem *base;
-
-	base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(base)) {
-		pr_err("%s(): ioremap failed\n", __func__);
-		return PTR_ERR(base);
-	}
-
-	clk_data = mtk_alloc_clk_data(CLK_MCU_NR_CLK);
-
-	r = mtk_clk_register_composites(&pdev->dev, mcu_muxes,
-					ARRAY_SIZE(mcu_muxes), base,
-					&mt2712_clk_lock, clk_data);
-	if (r)
-		dev_err(&pdev->dev, "Could not register composites: %d\n", r);
-
-	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
-
-	if (r != 0)
-		pr_err("%s(): could not register clock provider: %d\n",
-			__func__, r);
+static const struct mtk_clk_desc topck_desc = {
+	.clks = top_clks,
+	.num_clks = ARRAY_SIZE(top_clks),
+	.fixed_clks = top_fixed_clks,
+	.num_fixed_clks = ARRAY_SIZE(top_fixed_clks),
+	.factor_clks = top_divs,
+	.num_factor_clks = ARRAY_SIZE(top_divs),
+	.composite_clks = top_muxes,
+	.num_composite_clks = ARRAY_SIZE(top_muxes),
+	.divider_clks = top_adj_divs,
+	.num_divider_clks = ARRAY_SIZE(top_adj_divs),
+	.clk_lock = &mt2712_clk_lock,
+};
 
-	return r;
-}
+static const struct mtk_clk_desc mcu_desc = {
+	.composite_clks = mcu_muxes,
+	.num_composite_clks = ARRAY_SIZE(mcu_muxes),
+	.clk_lock = &mt2712_clk_lock,
+};
 
 static const struct of_device_id of_match_clk_mt2712[] = {
 	{
 		.compatible = "mediatek,mt2712-apmixedsys",
 		.data = clk_mt2712_apmixed_probe,
-	}, {
-		.compatible = "mediatek,mt2712-topckgen",
-		.data = clk_mt2712_top_probe,
-	}, {
-		.compatible = "mediatek,mt2712-mcucfg",
-		.data = clk_mt2712_mcu_probe,
 	}, {
 		/* sentinel */
 	}
@@ -1440,7 +1353,9 @@ static const struct mtk_clk_desc peri_desc = {
 
 static const struct of_device_id of_match_clk_mt2712_simple[] = {
 	{ .compatible = "mediatek,mt2712-infracfg", .data = &infra_desc },
+	{ .compatible = "mediatek,mt2712-mcucfg", .data = &mcu_desc },
 	{ .compatible = "mediatek,mt2712-pericfg", .data = &peri_desc, },
+	{ .compatible = "mediatek,mt2712-topckgen", .data = &topck_desc },
 	{ /* sentinel */ }
 };
 
-- 
2.39.1

On Mon, Feb 6, 2023 at 11:29 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Now that the common mtk_clk_simple_{probe,remove}() functions can deal
> with divider clocks it is possible to migrate more clock drivers to it:
> in this case, it's about topckgen.
> While at it, also perform a fast migration for mcucfg.
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>

> ---
>  drivers/clk/mediatek/clk-mt2712.c | 127 +++++-------------------------
>  1 file changed, 21 insertions(+), 106 deletions(-)
>
> diff --git a/drivers/clk/mediatek/clk-mt2712.c b/drivers/clk/mediatek/clk-mt2712.c
> index 94f8fc2a4f7b..db20c46e088b 100644
> --- a/drivers/clk/mediatek/clk-mt2712.c
> +++ b/drivers/clk/mediatek/clk-mt2712.c
> @@ -36,14 +36,11 @@ static const struct mtk_fixed_clk top_fixed_clks[] = {
>         FIXED_CLK(CLK_TOP_CVBSPLL, "cvbspll", NULL, 108000000),
>  };
>
> -static const struct mtk_fixed_factor top_early_divs[] = {
> +static const struct mtk_fixed_factor top_divs[] = {
>         FACTOR(CLK_TOP_SYS_26M, "sys_26m", "clk26m", 1,
>                 1),
>         FACTOR(CLK_TOP_CLK26M_D2, "clk26m_d2", "sys_26m", 1,
>                 2),
> -};
> -
> -static const struct mtk_fixed_factor top_divs[] = {
>         FACTOR(CLK_TOP_ARMCA35PLL, "armca35pll_ck", "armca35pll", 1,
>                 1),
>         FACTOR(CLK_TOP_ARMCA35PLL_600M, "armca35pll_600m", "armca35pll_ck", 1,
> @@ -1295,114 +1292,30 @@ static int clk_mt2712_apmixed_probe(struct platform_device *pdev)
>         return r;
>  }
>
> -static struct clk_hw_onecell_data *top_clk_data;
> -
> -static void clk_mt2712_top_init_early(struct device_node *node)
> -{
> -       int r, i;
> -
> -       if (!top_clk_data) {
> -               top_clk_data = mtk_alloc_clk_data(CLK_TOP_NR_CLK);
> -
> -               for (i = 0; i < CLK_TOP_NR_CLK; i++)
> -                       top_clk_data->hws[i] = ERR_PTR(-EPROBE_DEFER);
> -       }
> -
> -       mtk_clk_register_factors(top_early_divs, ARRAY_SIZE(top_early_divs),
> -                       top_clk_data);
> -
> -       r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, top_clk_data);
> -       if (r)
> -               pr_err("%s(): could not register clock provider: %d\n",
> -                       __func__, r);
> -}
> -
> -CLK_OF_DECLARE_DRIVER(mt2712_topckgen, "mediatek,mt2712-topckgen",
> -                       clk_mt2712_top_init_early);
> -
> -static int clk_mt2712_top_probe(struct platform_device *pdev)
> -{
> -       int r, i;
> -       struct device_node *node = pdev->dev.of_node;
> -       void __iomem *base;
> -
> -       base = devm_platform_ioremap_resource(pdev, 0);
> -       if (IS_ERR(base)) {
> -               pr_err("%s(): ioremap failed\n", __func__);
> -               return PTR_ERR(base);
> -       }
> -
> -       if (!top_clk_data) {
> -               top_clk_data = mtk_alloc_clk_data(CLK_TOP_NR_CLK);
> -       } else {
> -               for (i = 0; i < CLK_TOP_NR_CLK; i++) {
> -                       if (top_clk_data->hws[i] == ERR_PTR(-EPROBE_DEFER))
> -                               top_clk_data->hws[i] = ERR_PTR(-ENOENT);
> -               }
> -       }
> -
> -       mtk_clk_register_fixed_clks(top_fixed_clks, ARRAY_SIZE(top_fixed_clks),
> -                       top_clk_data);
> -       mtk_clk_register_factors(top_early_divs, ARRAY_SIZE(top_early_divs),
> -                       top_clk_data);
> -       mtk_clk_register_factors(top_divs, ARRAY_SIZE(top_divs), top_clk_data);
> -       mtk_clk_register_composites(&pdev->dev, top_muxes,
> -                                   ARRAY_SIZE(top_muxes), base,
> -                                   &mt2712_clk_lock, top_clk_data);
> -       mtk_clk_register_dividers(top_adj_divs, ARRAY_SIZE(top_adj_divs), base,
> -                       &mt2712_clk_lock, top_clk_data);
> -       mtk_clk_register_gates(&pdev->dev, node, top_clks,
> -                              ARRAY_SIZE(top_clks), top_clk_data);
> -
> -       r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, top_clk_data);
> -
> -       if (r != 0)
> -               pr_err("%s(): could not register clock provider: %d\n",
> -                       __func__, r);
> -
> -       return r;
> -}
> -
> -static int clk_mt2712_mcu_probe(struct platform_device *pdev)
> -{
> -       struct clk_hw_onecell_data *clk_data;
> -       int r;
> -       struct device_node *node = pdev->dev.of_node;
> -       void __iomem *base;
> -
> -       base = devm_platform_ioremap_resource(pdev, 0);
> -       if (IS_ERR(base)) {
> -               pr_err("%s(): ioremap failed\n", __func__);
> -               return PTR_ERR(base);
> -       }
> -
> -       clk_data = mtk_alloc_clk_data(CLK_MCU_NR_CLK);
> -
> -       r = mtk_clk_register_composites(&pdev->dev, mcu_muxes,
> -                                       ARRAY_SIZE(mcu_muxes), base,
> -                                       &mt2712_clk_lock, clk_data);
> -       if (r)
> -               dev_err(&pdev->dev, "Could not register composites: %d\n", r);
> -
> -       r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
> -
> -       if (r != 0)
> -               pr_err("%s(): could not register clock provider: %d\n",
> -                       __func__, r);
> +static const struct mtk_clk_desc topck_desc = {
> +       .clks = top_clks,
> +       .num_clks = ARRAY_SIZE(top_clks),
> +       .fixed_clks = top_fixed_clks,
> +       .num_fixed_clks = ARRAY_SIZE(top_fixed_clks),
> +       .factor_clks = top_divs,
> +       .num_factor_clks = ARRAY_SIZE(top_divs),
> +       .composite_clks = top_muxes,
> +       .num_composite_clks = ARRAY_SIZE(top_muxes),
> +       .divider_clks = top_adj_divs,
> +       .num_divider_clks = ARRAY_SIZE(top_adj_divs),
> +       .clk_lock = &mt2712_clk_lock,

At some point maybe we should look into splitting up the locks to one
per block, or converting everything to regmap.

ChenYu

Il 07/02/23 07:15, Chen-Yu Tsai ha scritto:
> On Mon, Feb 6, 2023 at 11:29 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Now that the common mtk_clk_simple_{probe,remove}() functions can deal
>> with divider clocks it is possible to migrate more clock drivers to it:
>> in this case, it's about topckgen.
>> While at it, also perform a fast migration for mcucfg.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> 
> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> 
>> ---
>>   drivers/clk/mediatek/clk-mt2712.c | 127 +++++-------------------------
>>   1 file changed, 21 insertions(+), 106 deletions(-)
>>

..snip..
                      __func__, r);
>> +static const struct mtk_clk_desc topck_desc = {
>> +       .clks = top_clks,
>> +       .num_clks = ARRAY_SIZE(top_clks),
>> +       .fixed_clks = top_fixed_clks,
>> +       .num_fixed_clks = ARRAY_SIZE(top_fixed_clks),
>> +       .factor_clks = top_divs,
>> +       .num_factor_clks = ARRAY_SIZE(top_divs),
>> +       .composite_clks = top_muxes,
>> +       .num_composite_clks = ARRAY_SIZE(top_muxes),
>> +       .divider_clks = top_adj_divs,
>> +       .num_divider_clks = ARRAY_SIZE(top_adj_divs),
>> +       .clk_lock = &mt2712_clk_lock,
> 
> At some point maybe we should look into splitting up the locks to one
> per block, or converting everything to regmap.
> 

I was thinking the same about the locks... but about regmap, that would
actually add up some overhead at every R/W operation and I would really
like to measure that precisely before doing any kind of regmap conversion
for the MediaTek clocks.

Perhaps I'll even find a way to avoid any kind of (even if small) overhead
while doing that sometime in the future, which wouldn't be benefitting only
MediaTek, but also other users like Qualcomm (as they have practically all
clocks on regmap!).

Cheers,
Angelo

On Tue, Feb 7, 2023 at 4:45 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 07/02/23 07:15, Chen-Yu Tsai ha scritto:
> > On Mon, Feb 6, 2023 at 11:29 PM AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@collabora.com> wrote:
> >>
> >> Now that the common mtk_clk_simple_{probe,remove}() functions can deal
> >> with divider clocks it is possible to migrate more clock drivers to it:
> >> in this case, it's about topckgen.
> >> While at it, also perform a fast migration for mcucfg.
> >>
> >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> >
> > Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> >
> >> ---
> >>   drivers/clk/mediatek/clk-mt2712.c | 127 +++++-------------------------
> >>   1 file changed, 21 insertions(+), 106 deletions(-)
> >>
>
> ..snip..
>                       __func__, r);
> >> +static const struct mtk_clk_desc topck_desc = {
> >> +       .clks = top_clks,
> >> +       .num_clks = ARRAY_SIZE(top_clks),
> >> +       .fixed_clks = top_fixed_clks,
> >> +       .num_fixed_clks = ARRAY_SIZE(top_fixed_clks),
> >> +       .factor_clks = top_divs,
> >> +       .num_factor_clks = ARRAY_SIZE(top_divs),
> >> +       .composite_clks = top_muxes,
> >> +       .num_composite_clks = ARRAY_SIZE(top_muxes),
> >> +       .divider_clks = top_adj_divs,
> >> +       .num_divider_clks = ARRAY_SIZE(top_adj_divs),
> >> +       .clk_lock = &mt2712_clk_lock,
> >
> > At some point maybe we should look into splitting up the locks to one
> > per block, or converting everything to regmap.
> >
>
> I was thinking the same about the locks... but about regmap, that would
> actually add up some overhead at every R/W operation and I would really
> like to measure that precisely before doing any kind of regmap conversion
> for the MediaTek clocks.
>
> Perhaps I'll even find a way to avoid any kind of (even if small) overhead
> while doing that sometime in the future, which wouldn't be benefitting only
> MediaTek, but also other users like Qualcomm (as they have practically all
> clocks on regmap!).

Stephen would likely appreciate a unified regmap clock library :D