Tim Chu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41232 )
Change subject: drivers/ocp/ipmi: Implement set POST start/end command ......................................................................
drivers/ocp/ipmi: Implement set POST start/end command
Implemented to send POST start and POST end command to BMC in ramstage.
Tested=Read POST command in OpenBMC, if success message may show as below,
root@bmc:~# cat /var/log/messages |grep -i "POST" Aug 10 00:11:24 bmc user.info ipmid: POST Start Event for Payload#1 Aug 10 00:11:28 bmc user.info ipmid: POST End Event for Payload#1 root@bmc:~#
Signed-off-by: TimChu Tim.Chu@quantatw.com Change-Id: I597924740e770a7880fd545ffda32c3ab9767ea1 --- A src/drivers/ocp/ipmi/Kconfig A src/drivers/ocp/ipmi/Makefile.inc A src/drivers/ocp/ipmi/ipmi_ocp.c A src/drivers/ocp/ipmi/ipmi_ocp.h M src/mainboard/ocp/tiogapass/Kconfig M src/mainboard/ocp/tiogapass/devicetree.cb 6 files changed, 136 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/41232/1
diff --git a/src/drivers/ocp/ipmi/Kconfig b/src/drivers/ocp/ipmi/Kconfig new file mode 100644 index 0000000..95c51c0 --- /dev/null +++ b/src/drivers/ocp/ipmi/Kconfig @@ -0,0 +1,3 @@ +config IPMI_OCP + bool + default n diff --git a/src/drivers/ocp/ipmi/Makefile.inc b/src/drivers/ocp/ipmi/Makefile.inc new file mode 100644 index 0000000..8291f82 --- /dev/null +++ b/src/drivers/ocp/ipmi/Makefile.inc @@ -0,0 +1 @@ +ramstage-$(CONFIG_IPMI_OCP) += ipmi_ocp.c diff --git a/src/drivers/ocp/ipmi/ipmi_ocp.c b/src/drivers/ocp/ipmi/ipmi_ocp.c new file mode 100644 index 0000000..773638f --- /dev/null +++ b/src/drivers/ocp/ipmi/ipmi_ocp.c @@ -0,0 +1,118 @@ +/* This file is part of the coreboot project. */ +/* + * Place in devicetree.cb: + * + * chip drivers/ocp/ipmi # OCP specific IPMI porting + device pnp ca2.1 on end + * end + */ + +#include <console/console.h> +#include <device/device.h> +#include <device/pnp.h> +#include "chip.h" +#include "drivers/ipmi/ipmi_kcs.h" +#include "ipmi_ocp.h" + +static int ipmi_set_post_start(struct device *dev, struct ipmi_rsp *rsp) +{ + int ret; + + ret = ipmi_kcs_message(dev->path.pnp.port, IPMI_NETFN_OEM, 0, + IPMI_BMC_SET_POST_START, NULL, 0, (u8 *)rsp, + sizeof(*rsp)); + + if (ret < sizeof(struct ipmi_rsp) || rsp->completion_code) { + printk(BIOS_ERR, "IPMI: %s command failed (ret=%d rsp=0x%x)\n", + __func__, ret, rsp->completion_code); + return 1; + } + if (ret != sizeof(*rsp)) { + printk(BIOS_ERR, "IPMI: %s response truncated\n", __func__); + return 1; + } + return 0; +} + +static int ipmi_set_post_end(struct device *dev, struct ipmi_rsp *rsp) +{ + int ret; + + ret = ipmi_kcs_message(dev->path.pnp.port, IPMI_NETFN_OEM, 0, + IPMI_BMC_SET_POST_END, NULL, 0, (u8 *)rsp, + sizeof(*rsp)); + + if (ret < sizeof(struct ipmi_rsp) || rsp->completion_code) { + printk(BIOS_ERR, "IPMI: %s command failed (ret=%d rsp=0x%x)\n", + __func__, ret, rsp->completion_code); + return 1; + } + if (ret != sizeof(*rsp)) { + printk(BIOS_ERR, "IPMI: %s response truncated\n", __func__); + return 1; + } + return 0; +} + +static void ipmi_ocp_init(struct device *dev) +{ + struct ipmi_rsp postrsp; + + /* Send POST start to BMC. */ + if (!ipmi_set_post_start(dev, &postrsp)) + printk(BIOS_DEBUG, "IPMI BMC POST is started\n"); +} + +static void ipmi_ocp_final(struct device *dev) +{ + struct ipmi_rsp postrsp; + + /* Send POST end to BMC. */ + if (!ipmi_set_post_end(dev, &postrsp)) + printk(BIOS_DEBUG, "IPMI BMC POST is ended\n"); +} + +static void ipmi_set_resources(struct device *dev) +{ + struct resource *res; + + for (res = dev->resource_list; res; res = res->next) { + if (!(res->flags & IORESOURCE_ASSIGNED)) + continue; + + res->flags |= IORESOURCE_STORED; + report_resource_stored(dev, res, ""); + } +} + +static void ipmi_read_resources(struct device *dev) +{ + struct resource *res = new_resource(dev, 0); + res->base = dev->path.pnp.port; + res->size = 2; + res->flags = IORESOURCE_IO | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; +} + +static struct device_operations ops = { + .read_resources = ipmi_read_resources, + .set_resources = ipmi_set_resources, + .init = ipmi_ocp_init, + .final = ipmi_ocp_final, +}; + +static void enable_dev(struct device *dev) +{ + if (dev->path.type != DEVICE_PATH_PNP) + printk(BIOS_ERR, "%s: Unsupported device type\n", + dev_path(dev)); + else if (dev->path.pnp.port & 1) + printk(BIOS_ERR, "%s: Base address needs to be aligned to 2\n", + dev_path(dev)); + else + dev->ops = &ops; +} + +struct chip_operations drivers_ocp_ipmi_ops = { + CHIP_NAME("IPMI OCP") + .enable_dev = enable_dev, +}; diff --git a/src/drivers/ocp/ipmi/ipmi_ocp.h b/src/drivers/ocp/ipmi/ipmi_ocp.h new file mode 100644 index 0000000..bcf0f9d --- /dev/null +++ b/src/drivers/ocp/ipmi/ipmi_ocp.h @@ -0,0 +1,10 @@ +/* This file is part of the coreboot project. */ + +#ifndef __IPMI_OCP_H +#define __IPMI_OCP_H + +#define IPMI_NETFN_OEM 0x30 +#define IPMI_BMC_SET_POST_START 0x73 +#define IPMI_BMC_SET_POST_END 0x74 + +#endif diff --git a/src/mainboard/ocp/tiogapass/Kconfig b/src/mainboard/ocp/tiogapass/Kconfig index 5cb2421..2ec47ed 100644 --- a/src/mainboard/ocp/tiogapass/Kconfig +++ b/src/mainboard/ocp/tiogapass/Kconfig @@ -9,6 +9,7 @@ select HAVE_ACPI_TABLES select MAINBOARD_USES_FSP2_0 select IPMI_KCS + select IPMI_OCP select SOC_INTEL_SKYLAKE_SP select SUPERIO_ASPEED_AST2400
diff --git a/src/mainboard/ocp/tiogapass/devicetree.cb b/src/mainboard/ocp/tiogapass/devicetree.cb index 19dba55..35e74b0 100644 --- a/src/mainboard/ocp/tiogapass/devicetree.cb +++ b/src/mainboard/ocp/tiogapass/devicetree.cb @@ -77,6 +77,9 @@ register "bmc_i2c_address" = "0x20" register "bmc_boot_timeout" = "60" end + chip drivers/ocp/ipmi # OCP specific IPMI porting + device pnp ca2.1 on end + end end # Intel Corporation C621 Series Chipset LPC/eSPI Controller device pci 1f.2 on end # Intel Corporation C620 Series Chipset Family Power Management Controller device pci 1f.4 on end # Intel Corporation C620 Series Chipset Family SMBus
Tim Chu has removed Patrick Georgi from this change. ( https://review.coreboot.org/c/coreboot/+/41232 )
Change subject: drivers/ocp/ipmi: Implement set POST start/end command ......................................................................
Removed reviewer Patrick Georgi.
Tim Chu has removed Martin Roth from this change. ( https://review.coreboot.org/c/coreboot/+/41232 )
Change subject: drivers/ocp/ipmi: Implement set POST start/end command ......................................................................
Removed reviewer Martin Roth.
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41232 )
Change subject: drivers/ocp/ipmi: Implement set POST start/end command ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41232/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41232/1//COMMIT_MSG@9 PS1, Line 9: Implemented to send POST start and POST end command to BMC in ramstage. Implemented to send --> Implemented sending.
Also please split this patch into two. One for drivers/ocp/ipmi, another for mb/ocp/tiogapass.
https://review.coreboot.org/c/coreboot/+/41232/1/src/drivers/ocp/ipmi/ipmi_o... File src/drivers/ocp/ipmi/ipmi_ocp.h:
https://review.coreboot.org/c/coreboot/+/41232/1/src/drivers/ocp/ipmi/ipmi_o... PS1, Line 1: /* This file is part of the coreboot project. */ Instead we just need this single line: /* SPDX-License-Identifier: GPL-2.0-or-later */
Tim Chu has removed David Hendricks from this change. ( https://review.coreboot.org/c/coreboot/+/41232 )
Change subject: drivers/ocp/ipmi: Implement set POST start/end command ......................................................................
Removed reviewer David Hendricks.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41232 )
Change subject: drivers/ocp/ipmi: Implement set POST start/end command ......................................................................
Patch Set 1:
Shouldn't "post start" be send in bootblock and "post end" before payload hand-off?
Tim Chu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41232 )
Change subject: drivers/ocp/ipmi: Implement set POST start/end command ......................................................................
Patch Set 1:
Patch Set 1:
Shouldn't "post start" be send in bootblock and "post end" before payload hand-off?
"post start" should wait for the finish of ipmi init, so I add this in ramstage. About "post end", do you mean to use call back of boot state to implement?
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41232 )
Change subject: drivers/ocp/ipmi: Implement set POST start/end command ......................................................................
Patch Set 1:
What's ipmi init? Do you have a specification that describes this more briefly?
Tim Chu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41232 )
Change subject: drivers/ocp/ipmi: Implement set POST start/end command ......................................................................
Patch Set 1:
Patch Set 1:
What's ipmi init? Do you have a specification that describes this more briefly?
"wait for the finish of ipmi init" is not suitable in my last reply. The reason I add this command in ramstage is that "post start" should wait BMC ready.
Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41232 )
Change subject: drivers/ocp/ipmi: Implement set POST start/end command ......................................................................
Patch Set 1:
Patch Set 1:
Patch Set 1:
What's ipmi init? Do you have a specification that describes this more briefly?
"wait for the finish of ipmi init" is not suitable in my last reply. The reason I add this command in ramstage is that "post start" should wait BMC ready.
About OEM IPMI start command, in CB:40234 it adds ipmi_kcs_premem_init() which waits for BMC ready in romstage, and is called in CB:39690. So I think calling IPMI start command can be done in romstage which is earlier than ramstage. The only thing is that in romstage I need to pass BMC KCS address to the function. Hi Jonathan, Should we put all common IPMI OEM commands in this driver? Such as OEM start, stop, set PPIN .,etc. For different mainboard code we can call this driver/library to avoid duplicating the same codes.
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41232 )
Change subject: drivers/ocp/ipmi: Implement set POST start/end command ......................................................................
Patch Set 1:
Patch Set 1:
Patch Set 1:
Patch Set 1:
What's ipmi init? Do you have a specification that describes this more briefly?
"wait for the finish of ipmi init" is not suitable in my last reply. The reason I add this command in ramstage is that "post start" should wait BMC ready.
About OEM IPMI start command, in CB:40234 it adds ipmi_kcs_premem_init() which waits for BMC ready in romstage, and is called in CB:39690. So I think calling IPMI start command can be done in romstage which is earlier than ramstage. The only thing is that in romstage I need to pass BMC KCS address to the function. Hi Jonathan, Should we put all common IPMI OEM commands in this driver? Such as OEM start, stop, set PPIN .,etc. For different mainboard code we can call this driver/library to avoid duplicating the same codes.
Hi Johnny, I agree that gradually we should put all common IPMI OEM commands (that are used by OCP servers) in this driver. Hi Tim, thanks for putting this patch together. Suggest to split this patch into two: the first one is for drivers/ocp/ipmi, another is for mb/ocp/tiogapass.
Tim Chu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41232 )
Change subject: drivers/ocp/ipmi: Implement set POST start/end command ......................................................................
Patch Set 1:
(2 comments)
Patch Set 1:
Patch Set 1:
Patch Set 1:
Patch Set 1:
What's ipmi init? Do you have a specification that describes this more briefly?
"wait for the finish of ipmi init" is not suitable in my last reply. The reason I add this command in ramstage is that "post start" should wait BMC ready.
About OEM IPMI start command, in CB:40234 it adds ipmi_kcs_premem_init() which waits for BMC ready in romstage, and is called in CB:39690. So I think calling IPMI start command can be done in romstage which is earlier than ramstage. The only thing is that in romstage I need to pass BMC KCS address to the function. Hi Jonathan, Should we put all common IPMI OEM commands in this driver? Such as OEM start, stop, set PPIN .,etc. For different mainboard code we can call this driver/library to avoid duplicating the same codes.
Hi Johnny, I agree that gradually we should put all common IPMI OEM commands (that are used by OCP servers) in this driver. Hi Tim, thanks for putting this patch together. Suggest to split this patch into two: the first one is for drivers/ocp/ipmi, another is for mb/ocp/tiogapass.
Hi Jonathan, I've split this patch into two. One is in CB:41604, another is in CB:41605.
https://review.coreboot.org/c/coreboot/+/41232/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41232/1//COMMIT_MSG@9 PS1, Line 9: Implemented to send POST start and POST end command to BMC in ramstage.
Implemented to send --> Implemented sending. […]
Done
https://review.coreboot.org/c/coreboot/+/41232/1/src/drivers/ocp/ipmi/ipmi_o... File src/drivers/ocp/ipmi/ipmi_ocp.h:
https://review.coreboot.org/c/coreboot/+/41232/1/src/drivers/ocp/ipmi/ipmi_o... PS1, Line 1: /* This file is part of the coreboot project. */
Instead we just need this single line: […]
Done
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41232 )
Change subject: drivers/ocp/ipmi: Implement set POST start/end command ......................................................................
Patch Set 1:
Patch Set 1:
(2 comments)
Patch Set 1:
Patch Set 1:
Patch Set 1:
Patch Set 1:
What's ipmi init? Do you have a specification that describes this more briefly?
"wait for the finish of ipmi init" is not suitable in my last reply. The reason I add this command in ramstage is that "post start" should wait BMC ready.
About OEM IPMI start command, in CB:40234 it adds ipmi_kcs_premem_init() which waits for BMC ready in romstage, and is called in CB:39690. So I think calling IPMI start command can be done in romstage which is earlier than ramstage. The only thing is that in romstage I need to pass BMC KCS address to the function. Hi Jonathan, Should we put all common IPMI OEM commands in this driver? Such as OEM start, stop, set PPIN .,etc. For different mainboard code we can call this driver/library to avoid duplicating the same codes.
Hi Johnny, I agree that gradually we should put all common IPMI OEM commands (that are used by OCP servers) in this driver. Hi Tim, thanks for putting this patch together. Suggest to split this patch into two: the first one is for drivers/ocp/ipmi, another is for mb/ocp/tiogapass.
Hi Jonathan, I've split this patch into two. One is in CB:41604, another is in CB:41605.
Nice. You can abandon this change then, so people will not get confused. When I split patch, I normally reuse the previous Change-ID in the first patch of the split.
Tim Chu has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/41232 )
Change subject: drivers/ocp/ipmi: Implement set POST start/end command ......................................................................
Abandoned
-
Johnny Lin (Code Review) -
Jonathan Zhang (Code Review)
-
Patrick Rudolph (Code Review)
-
Tim Chu (Code Review)