Re: [PATCH] clk: zynqmp: warn always when a clock op fails

[Stephen Boyd <sboyd@kernel.org>]

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.