| Message ID | 20220119115434.2042017-1-m.tretter@pengutronix.de (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [v2] clk: zynqmp: replace warn_once with pr_debug for failed clock ops | expand |
On 1/19/22 12:54, Michael Tretter wrote: > The warning that a clock operation failed is only printed once. However, > the function is called for various different clocks. The limit hides the > warnings if different clocks are affected by the failures. > > The clock ops might fail if the firmware that handles the clocks is > misconfigured. Therefore, replace the pr_warn_once with pr_debug to > allow the user to see all errors if necessary. By default, hide the > error messages and let drivers handle the errors. > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> > --- > This is v2 of the patch to improve the error messages of the ZynqMP > clock driver [0]. > > [0] https://lore.kernel.org/all/20220112141229.700708-1-m.tretter@pengutronix.de/ > > Changelog: > > v2: > > - Update commit message > - Use pr_debug instead of pr_warn > --- > 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..b89e55737198 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_debug("%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_debug("%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_debug("%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..60359333f26d 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_debug("%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_debug("%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..422ea79907dd 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_debug("%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_debug("%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_debug("%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..91a6b4cc910e 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_debug("%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_debug("%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_debug("%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_debug("%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_debug("%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_debug("%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_debug("%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_debug("%s() clock disable failed for %s, ret = %d\n", > + __func__, clk_name, ret); > } > > static const struct clk_ops zynqmp_pll_ops = { Acked-by: Michal Simek <michal.simek@xilinx.com> Thanks, Michal
Quoting Michael Tretter (2022-01-19 03:54:34) > The warning that a clock operation failed is only printed once. However, > the function is called for various different clocks. The limit hides the > warnings if different clocks are affected by the failures. > > The clock ops might fail if the firmware that handles the clocks is > misconfigured. Therefore, replace the pr_warn_once with pr_debug to > allow the user to see all errors if necessary. By default, hide the > error messages and let drivers handle the errors. > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> > --- Applied to clk-next
diff --git a/drivers/clk/zynqmp/clk-gate-zynqmp.c b/drivers/clk/zynqmp/clk-gate-zynqmp.c index 565ed67a0430..b89e55737198 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_debug("%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_debug("%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_debug("%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..60359333f26d 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_debug("%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_debug("%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..422ea79907dd 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_debug("%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_debug("%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_debug("%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..91a6b4cc910e 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_debug("%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_debug("%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_debug("%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_debug("%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_debug("%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_debug("%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_debug("%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_debug("%s() clock disable failed for %s, ret = %d\n", + __func__, clk_name, ret); } static const struct clk_ops zynqmp_pll_ops = {
The warning that a clock operation failed is only printed once. However, the function is called for various different clocks. The limit hides the warnings if different clocks are affected by the failures. The clock ops might fail if the firmware that handles the clocks is misconfigured. Therefore, replace the pr_warn_once with pr_debug to allow the user to see all errors if necessary. By default, hide the error messages and let drivers handle the errors. Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> --- This is v2 of the patch to improve the error messages of the ZynqMP clock driver [0]. [0] https://lore.kernel.org/all/20220112141229.700708-1-m.tretter@pengutronix.de/ Changelog: v2: - Update commit message - Use pr_debug instead of pr_warn --- 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(-)