Linux-Clk Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] clk: ast2600: enable BCLK for PCI/PCIe bus always
@ 2020-01-13 21:34 Jae Hyun Yoo
  2020-01-14  6:15 ` Joel Stanley
  0 siblings, 1 reply; 5+ messages in thread
From: Jae Hyun Yoo @ 2020-01-13 21:34 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Joel Stanley, Andrew Jeffery
  Cc: linux-clk, linux-aspeed, Jae Hyun Yoo

BCLK for PCI/PCIe bus should be enabled always with having the
CLK_IS_CRITICAL flag otherwise it will be disabled at kernel late
initcall phase as an unused clock, and eventually it causes
unexpected behavior on BMC features that are connected to the host
through PCI/PCIe bus.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 drivers/clk/clk-ast2600.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk-ast2600.c b/drivers/clk/clk-ast2600.c
index 392d01705b97..42bfdc16bf7a 100644
--- a/drivers/clk/clk-ast2600.c
+++ b/drivers/clk/clk-ast2600.c
@@ -64,7 +64,7 @@ static const struct aspeed_gate_data aspeed_g6_gates[] = {
 	[ASPEED_CLK_GATE_GCLK]		= {  2,  7, "gclk-gate",	NULL,	 0 },	/* 2D engine */
 	/* vclk parent - dclk/d1clk/hclk/mclk */
 	[ASPEED_CLK_GATE_VCLK]		= {  3,  6, "vclk-gate",	NULL,	 0 },	/* Video Capture */
-	[ASPEED_CLK_GATE_BCLK]		= {  4,  8, "bclk-gate",	"bclk",	 0 }, /* PCIe/PCI */
+	[ASPEED_CLK_GATE_BCLK]		= {  4,  8, "bclk-gate",	"bclk",	 CLK_IS_CRITICAL }, /* PCIe/PCI */
 	/* From dpll */
 	[ASPEED_CLK_GATE_DCLK]		= {  5, -1, "dclk-gate",	NULL,	 CLK_IS_CRITICAL }, /* DAC */
 	[ASPEED_CLK_GATE_REF0CLK]	= {  6, -1, "ref0clk-gate",	"clkin", CLK_IS_CRITICAL },
-- 
2.17.1


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

* Re: [PATCH] clk: ast2600: enable BCLK for PCI/PCIe bus always
  2020-01-13 21:34 [PATCH] clk: ast2600: enable BCLK for PCI/PCIe bus always Jae Hyun Yoo
@ 2020-01-14  6:15 ` Joel Stanley
  2020-01-14 19:53   ` Jae Hyun Yoo
  0 siblings, 1 reply; 5+ messages in thread
From: Joel Stanley @ 2020-01-14  6:15 UTC (permalink / raw)
  To: Jae Hyun Yoo
  Cc: Michael Turquette, Stephen Boyd, Andrew Jeffery, linux-clk, linux-aspeed

On Mon, 13 Jan 2020 at 21:33, Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> wrote:
>
> BCLK for PCI/PCIe bus should be enabled always with having the
> CLK_IS_CRITICAL flag otherwise it will be disabled at kernel late
> initcall phase as an unused clock, and eventually it causes
> unexpected behavior on BMC features that are connected to the host
> through PCI/PCIe bus.

This is true for systems that have PCIe connected. There are systems
that do not, and in that case we don't want to have the clock enabled.

Are you doing this to support the case where the PCIe device not load
a BMC driver? (eg for host VGA use). If not, then you can have the
driver you're loading request the BCLK.

If this is for the host VGA device, then you will need to come up with
a mechanism that makes the enabling of this clock depend on the device
tree.

Cheers,

Joel

>
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> ---
>  drivers/clk/clk-ast2600.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/clk-ast2600.c b/drivers/clk/clk-ast2600.c
> index 392d01705b97..42bfdc16bf7a 100644
> --- a/drivers/clk/clk-ast2600.c
> +++ b/drivers/clk/clk-ast2600.c
> @@ -64,7 +64,7 @@ static const struct aspeed_gate_data aspeed_g6_gates[] = {
>         [ASPEED_CLK_GATE_GCLK]          = {  2,  7, "gclk-gate",        NULL,    0 },   /* 2D engine */
>         /* vclk parent - dclk/d1clk/hclk/mclk */
>         [ASPEED_CLK_GATE_VCLK]          = {  3,  6, "vclk-gate",        NULL,    0 },   /* Video Capture */
> -       [ASPEED_CLK_GATE_BCLK]          = {  4,  8, "bclk-gate",        "bclk",  0 }, /* PCIe/PCI */
> +       [ASPEED_CLK_GATE_BCLK]          = {  4,  8, "bclk-gate",        "bclk",  CLK_IS_CRITICAL }, /* PCIe/PCI */
>         /* From dpll */
>         [ASPEED_CLK_GATE_DCLK]          = {  5, -1, "dclk-gate",        NULL,    CLK_IS_CRITICAL }, /* DAC */
>         [ASPEED_CLK_GATE_REF0CLK]       = {  6, -1, "ref0clk-gate",     "clkin", CLK_IS_CRITICAL },
> --
> 2.17.1
>

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

* Re: [PATCH] clk: ast2600: enable BCLK for PCI/PCIe bus always
  2020-01-14  6:15 ` Joel Stanley
@ 2020-01-14 19:53   ` Jae Hyun Yoo
  2020-01-15  5:43     ` Joel Stanley
  0 siblings, 1 reply; 5+ messages in thread
From: Jae Hyun Yoo @ 2020-01-14 19:53 UTC (permalink / raw)
  To: Joel Stanley
  Cc: Michael Turquette, Stephen Boyd, Andrew Jeffery, linux-clk, linux-aspeed

Hi Joel,

On 1/13/2020 10:15 PM, Joel Stanley wrote:
> On Mon, 13 Jan 2020 at 21:33, Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> wrote:
>>
>> BCLK for PCI/PCIe bus should be enabled always with having the
>> CLK_IS_CRITICAL flag otherwise it will be disabled at kernel late
>> initcall phase as an unused clock, and eventually it causes
>> unexpected behavior on BMC features that are connected to the host
>> through PCI/PCIe bus.
> 
> This is true for systems that have PCIe connected. There are systems
> that do not, and in that case we don't want to have the clock enabled.
> 
> Are you doing this to support the case where the PCIe device not load
> a BMC driver? (eg for host VGA use). If not, then you can have the
> driver you're loading request the BCLK.
> 
> If this is for the host VGA device, then you will need to come up with
> a mechanism that makes the enabling of this clock depend on the device
> tree.

Is there any way to enable clock by just adding a node in device tree?
Do you want me to add a simple driver module for host VGA just for
enabling BCLK?

Thanks,

Jae

> Cheers,
> 
> Joel
> 
>>
>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>> ---
>>   drivers/clk/clk-ast2600.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/clk-ast2600.c b/drivers/clk/clk-ast2600.c
>> index 392d01705b97..42bfdc16bf7a 100644
>> --- a/drivers/clk/clk-ast2600.c
>> +++ b/drivers/clk/clk-ast2600.c
>> @@ -64,7 +64,7 @@ static const struct aspeed_gate_data aspeed_g6_gates[] = {
>>          [ASPEED_CLK_GATE_GCLK]          = {  2,  7, "gclk-gate",        NULL,    0 },   /* 2D engine */
>>          /* vclk parent - dclk/d1clk/hclk/mclk */
>>          [ASPEED_CLK_GATE_VCLK]          = {  3,  6, "vclk-gate",        NULL,    0 },   /* Video Capture */
>> -       [ASPEED_CLK_GATE_BCLK]          = {  4,  8, "bclk-gate",        "bclk",  0 }, /* PCIe/PCI */
>> +       [ASPEED_CLK_GATE_BCLK]          = {  4,  8, "bclk-gate",        "bclk",  CLK_IS_CRITICAL }, /* PCIe/PCI */
>>          /* From dpll */
>>          [ASPEED_CLK_GATE_DCLK]          = {  5, -1, "dclk-gate",        NULL,    CLK_IS_CRITICAL }, /* DAC */
>>          [ASPEED_CLK_GATE_REF0CLK]       = {  6, -1, "ref0clk-gate",     "clkin", CLK_IS_CRITICAL },
>> --
>> 2.17.1
>>
> 

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

* Re: [PATCH] clk: ast2600: enable BCLK for PCI/PCIe bus always
  2020-01-14 19:53   ` Jae Hyun Yoo
@ 2020-01-15  5:43     ` Joel Stanley
  2020-01-15 20:21       ` Jae Hyun Yoo
  0 siblings, 1 reply; 5+ messages in thread
From: Joel Stanley @ 2020-01-15  5:43 UTC (permalink / raw)
  To: Jae Hyun Yoo
  Cc: Michael Turquette, Stephen Boyd, Andrew Jeffery, linux-clk, linux-aspeed

On Tue, 14 Jan 2020 at 19:53, Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> wrote:
>
> Hi Joel,
>
> On 1/13/2020 10:15 PM, Joel Stanley wrote:
> > On Mon, 13 Jan 2020 at 21:33, Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> wrote:
> >>
> >> BCLK for PCI/PCIe bus should be enabled always with having the
> >> CLK_IS_CRITICAL flag otherwise it will be disabled at kernel late
> >> initcall phase as an unused clock, and eventually it causes
> >> unexpected behavior on BMC features that are connected to the host
> >> through PCI/PCIe bus.
> >
> > This is true for systems that have PCIe connected. There are systems
> > that do not, and in that case we don't want to have the clock enabled.
> >
> > Are you doing this to support the case where the PCIe device not load
> > a BMC driver? (eg for host VGA use). If not, then you can have the
> > driver you're loading request the BCLK.
> >
> > If this is for the host VGA device, then you will need to come up with
> > a mechanism that makes the enabling of this clock depend on the device
> > tree.
>
> Is there any way to enable clock by just adding a node in device tree?
> Do you want me to add a simple driver module for host VGA just for
> enabling BCLK?

Okay, so this is for the case where we lack a BMC driver.

One option would be to do as you suggest, adding a simple driver that
claims essential clocks.

Another could be to add some extra properties that clock driver parses
and enables the clocks.

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

* Re: [PATCH] clk: ast2600: enable BCLK for PCI/PCIe bus always
  2020-01-15  5:43     ` Joel Stanley
@ 2020-01-15 20:21       ` Jae Hyun Yoo
  0 siblings, 0 replies; 5+ messages in thread
From: Jae Hyun Yoo @ 2020-01-15 20:21 UTC (permalink / raw)
  To: Joel Stanley
  Cc: Michael Turquette, Stephen Boyd, Andrew Jeffery, linux-clk, linux-aspeed

On 1/14/2020 9:43 PM, Joel Stanley wrote:
> On Tue, 14 Jan 2020 at 19:53, Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> wrote:
>>
>> Hi Joel,
>>
>> On 1/13/2020 10:15 PM, Joel Stanley wrote:
>>> On Mon, 13 Jan 2020 at 21:33, Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> wrote:
>>>>
>>>> BCLK for PCI/PCIe bus should be enabled always with having the
>>>> CLK_IS_CRITICAL flag otherwise it will be disabled at kernel late
>>>> initcall phase as an unused clock, and eventually it causes
>>>> unexpected behavior on BMC features that are connected to the host
>>>> through PCI/PCIe bus.
>>>
>>> This is true for systems that have PCIe connected. There are systems
>>> that do not, and in that case we don't want to have the clock enabled.
>>>
>>> Are you doing this to support the case where the PCIe device not load
>>> a BMC driver? (eg for host VGA use). If not, then you can have the
>>> driver you're loading request the BCLK.
>>>
>>> If this is for the host VGA device, then you will need to come up with
>>> a mechanism that makes the enabling of this clock depend on the device
>>> tree.
>>
>> Is there any way to enable clock by just adding a node in device tree?
>> Do you want me to add a simple driver module for host VGA just for
>> enabling BCLK?
> 
> Okay, so this is for the case where we lack a BMC driver.
> 
> One option would be to do as you suggest, adding a simple driver that
> claims essential clocks.
> 
> Another could be to add some extra properties that clock driver parses
> and enables the clocks.
> 

Looks like it needs changes in clk-ast2600 module for checking of
'clock-critical' device tree property. Let me try to make it.

Thanks,
Jae

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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-13 21:34 [PATCH] clk: ast2600: enable BCLK for PCI/PCIe bus always Jae Hyun Yoo
2020-01-14  6:15 ` Joel Stanley
2020-01-14 19:53   ` Jae Hyun Yoo
2020-01-15  5:43     ` Joel Stanley
2020-01-15 20:21       ` Jae Hyun Yoo

Linux-Clk Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-clk/0 linux-clk/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-clk linux-clk/ https://lore.kernel.org/linux-clk \
		linux-clk@vger.kernel.org
	public-inbox-index linux-clk

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-clk


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git