All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] clk: imx8mq: Mark AHB clock as critical
@ 2019-06-25  9:01 ` Abel Vesa
  0 siblings, 0 replies; 14+ 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] 14+ messages in thread

* [PATCH] clk: imx8mq: Mark AHB clock as critical
@ 2019-06-25  9:01 ` Abel Vesa
  0 siblings, 0 replies; 14+ 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: Linux Kernel Mailing List, Abel Vesa, NXP Linux Team,
	linux-arm-kernel, linux-clk

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] clk: imx8mq: Mark AHB clock as critical
  2019-06-25  9:01 ` Abel Vesa
@ 2019-06-25 22:32   ` Stephen Boyd
  -1 siblings, 0 replies; 14+ 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] 14+ messages in thread

* Re: [PATCH] clk: imx8mq: Mark AHB clock as critical
@ 2019-06-25 22:32   ` Stephen Boyd
  0 siblings, 0 replies; 14+ 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: Linux Kernel Mailing List, Abel Vesa, NXP Linux Team,
	linux-arm-kernel, linux-clk

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
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ 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
  -1 siblings, 0 replies; 14+ 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] 14+ messages in thread

* Re: [PATCH] clk: imx8mq: Mark AHB clock as critical
@ 2019-07-05  8:52     ` Abel Vesa
  0 siblings, 0 replies; 14+ messages in thread
From: Abel Vesa @ 2019-07-05  8:52 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Anson Huang, Mike Turquette, Linux Kernel Mailing List,
	dl-linux-imx, Sascha Hauer, Fabio Estevam, Shawn Guo, linux-clk,
	linux-arm-kernel

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
> > 
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ 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
  -1 siblings, 0 replies; 14+ 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] 14+ messages in thread

* Re: [PATCH] clk: imx8mq: Mark AHB clock as critical
@ 2019-07-22 21:25       ` Stephen Boyd
  0 siblings, 0 replies; 14+ messages in thread
From: Stephen Boyd @ 2019-07-22 21:25 UTC (permalink / raw)
  To: Abel Vesa
  Cc: Anson Huang, Mike Turquette, Linux Kernel Mailing List,
	dl-linux-imx, Sascha Hauer, Fabio Estevam, Shawn Guo, linux-clk,
	linux-arm-kernel

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ 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
  -1 siblings, 0 replies; 14+ 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] 14+ messages in thread

* Re: [PATCH] clk: imx8mq: Mark AHB clock as critical
@ 2019-07-27  6:30         ` Daniel Baluta
  0 siblings, 0 replies; 14+ messages in thread
From: Daniel Baluta @ 2019-07-27  6:30 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Abel Vesa, Anson Huang, Mike Turquette,
	Linux Kernel Mailing List, dl-linux-imx, Sascha Hauer,
	Fabio Estevam, Shawn Guo, linux-clk, linux-arm-kernel

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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ 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
  -1 siblings, 0 replies; 14+ 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] 14+ messages in thread

* Re: [PATCH] clk: imx8mq: Mark AHB clock as critical
@ 2019-08-03  7:27           ` Shawn Guo
  0 siblings, 0 replies; 14+ messages in thread
From: Shawn Guo @ 2019-08-03  7:27 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, 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ 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
  -1 siblings, 0 replies; 14+ 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] 14+ messages in thread

* Re: [PATCH] clk: imx8mq: Mark AHB clock as critical
@ 2019-08-03  8:00             ` Shawn Guo
  0 siblings, 0 replies; 14+ 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-08-03  8:00 UTC | newest]

Thread overview: 14+ 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  9:01 ` Abel Vesa
2019-06-25 22:32 ` Stephen Boyd
2019-06-25 22:32   ` Stephen Boyd
2019-07-05  8:52   ` Abel Vesa
2019-07-05  8:52     ` Abel Vesa
2019-07-22 21:25     ` Stephen Boyd
2019-07-22 21:25       ` Stephen Boyd
2019-07-27  6:30       ` Daniel Baluta
2019-07-27  6:30         ` Daniel Baluta
2019-08-03  7:27         ` Shawn Guo
2019-08-03  7:27           ` Shawn Guo
2019-08-03  8:00           ` Shawn Guo
2019-08-03  8:00             ` Shawn Guo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.