Hi Geert,
Just going to ack the other items you raised for now, but curious about 
this one:
On 06/12/2021 15:53, Geert Uytterhoeven wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

quoted

+static void mpfs_clk_unregister_cfg(struct device *dev, struct clk_hw *hw)
+{
+       struct mpfs_cfg_hw_clock *cfg_hw = to_mpfs_cfg_clk(hw);
+
+       devm_clk_hw_unregister(dev, hw);
+       kfree(cfg_hw);

This is freeing a part of the big array allocated with devm_kzalloc()?

I took a look at this, and I don't think it is freeing the devm 
allocated array.

To me, what is actually being freed is an element in the array of 
structs passed to mpfs_clk_register_cfgs in the probe function.
However, this struct is statically defined - so its elements shouldn't 
be freed at all?

drivers/clk/clk-bm1880.c has the same behaviour in the unregister 
function, where it calls kfree on the elements of a static array of clk 
structs. So if my understanding is correct it would need fixing there too.

Cheers,
Conor.

quoted

+}
+
+static struct clk_hw *mpfs_clk_register_cfg(struct device *dev,
+                                           struct mpfs_cfg_hw_clock *cfg_hw,
+                                           void __iomem *sys_base)
+{
+       struct clk_hw *hw;
+       int err;
+
+       cfg_hw->sys_base = sys_base;
+       cfg_hw->lock = &mpfs_clk_lock;
+
+       hw = &cfg_hw->hw;
+       err = devm_clk_hw_register(dev, hw);
+       if (err)
+               return ERR_PTR(err);
+
+       return hw;
+}
+
+static int mpfs_clk_register_cfgs(struct device *dev, struct mpfs_cfg_hw_clock *cfg_hws,
+                                 int num_clks, struct mpfs_clock_data *data)

unsigned int num_clks

quoted

+{
+       struct clk_hw *hw;
+       void __iomem *sys_base = data->base;
+       unsigned int i, id;
+
+       for (i = 0; i < num_clks; i++) {
+               struct mpfs_cfg_hw_clock *cfg_hw = &cfg_hws[i];
+
+               hw = mpfs_clk_register_cfg(dev, cfg_hw, sys_base);
+               if (IS_ERR(hw)) {
+                       dev_err(dev, "%s: failed to register clock %s\n", __func__,

I guess the __func__ can be dropped, as the clock name is unique?

quoted

+                               cfg_hw->cfg.name);
+                       goto err_clk;
+               }
+
+               id = cfg_hws[i].cfg.id;
+               data->hw_data.hws[id] = hw;
+       }
+
+       return 0;
+
+err_clk:
+       while (i--)
+               mpfs_clk_unregister_cfg(dev, data->hw_data.hws[cfg_hws[i].cfg.id]);
+
+       return PTR_ERR(hw);
+}