* [PATCH] clk: imx8mq: Mark AHB clock as critical
@ 2019-06-25 9:01 Abel Vesa
2019-06-25 22:32 ` Stephen Boyd
0 siblings, 1 reply; 7+ messages in thread
From: Abel Vesa @ 2019-06-25 9:01 UTC (permalink / raw)
To: Mike Turquette, Stephen Boyd, Shawn Guo, Sascha Hauer,
Fabio Estevam, Anson Huang
Cc: NXP Linux Team, linux-clk, linux-arm-kernel,
Linux Kernel Mailing List, Abel Vesa
Keep the AHB clock always on since there is no driver to control it and
all the other clocks that use it as parent rely on it being always enabled.
Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
drivers/clk/imx/clk-imx8mq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
index 5fbc2a7..b48268b 100644
--- a/drivers/clk/imx/clk-imx8mq.c
+++ b/drivers/clk/imx/clk-imx8mq.c
@@ -398,7 +398,7 @@ static int imx8mq_clocks_probe(struct platform_device *pdev)
clks[IMX8MQ_CLK_NOC_APB] = imx8m_clk_composite_critical("noc_apb", imx8mq_noc_apb_sels, base + 0x8d80);
/* AHB */
- clks[IMX8MQ_CLK_AHB] = imx8m_clk_composite("ahb", imx8mq_ahb_sels, base + 0x9000);
+ clks[IMX8MQ_CLK_AHB] = imx8m_clk_composite_critical("ahb", imx8mq_ahb_sels, base + 0x9000);
clks[IMX8MQ_CLK_AUDIO_AHB] = imx8m_clk_composite("audio_ahb", imx8mq_audio_ahb_sels, base + 0x9100);
/* IPG */
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] clk: imx8mq: Mark AHB clock as critical
2019-06-25 9:01 [PATCH] clk: imx8mq: Mark AHB clock as critical Abel Vesa
@ 2019-06-25 22:32 ` Stephen Boyd
2019-07-05 8:52 ` Abel Vesa
0 siblings, 1 reply; 7+ messages in thread
From: Stephen Boyd @ 2019-06-25 22:32 UTC (permalink / raw)
To: Abel Vesa, Anson Huang, Fabio Estevam, Mike Turquette,
Sascha Hauer, Shawn Guo
Cc: NXP Linux Team, linux-clk, linux-arm-kernel,
Linux Kernel Mailing List, Abel Vesa
Quoting Abel Vesa (2019-06-25 02:01:56)
> Keep the AHB clock always on since there is no driver to control it and
> all the other clocks that use it as parent rely on it being always enabled.
>
> Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
> ---
> drivers/clk/imx/clk-imx8mq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
> index 5fbc2a7..b48268b 100644
> --- a/drivers/clk/imx/clk-imx8mq.c
> +++ b/drivers/clk/imx/clk-imx8mq.c
> @@ -398,7 +398,7 @@ static int imx8mq_clocks_probe(struct platform_device *pdev)
> clks[IMX8MQ_CLK_NOC_APB] = imx8m_clk_composite_critical("noc_apb", imx8mq_noc_apb_sels, base + 0x8d80);
>
> /* AHB */
> - clks[IMX8MQ_CLK_AHB] = imx8m_clk_composite("ahb", imx8mq_ahb_sels, base + 0x9000);
> + clks[IMX8MQ_CLK_AHB] = imx8m_clk_composite_critical("ahb", imx8mq_ahb_sels, base + 0x9000);
Please add a comment into the code why it's critical.
> clks[IMX8MQ_CLK_AUDIO_AHB] = imx8m_clk_composite("audio_ahb", imx8mq_audio_ahb_sels, base + 0x9100);
>
> /* IPG */
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] clk: imx8mq: Mark AHB clock as critical
2019-06-25 22:32 ` Stephen Boyd
@ 2019-07-05 8:52 ` Abel Vesa
2019-07-22 21:25 ` Stephen Boyd
0 siblings, 1 reply; 7+ messages in thread
From: Abel Vesa @ 2019-07-05 8:52 UTC (permalink / raw)
To: Stephen Boyd
Cc: Anson Huang, Fabio Estevam, Mike Turquette, Sascha Hauer,
Shawn Guo, dl-linux-imx, linux-clk, linux-arm-kernel,
Linux Kernel Mailing List
On 19-06-25 15:32:22, Stephen Boyd wrote:
> Quoting Abel Vesa (2019-06-25 02:01:56)
> > Keep the AHB clock always on since there is no driver to control it and
> > all the other clocks that use it as parent rely on it being always enabled.
> >
> > Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
> > ---
> > drivers/clk/imx/clk-imx8mq.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
> > index 5fbc2a7..b48268b 100644
> > --- a/drivers/clk/imx/clk-imx8mq.c
> > +++ b/drivers/clk/imx/clk-imx8mq.c
> > @@ -398,7 +398,7 @@ static int imx8mq_clocks_probe(struct platform_device *pdev)
> > clks[IMX8MQ_CLK_NOC_APB] = imx8m_clk_composite_critical("noc_apb", imx8mq_noc_apb_sels, base + 0x8d80);
> >
> > /* AHB */
> > - clks[IMX8MQ_CLK_AHB] = imx8m_clk_composite("ahb", imx8mq_ahb_sels, base + 0x9000);
> > + clks[IMX8MQ_CLK_AHB] = imx8m_clk_composite_critical("ahb", imx8mq_ahb_sels, base + 0x9000);
>
> Please add a comment into the code why it's critical.
Comment explaining why the AHB bus clock is critical ?
Isn't that self-explanatory ?
>
> > clks[IMX8MQ_CLK_AUDIO_AHB] = imx8m_clk_composite("audio_ahb", imx8mq_audio_ahb_sels, base + 0x9100);
> >
> > /* IPG */
> > --
> > 2.7.4
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] clk: imx8mq: Mark AHB clock as critical
2019-07-05 8:52 ` Abel Vesa
@ 2019-07-22 21:25 ` Stephen Boyd
2019-07-27 6:30 ` Daniel Baluta
0 siblings, 1 reply; 7+ messages in thread
From: Stephen Boyd @ 2019-07-22 21:25 UTC (permalink / raw)
To: Abel Vesa
Cc: Anson Huang, Fabio Estevam, Mike Turquette, Sascha Hauer,
Shawn Guo, dl-linux-imx, linux-clk, linux-arm-kernel,
Linux Kernel Mailing List
Quoting Abel Vesa (2019-07-05 01:52:19)
> On 19-06-25 15:32:22, Stephen Boyd wrote:
> > Quoting Abel Vesa (2019-06-25 02:01:56)
> > > Keep the AHB clock always on since there is no driver to control it and
> > > all the other clocks that use it as parent rely on it being always enabled.
> > >
> > > Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
> > > ---
> > > drivers/clk/imx/clk-imx8mq.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
> > > index 5fbc2a7..b48268b 100644
> > > --- a/drivers/clk/imx/clk-imx8mq.c
> > > +++ b/drivers/clk/imx/clk-imx8mq.c
> > > @@ -398,7 +398,7 @@ static int imx8mq_clocks_probe(struct platform_device *pdev)
> > > clks[IMX8MQ_CLK_NOC_APB] = imx8m_clk_composite_critical("noc_apb", imx8mq_noc_apb_sels, base + 0x8d80);
> > >
> > > /* AHB */
> > > - clks[IMX8MQ_CLK_AHB] = imx8m_clk_composite("ahb", imx8mq_ahb_sels, base + 0x9000);
> > > + clks[IMX8MQ_CLK_AHB] = imx8m_clk_composite_critical("ahb", imx8mq_ahb_sels, base + 0x9000);
> >
> > Please add a comment into the code why it's critical.
>
> Comment explaining why the AHB bus clock is critical ?
> Isn't that self-explanatory ?
Nope, it isn't self-explanatory, because nothing on this line says "bus"
and it could be that someone reading this code isn't well versed in the
concepts of ARM world AHB to connect the two.
>
> >
> > > clks[IMX8MQ_CLK_AUDIO_AHB] = imx8m_clk_composite("audio_ahb", imx8mq_audio_ahb_sels, base + 0x9100);
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] clk: imx8mq: Mark AHB clock as critical
2019-07-22 21:25 ` Stephen Boyd
@ 2019-07-27 6:30 ` Daniel Baluta
2019-08-03 7:27 ` Shawn Guo
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Baluta @ 2019-07-27 6:30 UTC (permalink / raw)
To: Stephen Boyd
Cc: Abel Vesa, Anson Huang, Fabio Estevam, Mike Turquette,
Sascha Hauer, Shawn Guo, dl-linux-imx, linux-clk,
linux-arm-kernel, Linux Kernel Mailing List
On Tue, Jul 23, 2019 at 6:17 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Abel Vesa (2019-07-05 01:52:19)
> > On 19-06-25 15:32:22, Stephen Boyd wrote:
> > > Quoting Abel Vesa (2019-06-25 02:01:56)
> > > > Keep the AHB clock always on since there is no driver to control it and
> > > > all the other clocks that use it as parent rely on it being always enabled.
> > > >
> > > > Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
> > > > ---
> > > > drivers/clk/imx/clk-imx8mq.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
> > > > index 5fbc2a7..b48268b 100644
> > > > --- a/drivers/clk/imx/clk-imx8mq.c
> > > > +++ b/drivers/clk/imx/clk-imx8mq.c
> > > > @@ -398,7 +398,7 @@ static int imx8mq_clocks_probe(struct platform_device *pdev)
> > > > clks[IMX8MQ_CLK_NOC_APB] = imx8m_clk_composite_critical("noc_apb", imx8mq_noc_apb_sels, base + 0x8d80);
> > > >
> > > > /* AHB */
> > > > - clks[IMX8MQ_CLK_AHB] = imx8m_clk_composite("ahb", imx8mq_ahb_sels, base + 0x9000);
> > > > + clks[IMX8MQ_CLK_AHB] = imx8m_clk_composite_critical("ahb", imx8mq_ahb_sels, base + 0x9000);
> > >
> > > Please add a comment into the code why it's critical.
> >
> > Comment explaining why the AHB bus clock is critical ?
> > Isn't that self-explanatory ?
>
> Nope, it isn't self-explanatory, because nothing on this line says "bus"
> and it could be that someone reading this code isn't well versed in the
> concepts of ARM world AHB to connect the two.
Agree with Stephen. Commit message should try to give as much details
as possible
also maybe educate the readers who might not have that much knowledge.
Abel, I understand that for someone who works daily with this part of the kernel
this change might look trivial.
Also, without this patch linux-next hangs on imx8mq.
With the explanation added you can add my:
Tested-by: Daniel Baluta <daniel.baluta@nxp.com>
thanks,
Daniel.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] clk: imx8mq: Mark AHB clock as critical
2019-07-27 6:30 ` Daniel Baluta
@ 2019-08-03 7:27 ` Shawn Guo
2019-08-03 8:00 ` Shawn Guo
0 siblings, 1 reply; 7+ messages in thread
From: Shawn Guo @ 2019-08-03 7:27 UTC (permalink / raw)
To: Daniel Baluta
Cc: Stephen Boyd, Abel Vesa, Anson Huang, Fabio Estevam,
Mike Turquette, Sascha Hauer, dl-linux-imx, linux-clk,
linux-arm-kernel, Linux Kernel Mailing List
On Sat, Jul 27, 2019 at 09:30:57AM +0300, Daniel Baluta wrote:
> On Tue, Jul 23, 2019 at 6:17 AM Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Quoting Abel Vesa (2019-07-05 01:52:19)
> > > On 19-06-25 15:32:22, Stephen Boyd wrote:
> > > > Quoting Abel Vesa (2019-06-25 02:01:56)
> > > > > Keep the AHB clock always on since there is no driver to control it and
> > > > > all the other clocks that use it as parent rely on it being always enabled.
> > > > >
> > > > > Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
> > > > > ---
> > > > > drivers/clk/imx/clk-imx8mq.c | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
> > > > > index 5fbc2a7..b48268b 100644
> > > > > --- a/drivers/clk/imx/clk-imx8mq.c
> > > > > +++ b/drivers/clk/imx/clk-imx8mq.c
> > > > > @@ -398,7 +398,7 @@ static int imx8mq_clocks_probe(struct platform_device *pdev)
> > > > > clks[IMX8MQ_CLK_NOC_APB] = imx8m_clk_composite_critical("noc_apb", imx8mq_noc_apb_sels, base + 0x8d80);
> > > > >
> > > > > /* AHB */
> > > > > - clks[IMX8MQ_CLK_AHB] = imx8m_clk_composite("ahb", imx8mq_ahb_sels, base + 0x9000);
> > > > > + clks[IMX8MQ_CLK_AHB] = imx8m_clk_composite_critical("ahb", imx8mq_ahb_sels, base + 0x9000);
> > > >
> > > > Please add a comment into the code why it's critical.
> > >
> > > Comment explaining why the AHB bus clock is critical ?
> > > Isn't that self-explanatory ?
> >
> > Nope, it isn't self-explanatory, because nothing on this line says "bus"
> > and it could be that someone reading this code isn't well versed in the
> > concepts of ARM world AHB to connect the two.
>
> Agree with Stephen. Commit message should try to give as much details
> as possible
> also maybe educate the readers who might not have that much knowledge.
>
> Abel, I understand that for someone who works daily with this part of the kernel
> this change might look trivial.
>
> Also, without this patch linux-next hangs on imx8mq.
How does that happen? Mainline is fine there?
Shawn
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] clk: imx8mq: Mark AHB clock as critical
2019-08-03 7:27 ` Shawn Guo
@ 2019-08-03 8:00 ` Shawn Guo
0 siblings, 0 replies; 7+ messages in thread
From: Shawn Guo @ 2019-08-03 8:00 UTC (permalink / raw)
To: Daniel Baluta
Cc: Abel Vesa, Anson Huang, Stephen Boyd, Mike Turquette,
Linux Kernel Mailing List, dl-linux-imx, Sascha Hauer,
Fabio Estevam, linux-clk, linux-arm-kernel
On Sat, Aug 03, 2019 at 09:27:25AM +0200, Shawn Guo wrote:
> > Also, without this patch linux-next hangs on imx8mq.
>
> How does that happen? Mainline is fine there?
Okay, understood it by reading more threads.
Shawn
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-08-03 8:00 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-25 9:01 [PATCH] clk: imx8mq: Mark AHB clock as critical Abel Vesa
2019-06-25 22:32 ` Stephen Boyd
2019-07-05 8:52 ` Abel Vesa
2019-07-22 21:25 ` Stephen Boyd
2019-07-27 6:30 ` Daniel Baluta
2019-08-03 7:27 ` Shawn Guo
2019-08-03 8:00 ` Shawn Guo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).