[PATCH 31/31] clk: mediatek: Warn if clk IDs are duplicated

Chen-Yu Tsai posted 31 patches 2 years, 9 months ago
There is a newer version of this series
[PATCH 31/31] clk: mediatek: Warn if clk IDs are duplicated
Posted by Chen-Yu Tsai 2 years, 9 months ago
The Mediatek clk driver library handles duplicate clock IDs in two
different ways: either ignoring the duplicate entry, or overwriting
the old clk. Either way may cause unexpected behavior, and the latter
also causes an orphan clk that cannot be cleaned up.

Align the behavior so that later duplicate entries are ignored, and
a warning printed. The warning will also aid in making the issue
noticeable.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/clk/mediatek/clk-cpumux.c |  6 ++++++
 drivers/clk/mediatek/clk-gate.c   |  5 ++++-
 drivers/clk/mediatek/clk-mtk.c    | 18 ++++++++++++++----
 drivers/clk/mediatek/clk-mux.c    |  5 ++++-
 drivers/clk/mediatek/clk-pll.c    |  6 ++++++
 5 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/mediatek/clk-cpumux.c b/drivers/clk/mediatek/clk-cpumux.c
index 499c60432280..c11b3fae622e 100644
--- a/drivers/clk/mediatek/clk-cpumux.c
+++ b/drivers/clk/mediatek/clk-cpumux.c
@@ -120,6 +120,12 @@ int mtk_clk_register_cpumuxes(struct device_node *node,
 	for (i = 0; i < num; i++) {
 		const struct mtk_composite *mux = &clks[i];
 
+		if (!IS_ERR_OR_NULL(clk_data->clks[mux->id])) {
+			pr_warn("%pOF: Trying to register duplicate clock ID: %d\n",
+				node, mux->id);
+			continue;
+		}
+
 		clk = mtk_clk_register_cpumux(mux, regmap);
 		if (IS_ERR(clk)) {
 			pr_err("Failed to register clk %s: %pe\n", mux->name, clk);
diff --git a/drivers/clk/mediatek/clk-gate.c b/drivers/clk/mediatek/clk-gate.c
index 631ff170b7b9..da52023f8455 100644
--- a/drivers/clk/mediatek/clk-gate.c
+++ b/drivers/clk/mediatek/clk-gate.c
@@ -224,8 +224,11 @@ int mtk_clk_register_gates_with_dev(struct device_node *node,
 	for (i = 0; i < num; i++) {
 		const struct mtk_gate *gate = &clks[i];
 
-		if (!IS_ERR_OR_NULL(clk_data->clks[gate->id]))
+		if (!IS_ERR_OR_NULL(clk_data->clks[gate->id])) {
+			pr_warn("%pOF: Trying to register duplicate clock ID: %d\n",
+				node, gate->id);
 			continue;
+		}
 
 		clk = mtk_clk_register_gate(gate->name, gate->parent_name,
 					    regmap,
diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
index 6d0b8842971b..b2a3568922b2 100644
--- a/drivers/clk/mediatek/clk-mtk.c
+++ b/drivers/clk/mediatek/clk-mtk.c
@@ -65,8 +65,10 @@ int mtk_clk_register_fixed_clks(const struct mtk_fixed_clk *clks, int num,
 	for (i = 0; i < num; i++) {
 		const struct mtk_fixed_clk *rc = &clks[i];
 
-		if (!IS_ERR_OR_NULL(clk_data->clks[rc->id]))
+		if (!IS_ERR_OR_NULL(clk_data->clks[rc->id])) {
+			pr_warn("Trying to register duplicate clock ID: %d\n", rc->id);
 			continue;
+		}
 
 		clk = clk_register_fixed_rate(NULL, rc->name, rc->parent, 0,
 					      rc->rate);
@@ -128,8 +130,10 @@ int mtk_clk_register_factors(const struct mtk_fixed_factor *clks, int num,
 	for (i = 0; i < num; i++) {
 		const struct mtk_fixed_factor *ff = &clks[i];
 
-		if (!IS_ERR_OR_NULL(clk_data->clks[ff->id]))
+		if (!IS_ERR_OR_NULL(clk_data->clks[ff->id])) {
+			pr_warn("Trying to register duplicate clock ID: %d\n", ff->id);
 			continue;
+		}
 
 		clk = clk_register_fixed_factor(NULL, ff->name, ff->parent_name,
 				CLK_SET_RATE_PARENT, ff->mult, ff->div);
@@ -300,8 +304,11 @@ int mtk_clk_register_composites(const struct mtk_composite *mcs, int num,
 	for (i = 0; i < num; i++) {
 		const struct mtk_composite *mc = &mcs[i];
 
-		if (clk_data && !IS_ERR_OR_NULL(clk_data->clks[mc->id]))
+		if (!IS_ERR_OR_NULL(clk_data->clks[mc->id])) {
+			pr_warn("Trying to register duplicate clock ID: %d\n",
+				mc->id);
 			continue;
+		}
 
 		clk = mtk_clk_register_composite(mc, base, lock);
 
@@ -363,8 +370,11 @@ int mtk_clk_register_dividers(const struct mtk_clk_divider *mcds, int num,
 	for (i = 0; i <  num; i++) {
 		const struct mtk_clk_divider *mcd = &mcds[i];
 
-		if (!IS_ERR_OR_NULL(clk_data->clks[mcd->id]))
+		if (!IS_ERR_OR_NULL(clk_data->clks[mcd->id])) {
+			pr_warn("Trying to register duplicate clock ID: %d\n",
+				mcd->id);
 			continue;
+		}
 
 		clk = clk_register_divider(NULL, mcd->name, mcd->parent_name,
 			mcd->flags, base +  mcd->div_reg, mcd->div_shift,
diff --git a/drivers/clk/mediatek/clk-mux.c b/drivers/clk/mediatek/clk-mux.c
index f51e67650f03..21ad5a4afd65 100644
--- a/drivers/clk/mediatek/clk-mux.c
+++ b/drivers/clk/mediatek/clk-mux.c
@@ -208,8 +208,11 @@ int mtk_clk_register_muxes(const struct mtk_mux *muxes,
 	for (i = 0; i < num; i++) {
 		const struct mtk_mux *mux = &muxes[i];
 
-		if (!IS_ERR_OR_NULL(clk_data->clks[mux->id]))
+		if (!IS_ERR_OR_NULL(clk_data->clks[mux->id])) {
+			pr_warn("%pOF: Trying to register duplicate clock ID: %d\n",
+				node, mux->id);
 			continue;
+		}
 
 		clk = mtk_clk_register_mux(mux, regmap, lock);
 
diff --git a/drivers/clk/mediatek/clk-pll.c b/drivers/clk/mediatek/clk-pll.c
index 1dd15f560659..e5e9c188be99 100644
--- a/drivers/clk/mediatek/clk-pll.c
+++ b/drivers/clk/mediatek/clk-pll.c
@@ -386,6 +386,12 @@ int mtk_clk_register_plls(struct device_node *node,
 	for (i = 0; i < num_plls; i++) {
 		const struct mtk_pll_data *pll = &plls[i];
 
+		if (!IS_ERR_OR_NULL(clk_data->clks[pll->id])) {
+			pr_warn("%pOF: Trying to register duplicate clock ID: %d\n",
+				node, pll->id);
+			continue;
+		}
+
 		clk = mtk_clk_register_pll(pll, base);
 
 		if (IS_ERR(clk)) {
-- 
2.35.0.rc0.227.g00780c9af4-goog

Re: [PATCH 31/31] clk: mediatek: Warn if clk IDs are duplicated
Posted by Miles Chen 2 years, 9 months ago
>The Mediatek clk driver library handles duplicate clock IDs in two
>different ways: either ignoring the duplicate entry, or overwriting
>the old clk. Either way may cause unexpected behavior, and the latter
>also causes an orphan clk that cannot be cleaned up.
>
>Align the behavior so that later duplicate entries are ignored, and
>a warning printed. The warning will also aid in making the issue
>noticeable.
>
>Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
>---
> drivers/clk/mediatek/clk-cpumux.c |  6 ++++++
> drivers/clk/mediatek/clk-gate.c   |  5 ++++-
> drivers/clk/mediatek/clk-mtk.c    | 18 ++++++++++++++----
> drivers/clk/mediatek/clk-mux.c    |  5 ++++-
> drivers/clk/mediatek/clk-pll.c    |  6 ++++++
> 5 files changed, 34 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/clk/mediatek/clk-cpumux.c b/drivers/clk/mediatek/clk-cpumux.c
>index 499c60432280..c11b3fae622e 100644
>--- a/drivers/clk/mediatek/clk-cpumux.c
>+++ b/drivers/clk/mediatek/clk-cpumux.c
>@@ -120,6 +120,12 @@ int mtk_clk_register_cpumuxes(struct device_node *node,
> 	for (i = 0; i < num; i++) {
> 		const struct mtk_composite *mux = &clks[i];
> 
>+		if (!IS_ERR_OR_NULL(clk_data->clks[mux->id])) {
>+			pr_warn("%pOF: Trying to register duplicate clock ID: %d\n",
>+				node, mux->id);
>+			continue;

%pOF is an useful information when this happens.

Reviewed-by: Miles Chen <miles.chen@mediatek.com>

>+		}
>+
> 		clk = mtk_clk_register_cpumux(mux, regmap);
> 		if (IS_ERR(clk)) {
> 			pr_err("Failed to register clk %s: %pe\n", mux->name, clk);
>diff --git a/drivers/clk/mediatek/clk-gate.c b/drivers/clk/mediatek/clk-gate.c
>index 631ff170b7b9..da52023f8455 100644
>--- a/drivers/clk/mediatek/clk-gate.c
>+++ b/drivers/clk/mediatek/clk-gate.c
>@@ -224,8 +224,11 @@ int mtk_clk_register_gates_with_dev(struct device_node *node,
> 	for (i = 0; i < num; i++) {
> 		const struct mtk_gate *gate = &clks[i];
> 
>-		if (!IS_ERR_OR_NULL(clk_data->clks[gate->id]))
>+		if (!IS_ERR_OR_NULL(clk_data->clks[gate->id])) {
>+			pr_warn("%pOF: Trying to register duplicate clock ID: %d\n",
>+				node, gate->id);
> 			continue;
>+		}
> 
> 		clk = mtk_clk_register_gate(gate->name, gate->parent_name,
> 					    regmap,
>diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
>index 6d0b8842971b..b2a3568922b2 100644
>--- a/drivers/clk/mediatek/clk-mtk.c
>+++ b/drivers/clk/mediatek/clk-mtk.c
>@@ -65,8 +65,10 @@ int mtk_clk_register_fixed_clks(const struct mtk_fixed_clk *clks, int num,
> 	for (i = 0; i < num; i++) {
> 		const struct mtk_fixed_clk *rc = &clks[i];
> 
>-		if (!IS_ERR_OR_NULL(clk_data->clks[rc->id]))
>+		if (!IS_ERR_OR_NULL(clk_data->clks[rc->id])) {
>+			pr_warn("Trying to register duplicate clock ID: %d\n", rc->id);
> 			continue;
>+		}
> 
> 		clk = clk_register_fixed_rate(NULL, rc->name, rc->parent, 0,
> 					      rc->rate);
>@@ -128,8 +130,10 @@ int mtk_clk_register_factors(const struct mtk_fixed_factor *clks, int num,
> 	for (i = 0; i < num; i++) {
> 		const struct mtk_fixed_factor *ff = &clks[i];
> 
>-		if (!IS_ERR_OR_NULL(clk_data->clks[ff->id]))
>+		if (!IS_ERR_OR_NULL(clk_data->clks[ff->id])) {
>+			pr_warn("Trying to register duplicate clock ID: %d\n", ff->id);
> 			continue;
>+		}
> 
> 		clk = clk_register_fixed_factor(NULL, ff->name, ff->parent_name,
> 				CLK_SET_RATE_PARENT, ff->mult, ff->div);
>@@ -300,8 +304,11 @@ int mtk_clk_register_composites(const struct mtk_composite *mcs, int num,
> 	for (i = 0; i < num; i++) {
> 		const struct mtk_composite *mc = &mcs[i];
> 
>-		if (clk_data && !IS_ERR_OR_NULL(clk_data->clks[mc->id]))
>+		if (!IS_ERR_OR_NULL(clk_data->clks[mc->id])) {
>+			pr_warn("Trying to register duplicate clock ID: %d\n",
>+				mc->id);
> 			continue;
>+		}
> 
> 		clk = mtk_clk_register_composite(mc, base, lock);
> 
>@@ -363,8 +370,11 @@ int mtk_clk_register_dividers(const struct mtk_clk_divider *mcds, int num,
> 	for (i = 0; i <  num; i++) {
> 		const struct mtk_clk_divider *mcd = &mcds[i];
> 
>-		if (!IS_ERR_OR_NULL(clk_data->clks[mcd->id]))
>+		if (!IS_ERR_OR_NULL(clk_data->clks[mcd->id])) {
>+			pr_warn("Trying to register duplicate clock ID: %d\n",
>+				mcd->id);
> 			continue;
>+		}
> 
> 		clk = clk_register_divider(NULL, mcd->name, mcd->parent_name,
> 			mcd->flags, base +  mcd->div_reg, mcd->div_shift,
>diff --git a/drivers/clk/mediatek/clk-mux.c b/drivers/clk/mediatek/clk-mux.c
>index f51e67650f03..21ad5a4afd65 100644
>--- a/drivers/clk/mediatek/clk-mux.c
>+++ b/drivers/clk/mediatek/clk-mux.c
>@@ -208,8 +208,11 @@ int mtk_clk_register_muxes(const struct mtk_mux *muxes,
> 	for (i = 0; i < num; i++) {
> 		const struct mtk_mux *mux = &muxes[i];
> 
>-		if (!IS_ERR_OR_NULL(clk_data->clks[mux->id]))
>+		if (!IS_ERR_OR_NULL(clk_data->clks[mux->id])) {
>+			pr_warn("%pOF: Trying to register duplicate clock ID: %d\n",
>+				node, mux->id);
> 			continue;
>+		}
> 
> 		clk = mtk_clk_register_mux(mux, regmap, lock);
> 
>diff --git a/drivers/clk/mediatek/clk-pll.c b/drivers/clk/mediatek/clk-pll.c
>index 1dd15f560659..e5e9c188be99 100644
>--- a/drivers/clk/mediatek/clk-pll.c
>+++ b/drivers/clk/mediatek/clk-pll.c
>@@ -386,6 +386,12 @@ int mtk_clk_register_plls(struct device_node *node,
> 	for (i = 0; i < num_plls; i++) {
> 		const struct mtk_pll_data *pll = &plls[i];
> 
>+		if (!IS_ERR_OR_NULL(clk_data->clks[pll->id])) {
>+			pr_warn("%pOF: Trying to register duplicate clock ID: %d\n",
>+				node, pll->id);
>+			continue;
>+		}
>+
> 		clk = mtk_clk_register_pll(pll, base);
> 
> 		if (IS_ERR(clk)) {
>-- 
>2.35.0.rc0.227.g00780c9af4-goog
>
>