Re: [PATCH V3] clk: imx: Fix reparenting of UARTs not associated with sdout

From: Ahmad Fatoum <a.fatoum@pengutronix.de>
To: Adam Ford <aford173@gmail.com>, linux-arm-kernel@lists.infradead.org
Cc: Aisheng Dong <aisheng.dong@nxp.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Fabio Estevam <festevam@gmail.com>,
	Stephen Boyd <sboyd@kernel.org>, Shawn Guo <shawnguo@kernel.org>,
	Michael Turquette <mturquette@baylibre.com>,
	aford@beaconembedded.com, linux-clk@vger.kernel.org,
	NXP Linux Team <linux-imx@nxp.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	linux-kernel@vger.kernel.org,
	Jerome Brunet <jbrunet@baylibre.com>
Subject: Re: [PATCH V3] clk: imx: Fix reparenting of UARTs not associated with sdout
Date: Fri, 5 Mar 2021 13:21:33 +0100	[thread overview]
Message-ID: <fbe03824-1397-c1f8-ac97-f500eaf60338@pengutronix.de> (raw)
In-Reply-To: <20210115182909.314756-1-aford173@gmail.com>

Hello Adam,

On 15.01.21 19:29, Adam Ford wrote:
>  {
> +#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);
> +		imx_enabled_uart_clocks = 0;
> +
> +		for (i = 0; i < clk_count; i++) {
> +			imx_uart_clocks[imx_enabled_uart_clocks] = of_clk_get(of_stdout, i);

of_stdout may be NULL if there is no stdout-path. You should check that earlier.

With this fixed, feel free to add:

Tested-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> +
> +			/* Stop if there are no more of_stdout references */
> +			if (IS_ERR(imx_uart_clocks[imx_enabled_uart_clocks]))
> +				return;
> +
> +			/* Only enable the clock if it's not NULL */
> +			if (imx_uart_clocks[imx_enabled_uart_clocks])
> +				clk_prepare_enable(imx_uart_clocks[imx_enabled_uart_clocks++]);
> +		}
>  	}
> +#else
> +	/* i.MX boards use device trees now.  For build tests without CONFIG_OF, do nothing */
> +	imx_enabled_uart_clocks = 0;
> +#endif
>  }
>  
>  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);
>  	}
>  
>  	return 0;
> diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
> index 4f04c8287286..7571603bee23 100644
> --- a/drivers/clk/imx/clk.h
> +++ b/drivers/clk/imx/clk.h
> @@ -11,9 +11,9 @@ extern spinlock_t imx_ccm_lock;
>  void imx_check_clocks(struct clk *clks[], unsigned int count);
>  void imx_check_clk_hws(struct clk_hw *clks[], unsigned int count);
>  #ifndef MODULE
> -void imx_register_uart_clocks(struct clk ** const clks[]);
> +void imx_register_uart_clocks(unsigned int clk_count);
>  #else
> -static inline void imx_register_uart_clocks(struct clk ** const clks[])
> +static inline void imx_register_uart_clocks(unsigned int clk_count)
>  {
>  }
>  #endif
> 

-- 
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 |