Skip to contentSkip to conversation

eSPI: add Aspeed AST2500 eSPI driver to boot a host with PCH runs on eSPI

PATCH Superseded Haiyue Wang 3 participants 4 messages Jan 16, 2018 – Jan 25, 2018

Subsystems (4)
Device tree bindings Documentation Char and misc drivers Arm/Aspeed machine support

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Haiyue Wang

author
#0 lore raw

PATCH 1/1 eSPI: add Aspeed AST2500 eSPI driver to boot a host with PCH runs on eSPI

When PCH works under eSPI mode, the PMC (Power Management Controller) in PCH is waiting for SUS_ACK from BMC after it alerts SUS_WARN. It is in dead loop if no SUS_ACK assert. This is the basic requirement for the BMC works as eSPI slave.

Also for the host power on / off actions, from BMC side, the following VW (Virtual Wire) messages are done in firmware:
1. SLAVE_BOOT_LOAD_DONE / SLAVE_BOOT_LOAD_STATUS
2. SUS_ACK
3. OOB_RESET_ACK
4. HOST_RESET_ACK

Signed-off-by Haiyue Wang <[email protected]>
---
 .../devicetree/bindings/misc/aspeed-espi-slave.txt |  20 ++
 Documentation/misc-devices/espi-slave.rst          | 114 +++++++++
 drivers/misc/Kconfig                               |  11 +
 drivers/misc/Makefile                              |   1 +
 drivers/misc/aspeed-espi-slave.c                   | 266 +++++++++++++++++++++
 5 files changed, 412 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/aspeed-espi-slave.txt
 create mode 100644 Documentation/misc-devices/espi-slave.rst
 create mode 100644 drivers/misc/aspeed-espi-slave.c
Documentation/devicetree/bindings/misc/aspeed-espi-slave.txt new file +20 -0
new file mode 100644
index 0000000..35da26f
@@ -0,0 +1,20 @@
+Aspeed eSPI Slave Controller
+
+Required properties:
+ - compatible: must be one of:
+	- "aspeed,ast2500-espi-slave"
+
+ - reg: physical base address of the controller and length of memory mapped
+   region
+
+ - interrupts: interrupt generated by the controller
+
+Example:
+
+    espi: espi@1e6ee000 {
+        compatible = "aspeed,ast2500-espi-slave";
+        reg = <0x1e6ee000 0x100>;
+        interrupts = <23>;
+        status = "disabled";
+};
+
Documentation/misc-devices/espi-slave.rst new file +114 -0

114 lines not on this page · raw message

drivers/misc/Kconfig +11 -0
index f1a5c23..fec5783 100644
@@ -471,6 +471,17 @@ config VEXPRESS_SYSCFG
 	  ARM Ltd. Versatile Express uses specialised platform configuration
 	  bus. System Configuration interface is one of the possible means
 	  of generating transactions on this bus.
+config ASPEED_ESPI_SLAVE
+	depends on ARCH_ASPEED || COMPILE_TEST
+	select REGMAP_MMIO
+	tristate "Aspeed ast2500 eSPI slave device"
+	---help---
+	  This allows host to access Baseboard Management Controller (BMC) over the
+	  Enhanced Serial Peripheral Interface (eSPI) bus, which replaces the Low Pin
+	  Count (LPC) bus.
+
+	  Its interface supports peripheral, virtual wire, out-of-band, and flash
+	  sharing channels.
 
 config ASPEED_LPC_CTRL
 	depends on (ARCH_ASPEED || COMPILE_TEST) && REGMAP && MFD_SYSCON
drivers/misc/Makefile +1 -0
index 5ca5f64..a1081f4 100644
@@ -52,6 +52,7 @@ obj-$(CONFIG_GENWQE)		+= genwqe/
 obj-$(CONFIG_ECHO)		+= echo/
 obj-$(CONFIG_VEXPRESS_SYSCFG)	+= vexpress-syscfg.o
 obj-$(CONFIG_CXL_BASE)		+= cxl/
+obj-$(CONFIG_ASPEED_ESPI_SLAVE) += aspeed-espi-slave.o
 obj-$(CONFIG_ASPEED_LPC_CTRL)	+= aspeed-lpc-ctrl.o
 obj-$(CONFIG_ASPEED_LPC_SNOOP)	+= aspeed-lpc-snoop.o
 obj-$(CONFIG_PCI_ENDPOINT_TEST)	+= pci_endpoint_test.o
drivers/misc/aspeed-espi-slave.c new file +266 -0

266 lines not on this page · raw message

Signature · 1 line
2.7.4
maintainer
#1 lore raw
Quoted 12 lines · Haiyue Wang

When PCH works under eSPI mode, the PMC (Power Management Controller) in PCH is waiting for SUS_ACK from BMC after it alerts SUS_WARN. It is in dead loop if no SUS_ACK assert. This is the basic requirement for the BMC works as eSPI slave.

Also for the host power on / off actions, from BMC side, the following VW (Virtual Wire) messages are done in firmware:
1. SLAVE_BOOT_LOAD_DONE / SLAVE_BOOT_LOAD_STATUS
2. SUS_ACK
3. OOB_RESET_ACK
4. HOST_RESET_ACK

Quoted 4 lines
Signed-off-by Haiyue Wang <[email protected]>
---
 .../devicetree/bindings/misc/aspeed-espi-slave.txt |  20 ++
 Documentation/misc-devices/espi-slave.rst          | 114 +++++++++

DT files need to be split out into a separate patch so that the DT maintainers can properly review them.

drivers/misc/Kconfig config VEXPRESS_SYSCFG · line 474

Quoted 7 lines
drivers/misc/Kconfig +1 -0
@@ -471,6 +471,17 @@ config VEXPRESS_SYSCFG
 	  ARM Ltd. Versatile Express uses specialised platform configuration
 	  bus. System Configuration interface is one of the possible means
 	  of generating transactions on this bus.
+config ASPEED_ESPI_SLAVE

You need a blank line above this one please.

drivers/misc/Kconfig config VEXPRESS_SYSCFG · line 476

Quoted 2 lines
drivers/misc/Kconfig +2 -0
+	depends on ARCH_ASPEED || COMPILE_TEST
+	select REGMAP_MMIO

Select or depend?

drivers/misc/Kconfig config VEXPRESS_SYSCFG · line 484

Quoted 8 lines
drivers/misc/Kconfig +8 -0
+	tristate "Aspeed ast2500 eSPI slave device"
+	---help---
+	  This allows host to access Baseboard Management Controller (BMC) over the
+	  Enhanced Serial Peripheral Interface (eSPI) bus, which replaces the Low Pin
+	  Count (LPC) bus.
+
+	  Its interface supports peripheral, virtual wire, out-of-band, and flash
+	  sharing channels.

What is the module name?

drivers/misc/Makefile obj-$(CONFIG_GENWQE) += genwqe/ · line 55

Quoted 12 lines
drivers/misc/Kconfig +0 -0
 config ASPEED_LPC_CTRL
 	depends on (ARCH_ASPEED || COMPILE_TEST) && REGMAP && MFD_SYSCON
drivers/misc/Makefile +1 -0
index 5ca5f64..a1081f4 100644
@@ -52,6 +52,7 @@ obj-$(CONFIG_GENWQE)		+= genwqe/
 obj-$(CONFIG_ECHO)		+= echo/
 obj-$(CONFIG_VEXPRESS_SYSCFG)	+= vexpress-syscfg.o
 obj-$(CONFIG_CXL_BASE)		+= cxl/
+obj-$(CONFIG_ASPEED_ESPI_SLAVE) += aspeed-espi-slave.o

Why no tab?

thanks,

greg k-h

author
#2 lore raw
Quoted 20 lines · Greg KH
Quoted 12 lines · Haiyue Wang

When PCH works under eSPI mode, the PMC (Power Management Controller) in PCH is waiting for SUS_ACK from BMC after it alerts SUS_WARN. It is in dead loop if no SUS_ACK assert. This is the basic requirement for the BMC works as eSPI slave.

Also for the host power on / off actions, from BMC side, the following VW (Virtual Wire) messages are done in firmware:
1. SLAVE_BOOT_LOAD_DONE / SLAVE_BOOT_LOAD_STATUS
2. SUS_ACK
3. OOB_RESET_ACK
4. HOST_RESET_ACK

Quoted 4 lines
Signed-off-by Haiyue Wang <[email protected]>
---
  .../devicetree/bindings/misc/aspeed-espi-slave.txt |  20 ++
  Documentation/misc-devices/espi-slave.rst          | 114 +++++++++

DT files need to be split out into a separate patch so that the DT maintainers can properly review them.

Quoted 8 lines

drivers/misc/Kconfig config VEXPRESS_SYSCFG · line 474

Quoted 7 lines
drivers/misc/Kconfig +1 -0
@@ -471,6 +471,17 @@ config VEXPRESS_SYSCFG
  	  ARM Ltd. Versatile Express uses specialised platform configuration
  	  bus. System Configuration interface is one of the possible means
  	  of generating transactions on this bus.
+config ASPEED_ESPI_SLAVE

You need a blank line above this one please.

Fixed.

Quoted 3 lines

drivers/misc/Kconfig config VEXPRESS_SYSCFG · line 476

Quoted 2 lines
drivers/misc/Kconfig +2 -0
+	depends on ARCH_ASPEED || COMPILE_TEST
+	select REGMAP_MMIO

Select or depend?

Before, I used "depends on". I was told to to change "select". I'm not sure which is more better by practice ?

Quoted 9 lines

drivers/misc/Kconfig config VEXPRESS_SYSCFG · line 484

Quoted 8 lines
drivers/misc/Kconfig +8 -0
+	tristate "Aspeed ast2500 eSPI slave device"
+	---help---
+	  This allows host to access Baseboard Management Controller (BMC) over the
+	  Enhanced Serial Peripheral Interface (eSPI) bus, which replaces the Low Pin
+	  Count (LPC) bus.
+
+	  Its interface supports peripheral, virtual wire, out-of-band, and flash
+	  sharing channels.

What is the module name?

You means the --help-- description is not clear and specific ?

Quoted 13 lines

drivers/misc/Makefile obj-$(CONFIG_GENWQE) += genwqe/ · line 55

Quoted 12 lines
drivers/misc/Kconfig +0 -0
  config ASPEED_LPC_CTRL
  	depends on (ARCH_ASPEED || COMPILE_TEST) && REGMAP && MFD_SYSCON
drivers/misc/Makefile +1 -0
index 5ca5f64..a1081f4 100644
@@ -52,6 +52,7 @@ obj-$(CONFIG_GENWQE)		+= genwqe/
  obj-$(CONFIG_ECHO)		+= echo/
  obj-$(CONFIG_VEXPRESS_SYSCFG)	+= vexpress-syscfg.o
  obj-$(CONFIG_CXL_BASE)		+= cxl/
+obj-$(CONFIG_ASPEED_ESPI_SLAVE) += aspeed-espi-slave.o

Why no tab?

Fixed, thanks.

Quoted 3 lines

thanks,

greg k-h

#3 lore raw
Quoted 4 lines · Haiyue
Quoted 1 line · Greg KH

On Tue, Jan 16, 2018 at 07:52:32PM +0800, Haiyue Wang wrote:

Quoted 8 lines
Quoted 4 lines

drivers/misc/Kconfig config VEXPRESS_SYSCFG · line 476

Quoted 2 lines
drivers/misc/Kconfig +2 -0
+	depends on ARCH_ASPEED || COMPILE_TEST
+	select REGMAP_MMIO

Select or depend?

Before, I used "depends on". I was told to to change "select". I'm not
sure which is more better by practice ?

git grep will answer to this question, but also documentation is good to read.

REGMAP API is selected by users.

One case where it's not so looks like a bug.

Signature · 2 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy