> On 26/08/2020 16:55:14+0800, dinghao.liu@xxxxxxxxxx wrote: > > > On Sun, Aug 23, 2020 at 3:59 PM Dinghao Liu <dinghao.liu@xxxxxxxxxx> wrote: > > > > > > > > When clk_hw_register_fixed_rate_with_accuracy() fails, > > > > clk_data should be freed. It's the same for the subsequent > > > > error paths. > > > > > > I suppose you should also unregister the already registered clocks > > > in the latter two error paths? > > > > > > > Sounds reasonable. But I find that the existing kernel code takes different > > strategies for this case. of_sama5d4_sckc_setup() uses clk_hw_unregister() > > after clk_hw_register_fixed_rate_with_accuracy(), while _of_fixed_clk_setup() > > uses clk_hw_unregister_fixed_rate(). But at91sam926x_pmc_setup() just does > > nothing in this case. > > I guess you should use clk_hw_unregister_fixed_rate after > clk_hw_register_fixed_rate_with_accuracy. clk_hw_unregister will leak > the struct clk_fixed_rate. It doesn't matter too much for > of_sama5d4_sckc_setup and at91sam926x_pmc_setup because if th clock > can't be registered, the platform will not boot. Thank you for your advice! I will submit a new patch soon. Regards, Dinghao