Re: [PATCH v6 2/2] clk: microchip: Add driver for Microchip PolarFire SoC

[Geert Uytterhoeven <geert@linux-m68k.org>]

Hi Conor,

On Wed, Dec 8, 2021 at 4:30 PM <Conor.Dooley@microchip.com> 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:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >> +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