[PATCH 1/8] clk: Add range accessors

[Stephen Boyd]

Quoting Maxime Ripard (2021-03-03 00:45:27)
> Hi Stephen,
> 
> On Tue, Mar 02, 2021 at 03:18:58PM -0800, Stephen Boyd wrote:
> > Quoting Maxime Ripard (2021-02-25 07:59:02)
> > > Some devices might need to access the current available range of a clock
> > > to discover their capabilities. Let's add those accessors.
> > 
> > This needs more than two sentences to describe what's required.
> > 
> > > 
> > > Signed-off-by: Maxime Ripard <maxime at cerno.tech>
> > > ---
> > >  drivers/clk/clk.c   | 30 ++++++++++++++++++++++++++++++
> > >  include/linux/clk.h | 16 ++++++++++++++++
> > >  2 files changed, 46 insertions(+)
> > > 
> > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > > index 8c1d04db990d..b7307d4f090d 100644
> > > --- a/drivers/clk/clk.c
> > > +++ b/drivers/clk/clk.c
> > > @@ -2407,6 +2407,36 @@ int clk_set_max_rate(struct clk *clk, unsigned long rate)
> > >  }
> > >  EXPORT_SYMBOL_GPL(clk_set_max_rate);
> > >  
> > > +long clk_get_min_rate(struct clk *clk)
> > 
> > I need to read the rest of the patches but I don't see the justification
> > for this sort of API vs. having the consumer constrain the clk frequency
> > that it wants. Is the code that's setting the min/max constraints not
> > the same as the code that's calling this API? Would an OPP table better
> > serve this so the device knows what frequencies are valid?s Please
> > provide the use case/justification in the commit text.
> 
> The patch that uses it is the patch 4
> 
> The issue I'm trying to solve is that all the RaspberryPi have a
> configuration file for the firmware, and the firmware is in charge of
> the clocks communicating through a mailbox interface.
> 
> By default, one of the main clocks in the system can only reach 500MHz,
> and that's the range reported by the firmware when queried. However, in
> order to support display modes with a fairly high bandwidth such as 4k
> at 60Hz, that clock needs to be raised to at least 550MHz, and the
> firmware configuration has a special parameter for that one. Setting
> that parameter will increase the range of the clock to have proper
> boundaries for that display mode.
> 
> If a user doesn't enable it and tries to use those display modes, the
> display will be completely blank.
> 
> There's no way to query the firmware configuration directly, so we can
> instead query the range of the clock and see if the firmware enables us
> to use those modes, warn the user and ignore the modes that wouldn't
> work. That's what those accessors are here for

How does the clk driver query the firmware but it can't be done
directly by the drm driver? 

> 
> > Why two functions instead of one function to get both min and max?
> 
> Since we have clk_set_min_rate and clk_set_max_rate, it made sense to me
> to mirror that, but I'd be happy to change if you think otherwise
> 

Does using clk_round_rate() work just as well?