linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] clk: aspeed: Mark lclk (LPC) and espiclk (eSPI) as critical
@ 2018-12-04 18:50 Jae Hyun Yoo
  2018-12-04 20:50 ` Stephen Boyd
  2018-12-04 20:51 ` Joel Stanley
  0 siblings, 2 replies; 5+ messages in thread
From: Jae Hyun Yoo @ 2018-12-04 18:50 UTC (permalink / raw)
  To: Joel Stanley, Andrew Jeffery, Michael Turquette, Stephen Boyd
  Cc: Jae Hyun Yoo, linux-aspeed, openbmc, Samuel Jiang, stable,
	linux-clk, Vijay Khemka, linux-arm-kernel

These interfaces are used by host to talk to BMC, and the clock
source is from the host, usually from PCH. So this commit marks
the lclk as critical to make it able to be enabled. Also, it marks
espiclk too because eSPI is sharing the same interface with LPC.

Fixes: 15ed8ce5f84e ("clk: aspeed: Register gated clocks")
Cc: stable@vger.kernel.org
Cc: Joel Stanley <joel@jms.id.au>
Cc: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 drivers/clk/clk-aspeed.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
index 596136793fc4..df9504427246 100644
--- a/drivers/clk/clk-aspeed.c
+++ b/drivers/clk/clk-aspeed.c
@@ -95,7 +95,7 @@ static const struct aspeed_gate_data aspeed_gates[] = {
 	[ASPEED_CLK_GATE_DCLK] =	{  5, -1, "dclk-gate",		NULL,	CLK_IS_CRITICAL }, /* DAC */
 	[ASPEED_CLK_GATE_REFCLK] =	{  6, -1, "refclk-gate",	"clkin", CLK_IS_CRITICAL },
 	[ASPEED_CLK_GATE_USBPORT2CLK] =	{  7,  3, "usb-port2-gate",	NULL,	0 }, /* USB2.0 Host port 2 */
-	[ASPEED_CLK_GATE_LCLK] =	{  8,  5, "lclk-gate",		NULL,	0 }, /* LPC */
+	[ASPEED_CLK_GATE_LCLK] =	{  8,  5, "lclk-gate",		NULL,	CLK_IS_CRITICAL }, /* LPC clock from host. No parent dependency */
 	[ASPEED_CLK_GATE_USBUHCICLK] =	{  9, 15, "usb-uhci-gate",	NULL,	0 }, /* USB1.1 (requires port 2 enabled) */
 	[ASPEED_CLK_GATE_D1CLK] =	{ 10, 13, "d1clk-gate",		NULL,	0 }, /* GFX CRT */
 	[ASPEED_CLK_GATE_YCLK] =	{ 13,  4, "yclk-gate",		NULL,	0 }, /* HAC */
@@ -103,7 +103,7 @@ static const struct aspeed_gate_data aspeed_gates[] = {
 	[ASPEED_CLK_GATE_UART1CLK] =	{ 15, -1, "uart1clk-gate",	"uart",	0 }, /* UART1 */
 	[ASPEED_CLK_GATE_UART2CLK] =	{ 16, -1, "uart2clk-gate",	"uart",	0 }, /* UART2 */
 	[ASPEED_CLK_GATE_UART5CLK] =	{ 17, -1, "uart5clk-gate",	"uart",	0 }, /* UART5 */
-	[ASPEED_CLK_GATE_ESPICLK] =	{ 19, -1, "espiclk-gate",	NULL,	0 }, /* eSPI */
+	[ASPEED_CLK_GATE_ESPICLK] =	{ 19, -1, "espiclk-gate",	NULL,	CLK_IS_CRITICAL }, /* eSPI clock from host. No parent dependency */
 	[ASPEED_CLK_GATE_MAC1CLK] =	{ 20, 11, "mac1clk-gate",	"mac",	0 }, /* MAC1 */
 	[ASPEED_CLK_GATE_MAC2CLK] =	{ 21, 12, "mac2clk-gate",	"mac",	0 }, /* MAC2 */
 	[ASPEED_CLK_GATE_RSACLK] =	{ 24, -1, "rsaclk-gate",	NULL,	0 }, /* RSA */
-- 
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] 5+ messages in thread

* Re: [PATCH v2] clk: aspeed: Mark lclk (LPC) and espiclk (eSPI) as critical
  2018-12-04 18:50 [PATCH v2] clk: aspeed: Mark lclk (LPC) and espiclk (eSPI) as critical Jae Hyun Yoo
@ 2018-12-04 20:50 ` Stephen Boyd
  2018-12-04 20:51 ` Joel Stanley
  1 sibling, 0 replies; 5+ messages in thread
From: Stephen Boyd @ 2018-12-04 20:50 UTC (permalink / raw)
  To: Andrew Jeffery, Jae Hyun Yoo, Joel Stanley, Michael Turquette
  Cc: Jae Hyun Yoo, linux-aspeed, openbmc, Samuel Jiang, stable,
	linux-clk, Vijay Khemka, linux-arm-kernel

Quoting Jae Hyun Yoo (2018-12-04 10:50:53)
> These interfaces are used by host to talk to BMC, and the clock
> source is from the host, usually from PCH. So this commit marks
> the lclk as critical to make it able to be enabled. Also, it marks
> espiclk too because eSPI is sharing the same interface with LPC.
> 
> Fixes: 15ed8ce5f84e ("clk: aspeed: Register gated clocks")
> Cc: stable@vger.kernel.org
> Cc: Joel Stanley <joel@jms.id.au>
> Cc: Andrew Jeffery <andrew@aj.id.au>
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> ---
>  drivers/clk/clk-aspeed.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
> index 596136793fc4..df9504427246 100644
> --- a/drivers/clk/clk-aspeed.c
> +++ b/drivers/clk/clk-aspeed.c
> @@ -95,7 +95,7 @@ static const struct aspeed_gate_data aspeed_gates[] = {
>         [ASPEED_CLK_GATE_DCLK] =        {  5, -1, "dclk-gate",          NULL,   CLK_IS_CRITICAL }, /* DAC */
>         [ASPEED_CLK_GATE_REFCLK] =      {  6, -1, "refclk-gate",        "clkin", CLK_IS_CRITICAL },
>         [ASPEED_CLK_GATE_USBPORT2CLK] = {  7,  3, "usb-port2-gate",     NULL,   0 }, /* USB2.0 Host port 2 */
> -       [ASPEED_CLK_GATE_LCLK] =        {  8,  5, "lclk-gate",          NULL,   0 }, /* LPC */
> +       [ASPEED_CLK_GATE_LCLK] =        {  8,  5, "lclk-gate",          NULL,   CLK_IS_CRITICAL }, /* LPC clock from host. No parent dependency */

What does "No parent dependency" mean? I was hoping to see something
like the commit text, "lclk is used to talk to BMC which can happen at
anytime" or something like that.


_______________________________________________
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] 5+ messages in thread

* Re: [PATCH v2] clk: aspeed: Mark lclk (LPC) and espiclk (eSPI) as critical
  2018-12-04 18:50 [PATCH v2] clk: aspeed: Mark lclk (LPC) and espiclk (eSPI) as critical Jae Hyun Yoo
  2018-12-04 20:50 ` Stephen Boyd
@ 2018-12-04 20:51 ` Joel Stanley
  2018-12-04 20:54   ` Stephen Boyd
  1 sibling, 1 reply; 5+ messages in thread
From: Joel Stanley @ 2018-12-04 20:51 UTC (permalink / raw)
  To: Jae Hyun Yoo
  Cc: linux-aspeed, Andrew Jeffery, OpenBMC Maillist,
	Michael Turquette, chyishian.jiang, # 3.4.x, linux-clk,
	Stephen Boyd, vijaykhemka, Linux ARM

Hi Jae,

On Wed, 5 Dec 2018 at 05:20, Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> wrote:
>
> These interfaces are used by host to talk to BMC, and the clock
> source is from the host, usually from PCH. So this commit marks
> the lclk as critical to make it able to be enabled. Also, it marks
> espiclk too because eSPI is sharing the same interface with LPC.

While this is true on the platform you have in mind, on other
platforms this is not the case. They do not use eSPI, and LCLK is
enabled by the driver that is used to configure LPC
(drivers/misc/aspeed-lpc-ctrl.c):

 lpc_ctrl: lpc-ctrl@0 {
     compatible = "aspeed,ast2400-lpc-ctrl";
     reg = <0x0 0x80>;
     clocks = <&syscon ASPEED_CLK_GATE_LCLK>;
 }

I wonder if we need a device tree binding to describe which clocks are critical.

Stephen, please don't merge this patch yet.

Cheers,

Joel


>
> Fixes: 15ed8ce5f84e ("clk: aspeed: Register gated clocks")
> Cc: stable@vger.kernel.org
> Cc: Joel Stanley <joel@jms.id.au>
> Cc: Andrew Jeffery <andrew@aj.id.au>
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> ---
>  drivers/clk/clk-aspeed.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
> index 596136793fc4..df9504427246 100644
> --- a/drivers/clk/clk-aspeed.c
> +++ b/drivers/clk/clk-aspeed.c
> @@ -95,7 +95,7 @@ static const struct aspeed_gate_data aspeed_gates[] = {
>         [ASPEED_CLK_GATE_DCLK] =        {  5, -1, "dclk-gate",          NULL,   CLK_IS_CRITICAL }, /* DAC */
>         [ASPEED_CLK_GATE_REFCLK] =      {  6, -1, "refclk-gate",        "clkin", CLK_IS_CRITICAL },
>         [ASPEED_CLK_GATE_USBPORT2CLK] = {  7,  3, "usb-port2-gate",     NULL,   0 }, /* USB2.0 Host port 2 */
> -       [ASPEED_CLK_GATE_LCLK] =        {  8,  5, "lclk-gate",          NULL,   0 }, /* LPC */
> +       [ASPEED_CLK_GATE_LCLK] =        {  8,  5, "lclk-gate",          NULL,   CLK_IS_CRITICAL }, /* LPC clock from host. No parent dependency */
>         [ASPEED_CLK_GATE_USBUHCICLK] =  {  9, 15, "usb-uhci-gate",      NULL,   0 }, /* USB1.1 (requires port 2 enabled) */
>         [ASPEED_CLK_GATE_D1CLK] =       { 10, 13, "d1clk-gate",         NULL,   0 }, /* GFX CRT */
>         [ASPEED_CLK_GATE_YCLK] =        { 13,  4, "yclk-gate",          NULL,   0 }, /* HAC */
> @@ -103,7 +103,7 @@ static const struct aspeed_gate_data aspeed_gates[] = {
>         [ASPEED_CLK_GATE_UART1CLK] =    { 15, -1, "uart1clk-gate",      "uart", 0 }, /* UART1 */
>         [ASPEED_CLK_GATE_UART2CLK] =    { 16, -1, "uart2clk-gate",      "uart", 0 }, /* UART2 */
>         [ASPEED_CLK_GATE_UART5CLK] =    { 17, -1, "uart5clk-gate",      "uart", 0 }, /* UART5 */
> -       [ASPEED_CLK_GATE_ESPICLK] =     { 19, -1, "espiclk-gate",       NULL,   0 }, /* eSPI */
> +       [ASPEED_CLK_GATE_ESPICLK] =     { 19, -1, "espiclk-gate",       NULL,   CLK_IS_CRITICAL }, /* eSPI clock from host. No parent dependency */
>         [ASPEED_CLK_GATE_MAC1CLK] =     { 20, 11, "mac1clk-gate",       "mac",  0 }, /* MAC1 */
>         [ASPEED_CLK_GATE_MAC2CLK] =     { 21, 12, "mac2clk-gate",       "mac",  0 }, /* MAC2 */
>         [ASPEED_CLK_GATE_RSACLK] =      { 24, -1, "rsaclk-gate",        NULL,   0 }, /* RSA */
> --
> 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] 5+ messages in thread

* Re: [PATCH v2] clk: aspeed: Mark lclk (LPC) and espiclk (eSPI) as critical
  2018-12-04 20:51 ` Joel Stanley
@ 2018-12-04 20:54   ` Stephen Boyd
  2018-12-05 15:04     ` Jae Hyun Yoo
  0 siblings, 1 reply; 5+ messages in thread
From: Stephen Boyd @ 2018-12-04 20:54 UTC (permalink / raw)
  To: Jae Hyun Yoo, Joel Stanley
  Cc: linux-aspeed, Andrew Jeffery, OpenBMC Maillist,
	Michael Turquette, chyishian.jiang, # 3.4.x, linux-clk,
	vijaykhemka, Linux ARM

Quoting Joel Stanley (2018-12-04 12:51:43)
> Hi Jae,
> 
> On Wed, 5 Dec 2018 at 05:20, Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> wrote:
> >
> > These interfaces are used by host to talk to BMC, and the clock
> > source is from the host, usually from PCH. So this commit marks
> > the lclk as critical to make it able to be enabled. Also, it marks
> > espiclk too because eSPI is sharing the same interface with LPC.
> 
> While this is true on the platform you have in mind, on other
> platforms this is not the case. They do not use eSPI, and LCLK is
> enabled by the driver that is used to configure LPC
> (drivers/misc/aspeed-lpc-ctrl.c):
> 
>  lpc_ctrl: lpc-ctrl@0 {
>      compatible = "aspeed,ast2400-lpc-ctrl";
>      reg = <0x0 0x80>;
>      clocks = <&syscon ASPEED_CLK_GATE_LCLK>;
>  }
> 
> I wonder if we need a device tree binding to describe which clocks are critical.

A binding to describe critical clks has been rejected in the past. I
don't think we need to have it here either? More information on why
things are being marked critical will be helpful to see if we need to
re-open that discussion again.

> 
> Stephen, please don't merge this patch yet.
> 

Sure.


_______________________________________________
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] 5+ messages in thread

* Re: [PATCH v2] clk: aspeed: Mark lclk (LPC) and espiclk (eSPI) as critical
  2018-12-04 20:54   ` Stephen Boyd
@ 2018-12-05 15:04     ` Jae Hyun Yoo
  0 siblings, 0 replies; 5+ messages in thread
From: Jae Hyun Yoo @ 2018-12-05 15:04 UTC (permalink / raw)
  To: Stephen Boyd, Joel Stanley
  Cc: linux-aspeed, Andrew Jeffery, OpenBMC Maillist, chyishian.jiang,
	# 3.4.x, linux-clk, Linux ARM, Michael Turquette, vijaykhemka

On 12/4/2018 2:54 PM, Stephen Boyd wrote:
> Quoting Joel Stanley (2018-12-04 12:51:43)
>> Hi Jae,
>>
>> On Wed, 5 Dec 2018 at 05:20, Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> wrote:
>>>
>>> These interfaces are used by host to talk to BMC, and the clock
>>> source is from the host, usually from PCH. So this commit marks
>>> the lclk as critical to make it able to be enabled. Also, it marks
>>> espiclk too because eSPI is sharing the same interface with LPC.
>>
>> While this is true on the platform you have in mind, on other
>> platforms this is not the case. They do not use eSPI, and LCLK is
>> enabled by the driver that is used to configure LPC
>> (drivers/misc/aspeed-lpc-ctrl.c):
>>
>>   lpc_ctrl: lpc-ctrl@0 {
>>       compatible = "aspeed,ast2400-lpc-ctrl";
>>       reg = <0x0 0x80>;
>>       clocks = <&syscon ASPEED_CLK_GATE_LCLK>;
>>   }
>>
>> I wonder if we need a device tree binding to describe which clocks are critical.
> 
> A binding to describe critical clks has been rejected in the past. I
> don't think we need to have it here either? More information on why
> things are being marked critical will be helpful to see if we need to
> re-open that discussion again.
> 
>>
>> Stephen, please don't merge this patch yet.
>>
> 
> Sure.
> 

Hi Stephen and Joel,

Thanks for blocking this patch. We checked that LCLK gate can be opened
by enabling lpc-ctrl node without using this change. Please drop this
patch.

Thanks,
Jae

_______________________________________________
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] 5+ messages in thread

end of thread, other threads:[~2018-12-05 15:04 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-04 18:50 [PATCH v2] clk: aspeed: Mark lclk (LPC) and espiclk (eSPI) as critical Jae Hyun Yoo
2018-12-04 20:50 ` Stephen Boyd
2018-12-04 20:51 ` Joel Stanley
2018-12-04 20:54   ` Stephen Boyd
2018-12-05 15:04     ` Jae Hyun Yoo

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