[Linux-stm32] [PATCH v2 02/14] clk: stm32mp1: merge 'ck_hse_rtc' and 'ck_rtc' into one clock

[Stephen Boyd]

Quoting gabriel.fernandez at foss.st.com (2021-02-12 00:08:40)
> 
> On 2/9/21 9:00 AM, Stephen Boyd wrote:
> > Quoting gabriel.fernandez at foss.st.com (2021-01-26 01:01:08)
> >> From: Gabriel Fernandez <gabriel.fernandez at foss.st.com>
> >>
> >> 'ck_rtc' has multiple clocks as input (ck_hsi, ck_lsi, and ck_hse).
> >> A divider is available only on the specific rtc input for ck_hse.
> >> This Merge will facilitate to have a more coherent clock tree
> >> in no trusted / trusted world.
> >>
> >> Signed-off-by: Gabriel Fernandez <gabriel.fernandez at foss.st.com>
> >> ---
> >>   drivers/clk/clk-stm32mp1.c | 49 +++++++++++++++++++++++++++++++++-----
> >>   1 file changed, 43 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/clk/clk-stm32mp1.c b/drivers/clk/clk-stm32mp1.c
> >> index 35d5aee8f9b0..0e1d4427a8df 100644
> >> --- a/drivers/clk/clk-stm32mp1.c
> >> +++ b/drivers/clk/clk-stm32mp1.c
> >> @@ -245,7 +245,7 @@ static const char * const dsi_src[] = {
> >>   };
> >>   
> >>   static const char * const rtc_src[] = {
> >> -       "off", "ck_lse", "ck_lsi", "ck_hse_rtc"
> >> +       "off", "ck_lse", "ck_lsi", "ck_hse"
> >>   };
> >>   
> >>   static const char * const mco1_src[] = {
> >> @@ -1031,6 +1031,42 @@ static struct clk_hw *clk_register_cktim(struct device *dev, const char *name,
> >>          return hw;
> >>   }
> >>   
> >> +/* The divider of RTC clock concerns only ck_hse clock */
> >> +#define HSE_RTC 3
> >> +
> >> +static unsigned long clk_divider_rtc_recalc_rate(struct clk_hw *hw,
> >> +                                                unsigned long parent_rate)
> >> +{
> >> +       if (clk_hw_get_parent(hw) == clk_hw_get_parent_by_index(hw, HSE_RTC))
> >> +               return clk_divider_ops.recalc_rate(hw, parent_rate);
> >> +
> >> +       return parent_rate;
> >> +}
> >> +
> >> +static long clk_divider_rtc_round_rate(struct clk_hw *hw, unsigned long rate,
> >> +                                      unsigned long *prate)
> >> +{
> >> +       if (clk_hw_get_parent(hw) == clk_hw_get_parent_by_index(hw, HSE_RTC))
> > This clk op can be called at basically any time. Maybe this should use
> > the determine rate op and then look to see what the parent is that comes
> > in via the rate request structure? Or is the intention to keep this
> > pinned to one particular parent? Looking at this right now it doesn't
> > really make much sense why the current parent state should play into
> > what rate the clk can round to, unless there is some more clk flags
> > going on that constrain the ability to change this clk's parent.
> 
> Yes the intention is to keep this pinned for one particular parent.
> 
> This divider is only applied on the 4th input of the MUX of the RTC and
> 
> doesn't affect the HSE frequency for all the system.
> 
> 
> Oscillators
>   -----
> | lse |----------------+----------------> ck_lse
>   -----                 |
>   -----                 |
> | lsi |------------+--------------------> ck_lsi
>   -----             |   |
>                     |   |
>   -----             |   |
> | hse |----+-------|---|----------------> ck_hse
>   -----     |       |   |
>             |       |   |         |\ mux
>             |       |   |  OFF -->| \
>             |       |   |         |  \     gate
>             |       |   --------->|  |     ---
>             |       |             |  |--->|   |--> ck_rtc
>             |       ------------->|  |     ---
>             |    -----------      |  |
>              ----| % 1 to 64 |--->| /
>                   -----------     |/
>                     divider
> 
> I manage the RTC with a clock composite with a gate a mux and a specific 
> rate ops for hse input.
> 
> That why i need to the parent state.
> 

So would using determine_rate op instead of round_rate op help here? That
will provide the current parent rate and hw pointer in the rate request
structure.