Re: [PATCH V4] clk: imx: Fix reparenting of UARTs not associated with stdout

From: Adam Ford
Date: Sat Mar 20 2021 - 19:01:42 EST


On Sun, Mar 14, 2021 at 4:40 AM Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx> wrote:
>
> On 13.03.21 16:16, Ahmad Fatoum wrote:
> >> +/* i.MX boards use device trees now. For build tests without CONFIG_OF, do nothing */
> >> +#ifdef CONFIG_OF
> >> if (imx_keep_uart_clocks) {
> >> int i;
> >>
> >> - imx_uart_clocks = clks;
> >> - for (i = 0; imx_uart_clocks[i]; i++)
> >> - clk_prepare_enable(*imx_uart_clocks[i]);
> >> + imx_uart_clocks = kcalloc(clk_count, sizeof(struct clk *), GFP_KERNEL);
> >> +
> >> + if (!of_stdout)
> >> + return;
> >
> > Memory leak. Just do if (imx_keep_uart_clocks && of_stdout)
>
> Please dismiss. I overlooked that you free it in a later initcall.

Abel,

Are you OK with this? I also have a V5 posted [1] which does what
Ahmad suggested.

Either of these will fix reparenting issues, but I need this for
Bluetooth to operate correctly, because both beacon imx8mn and imx8mn
kits switch the UART parent to an 80MHz clock in order to run at
4Mbps.

thank you,

adam
>
> >> static int __init imx_clk_disable_uart(void)
> >> {
> >> - if (imx_keep_uart_clocks && imx_uart_clocks) {
> >> + if (imx_keep_uart_clocks && imx_enabled_uart_clocks) {
> >> int i;
> >>
> >> - for (i = 0; imx_uart_clocks[i]; i++)
> >> - clk_disable_unprepare(*imx_uart_clocks[i]);
> >> + for (i = 0; i < imx_enabled_uart_clocks; i++) {
> >> + clk_disable_unprepare(imx_uart_clocks[i]);
> >> + clk_put(imx_uart_clocks[i]);
> >> + };
> >> + kfree(imx_uart_clocks);
> >> }
>
> --
> Pengutronix e.K. | |
> Steuerwalder Str. 21 | http://www.pengutronix.de/ |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |