Hi Jae, On Wed, 5 Dec 2018 at 05:20, Jae Hyun Yoo <jae.hyun.yoo@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx > Cc: Joel Stanley <joel@xxxxxxxxx> > Cc: Andrew Jeffery <andrew@xxxxxxxx> > Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@xxxxxxxxxxxxxxx> > --- > 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 >