* [PATCH] clk: zynqmp: warn always when a clock op fails
@ 2022-01-12 14:12 ` Michael Tretter
0 siblings, 0 replies; 8+ messages in thread
From: Michael Tretter @ 2022-01-12 14:12 UTC (permalink / raw)
To: linux-clk, linux-arm-kernel
Cc: mturquette, sboyd, michal.simek, rajan.vaja, kernel, m.tretter
The warning that a clock operation failed is only printed once. However,
the function is called for various different clocks. The limit hides
warnings if different clock are affected by the failures.
Print the warning every time when a clock operation fails.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
drivers/clk/zynqmp/clk-gate-zynqmp.c | 12 +++++------
drivers/clk/zynqmp/clk-mux-zynqmp.c | 8 +++----
drivers/clk/zynqmp/divider.c | 12 +++++------
drivers/clk/zynqmp/pll.c | 32 ++++++++++++++--------------
4 files changed, 32 insertions(+), 32 deletions(-)
diff --git a/drivers/clk/zynqmp/clk-gate-zynqmp.c b/drivers/clk/zynqmp/clk-gate-zynqmp.c
index 565ed67a0430..0d9a39110f29 100644
--- a/drivers/clk/zynqmp/clk-gate-zynqmp.c
+++ b/drivers/clk/zynqmp/clk-gate-zynqmp.c
@@ -41,8 +41,8 @@ static int zynqmp_clk_gate_enable(struct clk_hw *hw)
ret = zynqmp_pm_clock_enable(clk_id);
if (ret)
- pr_warn_once("%s() clock enabled failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() clock enable failed for %s (id %d), ret = %d\n",
+ __func__, clk_name, clk_id, ret);
return ret;
}
@@ -61,8 +61,8 @@ static void zynqmp_clk_gate_disable(struct clk_hw *hw)
ret = zynqmp_pm_clock_disable(clk_id);
if (ret)
- pr_warn_once("%s() clock disable failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() clock disable failed for %s (id %d), ret = %d\n",
+ __func__, clk_name, clk_id, ret);
}
/**
@@ -80,8 +80,8 @@ static int zynqmp_clk_gate_is_enabled(struct clk_hw *hw)
ret = zynqmp_pm_clock_getstate(clk_id, &state);
if (ret) {
- pr_warn_once("%s() clock get state failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() clock get state failed for %s, ret = %d\n",
+ __func__, clk_name, ret);
return -EIO;
}
diff --git a/drivers/clk/zynqmp/clk-mux-zynqmp.c b/drivers/clk/zynqmp/clk-mux-zynqmp.c
index 17afce594f28..6bc16500231e 100644
--- a/drivers/clk/zynqmp/clk-mux-zynqmp.c
+++ b/drivers/clk/zynqmp/clk-mux-zynqmp.c
@@ -51,8 +51,8 @@ static u8 zynqmp_clk_mux_get_parent(struct clk_hw *hw)
ret = zynqmp_pm_clock_getparent(clk_id, &val);
if (ret) {
- pr_warn_once("%s() getparent failed for clock: %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() getparent failed for clock: %s, ret = %d\n",
+ __func__, clk_name, ret);
/*
* clk_core_get_parent_by_index() takes num_parents as incorrect
* index which is exactly what I want to return here
@@ -80,8 +80,8 @@ static int zynqmp_clk_mux_set_parent(struct clk_hw *hw, u8 index)
ret = zynqmp_pm_clock_setparent(clk_id, index);
if (ret)
- pr_warn_once("%s() set parent failed for clock: %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() set parent failed for clock: %s, ret = %d\n",
+ __func__, clk_name, ret);
return ret;
}
diff --git a/drivers/clk/zynqmp/divider.c b/drivers/clk/zynqmp/divider.c
index cb49281f9cf9..c6d6a2fb5866 100644
--- a/drivers/clk/zynqmp/divider.c
+++ b/drivers/clk/zynqmp/divider.c
@@ -89,8 +89,8 @@ static unsigned long zynqmp_clk_divider_recalc_rate(struct clk_hw *hw,
ret = zynqmp_pm_clock_getdivider(clk_id, &div);
if (ret)
- pr_warn_once("%s() get divider failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() get divider failed for %s, ret = %d\n",
+ __func__, clk_name, ret);
if (div_type == TYPE_DIV1)
value = div & 0xFFFF;
@@ -177,8 +177,8 @@ static long zynqmp_clk_divider_round_rate(struct clk_hw *hw,
ret = zynqmp_pm_clock_getdivider(clk_id, &bestdiv);
if (ret)
- pr_warn_once("%s() get divider failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() get divider failed for %s, ret = %d\n",
+ __func__, clk_name, ret);
if (div_type == TYPE_DIV1)
bestdiv = bestdiv & 0xFFFF;
else
@@ -244,8 +244,8 @@ static int zynqmp_clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
ret = zynqmp_pm_clock_setdivider(clk_id, div);
if (ret)
- pr_warn_once("%s() set divider failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() set divider failed for %s, ret = %d\n",
+ __func__, clk_name, ret);
return ret;
}
diff --git a/drivers/clk/zynqmp/pll.c b/drivers/clk/zynqmp/pll.c
index 036e4ff64a2f..6c4dfae3df1d 100644
--- a/drivers/clk/zynqmp/pll.c
+++ b/drivers/clk/zynqmp/pll.c
@@ -56,8 +56,8 @@ static inline enum pll_mode zynqmp_pll_get_mode(struct clk_hw *hw)
ret = zynqmp_pm_get_pll_frac_mode(clk_id, ret_payload);
if (ret) {
- pr_warn_once("%s() PLL get frac mode failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() PLL get frac mode failed for %s, ret = %d\n",
+ __func__, clk_name, ret);
return PLL_MODE_ERROR;
}
@@ -84,8 +84,8 @@ static inline void zynqmp_pll_set_mode(struct clk_hw *hw, bool on)
ret = zynqmp_pm_set_pll_frac_mode(clk_id, mode);
if (ret)
- pr_warn_once("%s() PLL set frac mode failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() PLL set frac mode failed for %s, ret = %d\n",
+ __func__, clk_name, ret);
else
clk->set_pll_mode = true;
}
@@ -145,8 +145,8 @@ static unsigned long zynqmp_pll_recalc_rate(struct clk_hw *hw,
ret = zynqmp_pm_clock_getdivider(clk_id, &fbdiv);
if (ret) {
- pr_warn_once("%s() get divider failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() get divider failed for %s, ret = %d\n",
+ __func__, clk_name, ret);
return 0ul;
}
@@ -200,8 +200,8 @@ static int zynqmp_pll_set_rate(struct clk_hw *hw, unsigned long rate,
WARN(1, "More than allowed devices are using the %s, which is forbidden\n",
clk_name);
else if (ret)
- pr_warn_once("%s() set divider failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() set divider failed for %s, ret = %d\n",
+ __func__, clk_name, ret);
zynqmp_pm_set_pll_frac_data(clk_id, f);
return rate + frac;
@@ -211,8 +211,8 @@ static int zynqmp_pll_set_rate(struct clk_hw *hw, unsigned long rate,
fbdiv = clamp_t(u32, fbdiv, PLL_FBDIV_MIN, PLL_FBDIV_MAX);
ret = zynqmp_pm_clock_setdivider(clk_id, fbdiv);
if (ret)
- pr_warn_once("%s() set divider failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() set divider failed for %s, ret = %d\n",
+ __func__, clk_name, ret);
return parent_rate * fbdiv;
}
@@ -233,8 +233,8 @@ static int zynqmp_pll_is_enabled(struct clk_hw *hw)
ret = zynqmp_pm_clock_getstate(clk_id, &state);
if (ret) {
- pr_warn_once("%s() clock get state failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() clock get state failed for %s, ret = %d\n",
+ __func__, clk_name, ret);
return -EIO;
}
@@ -265,8 +265,8 @@ static int zynqmp_pll_enable(struct clk_hw *hw)
ret = zynqmp_pm_clock_enable(clk_id);
if (ret)
- pr_warn_once("%s() clock enable failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() clock enable failed for %s, ret = %d\n",
+ __func__, clk_name, ret);
return ret;
}
@@ -287,8 +287,8 @@ static void zynqmp_pll_disable(struct clk_hw *hw)
ret = zynqmp_pm_clock_disable(clk_id);
if (ret)
- pr_warn_once("%s() clock disable failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() clock disable failed for %s, ret = %d\n",
+ __func__, clk_name, ret);
}
static const struct clk_ops zynqmp_pll_ops = {
--
2.30.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH] clk: zynqmp: warn always when a clock op fails
@ 2022-01-12 14:12 ` Michael Tretter
0 siblings, 0 replies; 8+ messages in thread
From: Michael Tretter @ 2022-01-12 14:12 UTC (permalink / raw)
To: linux-clk, linux-arm-kernel
Cc: mturquette, sboyd, michal.simek, rajan.vaja, kernel, m.tretter
The warning that a clock operation failed is only printed once. However,
the function is called for various different clocks. The limit hides
warnings if different clock are affected by the failures.
Print the warning every time when a clock operation fails.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
drivers/clk/zynqmp/clk-gate-zynqmp.c | 12 +++++------
drivers/clk/zynqmp/clk-mux-zynqmp.c | 8 +++----
drivers/clk/zynqmp/divider.c | 12 +++++------
drivers/clk/zynqmp/pll.c | 32 ++++++++++++++--------------
4 files changed, 32 insertions(+), 32 deletions(-)
diff --git a/drivers/clk/zynqmp/clk-gate-zynqmp.c b/drivers/clk/zynqmp/clk-gate-zynqmp.c
index 565ed67a0430..0d9a39110f29 100644
--- a/drivers/clk/zynqmp/clk-gate-zynqmp.c
+++ b/drivers/clk/zynqmp/clk-gate-zynqmp.c
@@ -41,8 +41,8 @@ static int zynqmp_clk_gate_enable(struct clk_hw *hw)
ret = zynqmp_pm_clock_enable(clk_id);
if (ret)
- pr_warn_once("%s() clock enabled failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() clock enable failed for %s (id %d), ret = %d\n",
+ __func__, clk_name, clk_id, ret);
return ret;
}
@@ -61,8 +61,8 @@ static void zynqmp_clk_gate_disable(struct clk_hw *hw)
ret = zynqmp_pm_clock_disable(clk_id);
if (ret)
- pr_warn_once("%s() clock disable failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() clock disable failed for %s (id %d), ret = %d\n",
+ __func__, clk_name, clk_id, ret);
}
/**
@@ -80,8 +80,8 @@ static int zynqmp_clk_gate_is_enabled(struct clk_hw *hw)
ret = zynqmp_pm_clock_getstate(clk_id, &state);
if (ret) {
- pr_warn_once("%s() clock get state failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() clock get state failed for %s, ret = %d\n",
+ __func__, clk_name, ret);
return -EIO;
}
diff --git a/drivers/clk/zynqmp/clk-mux-zynqmp.c b/drivers/clk/zynqmp/clk-mux-zynqmp.c
index 17afce594f28..6bc16500231e 100644
--- a/drivers/clk/zynqmp/clk-mux-zynqmp.c
+++ b/drivers/clk/zynqmp/clk-mux-zynqmp.c
@@ -51,8 +51,8 @@ static u8 zynqmp_clk_mux_get_parent(struct clk_hw *hw)
ret = zynqmp_pm_clock_getparent(clk_id, &val);
if (ret) {
- pr_warn_once("%s() getparent failed for clock: %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() getparent failed for clock: %s, ret = %d\n",
+ __func__, clk_name, ret);
/*
* clk_core_get_parent_by_index() takes num_parents as incorrect
* index which is exactly what I want to return here
@@ -80,8 +80,8 @@ static int zynqmp_clk_mux_set_parent(struct clk_hw *hw, u8 index)
ret = zynqmp_pm_clock_setparent(clk_id, index);
if (ret)
- pr_warn_once("%s() set parent failed for clock: %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() set parent failed for clock: %s, ret = %d\n",
+ __func__, clk_name, ret);
return ret;
}
diff --git a/drivers/clk/zynqmp/divider.c b/drivers/clk/zynqmp/divider.c
index cb49281f9cf9..c6d6a2fb5866 100644
--- a/drivers/clk/zynqmp/divider.c
+++ b/drivers/clk/zynqmp/divider.c
@@ -89,8 +89,8 @@ static unsigned long zynqmp_clk_divider_recalc_rate(struct clk_hw *hw,
ret = zynqmp_pm_clock_getdivider(clk_id, &div);
if (ret)
- pr_warn_once("%s() get divider failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() get divider failed for %s, ret = %d\n",
+ __func__, clk_name, ret);
if (div_type == TYPE_DIV1)
value = div & 0xFFFF;
@@ -177,8 +177,8 @@ static long zynqmp_clk_divider_round_rate(struct clk_hw *hw,
ret = zynqmp_pm_clock_getdivider(clk_id, &bestdiv);
if (ret)
- pr_warn_once("%s() get divider failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() get divider failed for %s, ret = %d\n",
+ __func__, clk_name, ret);
if (div_type == TYPE_DIV1)
bestdiv = bestdiv & 0xFFFF;
else
@@ -244,8 +244,8 @@ static int zynqmp_clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
ret = zynqmp_pm_clock_setdivider(clk_id, div);
if (ret)
- pr_warn_once("%s() set divider failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() set divider failed for %s, ret = %d\n",
+ __func__, clk_name, ret);
return ret;
}
diff --git a/drivers/clk/zynqmp/pll.c b/drivers/clk/zynqmp/pll.c
index 036e4ff64a2f..6c4dfae3df1d 100644
--- a/drivers/clk/zynqmp/pll.c
+++ b/drivers/clk/zynqmp/pll.c
@@ -56,8 +56,8 @@ static inline enum pll_mode zynqmp_pll_get_mode(struct clk_hw *hw)
ret = zynqmp_pm_get_pll_frac_mode(clk_id, ret_payload);
if (ret) {
- pr_warn_once("%s() PLL get frac mode failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() PLL get frac mode failed for %s, ret = %d\n",
+ __func__, clk_name, ret);
return PLL_MODE_ERROR;
}
@@ -84,8 +84,8 @@ static inline void zynqmp_pll_set_mode(struct clk_hw *hw, bool on)
ret = zynqmp_pm_set_pll_frac_mode(clk_id, mode);
if (ret)
- pr_warn_once("%s() PLL set frac mode failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() PLL set frac mode failed for %s, ret = %d\n",
+ __func__, clk_name, ret);
else
clk->set_pll_mode = true;
}
@@ -145,8 +145,8 @@ static unsigned long zynqmp_pll_recalc_rate(struct clk_hw *hw,
ret = zynqmp_pm_clock_getdivider(clk_id, &fbdiv);
if (ret) {
- pr_warn_once("%s() get divider failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() get divider failed for %s, ret = %d\n",
+ __func__, clk_name, ret);
return 0ul;
}
@@ -200,8 +200,8 @@ static int zynqmp_pll_set_rate(struct clk_hw *hw, unsigned long rate,
WARN(1, "More than allowed devices are using the %s, which is forbidden\n",
clk_name);
else if (ret)
- pr_warn_once("%s() set divider failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() set divider failed for %s, ret = %d\n",
+ __func__, clk_name, ret);
zynqmp_pm_set_pll_frac_data(clk_id, f);
return rate + frac;
@@ -211,8 +211,8 @@ static int zynqmp_pll_set_rate(struct clk_hw *hw, unsigned long rate,
fbdiv = clamp_t(u32, fbdiv, PLL_FBDIV_MIN, PLL_FBDIV_MAX);
ret = zynqmp_pm_clock_setdivider(clk_id, fbdiv);
if (ret)
- pr_warn_once("%s() set divider failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() set divider failed for %s, ret = %d\n",
+ __func__, clk_name, ret);
return parent_rate * fbdiv;
}
@@ -233,8 +233,8 @@ static int zynqmp_pll_is_enabled(struct clk_hw *hw)
ret = zynqmp_pm_clock_getstate(clk_id, &state);
if (ret) {
- pr_warn_once("%s() clock get state failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() clock get state failed for %s, ret = %d\n",
+ __func__, clk_name, ret);
return -EIO;
}
@@ -265,8 +265,8 @@ static int zynqmp_pll_enable(struct clk_hw *hw)
ret = zynqmp_pm_clock_enable(clk_id);
if (ret)
- pr_warn_once("%s() clock enable failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() clock enable failed for %s, ret = %d\n",
+ __func__, clk_name, ret);
return ret;
}
@@ -287,8 +287,8 @@ static void zynqmp_pll_disable(struct clk_hw *hw)
ret = zynqmp_pm_clock_disable(clk_id);
if (ret)
- pr_warn_once("%s() clock disable failed for %s, ret = %d\n",
- __func__, clk_name, ret);
+ pr_warn("%s() clock disable failed for %s, ret = %d\n",
+ __func__, clk_name, ret);
}
static const struct clk_ops zynqmp_pll_ops = {
--
2.30.2
_______________________________________________
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] 8+ messages in thread* Re: [PATCH] clk: zynqmp: warn always when a clock op fails
2022-01-12 14:12 ` Michael Tretter
@ 2022-01-12 20:29 ` Stephen Boyd
-1 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2022-01-12 20:29 UTC (permalink / raw)
To: Michael Tretter, linux-arm-kernel, linux-clk
Cc: mturquette, michal.simek, rajan.vaja, kernel, m.tretter
Quoting Michael Tretter (2022-01-12 06:12:29)
> The warning that a clock operation failed is only printed once. However,
> the function is called for various different clocks. The limit hides
> warnings if different clock are affected by the failures.
>
> Print the warning every time when a clock operation fails.
>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> drivers/clk/zynqmp/clk-gate-zynqmp.c | 12 +++++------
> drivers/clk/zynqmp/clk-mux-zynqmp.c | 8 +++----
> drivers/clk/zynqmp/divider.c | 12 +++++------
> drivers/clk/zynqmp/pll.c | 32 ++++++++++++++--------------
> 4 files changed, 32 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/clk/zynqmp/clk-gate-zynqmp.c b/drivers/clk/zynqmp/clk-gate-zynqmp.c
> index 565ed67a0430..0d9a39110f29 100644
> --- a/drivers/clk/zynqmp/clk-gate-zynqmp.c
> +++ b/drivers/clk/zynqmp/clk-gate-zynqmp.c
> @@ -41,8 +41,8 @@ static int zynqmp_clk_gate_enable(struct clk_hw *hw)
> ret = zynqmp_pm_clock_enable(clk_id);
>
> if (ret)
> - pr_warn_once("%s() clock enabled failed for %s, ret = %d\n",
> - __func__, clk_name, ret);
> + pr_warn("%s() clock enable failed for %s (id %d), ret = %d\n",
> + __func__, clk_name, clk_id, ret);
Can we just remove these prints entirely? The driver that calls
clk_enable() should be checking the return value and taking proper
action. What is the user going to do with these messages?
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] clk: zynqmp: warn always when a clock op fails
@ 2022-01-12 20:29 ` Stephen Boyd
0 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2022-01-12 20:29 UTC (permalink / raw)
To: Michael Tretter, linux-arm-kernel, linux-clk
Cc: mturquette, michal.simek, rajan.vaja, kernel, m.tretter
Quoting Michael Tretter (2022-01-12 06:12:29)
> The warning that a clock operation failed is only printed once. However,
> the function is called for various different clocks. The limit hides
> warnings if different clock are affected by the failures.
>
> Print the warning every time when a clock operation fails.
>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> drivers/clk/zynqmp/clk-gate-zynqmp.c | 12 +++++------
> drivers/clk/zynqmp/clk-mux-zynqmp.c | 8 +++----
> drivers/clk/zynqmp/divider.c | 12 +++++------
> drivers/clk/zynqmp/pll.c | 32 ++++++++++++++--------------
> 4 files changed, 32 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/clk/zynqmp/clk-gate-zynqmp.c b/drivers/clk/zynqmp/clk-gate-zynqmp.c
> index 565ed67a0430..0d9a39110f29 100644
> --- a/drivers/clk/zynqmp/clk-gate-zynqmp.c
> +++ b/drivers/clk/zynqmp/clk-gate-zynqmp.c
> @@ -41,8 +41,8 @@ static int zynqmp_clk_gate_enable(struct clk_hw *hw)
> ret = zynqmp_pm_clock_enable(clk_id);
>
> if (ret)
> - pr_warn_once("%s() clock enabled failed for %s, ret = %d\n",
> - __func__, clk_name, ret);
> + pr_warn("%s() clock enable failed for %s (id %d), ret = %d\n",
> + __func__, clk_name, clk_id, ret);
Can we just remove these prints entirely? The driver that calls
clk_enable() should be checking the return value and taking proper
action. What is the user going to do with these messages?
_______________________________________________
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] 8+ messages in thread* Re: [PATCH] clk: zynqmp: warn always when a clock op fails
2022-01-12 20:29 ` Stephen Boyd
@ 2022-01-14 8:14 ` Michael Tretter
-1 siblings, 0 replies; 8+ messages in thread
From: Michael Tretter @ 2022-01-14 8:14 UTC (permalink / raw)
To: Stephen Boyd
Cc: linux-arm-kernel, linux-clk, mturquette, michal.simek,
rajan.vaja, kernel
On Wed, 12 Jan 2022 12:29:35 -0800, Stephen Boyd wrote:
> Quoting Michael Tretter (2022-01-12 06:12:29)
> > The warning that a clock operation failed is only printed once. However,
> > the function is called for various different clocks. The limit hides
> > warnings if different clock are affected by the failures.
> >
> > Print the warning every time when a clock operation fails.
> >
> > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > ---
> > drivers/clk/zynqmp/clk-gate-zynqmp.c | 12 +++++------
> > drivers/clk/zynqmp/clk-mux-zynqmp.c | 8 +++----
> > drivers/clk/zynqmp/divider.c | 12 +++++------
> > drivers/clk/zynqmp/pll.c | 32 ++++++++++++++--------------
> > 4 files changed, 32 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/clk/zynqmp/clk-gate-zynqmp.c b/drivers/clk/zynqmp/clk-gate-zynqmp.c
> > index 565ed67a0430..0d9a39110f29 100644
> > --- a/drivers/clk/zynqmp/clk-gate-zynqmp.c
> > +++ b/drivers/clk/zynqmp/clk-gate-zynqmp.c
> > @@ -41,8 +41,8 @@ static int zynqmp_clk_gate_enable(struct clk_hw *hw)
> > ret = zynqmp_pm_clock_enable(clk_id);
> >
> > if (ret)
> > - pr_warn_once("%s() clock enabled failed for %s, ret = %d\n",
> > - __func__, clk_name, ret);
> > + pr_warn("%s() clock enable failed for %s (id %d), ret = %d\n",
> > + __func__, clk_name, clk_id, ret);
>
> Can we just remove these prints entirely? The driver that calls
> clk_enable() should be checking the return value and taking proper
> action. What is the user going to do with these messages?
>
The clocks are handled by a firmware, which checks if the Linux system has the
permission to change certain clocks. The warnings help users to identify an
unexpected configuration of the firmware. However, I guess it would make sense
to change the warnings to pr_debug.
Michael
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] clk: zynqmp: warn always when a clock op fails
@ 2022-01-14 8:14 ` Michael Tretter
0 siblings, 0 replies; 8+ messages in thread
From: Michael Tretter @ 2022-01-14 8:14 UTC (permalink / raw)
To: Stephen Boyd
Cc: linux-arm-kernel, linux-clk, mturquette, michal.simek,
rajan.vaja, kernel
On Wed, 12 Jan 2022 12:29:35 -0800, Stephen Boyd wrote:
> Quoting Michael Tretter (2022-01-12 06:12:29)
> > The warning that a clock operation failed is only printed once. However,
> > the function is called for various different clocks. The limit hides
> > warnings if different clock are affected by the failures.
> >
> > Print the warning every time when a clock operation fails.
> >
> > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > ---
> > drivers/clk/zynqmp/clk-gate-zynqmp.c | 12 +++++------
> > drivers/clk/zynqmp/clk-mux-zynqmp.c | 8 +++----
> > drivers/clk/zynqmp/divider.c | 12 +++++------
> > drivers/clk/zynqmp/pll.c | 32 ++++++++++++++--------------
> > 4 files changed, 32 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/clk/zynqmp/clk-gate-zynqmp.c b/drivers/clk/zynqmp/clk-gate-zynqmp.c
> > index 565ed67a0430..0d9a39110f29 100644
> > --- a/drivers/clk/zynqmp/clk-gate-zynqmp.c
> > +++ b/drivers/clk/zynqmp/clk-gate-zynqmp.c
> > @@ -41,8 +41,8 @@ static int zynqmp_clk_gate_enable(struct clk_hw *hw)
> > ret = zynqmp_pm_clock_enable(clk_id);
> >
> > if (ret)
> > - pr_warn_once("%s() clock enabled failed for %s, ret = %d\n",
> > - __func__, clk_name, ret);
> > + pr_warn("%s() clock enable failed for %s (id %d), ret = %d\n",
> > + __func__, clk_name, clk_id, ret);
>
> Can we just remove these prints entirely? The driver that calls
> clk_enable() should be checking the return value and taking proper
> action. What is the user going to do with these messages?
>
The clocks are handled by a firmware, which checks if the Linux system has the
permission to change certain clocks. The warnings help users to identify an
unexpected configuration of the firmware. However, I guess it would make sense
to change the warnings to pr_debug.
Michael
_______________________________________________
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] 8+ messages in thread* Re: [PATCH] clk: zynqmp: warn always when a clock op fails
2022-01-14 8:14 ` Michael Tretter
@ 2022-01-14 22:20 ` Stephen Boyd
-1 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2022-01-14 22:20 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-arm-kernel, linux-clk, mturquette, michal.simek,
rajan.vaja, kernel
Quoting Michael Tretter (2022-01-14 00:14:42)
> On Wed, 12 Jan 2022 12:29:35 -0800, Stephen Boyd wrote:
> > Quoting Michael Tretter (2022-01-12 06:12:29)
> > > The warning that a clock operation failed is only printed once. However,
> > > the function is called for various different clocks. The limit hides
> > > warnings if different clock are affected by the failures.
> > >
> > > Print the warning every time when a clock operation fails.
> > >
> > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > > ---
> > > drivers/clk/zynqmp/clk-gate-zynqmp.c | 12 +++++------
> > > drivers/clk/zynqmp/clk-mux-zynqmp.c | 8 +++----
> > > drivers/clk/zynqmp/divider.c | 12 +++++------
> > > drivers/clk/zynqmp/pll.c | 32 ++++++++++++++--------------
> > > 4 files changed, 32 insertions(+), 32 deletions(-)
> > >
> > > diff --git a/drivers/clk/zynqmp/clk-gate-zynqmp.c b/drivers/clk/zynqmp/clk-gate-zynqmp.c
> > > index 565ed67a0430..0d9a39110f29 100644
> > > --- a/drivers/clk/zynqmp/clk-gate-zynqmp.c
> > > +++ b/drivers/clk/zynqmp/clk-gate-zynqmp.c
> > > @@ -41,8 +41,8 @@ static int zynqmp_clk_gate_enable(struct clk_hw *hw)
> > > ret = zynqmp_pm_clock_enable(clk_id);
> > >
> > > if (ret)
> > > - pr_warn_once("%s() clock enabled failed for %s, ret = %d\n",
> > > - __func__, clk_name, ret);
> > > + pr_warn("%s() clock enable failed for %s (id %d), ret = %d\n",
> > > + __func__, clk_name, clk_id, ret);
> >
> > Can we just remove these prints entirely? The driver that calls
> > clk_enable() should be checking the return value and taking proper
> > action. What is the user going to do with these messages?
> >
>
> The clocks are handled by a firmware, which checks if the Linux system has the
> permission to change certain clocks. The warnings help users to identify an
> unexpected configuration of the firmware. However, I guess it would make sense
> to change the warnings to pr_debug.
>
Sure pr_debug() sounds fine.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] clk: zynqmp: warn always when a clock op fails
@ 2022-01-14 22:20 ` Stephen Boyd
0 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2022-01-14 22:20 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-arm-kernel, linux-clk, mturquette, michal.simek,
rajan.vaja, kernel
Quoting Michael Tretter (2022-01-14 00:14:42)
> On Wed, 12 Jan 2022 12:29:35 -0800, Stephen Boyd wrote:
> > Quoting Michael Tretter (2022-01-12 06:12:29)
> > > The warning that a clock operation failed is only printed once. However,
> > > the function is called for various different clocks. The limit hides
> > > warnings if different clock are affected by the failures.
> > >
> > > Print the warning every time when a clock operation fails.
> > >
> > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > > ---
> > > drivers/clk/zynqmp/clk-gate-zynqmp.c | 12 +++++------
> > > drivers/clk/zynqmp/clk-mux-zynqmp.c | 8 +++----
> > > drivers/clk/zynqmp/divider.c | 12 +++++------
> > > drivers/clk/zynqmp/pll.c | 32 ++++++++++++++--------------
> > > 4 files changed, 32 insertions(+), 32 deletions(-)
> > >
> > > diff --git a/drivers/clk/zynqmp/clk-gate-zynqmp.c b/drivers/clk/zynqmp/clk-gate-zynqmp.c
> > > index 565ed67a0430..0d9a39110f29 100644
> > > --- a/drivers/clk/zynqmp/clk-gate-zynqmp.c
> > > +++ b/drivers/clk/zynqmp/clk-gate-zynqmp.c
> > > @@ -41,8 +41,8 @@ static int zynqmp_clk_gate_enable(struct clk_hw *hw)
> > > ret = zynqmp_pm_clock_enable(clk_id);
> > >
> > > if (ret)
> > > - pr_warn_once("%s() clock enabled failed for %s, ret = %d\n",
> > > - __func__, clk_name, ret);
> > > + pr_warn("%s() clock enable failed for %s (id %d), ret = %d\n",
> > > + __func__, clk_name, clk_id, ret);
> >
> > Can we just remove these prints entirely? The driver that calls
> > clk_enable() should be checking the return value and taking proper
> > action. What is the user going to do with these messages?
> >
>
> The clocks are handled by a firmware, which checks if the Linux system has the
> permission to change certain clocks. The warnings help users to identify an
> unexpected configuration of the firmware. However, I guess it would make sense
> to change the warnings to pr_debug.
>
Sure pr_debug() sounds fine.
_______________________________________________
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] 8+ messages in thread
end of thread, other threads:[~2022-01-14 22:21 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-12 14:12 [PATCH] clk: zynqmp: warn always when a clock op fails Michael Tretter
2022-01-12 14:12 ` Michael Tretter
2022-01-12 20:29 ` Stephen Boyd
2022-01-12 20:29 ` Stephen Boyd
2022-01-14 8:14 ` Michael Tretter
2022-01-14 8:14 ` Michael Tretter
2022-01-14 22:20 ` Stephen Boyd
2022-01-14 22:20 ` Stephen Boyd
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.