Hi Conor,

On Wed, Dec 8, 2021 at 4:30 PM [off-list ref] wrote:

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:

quoted

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?

You're right, it's part of the static array.  I was misled by the
back-and-forth conversions between the different types.
Regardless, the kfree() calls should be removed.

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.

If that is true (didn't check), it needs to be fixed, too.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds