Re: [PATCH V4] clk: imx: Fix reparenting of UARTs not associated with stdout
On Mon, Mar 22, 2021 at 4:42 PM Abel Vesa wrote:
>
> On 21-03-13 06:28:17, Adam Ford wrote:
> > Most if not all i.MX SoC's call a function which enables all UARTS.
> > This is a problem for users who need to re-parent the clock source,
> > because any attempt to change the parent results in an busy error
> > due to the fact that the clocks have been enabled already.
> >
> > clk: failed to reparent uart1 to sys_pll1_80m: -16
> >
> > Instead of pre-initializing all UARTS, scan the device tree to see
> > which UART clocks are associated to stdout, and only enable those
> > UART clocks if it's needed early. This will move initialization of
> > the remaining clocks until after the parenting of the clocks.
> >
> > When the clocks are shutdown, this mechanism will also disable any
> > clocks that were pre-initialized.
> >
> > Fixes: 9461f7b33d11c ("clk: fix CLK_SET_RATE_GATE with clock rate
> > protection")
> > Suggested-by: Aisheng Dong
> > Signed-off-by: Adam Ford
> > Reviewed-by: Abel Vesa
> > Tested-by: Ahmad Fatoum
> >
> > ---
> > V4: Check if of_stdout is available before using it.
> > Re-align #ifdef to remove repeated code.
> > V3: Return a method more closely related to upstream kernel but
> > instead of passing an array of UART's, each SoC passes the max
> > number of UART clocks is has. The imx clock driver will create
> > an array to enable on startup, and disable later.
> > V2: Attempt to port driver from vendor kernel.
> > ---
> > drivers/clk/imx/clk-imx25.c | 12 +-
> > drivers/clk/imx/clk-imx27.c | 13 +--
> > drivers/clk/imx/clk-imx35.c | 10 +
> > drivers/clk/imx/clk-imx5.c| 30 +++--
> > drivers/clk/imx/clk-imx6q.c | 16 +-
> > drivers/clk/imx/clk-imx6sl.c | 16 +-
> > drivers/clk/imx/clk-imx6sll.c | 24 +---
> > drivers/clk/imx/clk-imx6sx.c | 16 +-
> > drivers/clk/imx/clk-imx7d.c | 22 +--
> > drivers/clk/imx/clk-imx7ulp.c | 31 ++
> > drivers/clk/imx/clk-imx8mm.c | 18 ++-
> > drivers/clk/imx/clk-imx8mn.c | 18 ++-
> > drivers/clk/imx/clk-imx8mp.c | 17 +--
> > drivers/clk/imx/clk-imx8mq.c | 18 ++-
> > drivers/clk/imx/clk.c | 41 +++
> > drivers/clk/imx/clk.h | 4 ++--
> > 16 files changed, 54 insertions(+), 252 deletions(-)
> >
> > diff --git a/drivers/clk/imx/clk-imx25.c b/drivers/clk/imx/clk-imx25.c
> > index a66cabfbf94f..66192fe0a898 100644
> > --- a/drivers/clk/imx/clk-imx25.c
> > +++ b/drivers/clk/imx/clk-imx25.c
> > @@ -73,16 +73,6 @@ enum mx25_clks {
> >
> > static struct clk *clk[clk_max];
> >
> > -static struct clk ** const uart_clks[] __initconst = {
> > - [uart_ipg_per],
> > - [uart1_ipg],
> > - [uart2_ipg],
> > - [uart3_ipg],
> > - [uart4_ipg],
> > - [uart5_ipg],
> > - NULL
> > -};
> > -
> > static int __init __mx25_clocks_init(void __iomem *ccm_base)
> > {
> > BUG_ON(!ccm_base);
> > @@ -228,7 +218,7 @@ static int __init __mx25_clocks_init(void __iomem
> > *ccm_base)
> >*/
> > clk_set_parent(clk[cko_sel], clk[ipg]);
> >
> > - imx_register_uart_clocks(uart_clks);
> > + imx_register_uart_clocks(6);
> >
> > return 0;
> > }
> > diff --git a/drivers/clk/imx/clk-imx27.c b/drivers/clk/imx/clk-imx27.c
> > index 5585ded8b8c6..56a5fc402b10 100644
> > --- a/drivers/clk/imx/clk-imx27.c
> > +++ b/drivers/clk/imx/clk-imx27.c
> > @@ -49,17 +49,6 @@ static const char *ssi_sel_clks[] = { "spll_gate",
> > "mpll", };
> > static struct clk *clk[IMX27_CLK_MAX];
> > static struct clk_onecell_data clk_data;
> >
> > -static struct clk ** const uart_clks[] __initconst = {
> > - [IMX27_CLK_PER1_GATE],
> > - [IMX27_CLK_UART1_IPG_GATE],
> > - [IMX27_CLK_UART2_IPG_GATE],
> > - [IMX27_CLK_UART3_IPG_GATE],
> > - [IMX27_CLK_UART4_IPG_GATE],
> > - [IMX27_CLK_UART5_IPG_GATE],
> > - [IMX27_CLK_UART6_IPG_GATE],
> > - NULL
> > -};
> > -
> > static void __init _mx27_clocks_init(unsigned long fref)
> > {
> > BUG_ON(!ccm);
> > @@ -176,7 +165,7 @@ static void __init _mx27_clocks_init(unsigned long fref)
> >
> > clk_prepare_enable(clk[IMX27_CLK_EMI_AHB_GATE]);
> >
> > - imx_register_uart_clocks(uart_clks);
> > + imx_register_uart_clocks(7);
> >
> > imx_print_silicon_rev("i.MX27", mx27_revision());
> > }
> > diff --git a/drivers/clk/imx/clk-imx35.c b/drivers/clk/imx/clk-imx35.c
> > index c1df03665c09..0fe5ac210156 100644
> > --- a/drivers/clk/imx/clk-imx35.c
> > +++ b/drivers/clk/imx/clk-imx35.c
> > @@ -82,14 +82,6 @@ enum mx35_clks {
> >
> > static struct clk *clk[clk_max];
> >
> > -static struct clk ** const uart_clks[] __initconst = {
> > - [ipg],
> > - [uart1_gate],
> > - [uart2_gate],
> > - [uart3_gate],
> > - NULL
> > -};
> > -
> > static void __init _mx35_clocks_init(void)
> > {
Re: [PATCH V4] clk: imx: Fix reparenting of UARTs not associated with stdout
On 21-03-13 06:28:17, Adam Ford wrote:
> Most if not all i.MX SoC's call a function which enables all UARTS.
> This is a problem for users who need to re-parent the clock source,
> because any attempt to change the parent results in an busy error
> due to the fact that the clocks have been enabled already.
>
> clk: failed to reparent uart1 to sys_pll1_80m: -16
>
> Instead of pre-initializing all UARTS, scan the device tree to see
> which UART clocks are associated to stdout, and only enable those
> UART clocks if it's needed early. This will move initialization of
> the remaining clocks until after the parenting of the clocks.
>
> When the clocks are shutdown, this mechanism will also disable any
> clocks that were pre-initialized.
>
> Fixes: 9461f7b33d11c ("clk: fix CLK_SET_RATE_GATE with clock rate protection")
> Suggested-by: Aisheng Dong
> Signed-off-by: Adam Ford
> Reviewed-by: Abel Vesa
> Tested-by: Ahmad Fatoum
>
> ---
> V4: Check if of_stdout is available before using it.
> Re-align #ifdef to remove repeated code.
> V3: Return a method more closely related to upstream kernel but
> instead of passing an array of UART's, each SoC passes the max
> number of UART clocks is has. The imx clock driver will create
> an array to enable on startup, and disable later.
> V2: Attempt to port driver from vendor kernel.
> ---
> drivers/clk/imx/clk-imx25.c | 12 +-
> drivers/clk/imx/clk-imx27.c | 13 +--
> drivers/clk/imx/clk-imx35.c | 10 +
> drivers/clk/imx/clk-imx5.c| 30 +++--
> drivers/clk/imx/clk-imx6q.c | 16 +-
> drivers/clk/imx/clk-imx6sl.c | 16 +-
> drivers/clk/imx/clk-imx6sll.c | 24 +---
> drivers/clk/imx/clk-imx6sx.c | 16 +-
> drivers/clk/imx/clk-imx7d.c | 22 +--
> drivers/clk/imx/clk-imx7ulp.c | 31 ++
> drivers/clk/imx/clk-imx8mm.c | 18 ++-
> drivers/clk/imx/clk-imx8mn.c | 18 ++-
> drivers/clk/imx/clk-imx8mp.c | 17 +--
> drivers/clk/imx/clk-imx8mq.c | 18 ++-
> drivers/clk/imx/clk.c | 41 +++
> drivers/clk/imx/clk.h | 4 ++--
> 16 files changed, 54 insertions(+), 252 deletions(-)
>
> diff --git a/drivers/clk/imx/clk-imx25.c b/drivers/clk/imx/clk-imx25.c
> index a66cabfbf94f..66192fe0a898 100644
> --- a/drivers/clk/imx/clk-imx25.c
> +++ b/drivers/clk/imx/clk-imx25.c
> @@ -73,16 +73,6 @@ enum mx25_clks {
>
> static struct clk *clk[clk_max];
>
> -static struct clk ** const uart_clks[] __initconst = {
> - [uart_ipg_per],
> - [uart1_ipg],
> - [uart2_ipg],
> - [uart3_ipg],
> - [uart4_ipg],
> - [uart5_ipg],
> - NULL
> -};
> -
> static int __init __mx25_clocks_init(void __iomem *ccm_base)
> {
> BUG_ON(!ccm_base);
> @@ -228,7 +218,7 @@ static int __init __mx25_clocks_init(void __iomem
> *ccm_base)
>*/
> clk_set_parent(clk[cko_sel], clk[ipg]);
>
> - imx_register_uart_clocks(uart_clks);
> + imx_register_uart_clocks(6);
>
> return 0;
> }
> diff --git a/drivers/clk/imx/clk-imx27.c b/drivers/clk/imx/clk-imx27.c
> index 5585ded8b8c6..56a5fc402b10 100644
> --- a/drivers/clk/imx/clk-imx27.c
> +++ b/drivers/clk/imx/clk-imx27.c
> @@ -49,17 +49,6 @@ static const char *ssi_sel_clks[] = { "spll_gate", "mpll",
> };
> static struct clk *clk[IMX27_CLK_MAX];
> static struct clk_onecell_data clk_data;
>
> -static struct clk ** const uart_clks[] __initconst = {
> - [IMX27_CLK_PER1_GATE],
> - [IMX27_CLK_UART1_IPG_GATE],
> - [IMX27_CLK_UART2_IPG_GATE],
> - [IMX27_CLK_UART3_IPG_GATE],
> - [IMX27_CLK_UART4_IPG_GATE],
> - [IMX27_CLK_UART5_IPG_GATE],
> - [IMX27_CLK_UART6_IPG_GATE],
> - NULL
> -};
> -
> static void __init _mx27_clocks_init(unsigned long fref)
> {
> BUG_ON(!ccm);
> @@ -176,7 +165,7 @@ static void __init _mx27_clocks_init(unsigned long fref)
>
> clk_prepare_enable(clk[IMX27_CLK_EMI_AHB_GATE]);
>
> - imx_register_uart_clocks(uart_clks);
> + imx_register_uart_clocks(7);
>
> imx_print_silicon_rev("i.MX27", mx27_revision());
> }
> diff --git a/drivers/clk/imx/clk-imx35.c b/drivers/clk/imx/clk-imx35.c
> index c1df03665c09..0fe5ac210156 100644
> --- a/drivers/clk/imx/clk-imx35.c
> +++ b/drivers/clk/imx/clk-imx35.c
> @@ -82,14 +82,6 @@ enum mx35_clks {
>
> static struct clk *clk[clk_max];
>
> -static struct clk ** const uart_clks[] __initconst = {
> - [ipg],
> - [uart1_gate],
> - [uart2_gate],
> - [uart3_gate],
> - NULL
> -};
> -
> static void __init _mx35_clocks_init(void)
> {
> void __iomem *base;
> @@ -243,7 +235,7 @@ static void __init _mx35_clocks_init(void)
>*/
> clk_prepare_enable(clk[scc_gate]);
>
> - imx_register_uart_clocks(uart_clks);
> + imx_register_uart_clocks(4);
>
> imx_print_silicon_rev("i.MX35",
Re: [PATCH V4] clk: imx: Fix reparenting of UARTs not associated with stdout
On 21-03-20 18:00:25, Adam Ford wrote:
> On Sun, Mar 14, 2021 at 4:40 AM Ahmad Fatoum 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.
>
I'm OK with this version. Applied it on my clk/imx branch:
git://git.kernel.org/pub/scm/linux/kernel/git/abelvesa/linux.git
Thanks a lot for all the effort.
> 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- |
Re: [PATCH V4] clk: imx: Fix reparenting of UARTs not associated with stdout
On Sun, Mar 14, 2021 at 4:40 AM Ahmad Fatoum 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- |
Re: [PATCH V4] clk: imx: Fix reparenting of UARTs not associated with stdout
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.
>> 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- |
Re: [PATCH V4] clk: imx: Fix reparenting of UARTs not associated with stdout
Hi,
On 13.03.21 13:28, Adam Ford wrote:
> Most if not all i.MX SoC's call a function which enables all UARTS.
> This is a problem for users who need to re-parent the clock source,
> because any attempt to change the parent results in an busy error
> due to the fact that the clocks have been enabled already.
>
> clk: failed to reparent uart1 to sys_pll1_80m: -16
>
> Instead of pre-initializing all UARTS, scan the device tree to see
> which UART clocks are associated to stdout, and only enable those
> UART clocks if it's needed early. This will move initialization of
> the remaining clocks until after the parenting of the clocks.
>
> When the clocks are shutdown, this mechanism will also disable any
> clocks that were pre-initialized.
>
> Fixes: 9461f7b33d11c ("clk: fix CLK_SET_RATE_GATE with clock rate protection")
> Suggested-by: Aisheng Dong
> Signed-off-by: Adam Ford
> Reviewed-by: Abel Vesa
> Tested-by: Ahmad Fatoum
>
> ---
> V4: Check if of_stdout is available before using it.
> Re-align #ifdef to remove repeated code.
> V3: Return a method more closely related to upstream kernel but
> instead of passing an array of UART's, each SoC passes the max
> number of UART clocks is has. The imx clock driver will create
> an array to enable on startup, and disable later.
> V2: Attempt to port driver from vendor kernel.
> ---
> drivers/clk/imx/clk-imx25.c | 12 +-
> drivers/clk/imx/clk-imx27.c | 13 +--
> drivers/clk/imx/clk-imx35.c | 10 +
> drivers/clk/imx/clk-imx5.c| 30 +++--
> drivers/clk/imx/clk-imx6q.c | 16 +-
> drivers/clk/imx/clk-imx6sl.c | 16 +-
> drivers/clk/imx/clk-imx6sll.c | 24 +---
> drivers/clk/imx/clk-imx6sx.c | 16 +-
> drivers/clk/imx/clk-imx7d.c | 22 +--
> drivers/clk/imx/clk-imx7ulp.c | 31 ++
> drivers/clk/imx/clk-imx8mm.c | 18 ++-
> drivers/clk/imx/clk-imx8mn.c | 18 ++-
> drivers/clk/imx/clk-imx8mp.c | 17 +--
> drivers/clk/imx/clk-imx8mq.c | 18 ++-
> drivers/clk/imx/clk.c | 41 +++
> drivers/clk/imx/clk.h | 4 ++--
> 16 files changed, 54 insertions(+), 252 deletions(-)
>
> diff --git a/drivers/clk/imx/clk-imx25.c b/drivers/clk/imx/clk-imx25.c
> index a66cabfbf94f..66192fe0a898 100644
> --- a/drivers/clk/imx/clk-imx25.c
> +++ b/drivers/clk/imx/clk-imx25.c
> @@ -73,16 +73,6 @@ enum mx25_clks {
>
> static struct clk *clk[clk_max];
>
> -static struct clk ** const uart_clks[] __initconst = {
> - [uart_ipg_per],
> - [uart1_ipg],
> - [uart2_ipg],
> - [uart3_ipg],
> - [uart4_ipg],
> - [uart5_ipg],
> - NULL
> -};
> -
> static int __init __mx25_clocks_init(void __iomem *ccm_base)
> {
> BUG_ON(!ccm_base);
> @@ -228,7 +218,7 @@ static int __init __mx25_clocks_init(void __iomem
> *ccm_base)
>*/
> clk_set_parent(clk[cko_sel], clk[ipg]);
>
> - imx_register_uart_clocks(uart_clks);
> + imx_register_uart_clocks(6);
>
> return 0;
> }
> diff --git a/drivers/clk/imx/clk-imx27.c b/drivers/clk/imx/clk-imx27.c
> index 5585ded8b8c6..56a5fc402b10 100644
> --- a/drivers/clk/imx/clk-imx27.c
> +++ b/drivers/clk/imx/clk-imx27.c
> @@ -49,17 +49,6 @@ static const char *ssi_sel_clks[] = { "spll_gate", "mpll",
> };
> static struct clk *clk[IMX27_CLK_MAX];
> static struct clk_onecell_data clk_data;
>
> -static struct clk ** const uart_clks[] __initconst = {
> - [IMX27_CLK_PER1_GATE],
> - [IMX27_CLK_UART1_IPG_GATE],
> - [IMX27_CLK_UART2_IPG_GATE],
> - [IMX27_CLK_UART3_IPG_GATE],
> - [IMX27_CLK_UART4_IPG_GATE],
> - [IMX27_CLK_UART5_IPG_GATE],
> - [IMX27_CLK_UART6_IPG_GATE],
> - NULL
> -};
> -
> static void __init _mx27_clocks_init(unsigned long fref)
> {
> BUG_ON(!ccm);
> @@ -176,7 +165,7 @@ static void __init _mx27_clocks_init(unsigned long fref)
>
> clk_prepare_enable(clk[IMX27_CLK_EMI_AHB_GATE]);
>
> - imx_register_uart_clocks(uart_clks);
> + imx_register_uart_clocks(7);
>
> imx_print_silicon_rev("i.MX27", mx27_revision());
> }
> diff --git a/drivers/clk/imx/clk-imx35.c b/drivers/clk/imx/clk-imx35.c
> index c1df03665c09..0fe5ac210156 100644
> --- a/drivers/clk/imx/clk-imx35.c
> +++ b/drivers/clk/imx/clk-imx35.c
> @@ -82,14 +82,6 @@ enum mx35_clks {
>
> static struct clk *clk[clk_max];
>
> -static struct clk ** const uart_clks[] __initconst = {
> - [ipg],
> - [uart1_gate],
> - [uart2_gate],
> - [uart3_gate],
> - NULL
> -};
> -
> static void __init _mx35_clocks_init(void)
> {
> void __iomem *base;
> @@ -243,7 +235,7 @@ static void __init _mx35_clocks_init(void)
>*/
> clk_prepare_enable(clk[scc_gate]);
>
> - imx_register_uart_clocks(uart_clks);
> + imx_register_uart_clocks(4);
>
> imx_print_silicon_rev("i.MX35",
[PATCH V4] clk: imx: Fix reparenting of UARTs not associated with stdout
Most if not all i.MX SoC's call a function which enables all UARTS.
This is a problem for users who need to re-parent the clock source,
because any attempt to change the parent results in an busy error
due to the fact that the clocks have been enabled already.
clk: failed to reparent uart1 to sys_pll1_80m: -16
Instead of pre-initializing all UARTS, scan the device tree to see
which UART clocks are associated to stdout, and only enable those
UART clocks if it's needed early. This will move initialization of
the remaining clocks until after the parenting of the clocks.
When the clocks are shutdown, this mechanism will also disable any
clocks that were pre-initialized.
Fixes: 9461f7b33d11c ("clk: fix CLK_SET_RATE_GATE with clock rate protection")
Suggested-by: Aisheng Dong
Signed-off-by: Adam Ford
Reviewed-by: Abel Vesa
Tested-by: Ahmad Fatoum
---
V4: Check if of_stdout is available before using it.
Re-align #ifdef to remove repeated code.
V3: Return a method more closely related to upstream kernel but
instead of passing an array of UART's, each SoC passes the max
number of UART clocks is has. The imx clock driver will create
an array to enable on startup, and disable later.
V2: Attempt to port driver from vendor kernel.
---
drivers/clk/imx/clk-imx25.c | 12 +-
drivers/clk/imx/clk-imx27.c | 13 +--
drivers/clk/imx/clk-imx35.c | 10 +
drivers/clk/imx/clk-imx5.c| 30 +++--
drivers/clk/imx/clk-imx6q.c | 16 +-
drivers/clk/imx/clk-imx6sl.c | 16 +-
drivers/clk/imx/clk-imx6sll.c | 24 +---
drivers/clk/imx/clk-imx6sx.c | 16 +-
drivers/clk/imx/clk-imx7d.c | 22 +--
drivers/clk/imx/clk-imx7ulp.c | 31 ++
drivers/clk/imx/clk-imx8mm.c | 18 ++-
drivers/clk/imx/clk-imx8mn.c | 18 ++-
drivers/clk/imx/clk-imx8mp.c | 17 +--
drivers/clk/imx/clk-imx8mq.c | 18 ++-
drivers/clk/imx/clk.c | 41 +++
drivers/clk/imx/clk.h | 4 ++--
16 files changed, 54 insertions(+), 252 deletions(-)
diff --git a/drivers/clk/imx/clk-imx25.c b/drivers/clk/imx/clk-imx25.c
index a66cabfbf94f..66192fe0a898 100644
--- a/drivers/clk/imx/clk-imx25.c
+++ b/drivers/clk/imx/clk-imx25.c
@@ -73,16 +73,6 @@ enum mx25_clks {
static struct clk *clk[clk_max];
-static struct clk ** const uart_clks[] __initconst = {
- [uart_ipg_per],
- [uart1_ipg],
- [uart2_ipg],
- [uart3_ipg],
- [uart4_ipg],
- [uart5_ipg],
- NULL
-};
-
static int __init __mx25_clocks_init(void __iomem *ccm_base)
{
BUG_ON(!ccm_base);
@@ -228,7 +218,7 @@ static int __init __mx25_clocks_init(void __iomem *ccm_base)
*/
clk_set_parent(clk[cko_sel], clk[ipg]);
- imx_register_uart_clocks(uart_clks);
+ imx_register_uart_clocks(6);
return 0;
}
diff --git a/drivers/clk/imx/clk-imx27.c b/drivers/clk/imx/clk-imx27.c
index 5585ded8b8c6..56a5fc402b10 100644
--- a/drivers/clk/imx/clk-imx27.c
+++ b/drivers/clk/imx/clk-imx27.c
@@ -49,17 +49,6 @@ static const char *ssi_sel_clks[] = { "spll_gate", "mpll", };
static struct clk *clk[IMX27_CLK_MAX];
static struct clk_onecell_data clk_data;
-static struct clk ** const uart_clks[] __initconst = {
- [IMX27_CLK_PER1_GATE],
- [IMX27_CLK_UART1_IPG_GATE],
- [IMX27_CLK_UART2_IPG_GATE],
- [IMX27_CLK_UART3_IPG_GATE],
- [IMX27_CLK_UART4_IPG_GATE],
- [IMX27_CLK_UART5_IPG_GATE],
- [IMX27_CLK_UART6_IPG_GATE],
- NULL
-};
-
static void __init _mx27_clocks_init(unsigned long fref)
{
BUG_ON(!ccm);
@@ -176,7 +165,7 @@ static void __init _mx27_clocks_init(unsigned long fref)
clk_prepare_enable(clk[IMX27_CLK_EMI_AHB_GATE]);
- imx_register_uart_clocks(uart_clks);
+ imx_register_uart_clocks(7);
imx_print_silicon_rev("i.MX27", mx27_revision());
}
diff --git a/drivers/clk/imx/clk-imx35.c b/drivers/clk/imx/clk-imx35.c
index c1df03665c09..0fe5ac210156 100644
--- a/drivers/clk/imx/clk-imx35.c
+++ b/drivers/clk/imx/clk-imx35.c
@@ -82,14 +82,6 @@ enum mx35_clks {
static struct clk *clk[clk_max];
-static struct clk ** const uart_clks[] __initconst = {
- [ipg],
- [uart1_gate],
- [uart2_gate],
- [uart3_gate],
- NULL
-};
-
static void __init _mx35_clocks_init(void)
{
void __iomem *base;
@@ -243,7 +235,7 @@ static void __init _mx35_clocks_init(void)
*/
clk_prepare_enable(clk[scc_gate]);
- imx_register_uart_clocks(uart_clks);
+ imx_register_uart_clocks(4);
imx_print_silicon_rev("i.MX35", mx35_revision());
}
diff --git a/drivers/clk/imx/clk-imx5.c b/drivers/clk/imx/clk-imx5.c
index 01e079b81026..e4493846454d 100644
--- a/drivers/clk/imx/clk-imx5.c
+++ b/drivers/clk/imx/clk-imx5.c
@@ -128,30 +128,6 @@ static const char *ieee1588_sels[] = {
