[HELP] ipmi-kcs didn't work

[HELP] ipmi-kcs didn't work

[qianlihu]

Hi,

I add ipmi kcs node,but it didn't work

here is the patch for dts.

        lpc_bmc: lpc-bmc@0 {
-               compatible = "aspeed,ast2500-lpc-bmc";
+               compatible = "aspeed,ast2500-lpc-bmc", "simple-mfd", "syscon";
+
                reg = <0x0 0x80>;
+               reg-io-width = <4>;
+
+               #address-cells = <1>;
+               #size-cells = <1>;
+               ranges = <0x0 0x0 0x80>;
+
+               kcs3: kcs3@0 {
+                       compatible = "aspeed,ast2500-kcs-bmc";
+                       reg = <0x0 0x80>;
+                       interrupts = <8>;
+                       kcs_chan = <3>;
+                       kcs_addr = <0xca2>;
+                       status = "disabled";
+
+               };
        };

+&kcs3 {
                         +       status = "okay";
        +};


The device node /dev/ipmi-kcs3 is created.


root@sanjose:/tmp# ./kcsbridged --vv --d /dev/ipmi-kcs3
Debug logging
Starting
Registering dbus methods/signals
Requesting dbus name: org.openbmc.HostIpmi
Getting dbus file descriptors
Opening /dev/ipmi-kcs3
Creating timer fd
Entering polling loop
Processed 1 dbus events
Processed 1 dbus events
Processed 1 dbus events
Processed 1 dbus events
Processed 1 dbus events
Processed 1 dbus events
Processed 1 dbus events

but there is nothing output.

Re: [HELP] ipmi-kcs didn't work

[Jae Hyun Yoo]

Hi,

On 11/14/2018 2:38 AM, qianlihu wrote:
> Hi,
> 
> I add ipmi kcs node,but it didn't work
> 
> here is the patch for dts.
> 
>          lpc_bmc: lpc-bmc@0 {
> -               compatible = "aspeed,ast2500-lpc-bmc";
> +               compatible = "aspeed,ast2500-lpc-bmc", "simple-mfd", "syscon";
> +
>                  reg = <0x0 0x80>;
> +               reg-io-width = <4>;
> +
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               ranges = <0x0 0x0 0x80>;
> +
> +               kcs3: kcs3@0 {
> +                       compatible = "aspeed,ast2500-kcs-bmc";
> +                       reg = <0x0 0x80>;
> +                       interrupts = <8>;
> +                       kcs_chan = <3>;
> +                       kcs_addr = <0xca2>;

Probably, you should add a clock setting for KCS H/W block into here.

	clocks = <&syscon ASPEED_CLK_GATE_LCLK>;

Cheers,

Jae

> +                       status = "disabled";
> +
> +               };
>          };
> 
> +&kcs3 {
>                           +       status = "okay";
>          +};
> 
> 
> The device node /dev/ipmi-kcs3 is created.
> 
> 
> root@sanjose:/tmp# ./kcsbridged --vv --d /dev/ipmi-kcs3
> Debug logging
> Starting
> Registering dbus methods/signals
> Requesting dbus name: org.openbmc.HostIpmi
> Getting dbus file descriptors
> Opening /dev/ipmi-kcs3
> Creating timer fd
> Entering polling loop
> Processed 1 dbus events
> Processed 1 dbus events
> Processed 1 dbus events
> Processed 1 dbus events
> Processed 1 dbus events
> Processed 1 dbus events
> Processed 1 dbus events
> 
> but there is nothing output.
> 

Re: [HELP] ipmi-kcs didn't work

[qianlihu]

Hi,Jae

>
> Hi,
>
> On 11/14/2018 2:38 AM, qianlihu wrote:
> > Hi,
> >
> > I add ipmi kcs node,but it didn't work
> >
> > here is the patch for dts.
> >
> >          lpc_bmc: lpc-bmc@0 {
> > -               compatible = "aspeed,ast2500-lpc-bmc";
> > +               compatible = "aspeed,ast2500-lpc-bmc", "simple-mfd", "syscon";
> > +
> >                  reg = <0x0 0x80>;
> > +               reg-io-width = <4>;
> > +
> > +               #address-cells = <1>;
> > +               #size-cells = <1>;
> > +               ranges = <0x0 0x0 0x80>;
> > +
> > +               kcs3: kcs3@0 {
> > +                       compatible = "aspeed,ast2500-kcs-bmc";
> > +                       reg = <0x0 0x80>;
> > +                       interrupts = <8>;
> > +                       kcs_chan = <3>;
> > +                       kcs_addr = <0xca2>;
>
> Probably, you should add a clock setting for KCS H/W block into here.
>
>         clocks = <&syscon ASPEED_CLK_GATE_LCLK>;
>
I tried ,but it didn't work．

> Cheers,
>
> Jae
>
> > +                       status = "disabled";
> > +
> > +               };
> >          };
> >
> > +&kcs3 {
> >                           +       status = "okay";
> >          +};
> >
> >
> > The device node /dev/ipmi-kcs3 is created.
> >
> >
> > root@sanjose:/tmp# ./kcsbridged --vv --d /dev/ipmi-kcs3
> > Debug logging
> > Starting
> > Registering dbus methods/signals
> > Requesting dbus name: org.openbmc.HostIpmi
> > Getting dbus file descriptors
> > Opening /dev/ipmi-kcs3
> > Creating timer fd
> > Entering polling loop
> > Processed 1 dbus events
> > Processed 1 dbus events
> > Processed 1 dbus events
> > Processed 1 dbus events
> > Processed 1 dbus events
> > Processed 1 dbus events
> > Processed 1 dbus events
> >
> > but there is nothing output.
> >

this is the dmesg info

root@sanjose:~# dmesg |grep kcs
[    2.052340] aspeed-kcs-bmc: channel=3 addr=0xca2 idr=0x2c odr=0x38 str=0x44

I also checked the hardtrap:
0x1e6e2070[bit25]  enabled the lpc mode

I'm sure the IO port is 0xca2.


should I config the pin for LPC,I checked the ibt node,it didn't.


thank you

--qianlihu

Re: [HELP] ipmi-kcs didn't work

[Jae Hyun Yoo]

On 11/14/2018 5:49 PM, qianlihu wrote:
> Hi,Jae
> 
>>
>> Hi,
>>
>> On 11/14/2018 2:38 AM, qianlihu wrote:
>>> Hi,
>>>
>>> I add ipmi kcs node,but it didn't work
>>>
>>> here is the patch for dts.
>>>
>>>           lpc_bmc: lpc-bmc@0 {
>>> -               compatible = "aspeed,ast2500-lpc-bmc";
>>> +               compatible = "aspeed,ast2500-lpc-bmc", "simple-mfd", "syscon";
>>> +
>>>                   reg = <0x0 0x80>;
>>> +               reg-io-width = <4>;
>>> +
>>> +               #address-cells = <1>;
>>> +               #size-cells = <1>;
>>> +               ranges = <0x0 0x0 0x80>;
>>> +
>>> +               kcs3: kcs3@0 {
>>> +                       compatible = "aspeed,ast2500-kcs-bmc";
>>> +                       reg = <0x0 0x80>;
>>> +                       interrupts = <8>;
>>> +                       kcs_chan = <3>;
>>> +                       kcs_addr = <0xca2>;
>>
>> Probably, you should add a clock setting for KCS H/W block into here.
>>
>>          clocks = <&syscon ASPEED_CLK_GATE_LCLK>;
>>
> I tried ,but it didn't work．
> 
>> Cheers,
>>
>> Jae
>>
>>> +                       status = "disabled";
>>> +
>>> +               };
>>>           };
>>>
>>> +&kcs3 {
>>>                            +       status = "okay";
>>>           +};
>>>
>>>
>>> The device node /dev/ipmi-kcs3 is created.
>>>
>>>
>>> root@sanjose:/tmp# ./kcsbridged --vv --d /dev/ipmi-kcs3
>>> Debug logging
>>> Starting
>>> Registering dbus methods/signals
>>> Requesting dbus name: org.openbmc.HostIpmi
>>> Getting dbus file descriptors
>>> Opening /dev/ipmi-kcs3
>>> Creating timer fd
>>> Entering polling loop
>>> Processed 1 dbus events
>>> Processed 1 dbus events
>>> Processed 1 dbus events
>>> Processed 1 dbus events
>>> Processed 1 dbus events
>>> Processed 1 dbus events
>>> Processed 1 dbus events
>>>
>>> but there is nothing output.
>>>
> 
> this is the dmesg info
> 
> root@sanjose:~# dmesg |grep kcs
> [    2.052340] aspeed-kcs-bmc: channel=3 addr=0xca2 idr=0x2c odr=0x38 str=0x44
> 
> I also checked the hardtrap:
> 0x1e6e2070[bit25]  enabled the lpc mode
> 
> I'm sure the IO port is 0xca2.
> 
> 
> should I config the pin for LPC,I checked the ibt node,it didn't.
> 
> 
> thank you
> 
> --qianlihu
> 

Try below fix on clk-aspeed.c

diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
index 596136793fc4..b55d35e3eb5e 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 */
         [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 */

In my experiments on my system, LCLK was not enabled due to the null
parent dependency so I added CLK_IS_CRITICAL flag to enable the clk.

LPC interface is default pin pad setting of AST2500 so you might not
need a pinctrl setting on it if you didn't change it's pin pads to GPIO
or eSPI.

Cheers,
Jae

Re: [HELP] ipmi-kcs didn't work

[qianlihu]

Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> 于2018年11月16日周五 上午3:30写道：
>
> On 11/14/2018 5:49 PM, qianlihu wrote:
> > Hi,Jae
> >
> >>
> >> Hi,
> >>
> >> On 11/14/2018 2:38 AM, qianlihu wrote:
> >>> Hi,
> >>>
> >>> I add ipmi kcs node,but it didn't work
> >>>
> >>> here is the patch for dts.
> >>>
> >>>           lpc_bmc: lpc-bmc@0 {
> >>> -               compatible = "aspeed,ast2500-lpc-bmc";
> >>> +               compatible = "aspeed,ast2500-lpc-bmc", "simple-mfd", "syscon";
> >>> +
> >>>                   reg = <0x0 0x80>;
> >>> +               reg-io-width = <4>;
> >>> +
> >>> +               #address-cells = <1>;
> >>> +               #size-cells = <1>;
> >>> +               ranges = <0x0 0x0 0x80>;
> >>> +
> >>> +               kcs3: kcs3@0 {
> >>> +                       compatible = "aspeed,ast2500-kcs-bmc";
> >>> +                       reg = <0x0 0x80>;
> >>> +                       interrupts = <8>;
> >>> +                       kcs_chan = <3>;
> >>> +                       kcs_addr = <0xca2>;
> >>
> >> Probably, you should add a clock setting for KCS H/W block into here.
> >>
> >>          clocks = <&syscon ASPEED_CLK_GATE_LCLK>;
> >>
> > I tried ,but it didn't work．
> >
> >> Cheers,
> >>
> >> Jae
> >>
> >>> +                       status = "disabled";
> >>> +
> >>> +               };
> >>>           };
> >>>
> >>> +&kcs3 {
> >>>                            +       status = "okay";
> >>>           +};
> >>>
> >>>
> >>> The device node /dev/ipmi-kcs3 is created.
> >>>
> >>>
> >>> root@sanjose:/tmp# ./kcsbridged --vv --d /dev/ipmi-kcs3
> >>> Debug logging
> >>> Starting
> >>> Registering dbus methods/signals
> >>> Requesting dbus name: org.openbmc.HostIpmi
> >>> Getting dbus file descriptors
> >>> Opening /dev/ipmi-kcs3
> >>> Creating timer fd
> >>> Entering polling loop
> >>> Processed 1 dbus events
> >>> Processed 1 dbus events
> >>> Processed 1 dbus events
> >>> Processed 1 dbus events
> >>> Processed 1 dbus events
> >>> Processed 1 dbus events
> >>> Processed 1 dbus events
> >>>
> >>> but there is nothing output.
> >>>
> >
> > this is the dmesg info
> >
> > root@sanjose:~# dmesg |grep kcs
> > [    2.052340] aspeed-kcs-bmc: channel=3 addr=0xca2 idr=0x2c odr=0x38 str=0x44
> >
> > I also checked the hardtrap:
> > 0x1e6e2070[bit25]  enabled the lpc mode
> >
> > I'm sure the IO port is 0xca2.
> >
> >
> > should I config the pin for LPC,I checked the ibt node,it didn't.
> >
> >
> > thank you
> >
> > --qianlihu
> >
>
> Try below fix on clk-aspeed.c
>
> diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
> index 596136793fc4..b55d35e3eb5e 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 */
>          [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 */
>
> In my experiments on my system, LCLK was not enabled due to the null
> parent dependency so I added CLK_IS_CRITICAL flag to enable the clk.
>
Worked! thanks
> LPC interface is default pin pad setting of AST2500 so you might not
> need a pinctrl setting on it if you didn't change it's pin pads to GPIO
> or eSPI.
>
> Cheers,
> Jae

Re: [HELP] ipmi-kcs didn't work

[Vijay Khemka]

    Try below fix on clk-aspeed.c
    
    diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
    index 596136793fc4..b55d35e3eb5e 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 */
             [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 */
    
    In my experiments on my system, LCLK was not enabled due to the null
    parent dependency so I added CLK_IS_CRITICAL flag to enable the clk.
    
    LPC interface is default pin pad setting of AST2500 so you might not
    need a pinctrl setting on it if you didn't change it's pin pads to GPIO
    or eSPI.
    
    Cheers,
    Jae

Jae,
Are you planning to upstream this patch as it is needed for kcsbridge to work. Also aspeed-g5 dtsi file.

Regards
-Vijay
    

Re: [HELP] ipmi-kcs didn't work

[Jae Hyun Yoo]

On 11/29/2018 12:42 PM, Vijay Khemka wrote:
>      Try below fix on clk-aspeed.c
>      
>      diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
>      index 596136793fc4..b55d35e3eb5e 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 */
>               [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 */
>      
>      In my experiments on my system, LCLK was not enabled due to the null
>      parent dependency so I added CLK_IS_CRITICAL flag to enable the clk.
>      
>      LPC interface is default pin pad setting of AST2500 so you might not
>      need a pinctrl setting on it if you didn't change it's pin pads to GPIO
>      or eSPI.
>      
>      Cheers,
>      Jae
> 
> Jae,
> Are you planning to upstream this patch as it is needed for kcsbridge to work. Also aspeed-g5 dtsi file.

Okay, I'll submit this patch and dtsi patch to community.

Thanks,
Jae

> 
> Regards
> -Vijay
>      
> 

Re: [HELP] ipmi-kcs didn't work

[Samuel Jiang]

[-- Attachment #1: Type: text/plain, Size: 2321 bytes --]

Hi Jae,

We also encountered the same situation a month ago.
We try to enable the LPC clock on devicetree to resolve this problem not direct modify clock driver.

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

However, I’m not sure which is better for LPC_CLCK setting.
Whether we should modify driver if original data is enabled or should enable on the device tree, correct?


Thanks,

Samuel Jiang
On Nov 30, 2018, 2:58 AM +0800, Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>, wrote:
> On 11/29/2018 12:42 PM, Vijay Khemka wrote:
> > Try below fix on clk-aspeed.c
> >
> > diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
> > index 596136793fc4..b55d35e3eb5e 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 */
> > [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 */
> >
> > In my experiments on my system, LCLK was not enabled due to the null
> > parent dependency so I added CLK_IS_CRITICAL flag to enable the clk.
> >
> > LPC interface is default pin pad setting of AST2500 so you might not
> > need a pinctrl setting on it if you didn't change it's pin pads to GPIO
> > or eSPI.
> >
> > Cheers,
> > Jae
> >
> > Jae,
> > Are you planning to upstream this patch as it is needed for kcsbridge to work. Also aspeed-g5 dtsi file.
>
> Okay, I'll submit this patch and dtsi patch to community.
>
> Thanks,
> Jae
>
> >
> > Regards
> > -Vijay
> >
> >

[-- Attachment #2: Type: text/html, Size: 3599 bytes --]

Re: [HELP] ipmi-kcs didn't work

[Jae Hyun Yoo]

On 11/29/2018 9:00 PM, Samuel Jiang wrote:
> Hi Jae,
> 
> We also encountered the same situation a month ago.
> We try to enable the LPC clock on devicetree to resolve this problem not 
> direct modify clock driver.
> 
> lpc_ctrl: lpc-ctrl@0 {
>   compatible = "aspeed,ast2500-lpc-ctrl”;
>   reg = <0x0 0x80>;
>   clocks = <&syscon ASPEED_CLK_GATE_LCLK>;
>      status = “okay”;
> };
> 
> However, I’m not sure which is better for LPC_CLCK setting.
> Whether we should modify driver if original data is enabled or should 
> enable on the device tree, correct?
> 
> 

Hi Samuel,

Yes, you are right. I realized that kcs_bmc_aspeed.c doesn't have any
clock control code so adding a clocks setting into kcs dtsi code doesn't
make any help. Also, it was the reason why the change is needed on
clk-aspeed.c. Enabling lpc_ctrl node you suggested seems a right way.

Hi Vijay,
Can you please try the way Samuel suggested after removing changes I
gave you. Samuel's solution looks good to me.

Thanks,
Jae

> Thanks,
> 
> Samuel Jiang
> On Nov 30, 2018, 2:58 AM +0800, Jae Hyun Yoo 
> <jae.hyun.yoo@linux.intel.com>, wrote:
>> On 11/29/2018 12:42 PM, Vijay Khemka wrote:
>>> Try below fix on clk-aspeed.c
>>>
>>> diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
>>> index 596136793fc4..b55d35e3eb5e 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 */
>>> [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 */
>>>
>>> In my experiments on my system, LCLK was not enabled due to the null
>>> parent dependency so I added CLK_IS_CRITICAL flag to enable the clk.
>>>
>>> LPC interface is default pin pad setting of AST2500 so you might not
>>> need a pinctrl setting on it if you didn't change it's pin pads to GPIO
>>> or eSPI.
>>>
>>> Cheers,
>>> Jae
>>>
>>> Jae,
>>> Are you planning to upstream this patch as it is needed for kcsbridge 
>>> to work. Also aspeed-g5 dtsi file.
>>
>> Okay, I'll submit this patch and dtsi patch to community.
>>
>> Thanks,
>> Jae
>>
>>>
>>> Regards
>>> -Vijay
>>>
>>>

Re: [HELP] ipmi-kcs didn't work

[Vijay Khemka]

On 11/30/18, 7:31 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> wrote:

    On 11/29/2018 9:00 PM, Samuel Jiang wrote:
    > Hi Jae,
    > 
    > We also encountered the same situation a month ago.
    > We try to enable the LPC clock on devicetree to resolve this problem not 
    > direct modify clock driver.
    > 
    > lpc_ctrl: lpc-ctrl@0 {
    >   compatible = "aspeed,ast2500-lpc-ctrl”;
    >   reg = <0x0 0x80>;
    >   clocks = <&syscon ASPEED_CLK_GATE_LCLK>;
    >      status = “okay”;
    > };
    > 
    > However, I’m not sure which is better for LPC_CLCK setting.
    > Whether we should modify driver if original data is enabled or should 
    > enable on the device tree, correct?
    > 
    > 
    
    Hi Samuel,
    
    Yes, you are right. I realized that kcs_bmc_aspeed.c doesn't have any
    clock control code so adding a clocks setting into kcs dtsi code doesn't
    make any help. Also, it was the reason why the change is needed on
    clk-aspeed.c. Enabling lpc_ctrl node you suggested seems a right way.
    
    Hi Vijay,
    Can you please try the way Samuel suggested after removing changes I
    gave you. Samuel's solution looks good to me.
    
    Thanks,
    Jae
    
Jae,
I can try. We can certainly enable lpc_ctrl node but in my understanding that 
we still need this below patch to redefine ASPEED_CLK_GATE_LCLK value to 
add CRITICAL. I might be missing something here.
    - [ASPEED_CLK_GATE_LCLK] = { 8, 5, "lclk-gate", NULL, 0 }, /* LPC */
   + [ASPEED_CLK_GATE_LCLK] = { 8, 5, "lclk-gate", NULL, CLK_IS_CRITICAL }, /* LPC */


Regards
-Vijay

Re: [HELP] ipmi-kcs didn't work

[Jae Hyun Yoo]

On 11/30/2018 12:57 PM, Vijay Khemka wrote:
> 
> On 11/30/18, 7:31 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> wrote:
> 
>      On 11/29/2018 9:00 PM, Samuel Jiang wrote:
>      > Hi Jae,
>      >
>      > We also encountered the same situation a month ago.
>      > We try to enable the LPC clock on devicetree to resolve this problem not
>      > direct modify clock driver.
>      >
>      > lpc_ctrl: lpc-ctrl@0 {
>      >   compatible = "aspeed,ast2500-lpc-ctrl”;
>      >   reg = <0x0 0x80>;
>      >   clocks = <&syscon ASPEED_CLK_GATE_LCLK>;
>      >      status = “okay”;
>      > };
>      >
>      > However, I’m not sure which is better for LPC_CLCK setting.
>      > Whether we should modify driver if original data is enabled or should
>      > enable on the device tree, correct?
>      >
>      >
>      
>      Hi Samuel,
>      
>      Yes, you are right. I realized that kcs_bmc_aspeed.c doesn't have any
>      clock control code so adding a clocks setting into kcs dtsi code doesn't
>      make any help. Also, it was the reason why the change is needed on
>      clk-aspeed.c. Enabling lpc_ctrl node you suggested seems a right way.
>      
>      Hi Vijay,
>      Can you please try the way Samuel suggested after removing changes I
>      gave you. Samuel's solution looks good to me.
>      
>      Thanks,
>      Jae
>      
> Jae,
> I can try. We can certainly enable lpc_ctrl node but in my understanding that
> we still need this below patch to redefine ASPEED_CLK_GATE_LCLK value to
> add CRITICAL. I might be missing something here.
>      - [ASPEED_CLK_GATE_LCLK] = { 8, 5, "lclk-gate", NULL, 0 }, /* LPC */
>     + [ASPEED_CLK_GATE_LCLK] = { 8, 5, "lclk-gate", NULL, CLK_IS_CRITICAL }, /* LPC */
> 

Vijay,

I checked that lpc_ctrl module has clocks dt property handling code in
the module so I think the LCLK would be enabled without the clk-aspeed.c
fix. I can't test it now because I'm in a business trip so please test
it. I'll also check it when I back to office a week after.

Thanks,
Jae

> 
> Regards
> -Vijay
> 

Re: [HELP] ipmi-kcs didn't work

[Vijay Khemka]

On 11/30/18, 11:29 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> wrote:

    On 11/30/2018 12:57 PM, Vijay Khemka wrote:
    > 
    > On 11/30/18, 7:31 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> wrote:
    > 
    >      On 11/29/2018 9:00 PM, Samuel Jiang wrote:
    >      > Hi Jae,
    >      >
    >      > We also encountered the same situation a month ago.
    >      > We try to enable the LPC clock on devicetree to resolve this problem not
    >      > direct modify clock driver.
    >      >
    >      > lpc_ctrl: lpc-ctrl@0 {
    >      >   compatible = "aspeed,ast2500-lpc-ctrl”;
    >      >   reg = <0x0 0x80>;
    >      >   clocks = <&syscon ASPEED_CLK_GATE_LCLK>;
    >      >      status = “okay”;
    >      > };
    >      >
    >      > However, I’m not sure which is better for LPC_CLCK setting.
    >      > Whether we should modify driver if original data is enabled or should
    >      > enable on the device tree, correct?
    >      >
    >      >
    >      
    >      Hi Samuel,
    >      
    >      Yes, you are right. I realized that kcs_bmc_aspeed.c doesn't have any
    >      clock control code so adding a clocks setting into kcs dtsi code doesn't
    >      make any help. Also, it was the reason why the change is needed on
    >      clk-aspeed.c. Enabling lpc_ctrl node you suggested seems a right way.
    >      
    >      Hi Vijay,
    >      Can you please try the way Samuel suggested after removing changes I
    >      gave you. Samuel's solution looks good to me.
    >      
    >      Thanks,
    >      Jae
    >      
    > Jae,
    > I can try. We can certainly enable lpc_ctrl node but in my understanding that
    > we still need this below patch to redefine ASPEED_CLK_GATE_LCLK value to
    > add CRITICAL. I might be missing something here.
    >      - [ASPEED_CLK_GATE_LCLK] = { 8, 5, "lclk-gate", NULL, 0 }, /* LPC */
    >     + [ASPEED_CLK_GATE_LCLK] = { 8, 5, "lclk-gate", NULL, CLK_IS_CRITICAL }, /* LPC */
    > 
    
    Vijay,
    
    I checked that lpc_ctrl module has clocks dt property handling code in
    the module so I think the LCLK would be enabled without the clk-aspeed.c
    fix. I can't test it now because I'm in a business trip so please test
    it. I'll also check it when I back to office a week after.
    
    Thanks,
    Jae
    
Jae,
I removed above aspeed clock driver patch, and enabled lpc-ctrl node in dts also removed clock definition from kcs. And I don't see host is getting powered on. It works only with clock driver change.

And I still didn't understand how enabling lpc-ctrl node will initialize ASPEED_CLK_GATE_LCLK to CRITICAL.

-Vijay

Re: [HELP] ipmi-kcs didn't work

[Jae Hyun Yoo]

On 11/30/2018 4:39 PM, Vijay Khemka wrote:
> 
> 
> On 11/30/18, 11:29 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> wrote:
> 
>      On 11/30/2018 12:57 PM, Vijay Khemka wrote:
>      >
>      > On 11/30/18, 7:31 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> wrote:
>      >
>      >      On 11/29/2018 9:00 PM, Samuel Jiang wrote:
>      >      > Hi Jae,
>      >      >
>      >      > We also encountered the same situation a month ago.
>      >      > We try to enable the LPC clock on devicetree to resolve this problem not
>      >      > direct modify clock driver.
>      >      >
>      >      > lpc_ctrl: lpc-ctrl@0 {
>      >      >   compatible = "aspeed,ast2500-lpc-ctrl”;
>      >      >   reg = <0x0 0x80>;
>      >      >   clocks = <&syscon ASPEED_CLK_GATE_LCLK>;
>      >      >      status = “okay”;
>      >      > };
>      >      >
>      >      > However, I’m not sure which is better for LPC_CLCK setting.
>      >      > Whether we should modify driver if original data is enabled or should
>      >      > enable on the device tree, correct?
>      >      >
>      >      >
>      >
>      >      Hi Samuel,
>      >
>      >      Yes, you are right. I realized that kcs_bmc_aspeed.c doesn't have any
>      >      clock control code so adding a clocks setting into kcs dtsi code doesn't
>      >      make any help. Also, it was the reason why the change is needed on
>      >      clk-aspeed.c. Enabling lpc_ctrl node you suggested seems a right way.
>      >
>      >      Hi Vijay,
>      >      Can you please try the way Samuel suggested after removing changes I
>      >      gave you. Samuel's solution looks good to me.
>      >
>      >      Thanks,
>      >      Jae
>      >
>      > Jae,
>      > I can try. We can certainly enable lpc_ctrl node but in my understanding that
>      > we still need this below patch to redefine ASPEED_CLK_GATE_LCLK value to
>      > add CRITICAL. I might be missing something here.
>      >      - [ASPEED_CLK_GATE_LCLK] = { 8, 5, "lclk-gate", NULL, 0 }, /* LPC */
>      >     + [ASPEED_CLK_GATE_LCLK] = { 8, 5, "lclk-gate", NULL, CLK_IS_CRITICAL }, /* LPC */
>      >
>      
>      Vijay,
>      
>      I checked that lpc_ctrl module has clocks dt property handling code in
>      the module so I think the LCLK would be enabled without the clk-aspeed.c
>      fix. I can't test it now because I'm in a business trip so please test
>      it. I'll also check it when I back to office a week after.
>      
>      Thanks,
>      Jae
>      
> Jae,
> I removed above aspeed clock driver patch, and enabled lpc-ctrl node in dts also removed clock definition from kcs. And I don't see host is getting powered on. It works only with clock driver change.
> 
> And I still didn't understand how enabling lpc-ctrl node will initialize ASPEED_CLK_GATE_LCLK to CRITICAL.
> 
> -Vijay
> 

Vijay,

Thanks Vijay for sharing the test result. I'll also check it on my
hardware when I get back to office. Will share my test result too.

Thanks again,
Jae

Re: [HELP] ipmi-kcs didn't work

[Samuel Jiang]

[-- Attachment #1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2: Type: text/html, Size: 105 bytes --]

Re: [HELP] ipmi-kcs didn't work

[Samuel Jiang]

[-- Attachment #1: Type: text/plain, Size: 3334 bytes --]

Apologize for sending out no content mail first.

Jae,
The aspeed_gates in clk-aspeed.c perhaps as todo suggest asking Aspeed the actual parent data for check initializing?

Vijay,
I traced lpc-ctrl module, it seems direct update the same ASPEED_CLK_GATE_LCLK register map bit to enable. If parent data is disabled, it could enable in dts.
The device tree detail, I reference it from aspeed-g5.dtsi. Hope it could help you for work.

Thanks,

Samuel Jiang
On Dec 1, 2018, 7:04 AM +0800, Jae Hyun Yoo , wrote:
> On 11/30/2018 4:39 PM, Vijay Khemka wrote:
> >
> >
> > On 11/30/18, 11:29 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> wrote:
> >
> > On 11/30/2018 12:57 PM, Vijay Khemka wrote:
> > >
> > > On 11/30/18, 7:31 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> wrote:
> > >
> > > On 11/29/2018 9:00 PM, Samuel Jiang wrote:
> > > > Hi Jae,
> > > >
> > > > We also encountered the same situation a month ago.
> > > > We try to enable the LPC clock on devicetree to resolve this problem not
> > > > direct modify clock driver.
> > > >
> > > > lpc_ctrl: lpc-ctrl@0 {
> > > >   compatible = "aspeed,ast2500-lpc-ctrl”;
> > > >   reg = <0x0 0x80>;
> > > >   clocks = <&syscon ASPEED_CLK_GATE_LCLK>;
> > > > status = “okay”;
> > > > };
> > > >
> > > > However, I’m not sure which is better for LPC_CLCK setting.
> > > > Whether we should modify driver if original data is enabled or should
> > > > enable on the device tree, correct?
> > > >
> > > >
> > >
> > > Hi Samuel,
> > >
> > > Yes, you are right. I realized that kcs_bmc_aspeed.c doesn't have any
> > > clock control code so adding a clocks setting into kcs dtsi code doesn't
> > > make any help. Also, it was the reason why the change is needed on
> > > clk-aspeed.c. Enabling lpc_ctrl node you suggested seems a right way.
> > >
> > > Hi Vijay,
> > > Can you please try the way Samuel suggested after removing changes I
> > > gave you. Samuel's solution looks good to me.
> > >
> > > Thanks,
> > > Jae
> > >
> > > Jae,
> > > I can try. We can certainly enable lpc_ctrl node but in my understanding that
> > > we still need this below patch to redefine ASPEED_CLK_GATE_LCLK value to
> > > add CRITICAL. I might be missing something here.
> > > - [ASPEED_CLK_GATE_LCLK] = { 8, 5, "lclk-gate", NULL, 0 }, /* LPC */
> > > + [ASPEED_CLK_GATE_LCLK] = { 8, 5, "lclk-gate", NULL, CLK_IS_CRITICAL }, /* LPC */
> > >
> >
> > Vijay,
> >
> > I checked that lpc_ctrl module has clocks dt property handling code in
> > the module so I think the LCLK would be enabled without the clk-aspeed.c
> > fix. I can't test it now because I'm in a business trip so please test
> > it. I'll also check it when I back to office a week after.
> >
> > Thanks,
> > Jae
> >
> > Jae,
> > I removed above aspeed clock driver patch, and enabled lpc-ctrl node in dts also removed clock definition from kcs. And I don't see host is getting powered on. It works only with clock driver change.
> >
> > And I still didn't understand how enabling lpc-ctrl node will initialize ASPEED_CLK_GATE_LCLK to CRITICAL.
> >
> > -Vijay
> >
>
> Vijay,
>
> Thanks Vijay for sharing the test result. I'll also check it on my
> hardware when I get back to office. Will share my test result too.
>
> Thanks again,
> Jae

[-- Attachment #2: Type: text/html, Size: 4553 bytes --]

Re: [HELP] ipmi-kcs didn't work

[Samuel Jiang]

[-- Attachment #1: Type: text/plain, Size: 3732 bytes --]

On Dec 1, 2018, 10:29 PM +0800, Samuel Jiang <chyishian.jiang@gmail.com>, wrote:
> Apologize for sending out no content mail first.
>
> Jae,
> The aspeed_gates in clk-aspeed.c perhaps as todo suggest asking Aspeed the actual parent data for check initializing?
>
> Vijay,
> I traced lpc-ctrl module, it seems direct update the same ASPEED_CLK_GATE_LCLK register map bit to enable. If parent data is disabled, it could enable in dts.
> The device tree detail, I reference it from aspeed-g5.dtsi. Hope it could help you for work.
>

Hi Vijay,

 Correct above content, how to enable LPC-ctrl module, we follow aspeed-bmc-opp-romulus.dts to adjust that.

Samuel


>  Thanks,
>
> Samuel Jiang
> On Dec 1, 2018, 7:04 AM +0800, Jae Hyun Yoo , wrote:
> > On 11/30/2018 4:39 PM, Vijay Khemka wrote:
> > >
> > >
> > > On 11/30/18, 11:29 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> wrote:
> > >
> > > On 11/30/2018 12:57 PM, Vijay Khemka wrote:
> > > >
> > > > On 11/30/18, 7:31 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> wrote:
> > > >
> > > > On 11/29/2018 9:00 PM, Samuel Jiang wrote:
> > > > > Hi Jae,
> > > > >
> > > > > We also encountered the same situation a month ago.
> > > > > We try to enable the LPC clock on devicetree to resolve this problem not
> > > > > direct modify clock driver.
> > > > >
> > > > > lpc_ctrl: lpc-ctrl@0 {
> > > > >   compatible = "aspeed,ast2500-lpc-ctrl”;
> > > > >   reg = <0x0 0x80>;
> > > > >   clocks = <&syscon ASPEED_CLK_GATE_LCLK>;
> > > > > status = “okay”;
> > > > > };
> > > > >
> > > > > However, I’m not sure which is better for LPC_CLCK setting.
> > > > > Whether we should modify driver if original data is enabled or should
> > > > > enable on the device tree, correct?
> > > > >
> > > > >
> > > >
> > > > Hi Samuel,
> > > >
> > > > Yes, you are right. I realized that kcs_bmc_aspeed.c doesn't have any
> > > > clock control code so adding a clocks setting into kcs dtsi code doesn't
> > > > make any help. Also, it was the reason why the change is needed on
> > > > clk-aspeed.c. Enabling lpc_ctrl node you suggested seems a right way.
> > > >
> > > > Hi Vijay,
> > > > Can you please try the way Samuel suggested after removing changes I
> > > > gave you. Samuel's solution looks good to me.
> > > >
> > > > Thanks,
> > > > Jae
> > > >
> > > > Jae,
> > > > I can try. We can certainly enable lpc_ctrl node but in my understanding that
> > > > we still need this below patch to redefine ASPEED_CLK_GATE_LCLK value to
> > > > add CRITICAL. I might be missing something here.
> > > > - [ASPEED_CLK_GATE_LCLK] = { 8, 5, "lclk-gate", NULL, 0 }, /* LPC */
> > > > + [ASPEED_CLK_GATE_LCLK] = { 8, 5, "lclk-gate", NULL, CLK_IS_CRITICAL }, /* LPC */
> > > >
> > >
> > > Vijay,
> > >
> > > I checked that lpc_ctrl module has clocks dt property handling code in
> > > the module so I think the LCLK would be enabled without the clk-aspeed.c
> > > fix. I can't test it now because I'm in a business trip so please test
> > > it. I'll also check it when I back to office a week after.
> > >
> > > Thanks,
> > > Jae
> > >
> > > Jae,
> > > I removed above aspeed clock driver patch, and enabled lpc-ctrl node in dts also removed clock definition from kcs. And I don't see host is getting powered on. It works only with clock driver change.
> > >
> > > And I still didn't understand how enabling lpc-ctrl node will initialize ASPEED_CLK_GATE_LCLK to CRITICAL.
> > >
> > > -Vijay
> > >
> >
> > Vijay,
> >
> > Thanks Vijay for sharing the test result. I'll also check it on my
> > hardware when I get back to office. Will share my test result too.
> >
> > Thanks again,
> > Jae

[-- Attachment #2: Type: text/html, Size: 5410 bytes --]

Re: [HELP] ipmi-kcs didn't work

[Jae Hyun Yoo]

On 12/1/2018 8:29 AM, Samuel Jiang wrote:
> Apologize for sending out no content mail first.
> 
> Jae,
> The aspeed_gates in clk-aspeed.c perhaps as todo suggest asking Aspeed 
> the actual parent data for check initializing?
> 

Yes, that makes sense.

Hi Gary and Ryan,

Can you please tell us what is the actual parent clock source of LPC IP?
I mean the operational clock of LPC IP hardware block, not the interface
clock.

Thanks,
Jae

> Vijay,
> I traced lpc-ctrl module, it seems direct update the same 
> ASPEED_CLK_GATE_LCLK register map bit to enable. If parent data is 
> disabled, it could enable in dts.
> The device tree detail, I reference it from aspeed-g5.dtsi. Hope it 
> could help you for work.
> 
> Thanks,
> 
> Samuel Jiang
> On Dec 1, 2018, 7:04 AM +0800, Jae Hyun Yoo , wrote:
>> On 11/30/2018 4:39 PM, Vijay Khemka wrote:
>>>
>>>
>>> On 11/30/18, 11:29 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> 
>>> wrote:
>>>
>>> On 11/30/2018 12:57 PM, Vijay Khemka wrote:
>>>>
>>>> On 11/30/18, 7:31 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> 
>>>> wrote:
>>>>
>>>> On 11/29/2018 9:00 PM, Samuel Jiang wrote:
>>>>> Hi Jae,
>>>>>
>>>>> We also encountered the same situation a month ago.
>>>>> We try to enable the LPC clock on devicetree to resolve this 
>>>>> problem not
>>>>> direct modify clock driver.
>>>>>
>>>>> lpc_ctrl: lpc-ctrl@0 {
>>>>>   compatible = "aspeed,ast2500-lpc-ctrl”;
>>>>>   reg = <0x0 0x80>;
>>>>>   clocks = <&syscon ASPEED_CLK_GATE_LCLK>;
>>>>> status = “okay”;
>>>>> };
>>>>>
>>>>> However, I’m not sure which is better for LPC_CLCK setting.
>>>>> Whether we should modify driver if original data is enabled or should
>>>>> enable on the device tree, correct?
>>>>>
>>>>>
>>>>
>>>> Hi Samuel,
>>>>
>>>> Yes, you are right. I realized that kcs_bmc_aspeed.c doesn't have any
>>>> clock control code so adding a clocks setting into kcs dtsi code doesn't
>>>> make any help. Also, it was the reason why the change is needed on
>>>> clk-aspeed.c. Enabling lpc_ctrl node you suggested seems a right way.
>>>>
>>>> Hi Vijay,
>>>> Can you please try the way Samuel suggested after removing changes I
>>>> gave you. Samuel's solution looks good to me.
>>>>
>>>> Thanks,
>>>> Jae
>>>>
>>>> Jae,
>>>> I can try. We can certainly enable lpc_ctrl node but in my 
>>>> understanding that
>>>> we still need this below patch to redefine ASPEED_CLK_GATE_LCLK value to
>>>> add CRITICAL. I might be missing something here.
>>>> - [ASPEED_CLK_GATE_LCLK] = { 8, 5, "lclk-gate", NULL, 0 }, /* LPC */
>>>> + [ASPEED_CLK_GATE_LCLK] = { 8, 5, "lclk-gate", NULL, 
>>>> CLK_IS_CRITICAL }, /* LPC */
>>>>
>>>
>>> Vijay,
>>>
>>> I checked that lpc_ctrl module has clocks dt property handling code in
>>> the module so I think the LCLK would be enabled without the clk-aspeed.c
>>> fix. I can't test it now because I'm in a business trip so please test
>>> it. I'll also check it when I back to office a week after.
>>>
>>> Thanks,
>>> Jae
>>>
>>> Jae,
>>> I removed above aspeed clock driver patch, and enabled lpc-ctrl node 
>>> in dts also removed clock definition from kcs. And I don't see host 
>>> is getting powered on. It works only with clock driver change.
>>>
>>> And I still didn't understand how enabling lpc-ctrl node will 
>>> initialize ASPEED_CLK_GATE_LCLK to CRITICAL.
>>>
>>> -Vijay
>>>
>>
>> Vijay,
>>
>> Thanks Vijay for sharing the test result. I'll also check it on my
>> hardware when I get back to office. Will share my test result too.
>>
>> Thanks again,
>> Jae

Re: [HELP] ipmi-kcs didn't work

[Vijay Khemka]

On 12/3/18, 7:54 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> wrote:

    On 12/1/2018 8:29 AM, Samuel Jiang wrote:
    > Apologize for sending out no content mail first.
    > 
    > Jae,
    > The aspeed_gates in clk-aspeed.c perhaps as todo suggest asking Aspeed 
    > the actual parent data for check initializing?
    > 
    
    Yes, that makes sense.
    
    Hi Gary and Ryan,
    
    Can you please tell us what is the actual parent clock source of LPC IP?
    I mean the operational clock of LPC IP hardware block, not the interface
    clock.
    
    Thanks,
    Jae
    
    > Vijay,
    > I traced lpc-ctrl module, it seems direct update the same 
    > ASPEED_CLK_GATE_LCLK register map bit to enable. If parent data is 
    > disabled, it could enable in dts.
    > The device tree detail, I reference it from aspeed-g5.dtsi. Hope it 
    > could help you for work.
    > 
    > Thanks,
    > 
    > Samuel Jiang

Samual/Jay,
In my case if I don't initialize LPC clock in driver, Bios on host side wait and doesn't boot.
I don't understand here what is holding Bios here but by initializing this LPC clock let bios boot.

Regards
-Vijay 

Re: [HELP] ipmi-kcs didn't work

[Jae Hyun Yoo]

On 12/3/2018 1:37 PM, Vijay Khemka wrote:
> 
> 
> On 12/3/18, 7:54 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> wrote:
> 
>      On 12/1/2018 8:29 AM, Samuel Jiang wrote:
>      > Apologize for sending out no content mail first.
>      >
>      > Jae,
>      > The aspeed_gates in clk-aspeed.c perhaps as todo suggest asking Aspeed
>      > the actual parent data for check initializing?
>      >
>      
>      Yes, that makes sense.
>      
>      Hi Gary and Ryan,
>      
>      Can you please tell us what is the actual parent clock source of LPC IP?
>      I mean the operational clock of LPC IP hardware block, not the interface
>      clock.
>      
>      Thanks,
>      Jae
>      
>      > Vijay,
>      > I traced lpc-ctrl module, it seems direct update the same
>      > ASPEED_CLK_GATE_LCLK register map bit to enable. If parent data is
>      > disabled, it could enable in dts.
>      > The device tree detail, I reference it from aspeed-g5.dtsi. Hope it
>      > could help you for work.
>      >
>      > Thanks,
>      >
>      > Samuel Jiang
> 
> Samual/Jay,
> In my case if I don't initialize LPC clock in driver, Bios on host side wait and doesn't boot.
> I don't understand here what is holding Bios here but by initializing this LPC clock let bios boot.
> 

Hi Vijay,
I also need to test Samuel's patch on real hardware. Probably, I could
share my test result next week because I'm in a business trip. Will
share my test result as soon as I back to office. Also, I'm currently
asking Aspeed engineers to check the parent clock of the LCLK to fill
out the parent clock info in clk-aspeed.c. If it is available, we would
not need to use CLK_IS_CRITICAL flag, I'm guessing. Will update this
thread as soon as possible.

Thanks,
Jae

> Regards
> -Vijay
> 

RE: [HELP] ipmi-kcs didn't work

[Ryan Chen]

Hello Jae,
	ASPEED LPC IP HW block have serval clk input. 
	Most important is LCLK is come from LPC Host.
	The others is not controllable by register. 
Ryan 

-----Original Message-----
From: Vijay Khemka [mailto:vijaykhemka@fb.com] 
Sent: Tuesday, December 4, 2018 3:38 AM
To: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>; Samuel Jiang <chyishian.jiang@gmail.com>; qianlihu <wangzhiqiang8906@gmail.com>; Gary Hsu <gary_hsu@aspeedtech.com>; Ryan Chen <ryan_chen@aspeedtech.com>
Cc: openbmc@lists.ozlabs.org
Subject: Re: [HELP] ipmi-kcs didn't work



On 12/3/18, 7:54 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> wrote:

    On 12/1/2018 8:29 AM, Samuel Jiang wrote:
    > Apologize for sending out no content mail first.
    > 
    > Jae,
    > The aspeed_gates in clk-aspeed.c perhaps as todo suggest asking Aspeed 
    > the actual parent data for check initializing?
    > 
    
    Yes, that makes sense.
    
    Hi Gary and Ryan,
    
    Can you please tell us what is the actual parent clock source of LPC IP?
    I mean the operational clock of LPC IP hardware block, not the interface
    clock.
    
    Thanks,
    Jae
    
    > Vijay,
    > I traced lpc-ctrl module, it seems direct update the same 
    > ASPEED_CLK_GATE_LCLK register map bit to enable. If parent data is 
    > disabled, it could enable in dts.
    > The device tree detail, I reference it from aspeed-g5.dtsi. Hope it 
    > could help you for work.
    > 
    > Thanks,
    > 
    > Samuel Jiang

Samual/Jay,
In my case if I don't initialize LPC clock in driver, Bios on host side wait and doesn't boot.
I don't understand here what is holding Bios here but by initializing this LPC clock let bios boot.

Regards
-Vijay 

Re: [HELP] ipmi-kcs didn't work

[Jae Hyun Yoo]

On 12/4/2018 12:18 AM, Ryan Chen wrote:
> Hello Jae,
> 	ASPEED LPC IP HW block have serval clk input.
> 	Most important is LCLK is come from LPC Host.
> 	The others is not controllable by register.
> Ryan
> 

Hi Ryan,

Thanks for the information. So clock source of LCLK is from external
usually from PCH, means that it doesn't have any parent dependency
within BMC. In this case, we need to mark CLK_IS_CRITICAL flag to enable
it. Also, we should mark the flag for ESPICLK too because eSPI is
sharing the same interface with LPC.

I'll make and upstream a patch.

Thanks,
Jae

> -----Original Message-----
> From: Vijay Khemka [mailto:vijaykhemka@fb.com]
> Sent: Tuesday, December 4, 2018 3:38 AM
> To: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>; Samuel Jiang <chyishian.jiang@gmail.com>; qianlihu <wangzhiqiang8906@gmail.com>; Gary Hsu <gary_hsu@aspeedtech.com>; Ryan Chen <ryan_chen@aspeedtech.com>
> Cc: openbmc@lists.ozlabs.org
> Subject: Re: [HELP] ipmi-kcs didn't work
> 
> 
> 
> On 12/3/18, 7:54 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> wrote:
> 
>      On 12/1/2018 8:29 AM, Samuel Jiang wrote:
>      > Apologize for sending out no content mail first.
>      >
>      > Jae,
>      > The aspeed_gates in clk-aspeed.c perhaps as todo suggest asking Aspeed
>      > the actual parent data for check initializing?
>      >
>      
>      Yes, that makes sense.
>      
>      Hi Gary and Ryan,
>      
>      Can you please tell us what is the actual parent clock source of LPC IP?
>      I mean the operational clock of LPC IP hardware block, not the interface
>      clock.
>      
>      Thanks,
>      Jae
>      
>      > Vijay,
>      > I traced lpc-ctrl module, it seems direct update the same
>      > ASPEED_CLK_GATE_LCLK register map bit to enable. If parent data is
>      > disabled, it could enable in dts.
>      > The device tree detail, I reference it from aspeed-g5.dtsi. Hope it
>      > could help you for work.
>      >
>      > Thanks,
>      >
>      > Samuel Jiang
> 
> Samual/Jay,
> In my case if I don't initialize LPC clock in driver, Bios on host side wait and doesn't boot.
> I don't understand here what is holding Bios here but by initializing this LPC clock let bios boot.
> 
> Regards
> -Vijay
> 

Re: [HELP] ipmi-kcs didn't work

[Vijay Khemka]

On 12/4/18, 7:44 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> wrote:

    On 12/4/2018 12:18 AM, Ryan Chen wrote:
    > Hello Jae,
    > 	ASPEED LPC IP HW block have serval clk input.
    > 	Most important is LCLK is come from LPC Host.
    > 	The others is not controllable by register.
    > Ryan
    > 
    
    Hi Ryan,
    
    Thanks for the information. So clock source of LCLK is from external
    usually from PCH, means that it doesn't have any parent dependency
    within BMC. In this case, we need to mark CLK_IS_CRITICAL flag to enable
    it. Also, we should mark the flag for ESPICLK too because eSPI is
    sharing the same interface with LPC.
    
    I'll make and upstream a patch.
    
    Thanks,
    Jae
Jae,
But I am able to enable clock through lpc-ctrl in dts. Which does same thing enabling clock for lpc. I don't think we need this patch.

Regards
-Vijay 

RE: [HELP] ipmi-kcs didn't work

[Ryan Chen]

Hello Jae,
		Yes, ESPICLK should be the same flag. 
		Thanks. 
Ryan

On 12/4/18, 7:44 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> wrote:

    On 12/4/2018 12:18 AM, Ryan Chen wrote:
    > Hello Jae,
    > 	ASPEED LPC IP HW block have serval clk input.
    > 	Most important is LCLK is come from LPC Host.
    > 	The others is not controllable by register.
    > Ryan
    > 
    
    Hi Ryan,
    
    Thanks for the information. So clock source of LCLK is from external
    usually from PCH, means that it doesn't have any parent dependency
    within BMC. In this case, we need to mark CLK_IS_CRITICAL flag to enable
    it. Also, we should mark the flag for ESPICLK too because eSPI is
    sharing the same interface with LPC.
    
    I'll make and upstream a patch.
    
    Thanks,
    Jae
Jae,
But I am able to enable clock through lpc-ctrl in dts. Which does same thing enabling clock for lpc. I don't think we need this patch.

Regards
-Vijay 

Re: [HELP] ipmi-kcs didn't work

[Jae Hyun Yoo]

On 12/4/2018 7:04 PM, Ryan Chen wrote:
> Hello Jae,
> 		Yes, ESPICLK should be the same flag.
> 		Thanks.
> Ryan
> 

Thanks Ryan for the confirmation. We checked that LCLK gate can be
opened by enabling lpc-ctrl node which has clock enabling code in
itself. ESPICLK gate could be opened using the same way, I believe.

Thanks,
Jae

> On 12/4/18, 7:44 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> wrote:
> 
>      On 12/4/2018 12:18 AM, Ryan Chen wrote:
>      > Hello Jae,
>      > 	ASPEED LPC IP HW block have serval clk input.
>      > 	Most important is LCLK is come from LPC Host.
>      > 	The others is not controllable by register.
>      > Ryan
>      >
>      
>      Hi Ryan,
>      
>      Thanks for the information. So clock source of LCLK is from external
>      usually from PCH, means that it doesn't have any parent dependency
>      within BMC. In this case, we need to mark CLK_IS_CRITICAL flag to enable
>      it. Also, we should mark the flag for ESPICLK too because eSPI is
>      sharing the same interface with LPC.
>      
>      I'll make and upstream a patch.
>      
>      Thanks,
>      Jae
> Jae,
> But I am able to enable clock through lpc-ctrl in dts. Which does same thing enabling clock for lpc. I don't think we need this patch.
> 
> Regards
> -Vijay
> 

Re: [HELP] ipmi-kcs didn't work

[Jae Hyun Yoo]

On 12/4/2018 5:59 PM, Vijay Khemka wrote:
> 
> 
> On 12/4/18, 7:44 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> wrote:
> 
>      On 12/4/2018 12:18 AM, Ryan Chen wrote:
>      > Hello Jae,
>      > 	ASPEED LPC IP HW block have serval clk input.
>      > 	Most important is LCLK is come from LPC Host.
>      > 	The others is not controllable by register.
>      > Ryan
>      >
>      
>      Hi Ryan,
>      
>      Thanks for the information. So clock source of LCLK is from external
>      usually from PCH, means that it doesn't have any parent dependency
>      within BMC. In this case, we need to mark CLK_IS_CRITICAL flag to enable
>      it. Also, we should mark the flag for ESPICLK too because eSPI is
>      sharing the same interface with LPC.
>      
>      I'll make and upstream a patch.
>      
>      Thanks,
>      Jae
> Jae,
> But I am able to enable clock through lpc-ctrl in dts. Which does same thing enabling clock for lpc. I don't think we need this patch.
> 

Okay, you confirmed it. I believe that we don't need to use the critical
flag setting on LCLK then. Will drop the upstreaming patch.

Thanks,
Jae

> Regards
> -Vijay
> 

RE: [HELP] ipmi-kcs didn't work

[Samuel Jiang]

[-- Attachment #1: Type: text/plain, Size: 2123 bytes --]

Hi Ryan,

I want to confirm the register default set of LCLK is running or stop for Aspped SoC?

Hi Jae,
I want to confirm that does the aspeed_gates is set for matching with the SoC default clock setting?


I need to these things information for joining the discussion to assist patch to send upstream.

Thanks,

Samuel Jiang

On Dec 4, 2018, 2:17 PM +0800, Ryan Chen , wrote:
> Hello Jae,
> ASPEED LPC IP HW block have serval clk input.
> Most important is LCLK is come from LPC Host.
> The others is not controllable by register.
> Ryan
>
> -----Original Message-----
> From: Vijay Khemka [mailto:vijaykhemka@fb.com]
> Sent: Tuesday, December 4, 2018 3:38 AM
> To: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>; Samuel Jiang <chyishian.jiang@gmail.com>; qianlihu <wangzhiqiang8906@gmail.com>; Gary Hsu <gary_hsu@aspeedtech.com>; Ryan Chen <ryan_chen@aspeedtech.com>
> Cc: openbmc@lists.ozlabs.org
> Subject: Re: [HELP] ipmi-kcs didn't work
>
>
>
> On 12/3/18, 7:54 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> wrote:
>
> On 12/1/2018 8:29 AM, Samuel Jiang wrote:
> > Apologize for sending out no content mail first.
> >
> > Jae,
> > The aspeed_gates in clk-aspeed.c perhaps as todo suggest asking Aspeed
> > the actual parent data for check initializing?
> >
>
> Yes, that makes sense.
>
> Hi Gary and Ryan,
>
> Can you please tell us what is the actual parent clock source of LPC IP?
> I mean the operational clock of LPC IP hardware block, not the interface
> clock.
>
> Thanks,
> Jae
>
> > Vijay,
> > I traced lpc-ctrl module, it seems direct update the same
> > ASPEED_CLK_GATE_LCLK register map bit to enable. If parent data is
> > disabled, it could enable in dts.
> > The device tree detail, I reference it from aspeed-g5.dtsi. Hope it
> > could help you for work.
> >
> > Thanks,
> >
> > Samuel Jiang
>
> Samual/Jay,
> In my case if I don't initialize LPC clock in driver, Bios on host side wait and doesn't boot.
> I don't understand here what is holding Bios here but by initializing this LPC clock let bios boot.
>
> Regards
> -Vijay
>

[-- Attachment #2: Type: text/html, Size: 3726 bytes --]

RE: [HELP] ipmi-kcs didn't work

[Ryan Chen]

[-- Attachment #1: Type: text/plain, Size: 2309 bytes --]

Hello,
        The default is running, not to stop.

Ryan

Hi Ryan,

I want to confirm the register default set of LCLK is running or stop for Aspped SoC?

Hi Jae,
I want to confirm that does the aspeed_gates is set for matching with the SoC default clock setting?


I need to these things information for joining the discussion to assist patch to send upstream.

Thanks,

Samuel Jiang

On Dec 4, 2018, 2:17 PM +0800, Ryan Chen , wrote:

Hello Jae,
ASPEED LPC IP HW block have serval clk input.
Most important is LCLK is come from LPC Host.
The others is not controllable by register.
Ryan

-----Original Message-----
From: Vijay Khemka [mailto:vijaykhemka@fb.com]
Sent: Tuesday, December 4, 2018 3:38 AM
To: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com<mailto:jae.hyun.yoo@linux.intel.com>>; Samuel Jiang <chyishian.jiang@gmail.com<mailto:chyishian.jiang@gmail.com>>; qianlihu <wangzhiqiang8906@gmail.com<mailto:wangzhiqiang8906@gmail.com>>; Gary Hsu <gary_hsu@aspeedtech.com<mailto:gary_hsu@aspeedtech.com>>; Ryan Chen <ryan_chen@aspeedtech.com<mailto:ryan_chen@aspeedtech.com>>
Cc: openbmc@lists.ozlabs.org<mailto:openbmc@lists.ozlabs.org>
Subject: Re: [HELP] ipmi-kcs didn't work



On 12/3/18, 7:54 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com<mailto:jae.hyun.yoo@linux.intel.com>> wrote:

On 12/1/2018 8:29 AM, Samuel Jiang wrote:

Apologize for sending out no content mail first.

Jae,
The aspeed_gates in clk-aspeed.c perhaps as todo suggest asking Aspeed
the actual parent data for check initializing?

Yes, that makes sense.

Hi Gary and Ryan,

Can you please tell us what is the actual parent clock source of LPC IP?
I mean the operational clock of LPC IP hardware block, not the interface
clock.

Thanks,
Jae


Vijay,
I traced lpc-ctrl module, it seems direct update the same
ASPEED_CLK_GATE_LCLK register map bit to enable. If parent data is
disabled, it could enable in dts.
The device tree detail, I reference it from aspeed-g5.dtsi. Hope it
could help you for work.

Thanks,

Samuel Jiang

Samual/Jay,
In my case if I don't initialize LPC clock in driver, Bios on host side wait and doesn't boot.
I don't understand here what is holding Bios here but by initializing this LPC clock let bios boot.

Regards
-Vijay

[-- Attachment #2: Type: text/html, Size: 8411 bytes --]

Re: [HELP] ipmi-kcs didn't work

[Jae Hyun Yoo]

On 12/5/2018 12:45 AM, Samuel Jiang wrote:
> Hi Ryan,
> 
> I want to confirm the register default set of LCLK is running or stop 
> for Aspped SoC?
> 
> Hi Jae,
> I want to confirm that does the aspeed_gates is set for matching with 
> the SoC default clock setting?
> 

Why should the aspeed_gates match with SoC default? All clocks will be
set newly according to the dts setting at kernel booting time or at
module probing time.

Jae

> I need to these things information for joining the discussion to assist 
> patch to send upstream.
> 
> Thanks,
> 
> Samuel Jiang
> 
> On Dec 4, 2018, 2:17 PM +0800, Ryan Chen , wrote:
>> Hello Jae,
>> ASPEED LPC IP HW block have serval clk input.
>> Most important is LCLK is come from LPC Host.
>> The others is not controllable by register.
>> Ryan
>>
>> -----Original Message-----
>> From: Vijay Khemka [mailto:vijaykhemka@fb.com]
>> Sent: Tuesday, December 4, 2018 3:38 AM
>> To: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>; Samuel Jiang 
>> <chyishian.jiang@gmail.com>; qianlihu <wangzhiqiang8906@gmail.com>; 
>> Gary Hsu <gary_hsu@aspeedtech.com>; Ryan Chen <ryan_chen@aspeedtech.com>
>> Cc: openbmc@lists.ozlabs.org
>> Subject: Re: [HELP] ipmi-kcs didn't work
>>
>>
>>
>> On 12/3/18, 7:54 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> wrote:
>>
>> On 12/1/2018 8:29 AM, Samuel Jiang wrote:
>>> Apologize for sending out no content mail first.
>>>
>>> Jae,
>>> The aspeed_gates in clk-aspeed.c perhaps as todo suggest asking Aspeed
>>> the actual parent data for check initializing?
>>>
>>
>> Yes, that makes sense.
>>
>> Hi Gary and Ryan,
>>
>> Can you please tell us what is the actual parent clock source of LPC IP?
>> I mean the operational clock of LPC IP hardware block, not the interface
>> clock.
>>
>> Thanks,
>> Jae
>>
>>> Vijay,
>>> I traced lpc-ctrl module, it seems direct update the same
>>> ASPEED_CLK_GATE_LCLK register map bit to enable. If parent data is
>>> disabled, it could enable in dts.
>>> The device tree detail, I reference it from aspeed-g5.dtsi. Hope it
>>> could help you for work.
>>>
>>> Thanks,
>>>
>>> Samuel Jiang
>>
>> Samual/Jay,
>> In my case if I don't initialize LPC clock in driver, Bios on host 
>> side wait and doesn't boot.
>> I don't understand here what is holding Bios here but by initializing 
>> this LPC clock let bios boot.
>>
>> Regards
>> -Vijay
>>

Re: [HELP] ipmi-kcs didn't work

[Samuel Jiang]

[-- Attachment #1: Type: text/plain, Size: 3047 bytes --]


> Why should the aspeed_gates match with SoC default? All clocks will be
> set newly according to the dts setting at kernel booting time or at
> module probing time.
>
> Jae

Sorry, I misunderstand the aspeed_gates purpose.  Ignore my question and drop the patch.

Thanks,

Samuel Jiang
On Dec 5, 2018, 11:01 PM +0800, Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>, wrote:
> On 12/5/2018 12:45 AM, Samuel Jiang wrote:
> > Hi Ryan,
> >
> > I want to confirm the register default set of LCLK is running or stop
> > for Aspped SoC?
> >
> > Hi Jae,
> > I want to confirm that does the aspeed_gates is set for matching with
> > the SoC default clock setting?
> >
>
> Why should the aspeed_gates match with SoC default? All clocks will be
> set newly according to the dts setting at kernel booting time or at
> module probing time.
>
> Jae
>
> > I need to these things information for joining the discussion to assist
> > patch to send upstream.
> >
> > Thanks,
> >
> > Samuel Jiang
> >
> > On Dec 4, 2018, 2:17 PM +0800, Ryan Chen , wrote:
> > > Hello Jae,
> > > ASPEED LPC IP HW block have serval clk input.
> > > Most important is LCLK is come from LPC Host.
> > > The others is not controllable by register.
> > > Ryan
> > >
> > > -----Original Message-----
> > > From: Vijay Khemka [mailto:vijaykhemka@fb.com]
> > > Sent: Tuesday, December 4, 2018 3:38 AM
> > > To: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>; Samuel Jiang
> > > <chyishian.jiang@gmail.com>; qianlihu <wangzhiqiang8906@gmail.com>;
> > > Gary Hsu <gary_hsu@aspeedtech.com>; Ryan Chen <ryan_chen@aspeedtech.com>
> > > Cc: openbmc@lists.ozlabs.org
> > > Subject: Re: [HELP] ipmi-kcs didn't work
> > >
> > >
> > >
> > > On 12/3/18, 7:54 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> wrote:
> > >
> > > On 12/1/2018 8:29 AM, Samuel Jiang wrote:
> > > > Apologize for sending out no content mail first.
> > > >
> > > > Jae,
> > > > The aspeed_gates in clk-aspeed.c perhaps as todo suggest asking Aspeed
> > > > the actual parent data for check initializing?
> > > >
> > >
> > > Yes, that makes sense.
> > >
> > > Hi Gary and Ryan,
> > >
> > > Can you please tell us what is the actual parent clock source of LPC IP?
> > > I mean the operational clock of LPC IP hardware block, not the interface
> > > clock.
> > >
> > > Thanks,
> > > Jae
> > >
> > > > Vijay,
> > > > I traced lpc-ctrl module, it seems direct update the same
> > > > ASPEED_CLK_GATE_LCLK register map bit to enable. If parent data is
> > > > disabled, it could enable in dts.
> > > > The device tree detail, I reference it from aspeed-g5.dtsi. Hope it
> > > > could help you for work.
> > > >
> > > > Thanks,
> > > >
> > > > Samuel Jiang
> > >
> > > Samual/Jay,
> > > In my case if I don't initialize LPC clock in driver, Bios on host
> > > side wait and doesn't boot.
> > > I don't understand here what is holding Bios here but by initializing
> > > this LPC clock let bios boot.
> > >
> > > Regards
> > > -Vijay
> > >

[-- Attachment #2: Type: text/html, Size: 4575 bytes --]

Re: [HELP] ipmi-kcs didn't work

[Vijay Khemka]

Hi Jay,
I found issue, As lpc-ctrl driver probe is failing because I didn't define flash and memory-region in DT for lpc-ctrl which probe look for. By defining these lpc-ctrl driver comes up and enable lpc clock. It is working for me now with this DT change.

Regards
-Vijay

On 12/3/18, 7:54 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> wrote:

    On 12/1/2018 8:29 AM, Samuel Jiang wrote:
    > Apologize for sending out no content mail first.
    > 
    > Jae,
    > The aspeed_gates in clk-aspeed.c perhaps as todo suggest asking Aspeed 
    > the actual parent data for check initializing?
    > 
    
    Yes, that makes sense.
    
    Hi Gary and Ryan,
    
    Can you please tell us what is the actual parent clock source of LPC IP?
    I mean the operational clock of LPC IP hardware block, not the interface
    clock.
    
    Thanks,
    Jae
    
    > Vijay,
    > I traced lpc-ctrl module, it seems direct update the same 
    > ASPEED_CLK_GATE_LCLK register map bit to enable. If parent data is 
    > disabled, it could enable in dts.
    > The device tree detail, I reference it from aspeed-g5.dtsi. Hope it 
    > could help you for work.
    > 
    > Thanks,
    > 
    > Samuel Jiang
    > On Dec 1, 2018, 7:04 AM +0800, Jae Hyun Yoo , wrote:
    >> On 11/30/2018 4:39 PM, Vijay Khemka wrote:
    >>>
    >>>
    >>> On 11/30/18, 11:29 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> 
    >>> wrote:
    >>>
    >>> On 11/30/2018 12:57 PM, Vijay Khemka wrote:
    >>>>
    >>>> On 11/30/18, 7:31 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> 
    >>>> wrote:
    >>>>
    >>>> On 11/29/2018 9:00 PM, Samuel Jiang wrote:
    >>>>> Hi Jae,
    >>>>>
    >>>>> We also encountered the same situation a month ago.
    >>>>> We try to enable the LPC clock on devicetree to resolve this 
    >>>>> problem not
    >>>>> direct modify clock driver.
    >>>>>
    >>>>> lpc_ctrl: lpc-ctrl@0 {
    >>>>>   compatible = "aspeed,ast2500-lpc-ctrl”;
    >>>>>   reg = <0x0 0x80>;
    >>>>>   clocks = <&syscon ASPEED_CLK_GATE_LCLK>;
    >>>>> status = “okay”;
    >>>>> };
    >>>>>
    >>>>> However, I’m not sure which is better for LPC_CLCK setting.
    >>>>> Whether we should modify driver if original data is enabled or should
    >>>>> enable on the device tree, correct?
    >>>>>
    >>>>>
    >>>>
    >>>> Hi Samuel,
    >>>>
    >>>> Yes, you are right. I realized that kcs_bmc_aspeed.c doesn't have any
    >>>> clock control code so adding a clocks setting into kcs dtsi code doesn't
    >>>> make any help. Also, it was the reason why the change is needed on
    >>>> clk-aspeed.c. Enabling lpc_ctrl node you suggested seems a right way.
    >>>>
    >>>> Hi Vijay,
    >>>> Can you please try the way Samuel suggested after removing changes I
    >>>> gave you. Samuel's solution looks good to me.
    >>>>
    >>>> Thanks,
    >>>> Jae
    >>>>
    >>>> Jae,
    >>>> I can try. We can certainly enable lpc_ctrl node but in my 
    >>>> understanding that
    >>>> we still need this below patch to redefine ASPEED_CLK_GATE_LCLK value to
    >>>> add CRITICAL. I might be missing something here.
    >>>> - [ASPEED_CLK_GATE_LCLK] = { 8, 5, "lclk-gate", NULL, 0 }, /* LPC */
    >>>> + [ASPEED_CLK_GATE_LCLK] = { 8, 5, "lclk-gate", NULL, 
    >>>> CLK_IS_CRITICAL }, /* LPC */
    >>>>
    >>>
    >>> Vijay,
    >>>
    >>> I checked that lpc_ctrl module has clocks dt property handling code in
    >>> the module so I think the LCLK would be enabled without the clk-aspeed.c
    >>> fix. I can't test it now because I'm in a business trip so please test
    >>> it. I'll also check it when I back to office a week after.
    >>>
    >>> Thanks,
    >>> Jae
    >>>
    >>> Jae,
    >>> I removed above aspeed clock driver patch, and enabled lpc-ctrl node 
    >>> in dts also removed clock definition from kcs. And I don't see host 
    >>> is getting powered on. It works only with clock driver change.
    >>>
    >>> And I still didn't understand how enabling lpc-ctrl node will 
    >>> initialize ASPEED_CLK_GATE_LCLK to CRITICAL.
    >>>
    >>> -Vijay
    >>>
    >>
    >> Vijay,
    >>
    >> Thanks Vijay for sharing the test result. I'll also check it on my
    >> hardware when I get back to office. Will share my test result too.
    >>
    >> Thanks again,
    >> Jae
    

Re: [HELP] ipmi-kcs didn't work

[qianlihu]

On Sat, Dec 1, 2018 at 3:29 AM Jae Hyun Yoo
<jae.hyun.yoo@linux.intel.com> wrote:
>
> On 11/30/2018 12:57 PM, Vijay Khemka wrote:
> >
> > On 11/30/18, 7:31 AM, "Jae Hyun Yoo" <jae.hyun.yoo@linux.intel.com> wrote:
> >
> >      On 11/29/2018 9:00 PM, Samuel Jiang wrote:
> >      > Hi Jae,
> >      >
> >      > We also encountered the same situation a month ago.
> >      > We try to enable the LPC clock on devicetree to resolve this problem not
> >      > direct modify clock driver.
> >      >
> >      > lpc_ctrl: lpc-ctrl@0 {
> >      >  compatible = "aspeed,ast2500-lpc-ctrl”;
> >      >  reg = <0x0 0x80>;
> >      >  clocks = <&syscon ASPEED_CLK_GATE_LCLK>;
> >      >      status = “okay”;
> >      > };
> >      >
> >      > However, I’m not sure which is better for LPC_CLCK setting.
> >      > Whether we should modify driver if original data is enabled or should
> >      > enable on the device tree, correct?

Samuel,
thanks for your solution. It is good for me .

-qianlihu
> >      >
> >      >


> >
> >      Hi Samuel,
> >
> >      Yes, you are right. I realized that kcs_bmc_aspeed.c doesn't have any
> >      clock control code so adding a clocks setting into kcs dtsi code doesn't
> >      make any help. Also, it was the reason why the change is needed on
> >      clk-aspeed.c. Enabling lpc_ctrl node you suggested seems a right way.
> >
> >      Hi Vijay,
> >      Can you please try the way Samuel suggested after removing changes I
> >      gave you. Samuel's solution looks good to me.
> >
> >      Thanks,
> >      Jae
> >
> > Jae,
> > I can try. We can certainly enable lpc_ctrl node but in my understanding that
> > we still need this below patch to redefine ASPEED_CLK_GATE_LCLK value to
> > add CRITICAL. I might be missing something here.
> >      - [ASPEED_CLK_GATE_LCLK] = { 8, 5, "lclk-gate", NULL, 0 }, /* LPC */
> >     + [ASPEED_CLK_GATE_LCLK] = { 8, 5, "lclk-gate", NULL, CLK_IS_CRITICAL }, /* LPC */
> >
>
> Vijay,
>
> I checked that lpc_ctrl module has clocks dt property handling code in
> the module so I think the LCLK would be enabled without the clk-aspeed.c
> fix. I can't test it now because I'm in a business trip so please test
> it. I'll also check it when I back to office a week after.

Jae,
I tested it on my platform, it worked.

-qianlihu
--
> Thanks,
> Jae
>
> >
> > Regards
> > -Vijay
> >