linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] clk: mediatek: More cleanups
@ 2022-09-26 10:25 Chen-Yu Tsai
  2022-09-26 10:25 ` [PATCH 1/6] clk: mediatek: fix unregister function in mtk_clk_register_dividers cleanup Chen-Yu Tsai
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: Chen-Yu Tsai @ 2022-09-26 10:25 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: Chen-Yu Tsai, Matthias Brugger, AngeloGioacchino Del Regno,
	Miles Chen, linux-clk, linux-arm-kernel, linux-mediatek,
	linux-kernel

Here's some more cleanups for the MedaiTek clk drivers.

Patches 1 and 2 fixes and cleans up some parts that were missed in the
previous clk_hw conversion series.

Patch 3 drops the duplicate registration of the top_early_divs clks for
MT8192. Currently this only contains a fixed divider.

Patch 4 prevents having a duplicate OF clk provider for the topckgen
controller, but removing the previously registered instance.

Patch 5 deduplicates the parent clock lists for MT8192.

Patch 6 adds proper error handling to the clock probe functions for
MT8192.


Please have a look.


Thanks
ChenYu


Chen-Yu Tsai (6):
  clk: mediatek: fix unregister function in mtk_clk_register_dividers
    cleanup
  clk: mediatek: Migrate remaining clk_unregister_*() to
    clk_hw_unregister_*()
  clk: mediatek: mt8192: Do not re-register top_early_divs in probe
    function
  clk: mediatek: mt8192: Avoid duplicate OF clk provider for topckgen
  clk: mediatek: mt8192: deduplicate parent clock lists
  clk: mediatek: mt8192: Implement error handling in probe functions

 drivers/clk/mediatek/clk-mt8192-aud.c |  15 +-
 drivers/clk/mediatek/clk-mt8192-mm.c  |  17 +-
 drivers/clk/mediatek/clk-mt8192.c     | 296 +++++++++-----------------
 drivers/clk/mediatek/clk-mtk.c        |  12 +-
 4 files changed, 130 insertions(+), 210 deletions(-)

-- 
2.37.3.998.g577e59143f-goog


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/6] clk: mediatek: fix unregister function in mtk_clk_register_dividers cleanup
  2022-09-26 10:25 [PATCH 0/6] clk: mediatek: More cleanups Chen-Yu Tsai
@ 2022-09-26 10:25 ` Chen-Yu Tsai
  2022-09-27 10:39   ` AngeloGioacchino Del Regno
  2022-09-26 10:25 ` [PATCH 2/6] clk: mediatek: Migrate remaining clk_unregister_*() to clk_hw_unregister_*() Chen-Yu Tsai
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Chen-Yu Tsai @ 2022-09-26 10:25 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: Chen-Yu Tsai, Matthias Brugger, AngeloGioacchino Del Regno,
	Miles Chen, linux-clk, linux-arm-kernel, linux-mediatek,
	linux-kernel

When the cleanup paths for the various clk register APIs in the MediaTek
clk library were added, the one in the dividers type used the wrong type
of unregister function. This would result in incorrect dereferencing of
the clk pointer and freeing of invalid pointers.

Fix this by switching to the correct type of clk unregistration call.

Fixes: 3c3ba2ab0226 ("clk: mediatek: mtk: Implement error handling in register APIs")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/clk/mediatek/clk-mtk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
index 3a8875b6c37f..174d0645be38 100644
--- a/drivers/clk/mediatek/clk-mtk.c
+++ b/drivers/clk/mediatek/clk-mtk.c
@@ -393,7 +393,7 @@ int mtk_clk_register_dividers(const struct mtk_clk_divider *mcds, int num,
 		if (IS_ERR_OR_NULL(clk_data->hws[mcd->id]))
 			continue;
 
-		mtk_clk_unregister_composite(clk_data->hws[mcd->id]);
+		clk_hw_unregister_divider(clk_data->hws[mcd->id]);
 		clk_data->hws[mcd->id] = ERR_PTR(-ENOENT);
 	}
 
-- 
2.37.3.998.g577e59143f-goog


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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/6] clk: mediatek: Migrate remaining clk_unregister_*() to clk_hw_unregister_*()
  2022-09-26 10:25 [PATCH 0/6] clk: mediatek: More cleanups Chen-Yu Tsai
  2022-09-26 10:25 ` [PATCH 1/6] clk: mediatek: fix unregister function in mtk_clk_register_dividers cleanup Chen-Yu Tsai
@ 2022-09-26 10:25 ` Chen-Yu Tsai
  2022-09-27 10:40   ` AngeloGioacchino Del Regno
  2022-09-26 10:25 ` [PATCH 3/6] clk: mediatek: mt8192: Do not re-register top_early_divs in probe function Chen-Yu Tsai
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Chen-Yu Tsai @ 2022-09-26 10:25 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: Chen-Yu Tsai, Matthias Brugger, AngeloGioacchino Del Regno,
	Miles Chen, linux-clk, linux-arm-kernel, linux-mediatek,
	linux-kernel

During the previous |struct clk| to |struct clk_hw| clk provider API
migration in commit 6f691a586296 ("clk: mediatek: Switch to clk_hw
provider APIs"), a few clk_unregister_*() calls were missed.

Migrate the remaining ones to the |struct clk_hw| provider API, i.e.
change clk_unregister_*() to clk_hw_unregister_*().

Fixes: 6f691a586296 ("clk: mediatek: Switch to clk_hw provider APIs")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/clk/mediatek/clk-mtk.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
index 174d0645be38..a8ae65302837 100644
--- a/drivers/clk/mediatek/clk-mtk.c
+++ b/drivers/clk/mediatek/clk-mtk.c
@@ -80,7 +80,7 @@ int mtk_clk_register_fixed_clks(const struct mtk_fixed_clk *clks, int num,
 		if (IS_ERR_OR_NULL(clk_data->hws[rc->id]))
 			continue;
 
-		clk_unregister_fixed_rate(clk_data->hws[rc->id]->clk);
+		clk_hw_unregister_fixed_rate(clk_data->hws[rc->id]);
 		clk_data->hws[rc->id] = ERR_PTR(-ENOENT);
 	}
 
@@ -102,7 +102,7 @@ void mtk_clk_unregister_fixed_clks(const struct mtk_fixed_clk *clks, int num,
 		if (IS_ERR_OR_NULL(clk_data->hws[rc->id]))
 			continue;
 
-		clk_unregister_fixed_rate(clk_data->hws[rc->id]->clk);
+		clk_hw_unregister_fixed_rate(clk_data->hws[rc->id]);
 		clk_data->hws[rc->id] = ERR_PTR(-ENOENT);
 	}
 }
@@ -146,7 +146,7 @@ int mtk_clk_register_factors(const struct mtk_fixed_factor *clks, int num,
 		if (IS_ERR_OR_NULL(clk_data->hws[ff->id]))
 			continue;
 
-		clk_unregister_fixed_factor(clk_data->hws[ff->id]->clk);
+		clk_hw_unregister_fixed_factor(clk_data->hws[ff->id]);
 		clk_data->hws[ff->id] = ERR_PTR(-ENOENT);
 	}
 
@@ -168,7 +168,7 @@ void mtk_clk_unregister_factors(const struct mtk_fixed_factor *clks, int num,
 		if (IS_ERR_OR_NULL(clk_data->hws[ff->id]))
 			continue;
 
-		clk_unregister_fixed_factor(clk_data->hws[ff->id]->clk);
+		clk_hw_unregister_fixed_factor(clk_data->hws[ff->id]);
 		clk_data->hws[ff->id] = ERR_PTR(-ENOENT);
 	}
 }
@@ -414,7 +414,7 @@ void mtk_clk_unregister_dividers(const struct mtk_clk_divider *mcds, int num,
 		if (IS_ERR_OR_NULL(clk_data->hws[mcd->id]))
 			continue;
 
-		clk_unregister_divider(clk_data->hws[mcd->id]->clk);
+		clk_hw_unregister_divider(clk_data->hws[mcd->id]);
 		clk_data->hws[mcd->id] = ERR_PTR(-ENOENT);
 	}
 }
-- 
2.37.3.998.g577e59143f-goog


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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/6] clk: mediatek: mt8192: Do not re-register top_early_divs in probe function
  2022-09-26 10:25 [PATCH 0/6] clk: mediatek: More cleanups Chen-Yu Tsai
  2022-09-26 10:25 ` [PATCH 1/6] clk: mediatek: fix unregister function in mtk_clk_register_dividers cleanup Chen-Yu Tsai
  2022-09-26 10:25 ` [PATCH 2/6] clk: mediatek: Migrate remaining clk_unregister_*() to clk_hw_unregister_*() Chen-Yu Tsai
@ 2022-09-26 10:25 ` Chen-Yu Tsai
  2022-09-27 10:39   ` AngeloGioacchino Del Regno
  2022-09-26 10:25 ` [PATCH 4/6] clk: mediatek: mt8192: Avoid duplicate OF clk provider for topckgen Chen-Yu Tsai
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Chen-Yu Tsai @ 2022-09-26 10:25 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: Chen-Yu Tsai, Matthias Brugger, AngeloGioacchino Del Regno,
	Miles Chen, linux-clk, linux-arm-kernel, linux-mediatek,
	linux-kernel

top_early_divs are registered in the CLK_OF_DECLARE_DRIVER() half of the
topckgen clk driver. Don't try to register it again in the actual probe
function. This gets rid of the "Trying to register duplicate clock ..."
warning.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/clk/mediatek/clk-mt8192.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/clk/mediatek/clk-mt8192.c b/drivers/clk/mediatek/clk-mt8192.c
index ebbd2798d9a3..e39012583675 100644
--- a/drivers/clk/mediatek/clk-mt8192.c
+++ b/drivers/clk/mediatek/clk-mt8192.c
@@ -1235,7 +1235,6 @@ static int clk_mt8192_top_probe(struct platform_device *pdev)
 		return PTR_ERR(base);
 
 	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_muxes(top_mtk_muxes, ARRAY_SIZE(top_mtk_muxes), node, &mt8192_clk_lock,
 			       top_clk_data);
-- 
2.37.3.998.g577e59143f-goog


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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/6] clk: mediatek: mt8192: Avoid duplicate OF clk provider for topckgen
  2022-09-26 10:25 [PATCH 0/6] clk: mediatek: More cleanups Chen-Yu Tsai
                   ` (2 preceding siblings ...)
  2022-09-26 10:25 ` [PATCH 3/6] clk: mediatek: mt8192: Do not re-register top_early_divs in probe function Chen-Yu Tsai
@ 2022-09-26 10:25 ` Chen-Yu Tsai
  2022-09-26 10:25 ` [PATCH 5/6] clk: mediatek: mt8192: deduplicate parent clock lists Chen-Yu Tsai
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Chen-Yu Tsai @ 2022-09-26 10:25 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: Chen-Yu Tsai, Matthias Brugger, AngeloGioacchino Del Regno,
	Miles Chen, linux-clk, linux-arm-kernel, linux-mediatek,
	linux-kernel

The MT8192 topckgen clock driver is split into two parts, an early
CLK_OF_DECLARE_DRIVER() part which registers one clock solely for the
system timer, and a standard platform driver part that handles the rest.

In both parts, of_clk_hw_add_provider() is called, causing the clk
provider to be added twice. While this doesn't cause issues, it isn't
clean either.

Remove the existing entry before calling of_clk_hw_add_provider() in
the platform driver probe function. This ensures that there is only
one entry, and the OF related code still runs on the full set of
clocks.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/clk/mediatek/clk-mt8192.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/clk/mediatek/clk-mt8192.c b/drivers/clk/mediatek/clk-mt8192.c
index e39012583675..c2ce72df6db0 100644
--- a/drivers/clk/mediatek/clk-mt8192.c
+++ b/drivers/clk/mediatek/clk-mt8192.c
@@ -1246,6 +1246,12 @@ static int clk_mt8192_top_probe(struct platform_device *pdev)
 	if (r)
 		return r;
 
+	/*
+	 * Remove clock provider set in clk_mt8192_top_init_early() first
+	 * to avoid duplicate entry, and re-add it so the OF related code
+	 * gets run again with the full set of clocks.
+	 */
+	of_clk_del_provider(node);
 	return of_clk_add_hw_provider(node, of_clk_hw_onecell_get,
 				      top_clk_data);
 }
-- 
2.37.3.998.g577e59143f-goog


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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 5/6] clk: mediatek: mt8192: deduplicate parent clock lists
  2022-09-26 10:25 [PATCH 0/6] clk: mediatek: More cleanups Chen-Yu Tsai
                   ` (3 preceding siblings ...)
  2022-09-26 10:25 ` [PATCH 4/6] clk: mediatek: mt8192: Avoid duplicate OF clk provider for topckgen Chen-Yu Tsai
@ 2022-09-26 10:25 ` Chen-Yu Tsai
  2022-09-27 10:39   ` AngeloGioacchino Del Regno
  2022-09-26 10:25 ` [PATCH 6/6] clk: mediatek: mt8192: Implement error handling in probe functions Chen-Yu Tsai
  2022-09-29  4:36 ` [PATCH 0/6] clk: mediatek: More cleanups Chen-Yu Tsai
  6 siblings, 1 reply; 17+ messages in thread
From: Chen-Yu Tsai @ 2022-09-26 10:25 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: Chen-Yu Tsai, Matthias Brugger, AngeloGioacchino Del Regno,
	Miles Chen, linux-clk, linux-arm-kernel, linux-mediatek,
	linux-kernel

Some groups of clocks of the same type share the same list of parents.
These lists were declared separately for each clock in older drivers,
bloating the code.

Merge some obvious duplicate parent clock lists in the MT8192 clock
driver together to reduce the code size. These include:

- apll_i2s*_m_parents into one as apll_i2s_m_parents
- img1_parents & img2_parents into one as img_parents
- msdc30_*_parents into one as msdc30_parents
- camtg*_parents into cam_tg_parents
- seninf*_parents into seninf_parents

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/clk/mediatek/clk-mt8192.c | 206 ++++--------------------------
 1 file changed, 25 insertions(+), 181 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mt8192.c b/drivers/clk/mediatek/clk-mt8192.c
index c2ce72df6db0..d3f57fb73c49 100644
--- a/drivers/clk/mediatek/clk-mt8192.c
+++ b/drivers/clk/mediatek/clk-mt8192.c
@@ -167,22 +167,7 @@ static const char * const mdp_parents[] = {
 	"mmpll_d5_d2"
 };
 
-static const char * const img1_parents[] = {
-	"clk26m",
-	"univpll_d4",
-	"tvdpll_ck",
-	"mainpll_d4",
-	"univpll_d5",
-	"mmpll_d6",
-	"univpll_d6",
-	"mainpll_d6",
-	"mmpll_d4_d2",
-	"mainpll_d4_d2",
-	"mmpll_d6_d2",
-	"mmpll_d5_d2"
-};
-
-static const char * const img2_parents[] = {
+static const char * const img_parents[] = {
 	"clk26m",
 	"univpll_d4",
 	"tvdpll_ck",
@@ -280,61 +265,6 @@ static const char * const camtg_parents[] = {
 	"univpll_192m_d32"
 };
 
-static const char * const camtg2_parents[] = {
-	"clk26m",
-	"univpll_192m_d8",
-	"univpll_d6_d8",
-	"univpll_192m_d4",
-	"univpll_d6_d16",
-	"csw_f26m_d2",
-	"univpll_192m_d16",
-	"univpll_192m_d32"
-};
-
-static const char * const camtg3_parents[] = {
-	"clk26m",
-	"univpll_192m_d8",
-	"univpll_d6_d8",
-	"univpll_192m_d4",
-	"univpll_d6_d16",
-	"csw_f26m_d2",
-	"univpll_192m_d16",
-	"univpll_192m_d32"
-};
-
-static const char * const camtg4_parents[] = {
-	"clk26m",
-	"univpll_192m_d8",
-	"univpll_d6_d8",
-	"univpll_192m_d4",
-	"univpll_d6_d16",
-	"csw_f26m_d2",
-	"univpll_192m_d16",
-	"univpll_192m_d32"
-};
-
-static const char * const camtg5_parents[] = {
-	"clk26m",
-	"univpll_192m_d8",
-	"univpll_d6_d8",
-	"univpll_192m_d4",
-	"univpll_d6_d16",
-	"csw_f26m_d2",
-	"univpll_192m_d16",
-	"univpll_192m_d32"
-};
-
-static const char * const camtg6_parents[] = {
-	"clk26m",
-	"univpll_192m_d8",
-	"univpll_d6_d8",
-	"univpll_192m_d4",
-	"univpll_d6_d16",
-	"csw_f26m_d2",
-	"univpll_192m_d16",
-	"univpll_192m_d32"
-};
-
 static const char * const uart_parents[] = {
 	"clk26m",
 	"univpll_d6_d8"
@@ -362,15 +292,7 @@ static const char * const msdc50_0_parents[] = {
 	"univpll_d4_d2"
 };
 
-static const char * const msdc30_1_parents[] = {
-	"clk26m",
-	"univpll_d6_d2",
-	"mainpll_d6_d2",
-	"mainpll_d7_d2",
-	"msdcpll_d2"
-};
-
-static const char * const msdc30_2_parents[] = {
+static const char * const msdc30_parents[] = {
 	"clk26m",
 	"univpll_d6_d2",
 	"mainpll_d6_d2",
@@ -457,39 +379,6 @@ static const char * const seninf_parents[] = {
 	"univpll_d5"
 };
 
-static const char * const seninf1_parents[] = {
-	"clk26m",
-	"univpll_d4_d4",
-	"univpll_d6_d2",
-	"univpll_d4_d2",
-	"univpll_d7",
-	"univpll_d6",
-	"mmpll_d6",
-	"univpll_d5"
-};
-
-static const char * const seninf2_parents[] = {
-	"clk26m",
-	"univpll_d4_d4",
-	"univpll_d6_d2",
-	"univpll_d4_d2",
-	"univpll_d7",
-	"univpll_d6",
-	"mmpll_d6",
-	"univpll_d5"
-};
-
-static const char * const seninf3_parents[] = {
-	"clk26m",
-	"univpll_d4_d4",
-	"univpll_d6_d2",
-	"univpll_d4_d2",
-	"univpll_d7",
-	"univpll_d6",
-	"mmpll_d6",
-	"univpll_d5"
-};
-
 static const char * const tl_parents[] = {
 	"clk26m",
 	"univpll_192m_d2",
@@ -649,52 +538,7 @@ static const char * const sflash_parents[] = {
 	"univpll_d5_d8"
 };
 
-static const char * const apll_i2s0_m_parents[] = {
-	"aud_1_sel",
-	"aud_2_sel"
-};
-
-static const char * const apll_i2s1_m_parents[] = {
-	"aud_1_sel",
-	"aud_2_sel"
-};
-
-static const char * const apll_i2s2_m_parents[] = {
-	"aud_1_sel",
-	"aud_2_sel"
-};
-
-static const char * const apll_i2s3_m_parents[] = {
-	"aud_1_sel",
-	"aud_2_sel"
-};
-
-static const char * const apll_i2s4_m_parents[] = {
-	"aud_1_sel",
-	"aud_2_sel"
-};
-
-static const char * const apll_i2s5_m_parents[] = {
-	"aud_1_sel",
-	"aud_2_sel"
-};
-
-static const char * const apll_i2s6_m_parents[] = {
-	"aud_1_sel",
-	"aud_2_sel"
-};
-
-static const char * const apll_i2s7_m_parents[] = {
-	"aud_1_sel",
-	"aud_2_sel"
-};
-
-static const char * const apll_i2s8_m_parents[] = {
-	"aud_1_sel",
-	"aud_2_sel"
-};
-
-static const char * const apll_i2s9_m_parents[] = {
+static const char * const apll_i2s_m_parents[] = {
 	"aud_1_sel",
 	"aud_2_sel"
 };
@@ -724,9 +568,9 @@ static const struct mtk_mux top_mtk_muxes[] = {
 	MUX_GATE_CLR_SET_UPD(CLK_TOP_MDP_SEL, "mdp_sel",
 			     mdp_parents, 0x020, 0x024, 0x028, 8, 4, 15, 0x004, 5),
 	MUX_GATE_CLR_SET_UPD(CLK_TOP_IMG1_SEL, "img1_sel",
-			     img1_parents, 0x020, 0x024, 0x028, 16, 4, 23, 0x004, 6),
+			     img_parents, 0x020, 0x024, 0x028, 16, 4, 23, 0x004, 6),
 	MUX_GATE_CLR_SET_UPD(CLK_TOP_IMG2_SEL, "img2_sel",
-			     img2_parents, 0x020, 0x024, 0x028, 24, 4, 31, 0x004, 7),
+			     img_parents, 0x020, 0x024, 0x028, 24, 4, 31, 0x004, 7),
 	/* CLK_CFG_2 */
 	MUX_GATE_CLR_SET_UPD(CLK_TOP_IPE_SEL, "ipe_sel",
 			     ipe_parents, 0x030, 0x034, 0x038, 0, 4, 7, 0x004, 8),
@@ -747,16 +591,16 @@ static const struct mtk_mux top_mtk_muxes[] = {
 			     camtg_parents, 0x050, 0x054, 0x058, 24, 3, 31, 0x004, 19),
 	/* CLK_CFG_5 */
 	MUX_GATE_CLR_SET_UPD(CLK_TOP_CAMTG2_SEL, "camtg2_sel",
-			     camtg2_parents, 0x060, 0x064, 0x068, 0, 3, 7, 0x004, 20),
+			     camtg_parents, 0x060, 0x064, 0x068, 0, 3, 7, 0x004, 20),
 	MUX_GATE_CLR_SET_UPD(CLK_TOP_CAMTG3_SEL, "camtg3_sel",
-			     camtg3_parents, 0x060, 0x064, 0x068, 8, 3, 15, 0x004, 21),
+			     camtg_parents, 0x060, 0x064, 0x068, 8, 3, 15, 0x004, 21),
 	MUX_GATE_CLR_SET_UPD(CLK_TOP_CAMTG4_SEL, "camtg4_sel",
-			     camtg4_parents, 0x060, 0x064, 0x068, 16, 3, 23, 0x004, 22),
+			     camtg_parents, 0x060, 0x064, 0x068, 16, 3, 23, 0x004, 22),
 	MUX_GATE_CLR_SET_UPD(CLK_TOP_CAMTG5_SEL, "camtg5_sel",
-			     camtg5_parents, 0x060, 0x064, 0x068, 24, 3, 31, 0x004, 23),
+			     camtg_parents, 0x060, 0x064, 0x068, 24, 3, 31, 0x004, 23),
 	/* CLK_CFG_6 */
 	MUX_GATE_CLR_SET_UPD(CLK_TOP_CAMTG6_SEL, "camtg6_sel",
-			     camtg6_parents, 0x070, 0x074, 0x078, 0, 3, 7, 0x004, 24),
+			     camtg_parents, 0x070, 0x074, 0x078, 0, 3, 7, 0x004, 24),
 	MUX_GATE_CLR_SET_UPD(CLK_TOP_UART_SEL, "uart_sel",
 			     uart_parents, 0x070, 0x074, 0x078, 8, 1, 15, 0x004, 25),
 	MUX_GATE_CLR_SET_UPD(CLK_TOP_SPI_SEL, "spi_sel",
@@ -767,9 +611,9 @@ static const struct mtk_mux top_mtk_muxes[] = {
 	MUX_GATE_CLR_SET_UPD(CLK_TOP_MSDC50_0_SEL, "msdc50_0_sel",
 			     msdc50_0_parents, 0x080, 0x084, 0x088, 0, 3, 7, 0x004, 28),
 	MUX_GATE_CLR_SET_UPD(CLK_TOP_MSDC30_1_SEL, "msdc30_1_sel",
-			     msdc30_1_parents, 0x080, 0x084, 0x088, 8, 3, 15, 0x004, 29),
+			     msdc30_parents, 0x080, 0x084, 0x088, 8, 3, 15, 0x004, 29),
 	MUX_GATE_CLR_SET_UPD(CLK_TOP_MSDC30_2_SEL, "msdc30_2_sel",
-			     msdc30_2_parents, 0x080, 0x084, 0x088, 16, 3, 23, 0x004, 30),
+			     msdc30_parents, 0x080, 0x084, 0x088, 16, 3, 23, 0x004, 30),
 	MUX_GATE_CLR_SET_UPD(CLK_TOP_AUDIO_SEL, "audio_sel",
 			     audio_parents, 0x080, 0x084, 0x088, 24, 2, 31, 0x008, 0),
 	/* CLK_CFG_8 */
@@ -796,12 +640,12 @@ static const struct mtk_mux top_mtk_muxes[] = {
 	MUX_GATE_CLR_SET_UPD(CLK_TOP_SENINF_SEL, "seninf_sel",
 			     seninf_parents, 0x0b0, 0x0b4, 0x0b8, 16, 3, 23, 0x008, 11),
 	MUX_GATE_CLR_SET_UPD(CLK_TOP_SENINF1_SEL, "seninf1_sel",
-			     seninf1_parents, 0x0b0, 0x0b4, 0x0b8, 24, 3, 31, 0x008, 12),
+			     seninf_parents, 0x0b0, 0x0b4, 0x0b8, 24, 3, 31, 0x008, 12),
 	/* CLK_CFG_11 */
 	MUX_GATE_CLR_SET_UPD(CLK_TOP_SENINF2_SEL, "seninf2_sel",
-			     seninf2_parents, 0x0c0, 0x0c4, 0x0c8, 0, 3, 7, 0x008, 13),
+			     seninf_parents, 0x0c0, 0x0c4, 0x0c8, 0, 3, 7, 0x008, 13),
 	MUX_GATE_CLR_SET_UPD(CLK_TOP_SENINF3_SEL, "seninf3_sel",
-			     seninf3_parents, 0x0c0, 0x0c4, 0x0c8, 8, 3, 15, 0x008, 14),
+			     seninf_parents, 0x0c0, 0x0c4, 0x0c8, 8, 3, 15, 0x008, 14),
 	MUX_GATE_CLR_SET_UPD(CLK_TOP_TL_SEL, "tl_sel",
 			     tl_parents, 0x0c0, 0x0c4, 0x0c8, 16, 2, 23, 0x008, 15),
 	MUX_GATE_CLR_SET_UPD(CLK_TOP_DXCC_SEL, "dxcc_sel",
@@ -847,16 +691,16 @@ static const struct mtk_mux top_mtk_muxes[] = {
 
 static struct mtk_composite top_muxes[] = {
 	/* CLK_AUDDIV_0 */
-	MUX(CLK_TOP_APLL_I2S0_M_SEL, "apll_i2s0_m_sel", apll_i2s0_m_parents, 0x320, 16, 1),
-	MUX(CLK_TOP_APLL_I2S1_M_SEL, "apll_i2s1_m_sel", apll_i2s1_m_parents, 0x320, 17, 1),
-	MUX(CLK_TOP_APLL_I2S2_M_SEL, "apll_i2s2_m_sel", apll_i2s2_m_parents, 0x320, 18, 1),
-	MUX(CLK_TOP_APLL_I2S3_M_SEL, "apll_i2s3_m_sel", apll_i2s3_m_parents, 0x320, 19, 1),
-	MUX(CLK_TOP_APLL_I2S4_M_SEL, "apll_i2s4_m_sel", apll_i2s4_m_parents, 0x320, 20, 1),
-	MUX(CLK_TOP_APLL_I2S5_M_SEL, "apll_i2s5_m_sel", apll_i2s5_m_parents, 0x320, 21, 1),
-	MUX(CLK_TOP_APLL_I2S6_M_SEL, "apll_i2s6_m_sel", apll_i2s6_m_parents, 0x320, 22, 1),
-	MUX(CLK_TOP_APLL_I2S7_M_SEL, "apll_i2s7_m_sel", apll_i2s7_m_parents, 0x320, 23, 1),
-	MUX(CLK_TOP_APLL_I2S8_M_SEL, "apll_i2s8_m_sel", apll_i2s8_m_parents, 0x320, 24, 1),
-	MUX(CLK_TOP_APLL_I2S9_M_SEL, "apll_i2s9_m_sel", apll_i2s9_m_parents, 0x320, 25, 1),
+	MUX(CLK_TOP_APLL_I2S0_M_SEL, "apll_i2s0_m_sel", apll_i2s_m_parents, 0x320, 16, 1),
+	MUX(CLK_TOP_APLL_I2S1_M_SEL, "apll_i2s1_m_sel", apll_i2s_m_parents, 0x320, 17, 1),
+	MUX(CLK_TOP_APLL_I2S2_M_SEL, "apll_i2s2_m_sel", apll_i2s_m_parents, 0x320, 18, 1),
+	MUX(CLK_TOP_APLL_I2S3_M_SEL, "apll_i2s3_m_sel", apll_i2s_m_parents, 0x320, 19, 1),
+	MUX(CLK_TOP_APLL_I2S4_M_SEL, "apll_i2s4_m_sel", apll_i2s_m_parents, 0x320, 20, 1),
+	MUX(CLK_TOP_APLL_I2S5_M_SEL, "apll_i2s5_m_sel", apll_i2s_m_parents, 0x320, 21, 1),
+	MUX(CLK_TOP_APLL_I2S6_M_SEL, "apll_i2s6_m_sel", apll_i2s_m_parents, 0x320, 22, 1),
+	MUX(CLK_TOP_APLL_I2S7_M_SEL, "apll_i2s7_m_sel", apll_i2s_m_parents, 0x320, 23, 1),
+	MUX(CLK_TOP_APLL_I2S8_M_SEL, "apll_i2s8_m_sel", apll_i2s_m_parents, 0x320, 24, 1),
+	MUX(CLK_TOP_APLL_I2S9_M_SEL, "apll_i2s9_m_sel", apll_i2s_m_parents, 0x320, 25, 1),
 };
 
 static const struct mtk_composite top_adj_divs[] = {
-- 
2.37.3.998.g577e59143f-goog


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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 6/6] clk: mediatek: mt8192: Implement error handling in probe functions
  2022-09-26 10:25 [PATCH 0/6] clk: mediatek: More cleanups Chen-Yu Tsai
                   ` (4 preceding siblings ...)
  2022-09-26 10:25 ` [PATCH 5/6] clk: mediatek: mt8192: deduplicate parent clock lists Chen-Yu Tsai
@ 2022-09-26 10:25 ` Chen-Yu Tsai
  2022-09-27 10:39   ` AngeloGioacchino Del Regno
  2022-09-29  4:36 ` [PATCH 0/6] clk: mediatek: More cleanups Chen-Yu Tsai
  6 siblings, 1 reply; 17+ messages in thread
From: Chen-Yu Tsai @ 2022-09-26 10:25 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: Chen-Yu Tsai, Matthias Brugger, AngeloGioacchino Del Regno,
	Miles Chen, linux-clk, linux-arm-kernel, linux-mediatek,
	linux-kernel

This is similar to commit f3e690b00b86 ("clk: mediatek: mt8195:
Implement error handling in probe functions").

Until now the mediatek clk driver library did not have any way to
unregister clks, and so all drivers did not do proper cleanup in
their error paths.

Now that the library does have APIs to unregister clks, use them
in the error path of the probe functions for the mt8192 clk drivers
to do proper cleanup.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/clk/mediatek/clk-mt8192-aud.c | 15 ++++-
 drivers/clk/mediatek/clk-mt8192-mm.c  | 17 +++++-
 drivers/clk/mediatek/clk-mt8192.c     | 83 +++++++++++++++++++++------
 3 files changed, 93 insertions(+), 22 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mt8192-aud.c b/drivers/clk/mediatek/clk-mt8192-aud.c
index 8c989bffd8c7..825b80fc403d 100644
--- a/drivers/clk/mediatek/clk-mt8192-aud.c
+++ b/drivers/clk/mediatek/clk-mt8192-aud.c
@@ -89,15 +89,24 @@ static int clk_mt8192_aud_probe(struct platform_device *pdev)
 
 	r = mtk_clk_register_gates(node, aud_clks, ARRAY_SIZE(aud_clks), clk_data);
 	if (r)
-		return r;
+		goto free_data;
 
 	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
 	if (r)
-		return r;
+		goto unregister_gates;
 
 	r = devm_of_platform_populate(&pdev->dev);
 	if (r)
-		of_clk_del_provider(node);
+		goto remove_provider;
+
+	return 0;
+
+remove_provider:
+	of_clk_del_provider(node);
+unregister_gates:
+	mtk_clk_unregister_gates(aud_clks, ARRAY_SIZE(aud_clks), clk_data);
+free_data:
+	mtk_free_clk_data(clk_data);
 
 	return r;
 }
diff --git a/drivers/clk/mediatek/clk-mt8192-mm.c b/drivers/clk/mediatek/clk-mt8192-mm.c
index 1be3ff4d407d..4c90e0cd9f7c 100644
--- a/drivers/clk/mediatek/clk-mt8192-mm.c
+++ b/drivers/clk/mediatek/clk-mt8192-mm.c
@@ -93,9 +93,22 @@ static int clk_mt8192_mm_probe(struct platform_device *pdev)
 
 	r = mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks), clk_data);
 	if (r)
-		return r;
+		goto free_clk_data;
 
-	return of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
+	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
+	if (r)
+		goto unregister_gates;
+
+	platform_set_drvdata(pdev, clk_data);
+
+	return 0;
+
+unregister_gates:
+	mtk_clk_unregister_gates(mm_clks, ARRAY_SIZE(mm_clks), clk_data);
+free_clk_data:
+	mtk_free_clk_data(clk_data);
+
+	return r;
 }
 
 static struct platform_driver clk_mt8192_mm_drv = {
diff --git a/drivers/clk/mediatek/clk-mt8192.c b/drivers/clk/mediatek/clk-mt8192.c
index d3f57fb73c49..94aab61193a0 100644
--- a/drivers/clk/mediatek/clk-mt8192.c
+++ b/drivers/clk/mediatek/clk-mt8192.c
@@ -1078,26 +1078,64 @@ static int clk_mt8192_top_probe(struct platform_device *pdev)
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
-	mtk_clk_register_fixed_clks(top_fixed_clks, ARRAY_SIZE(top_fixed_clks), top_clk_data);
-	mtk_clk_register_factors(top_divs, ARRAY_SIZE(top_divs), top_clk_data);
-	mtk_clk_register_muxes(top_mtk_muxes, ARRAY_SIZE(top_mtk_muxes), node, &mt8192_clk_lock,
-			       top_clk_data);
-	mtk_clk_register_composites(top_muxes, ARRAY_SIZE(top_muxes), base, &mt8192_clk_lock,
-				    top_clk_data);
-	mtk_clk_register_composites(top_adj_divs, ARRAY_SIZE(top_adj_divs), base, &mt8192_clk_lock,
-				    top_clk_data);
-	r = mtk_clk_register_gates(node, top_clks, ARRAY_SIZE(top_clks), top_clk_data);
+	r = mtk_clk_register_fixed_clks(top_fixed_clks, ARRAY_SIZE(top_fixed_clks), top_clk_data);
 	if (r)
 		return r;
 
+	r = mtk_clk_register_factors(top_divs, ARRAY_SIZE(top_divs), top_clk_data);
+	if (r)
+		goto unregister_fixed;
+
+	r = mtk_clk_register_muxes(top_mtk_muxes, ARRAY_SIZE(top_mtk_muxes), node,
+				   &mt8192_clk_lock, top_clk_data);
+	if (r)
+		goto unregister_factors;
+
+	r = mtk_clk_register_composites(top_muxes, ARRAY_SIZE(top_muxes), base, &mt8192_clk_lock,
+					top_clk_data);
+	if (r)
+		goto unregister_mtk_muxes;
+
+	r = mtk_clk_register_composites(top_adj_divs, ARRAY_SIZE(top_adj_divs), base,
+					&mt8192_clk_lock, top_clk_data);
+	if (r)
+		goto unregister_muxes;
+
+	r = mtk_clk_register_gates(node, top_clks, ARRAY_SIZE(top_clks), top_clk_data);
+	if (r)
+		goto unregister_adj_divs;
+
 	/*
 	 * Remove clock provider set in clk_mt8192_top_init_early() first
 	 * to avoid duplicate entry, and re-add it so the OF related code
 	 * gets run again with the full set of clocks.
 	 */
 	of_clk_del_provider(node);
-	return of_clk_add_hw_provider(node, of_clk_hw_onecell_get,
-				      top_clk_data);
+	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, top_clk_data);
+	if (r)
+		goto unregister_gates;
+
+	return 0;
+
+unregister_gates:
+	mtk_clk_unregister_gates(top_clks, ARRAY_SIZE(top_clks), top_clk_data);
+unregister_adj_divs:
+	mtk_clk_unregister_composites(top_adj_divs, ARRAY_SIZE(top_adj_divs), top_clk_data);
+unregister_muxes:
+	mtk_clk_unregister_composites(top_muxes, ARRAY_SIZE(top_muxes), top_clk_data);
+unregister_mtk_muxes:
+	mtk_clk_unregister_muxes(top_mtk_muxes, ARRAY_SIZE(top_mtk_muxes), top_clk_data);
+unregister_factors:
+	mtk_clk_unregister_factors(top_divs, ARRAY_SIZE(top_divs), top_clk_data);
+unregister_fixed:
+	mtk_clk_unregister_fixed_clks(top_fixed_clks, ARRAY_SIZE(top_fixed_clks), top_clk_data);
+	/*
+	 * top_clk_data is not freed, as it is not allocated by the probe
+	 * function, and it is potentially still used through the
+	 * of_clk_add_hw_provider() call in clk_mt8192_top_init_early().
+	 */
+
+	return r;
 }
 
 static int clk_mt8192_infra_probe(struct platform_device *pdev)
@@ -1116,14 +1154,16 @@ static int clk_mt8192_infra_probe(struct platform_device *pdev)
 
 	r = mtk_register_reset_controller_with_dev(&pdev->dev, &clk_rst_desc);
 	if (r)
-		goto free_clk_data;
+		goto unregister_gates;
 
 	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
 	if (r)
-		goto free_clk_data;
+		goto unregister_gates;
 
 	return r;
 
+unregister_gates:
+	mtk_clk_unregister_gates(infra_clks, ARRAY_SIZE(infra_clks), clk_data);
 free_clk_data:
 	mtk_free_clk_data(clk_data);
 	return r;
@@ -1145,10 +1185,12 @@ static int clk_mt8192_peri_probe(struct platform_device *pdev)
 
 	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
 	if (r)
-		goto free_clk_data;
+		goto unregister_gates;
 
 	return r;
 
+unregister_gates:
+	mtk_clk_unregister_gates(peri_clks, ARRAY_SIZE(peri_clks), clk_data);
 free_clk_data:
 	mtk_free_clk_data(clk_data);
 	return r;
@@ -1164,17 +1206,24 @@ static int clk_mt8192_apmixed_probe(struct platform_device *pdev)
 	if (!clk_data)
 		return -ENOMEM;
 
-	mtk_clk_register_plls(node, plls, ARRAY_SIZE(plls), clk_data);
-	r = mtk_clk_register_gates(node, apmixed_clks, ARRAY_SIZE(apmixed_clks), clk_data);
+	r = mtk_clk_register_plls(node, plls, ARRAY_SIZE(plls), clk_data);
 	if (r)
 		goto free_clk_data;
 
+	r = mtk_clk_register_gates(node, apmixed_clks, ARRAY_SIZE(apmixed_clks), clk_data);
+	if (r)
+		goto unregister_plls;
+
 	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
 	if (r)
-		goto free_clk_data;
+		goto unregister_gates;
 
 	return r;
 
+unregister_gates:
+	mtk_clk_unregister_gates(apmixed_clks, ARRAY_SIZE(apmixed_clks), clk_data);
+unregister_plls:
+	mtk_clk_unregister_plls(plls, ARRAY_SIZE(plls), clk_data);
 free_clk_data:
 	mtk_free_clk_data(clk_data);
 	return r;
-- 
2.37.3.998.g577e59143f-goog


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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 6/6] clk: mediatek: mt8192: Implement error handling in probe functions
  2022-09-26 10:25 ` [PATCH 6/6] clk: mediatek: mt8192: Implement error handling in probe functions Chen-Yu Tsai
@ 2022-09-27 10:39   ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 17+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-09-27 10:39 UTC (permalink / raw)
  To: Chen-Yu Tsai, Michael Turquette, Stephen Boyd
  Cc: Matthias Brugger, Miles Chen, linux-clk, linux-arm-kernel,
	linux-mediatek, linux-kernel

Il 26/09/22 12:25, Chen-Yu Tsai ha scritto:
> This is similar to commit f3e690b00b86 ("clk: mediatek: mt8195:
> Implement error handling in probe functions").
> 
> Until now the mediatek clk driver library did not have any way to
> unregister clks, and so all drivers did not do proper cleanup in
> their error paths.
> 
> Now that the library does have APIs to unregister clks, use them
> in the error path of the probe functions for the mt8192 clk drivers
> to do proper cleanup.
> 
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

I can't give you my Reviewed-by on this patch only because of the comment
in patch [3/6] - anyway, LGTM.

Regards,
Angelo


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/6] clk: mediatek: mt8192: Do not re-register top_early_divs in probe function
  2022-09-26 10:25 ` [PATCH 3/6] clk: mediatek: mt8192: Do not re-register top_early_divs in probe function Chen-Yu Tsai
@ 2022-09-27 10:39   ` AngeloGioacchino Del Regno
  2022-09-28  1:55     ` Miles Chen
  2022-09-28  6:51     ` Chen-Yu Tsai
  0 siblings, 2 replies; 17+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-09-27 10:39 UTC (permalink / raw)
  To: Chen-Yu Tsai, Michael Turquette, Stephen Boyd
  Cc: Matthias Brugger, Miles Chen, linux-clk, linux-arm-kernel,
	linux-mediatek, linux-kernel

Il 26/09/22 12:25, Chen-Yu Tsai ha scritto:
> top_early_divs are registered in the CLK_OF_DECLARE_DRIVER() half of the
> topckgen clk driver. Don't try to register it again in the actual probe
> function. This gets rid of the "Trying to register duplicate clock ..."
> warning.
> 
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

Can't we simply remove the CLK_OF_DECLARE_DRIVER() and top_init_early entirely,
and transfer TOP_CSW_F26M_D2 to top_divs[] instead?
I get that systimer concern and we have something similar in MT8195, where the
TOP_CLK26M_D2 is registered "late".

Getting back to MT8192, TOP_CSW_F26M_D2 seems to be used only for:
1. systimer
2. SPMI MST (registered "late").

Being it a fixed factor clock, parented to another fixed clock, it doesn't
even have any ON/OFF switch, so I think it would be actually possible to go
for the proposed removal... which would further improve this cleanup.

Regards,
Angelo



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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/6] clk: mediatek: mt8192: deduplicate parent clock lists
  2022-09-26 10:25 ` [PATCH 5/6] clk: mediatek: mt8192: deduplicate parent clock lists Chen-Yu Tsai
@ 2022-09-27 10:39   ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 17+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-09-27 10:39 UTC (permalink / raw)
  To: Chen-Yu Tsai, Michael Turquette, Stephen Boyd
  Cc: Matthias Brugger, Miles Chen, linux-clk, linux-arm-kernel,
	linux-mediatek, linux-kernel

Il 26/09/22 12:25, Chen-Yu Tsai ha scritto:
> Some groups of clocks of the same type share the same list of parents.
> These lists were declared separately for each clock in older drivers,
> bloating the code.
> 
> Merge some obvious duplicate parent clock lists in the MT8192 clock
> driver together to reduce the code size. These include:
> 
> - apll_i2s*_m_parents into one as apll_i2s_m_parents
> - img1_parents & img2_parents into one as img_parents
> - msdc30_*_parents into one as msdc30_parents
> - camtg*_parents into cam_tg_parents
> - seninf*_parents into seninf_parents
> 
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

Makes a lot of sense, agreed.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>



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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/6] clk: mediatek: fix unregister function in mtk_clk_register_dividers cleanup
  2022-09-26 10:25 ` [PATCH 1/6] clk: mediatek: fix unregister function in mtk_clk_register_dividers cleanup Chen-Yu Tsai
@ 2022-09-27 10:39   ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 17+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-09-27 10:39 UTC (permalink / raw)
  To: Chen-Yu Tsai, Michael Turquette, Stephen Boyd
  Cc: Matthias Brugger, Miles Chen, linux-clk, linux-arm-kernel,
	linux-mediatek, linux-kernel

Il 26/09/22 12:25, Chen-Yu Tsai ha scritto:
> When the cleanup paths for the various clk register APIs in the MediaTek
> clk library were added, the one in the dividers type used the wrong type
> of unregister function. This would result in incorrect dereferencing of
> the clk pointer and freeing of invalid pointers.
> 
> Fix this by switching to the correct type of clk unregistration call.
> 
> Fixes: 3c3ba2ab0226 ("clk: mediatek: mtk: Implement error handling in register APIs")
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>



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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/6] clk: mediatek: Migrate remaining clk_unregister_*() to clk_hw_unregister_*()
  2022-09-26 10:25 ` [PATCH 2/6] clk: mediatek: Migrate remaining clk_unregister_*() to clk_hw_unregister_*() Chen-Yu Tsai
@ 2022-09-27 10:40   ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 17+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-09-27 10:40 UTC (permalink / raw)
  To: Chen-Yu Tsai, Michael Turquette, Stephen Boyd
  Cc: Matthias Brugger, Miles Chen, linux-clk, linux-arm-kernel,
	linux-mediatek, linux-kernel

Il 26/09/22 12:25, Chen-Yu Tsai ha scritto:
> During the previous |struct clk| to |struct clk_hw| clk provider API
> migration in commit 6f691a586296 ("clk: mediatek: Switch to clk_hw
> provider APIs"), a few clk_unregister_*() calls were missed.
> 
> Migrate the remaining ones to the |struct clk_hw| provider API, i.e.
> change clk_unregister_*() to clk_hw_unregister_*().
> 
> Fixes: 6f691a586296 ("clk: mediatek: Switch to clk_hw provider APIs")
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>



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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/6] clk: mediatek: mt8192: Do not re-register top_early_divs in probe function
  2022-09-27 10:39   ` AngeloGioacchino Del Regno
@ 2022-09-28  1:55     ` Miles Chen
  2022-09-28  7:50       ` Chen-Yu Tsai
  2022-09-28  6:51     ` Chen-Yu Tsai
  1 sibling, 1 reply; 17+ messages in thread
From: Miles Chen @ 2022-09-28  1:55 UTC (permalink / raw)
  To: angelogioacchino.delregno
  Cc: linux-arm-kernel, linux-clk, linux-kernel, linux-mediatek,
	matthias.bgg, miles.chen, mturquette, sboyd, wenst

>> top_early_divs are registered in the CLK_OF_DECLARE_DRIVER() half of the
>> topckgen clk driver. Don't try to register it again in the actual probe
>> function. This gets rid of the "Trying to register duplicate clock ..."
>> warning.
>> 
>> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
>
>Can't we simply remove the CLK_OF_DECLARE_DRIVER() and top_init_early entirely,
>and transfer TOP_CSW_F26M_D2 to top_divs[] instead?
>I get that systimer concern and we have something similar in MT8195, where the
>TOP_CLK26M_D2 is registered "late".

Another reason for this:
Removing the CLK_OF_DECLARE_DRIVER() is good when we want to build our driver as
kernel modules because it does not work with kernel modules.

thanks,
Miles
>
>Getting back to MT8192, TOP_CSW_F26M_D2 seems to be used only for:
>1. systimer
>2. SPMI MST (registered "late").
>
>Being it a fixed factor clock, parented to another fixed clock, it doesn't
>even have any ON/OFF switch, so I think it would be actually possible to go
>for the proposed removal... which would further improve this cleanup.
>
>Regards,
>Angelo
>

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/6] clk: mediatek: mt8192: Do not re-register top_early_divs in probe function
  2022-09-27 10:39   ` AngeloGioacchino Del Regno
  2022-09-28  1:55     ` Miles Chen
@ 2022-09-28  6:51     ` Chen-Yu Tsai
  1 sibling, 0 replies; 17+ messages in thread
From: Chen-Yu Tsai @ 2022-09-28  6:51 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno
  Cc: Michael Turquette, Stephen Boyd, Matthias Brugger, Miles Chen,
	linux-clk, linux-arm-kernel, linux-mediatek, linux-kernel

On Tue, Sep 27, 2022 at 6:39 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 26/09/22 12:25, Chen-Yu Tsai ha scritto:
> > top_early_divs are registered in the CLK_OF_DECLARE_DRIVER() half of the
> > topckgen clk driver. Don't try to register it again in the actual probe
> > function. This gets rid of the "Trying to register duplicate clock ..."
> > warning.
> >
> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
>
> Can't we simply remove the CLK_OF_DECLARE_DRIVER() and top_init_early entirely,
> and transfer TOP_CSW_F26M_D2 to top_divs[] instead?
> I get that systimer concern and we have something similar in MT8195, where the
> TOP_CLK26M_D2 is registered "late".

That was what I initially wanted to do. However I asked MTK whether the
system would work fully without systimer, and apparently it is used during
suspend (presumably it is supposed to be running?), so making it not
functional was a bit concerning.

That said, I do plan to rework the systimer stuff. The /2 divider is
actually internal to the systimer block, and should not have been modeled
the way it is now. Notably, the divider is actually variable. It is
only configured and locked by the bootloader.

For this I think we have two options:

a. Move the /2 fixed factor clock into a standalone device node, like
   what was done for the MT8195

b. Rework the systimer to internalize the divider, and thus moving the
   systimer input clock to osc26M.

Either one is outside the scope of this series. Option a. works especially
well for MT8192, as the configurable divider was removed from the systimer
block (only for this chip).

> Getting back to MT8192, TOP_CSW_F26M_D2 seems to be used only for:
> 1. systimer
> 2. SPMI MST (registered "late").
>
> Being it a fixed factor clock, parented to another fixed clock, it doesn't
> even have any ON/OFF switch, so I think it would be actually possible to go
> for the proposed removal... which would further improve this cleanup.

As mentioned above, I do have some plans to rework the stuff, but it is
kind of beyond the scope of this series, as it changes the device tree
binding and ABI.

Regards
ChenYu

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/6] clk: mediatek: mt8192: Do not re-register top_early_divs in probe function
  2022-09-28  1:55     ` Miles Chen
@ 2022-09-28  7:50       ` Chen-Yu Tsai
  2022-09-30  7:09         ` Miles Chen
  0 siblings, 1 reply; 17+ messages in thread
From: Chen-Yu Tsai @ 2022-09-28  7:50 UTC (permalink / raw)
  To: Miles Chen
  Cc: angelogioacchino.delregno, linux-arm-kernel, linux-clk,
	linux-kernel, linux-mediatek, matthias.bgg, mturquette, sboyd

On Wed, Sep 28, 2022 at 9:55 AM Miles Chen <miles.chen@mediatek.com> wrote:
>
> >> top_early_divs are registered in the CLK_OF_DECLARE_DRIVER() half of the
> >> topckgen clk driver. Don't try to register it again in the actual probe
> >> function. This gets rid of the "Trying to register duplicate clock ..."
> >> warning.
> >>
> >> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> >
> >Can't we simply remove the CLK_OF_DECLARE_DRIVER() and top_init_early entirely,
> >and transfer TOP_CSW_F26M_D2 to top_divs[] instead?
> >I get that systimer concern and we have something similar in MT8195, where the
> >TOP_CLK26M_D2 is registered "late".
>
> Another reason for this:
> Removing the CLK_OF_DECLARE_DRIVER() is good when we want to build our driver as
> kernel modules because it does not work with kernel modules.

I agree. But as I mentioned in my other reply, we need to fix the clock
user first before dropping that clock. And there's also the matter of
DT backward compatibility. So we need to do it incrementally.


ChenYu

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/6] clk: mediatek: More cleanups
  2022-09-26 10:25 [PATCH 0/6] clk: mediatek: More cleanups Chen-Yu Tsai
                   ` (5 preceding siblings ...)
  2022-09-26 10:25 ` [PATCH 6/6] clk: mediatek: mt8192: Implement error handling in probe functions Chen-Yu Tsai
@ 2022-09-29  4:36 ` Chen-Yu Tsai
  6 siblings, 0 replies; 17+ messages in thread
From: Chen-Yu Tsai @ 2022-09-29  4:36 UTC (permalink / raw)
  To: Stephen Boyd, AngeloGioacchino Del Regno
  Cc: Michael Turquette, Matthias Brugger, Miles Chen, linux-clk,
	linux-arm-kernel, linux-mediatek, linux-kernel

On Mon, Sep 26, 2022 at 6:25 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
>
> Here's some more cleanups for the MedaiTek clk drivers.
>
> Patches 1 and 2 fixes and cleans up some parts that were missed in the
> previous clk_hw conversion series.
>
> Patch 3 drops the duplicate registration of the top_early_divs clks for
> MT8192. Currently this only contains a fixed divider.
>
> Patch 4 prevents having a duplicate OF clk provider for the topckgen
> controller, but removing the previously registered instance.
>
> Patch 5 deduplicates the parent clock lists for MT8192.
>
> Patch 6 adds proper error handling to the clock probe functions for
> MT8192.
>
>
> Please have a look.
>
>
> Thanks
> ChenYu
>
>
> Chen-Yu Tsai (6):
>   clk: mediatek: fix unregister function in mtk_clk_register_dividers
>     cleanup
>   clk: mediatek: Migrate remaining clk_unregister_*() to
>     clk_hw_unregister_*()
>   clk: mediatek: mt8192: Do not re-register top_early_divs in probe
>     function
>   clk: mediatek: mt8192: Avoid duplicate OF clk provider for topckgen
>   clk: mediatek: mt8192: deduplicate parent clock lists
>   clk: mediatek: mt8192: Implement error handling in probe functions

I've queued patches 1, 2 and 5 up here [1] and will send a pull request
to the clock maintainer later this week.

The remaining three are related and still under discussion.


ChenYu

[1] https://git.kernel.org/pub/scm/linux/kernel/git/wens/linux.git/log/?h=clk-mtk-for-6.1

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/6] clk: mediatek: mt8192: Do not re-register top_early_divs in probe function
  2022-09-28  7:50       ` Chen-Yu Tsai
@ 2022-09-30  7:09         ` Miles Chen
  0 siblings, 0 replies; 17+ messages in thread
From: Miles Chen @ 2022-09-30  7:09 UTC (permalink / raw)
  To: wenst
  Cc: angelogioacchino.delregno, linux-arm-kernel, linux-clk,
	linux-kernel, linux-mediatek, matthias.bgg, miles.chen,
	mturquette, sboyd

>On Wed, Sep 28, 2022 at 9:55 AM Miles Chen <miles.chen@mediatek.com> wrote:
>>
>> >> top_early_divs are registered in the CLK_OF_DECLARE_DRIVER() half of the
>> >> topckgen clk driver. Don't try to register it again in the actual probe
>> >> function. This gets rid of the "Trying to register duplicate clock ..."
>> >> warning.
>> >>
>> >> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
>> >
>> >Can't we simply remove the CLK_OF_DECLARE_DRIVER() and top_init_early entirely,
>> >and transfer TOP_CSW_F26M_D2 to top_divs[] instead?
>> >I get that systimer concern and we have something similar in MT8195, where the
>> >TOP_CLK26M_D2 is registered "late".
>>
>> Another reason for this:
>> Removing the CLK_OF_DECLARE_DRIVER() is good when we want to build our driver as
>> kernel modules because it does not work with kernel modules.
>
>I agree. But as I mentioned in my other reply, we need to fix the clock
>user first before dropping that clock. And there's also the matter of
>DT backward compatibility. So we need to do it incrementally.
>
>
>ChenYu

Got it. thanks for doing this.

thanks,
Miles

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2022-09-30  8:01 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-26 10:25 [PATCH 0/6] clk: mediatek: More cleanups Chen-Yu Tsai
2022-09-26 10:25 ` [PATCH 1/6] clk: mediatek: fix unregister function in mtk_clk_register_dividers cleanup Chen-Yu Tsai
2022-09-27 10:39   ` AngeloGioacchino Del Regno
2022-09-26 10:25 ` [PATCH 2/6] clk: mediatek: Migrate remaining clk_unregister_*() to clk_hw_unregister_*() Chen-Yu Tsai
2022-09-27 10:40   ` AngeloGioacchino Del Regno
2022-09-26 10:25 ` [PATCH 3/6] clk: mediatek: mt8192: Do not re-register top_early_divs in probe function Chen-Yu Tsai
2022-09-27 10:39   ` AngeloGioacchino Del Regno
2022-09-28  1:55     ` Miles Chen
2022-09-28  7:50       ` Chen-Yu Tsai
2022-09-30  7:09         ` Miles Chen
2022-09-28  6:51     ` Chen-Yu Tsai
2022-09-26 10:25 ` [PATCH 4/6] clk: mediatek: mt8192: Avoid duplicate OF clk provider for topckgen Chen-Yu Tsai
2022-09-26 10:25 ` [PATCH 5/6] clk: mediatek: mt8192: deduplicate parent clock lists Chen-Yu Tsai
2022-09-27 10:39   ` AngeloGioacchino Del Regno
2022-09-26 10:25 ` [PATCH 6/6] clk: mediatek: mt8192: Implement error handling in probe functions Chen-Yu Tsai
2022-09-27 10:39   ` AngeloGioacchino Del Regno
2022-09-29  4:36 ` [PATCH 0/6] clk: mediatek: More cleanups Chen-Yu Tsai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).