Re: [PATCH 1/3] clk: sunxi-ng: add support for rate resetting notifier

[Frank Oltmanns <frank@oltmanns.dev>]

On 2023-08-07 at 17:42:22 +0800, Icenowy Zheng <uwu@icenowy.me> wrote:
> 在 2023-08-07星期一的 11:36 +0200，Frank Oltmanns写道：
>> From: Icenowy Zheng <icenowy@aosc.io>
>>
>> In some situaitons, we will want a clock rate be kept while its
>> parent
>> can change, for example, to make dual-head work on A64, TCON0 clock
>> needs to be kept for LCD display and its parent (or grandparent)
>> PLL-Video0 need to be changed for HDMI display. (There's a quirk on
>> A64
>> that HDMI PHY can only use PLL-Video0, not PLL-Video1).
>>
>> Add a notifier helper to create such kind of rate keeping notifier by
>> reset the rate after the parent changed.
>>
>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> ---
>>  drivers/clk/sunxi-ng/ccu_common.c | 22 ++++++++++++++++++++++
>>  drivers/clk/sunxi-ng/ccu_common.h | 12 ++++++++++++
>>  2 files changed, 34 insertions(+)
>>
>> diff --git a/drivers/clk/sunxi-ng/ccu_common.c b/drivers/clk/sunxi-
>> ng/ccu_common.c
>> index 8d28a7a079d0..434fa46ad460 100644
>> --- a/drivers/clk/sunxi-ng/ccu_common.c
>> +++ b/drivers/clk/sunxi-ng/ccu_common.c
>> @@ -87,6 +87,28 @@ int ccu_pll_notifier_register(struct ccu_pll_nb
>> *pll_nb)
>>  }
>>  EXPORT_SYMBOL_NS_GPL(ccu_pll_notifier_register, SUNXI_CCU);
>>  
>> +static int ccu_rate_reset_notifier_cb(struct notifier_block *nb,
>> +                                     unsigned long event, void
>> *data)
>> +{
>> +       struct ccu_rate_reset_nb *rate_reset =
>> to_ccu_rate_reset_nb(nb);
>> +
>> +       if (event == PRE_RATE_CHANGE) {
>> +               rate_reset->saved_rate = clk_get_rate(rate_reset-
>> >target_clk);
>
> In fact I think we should have a better way to save the intended clock
> rate ;-)
>

Are you referring to struct clk_core's clk_req member [1]? Maxime Ripard
also mentioned it to me [2]? Or do you have something else in mind?

Note that I have a patchset on the way [3], that will improve rate
selection for pll-video0 and its descendants. I'll double-check if that
improves the situation in a way so that the clk_get_rate() might be good
enough.

Thanks,
  Frank

[1]: https://elixir.bootlin.com/linux/latest/source/drivers/clk/clk.c#L68
[2]: https://lore.kernel.org/linux-clk/xcgmqvdoip53yao4sfoznnppauhmsmdablwoewh43zjv3bhidp@d7pxqohxydve/
[3]: https://lore.kernel.org/linux-clk/20230806-pll-mipi_set_rate_parent-v5-0-db4f5ca33fc3@oltmanns.dev/

>
>> +       } else if (event == POST_RATE_CHANGE) {
>> +               clk_set_rate(rate_reset->target_clk, rate_reset-
>> >saved_rate);
>> +       }
>> +
>> +       return NOTIFY_DONE;
>> +}
>> +
>> +int ccu_rate_reset_notifier_register(struct ccu_rate_reset_nb
>> *rate_reset_nb)
>> +{
>> +       rate_reset_nb->clk_nb.notifier_call =
>> ccu_rate_reset_notifier_cb;
>> +
>> +       return clk_notifier_register(rate_reset_nb->common->hw.clk,
>> +                                    &rate_reset_nb->clk_nb);
>> +}
>> +
>>  static int sunxi_ccu_probe(struct sunxi_ccu *ccu, struct device
>> *dev,
>>                            struct device_node *node, void __iomem
>> *reg,
>>                            const struct sunxi_ccu_desc *desc)
>> diff --git a/drivers/clk/sunxi-ng/ccu_common.h b/drivers/clk/sunxi-
>> ng/ccu_common.h
>> index fbf16c6b896d..6b0b05fae123 100644
>> --- a/drivers/clk/sunxi-ng/ccu_common.h
>> +++ b/drivers/clk/sunxi-ng/ccu_common.h
>> @@ -69,4 +69,16 @@ int devm_sunxi_ccu_probe(struct device *dev, void
>> __iomem *reg,
>>  void of_sunxi_ccu_probe(struct device_node *node, void __iomem *reg,
>>                         const struct sunxi_ccu_desc *desc);
>>  
>> +struct ccu_rate_reset_nb {
>> +       struct notifier_block   clk_nb;
>> +       struct ccu_common       *common;
>> +
>> +       struct clk              *target_clk;
>> +       unsigned long           saved_rate;
>> +};
>> +
>> +#define to_ccu_rate_reset_nb(_nb) container_of(_nb, struct
>> ccu_rate_reset_nb, clk_nb)
>> +
>> +int ccu_rate_reset_notifier_register(struct ccu_rate_reset_nb
>> *rate_reset_nb);
>> +
>>  #endif /* _COMMON_H_ */
>>