Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210313122818.445228-1-aford173@gmail.com> <e754a185-4aa2-295a-37a8-dd0c7ebc289f@pengutronix.de>
 <0361ab5b-a26e-7631-9bae-8909715f6354@pengutronix.de>
In-Reply-To: <0361ab5b-a26e-7631-9bae-8909715f6354@pengutronix.de>
From:   Adam Ford <aford173@gmail.com>
Date:   Sat, 20 Mar 2021 18:00:25 -0500
Message-ID: <CAHCN7xLSMGOe6E0YQU=n0aKX53_CV2cz9BgqO2OA96S1-eTMgg@mail.gmail.com>
Subject: Re: [PATCH V4] clk: imx: Fix reparenting of UARTs not associated with stdout
To:     Ahmad Fatoum <a.fatoum@pengutronix.de>
Cc:     linux-clk <linux-clk@vger.kernel.org>,
        Aisheng Dong <aisheng.dong@nxp.com>,
        Linus Walleij <linus.walleij@linaro.org>,
        Abel Vesa <abel.vesa@nxp.com>, Stephen Boyd <sboyd@kernel.org>,
        Fabio Estevam <festevam@gmail.com>,
        Michael Turquette <mturquette@baylibre.com>,
        Adam Ford-BE <aford@beaconembedded.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        NXP Linux Team <linux-imx@nxp.com>,
        Pengutronix Kernel Team <kernel@pengutronix.de>,
        Shawn Guo <shawnguo@kernel.org>,
        Sascha Hauer <s.hauer@pengutronix.de>,
        arm-soc <linux-arm-kernel@lists.infradead.org>,
        Jerome Brunet <jbrunet@baylibre.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sun, Mar 14, 2021 at 4:40 AM Ahmad Fatoum <a.fatoum@pengutronix.de> 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 |