eSPI: add ASPEED AST2500 eSPI driver to boot a host with PCH runs on eSPI
PATCH v2 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
---
v1 -> v2:
- Fix the checkpatch.pl warning in file espi-slave.rst Missing or malformed SPDX-License-Identifier tag in line 1 #71: FILE: Documentation/misc-devices/espi-slave.rst:1:
- Make the Kconfig desciption to be more accurate.
--- .../devicetree/bindings/misc/aspeed,espi-slave.txt | 20 ++ Documentation/misc-devices/espi-slave.rst | 119 ++++++++++ drivers/misc/Kconfig | 7 + drivers/misc/Makefile | 1 + drivers/misc/aspeed-espi-slave.c | 263 +++++++++++++++++++++ 5 files changed, 410 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
@@ -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 +119 -0
119 lines not on this page · raw message
drivers/misc/Kconfig +7 -0
@@ -472,6 +472,13 @@ config VEXPRESS_SYSCFG 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) && REGMAP_MMIO + tristate "Aspeed ast2500 eSPI slave device driver" + ---help--- + Control Aspeed ast2500 eSPI slave controller to handle event + which needs the firmware's processing. + config ASPEED_LPC_CTRL depends on (ARCH_ASPEED || COMPILE_TEST) && REGMAP && MFD_SYSCON tristate "Aspeed ast2400/2500 HOST LPC to BMC bridge control"
drivers/misc/Makefile +1 -0
@@ -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 +263 -0
263 lines not on this page · raw message
Signature · 1 line
2.7.4
Quoted 6 lines · Haiyue Wang
When PCH works under eSPI mode, the PMC (Power Management Controller) in
PCH is waiting forSUS_ACKfrom BMC after it alertsSUS_WARN. It is in dead loop if noSUS_ACKassert. This is the basic requirement for the BMC
works as eSPI slave.
So, do we have an agreement that the driver should go in this shape w/o interacting with SPI subsystem?
Also few comments below.
drivers/misc/Kconfig config VEXPRESS_SYSCFG · line 476
Quoted 3 lines
drivers/misc/Kconfig +2
-0+config ASPEED_ESPI_SLAVE + depends on (ARCH_ASPEED || COMPILE_TEST) && REGMAP_MMIO
I would rather split this to two
depends on REGMAP_MMIO depends on ARCH_ASPEED || COMPILE_TEST
Review
Quoted 7 lines
drivers/misc/Kconfig +3
-0+ tristate "Aspeed ast2500 eSPI slave device driver" + ---help--- + Control Aspeed ast2500 eSPI slave controller to handleevent
(quoted diff) +2
-0+ which needs the firmware's processing. +#include <linux/of.h>
What exactly requires this header?
drivers/misc/aspeed-espi-slave.c line 219
Quoted 10 lines
drivers/misc/aspeed-espi-slave.c new file +9
-0+static int aspeed_espi_slave_probe (struct platform_device *pdev) +{ +struct aspeed_espi_slave_data *priv; +struct device *dev = &pdev->dev; +struct resource *res; +void __iomem *regs; +int rc; + +dev_set_name (dev,DEVICE_NAME );
Do this after checks and memory allocations.
Quoted 12 lines
drivers/misc/aspeed-espi-slave.c new file +11
-0+ + res =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); + regs =devm_ioremap_resource (dev, res); +if (IS_ERR (regs)) +return PTR_ERR (regs); + + priv =devm_kzalloc (dev,sizeof (*priv),GFP_KERNEL ); +if (!priv) +return -ENOMEM ; + + priv->map = devm_regmap_init_mmio(dev, regs,
&espi_slave_regmap_cfg);
drivers/misc/aspeed-espi-slave.c line 260
Quoted 10 lines
drivers/misc/aspeed-espi-slave.c new file +8
-0+if (IS_ERR (priv->map)) +return PTR_ERR (priv->map); + +static const struct of_device_id of_espi_slave_match_table[] = { + { .compatible ="aspeed,ast2500-espi-slave" }, + { } +}; +MODULE_DEVICE_TABLE (of, of_espi_slave_match_table);
This one should be closer to the struct of_device_id.
Signature · 2 lines
Andy Shevchenko <[email protected]> Intel Finland Oy
Quoted 9 lines · Andy Shevchenko
Quoted 6 lines · Haiyue Wang
When PCH works under eSPI mode, the PMC (Power Management Controller) in
PCH is waiting forSUS_ACKfrom BMC after it alertsSUS_WARN. It is in dead loop if noSUS_ACKassert. This is the basic requirement for the BMC
works as eSPI slave.So, do we have an agreement that the driver should go in this shape w/o interacting with SPI subsystem?
Not sure, I've added the specification of eSPI, hope people don't feel confused with SPI. :-)
Quoted 7 lines
Also few comments below.
drivers/misc/Kconfig config VEXPRESS_SYSCFG · line 476
Quoted 2 lines
drivers/misc/Kconfig +2
-0+config ASPEED_ESPI_SLAVE + depends on (ARCH_ASPEED || COMPILE_TEST) && REGMAP_MMIOI would rather split this to two
depends on REGMAP_MMIO depends on ARCH_ASPEED || COMPILE_TEST
OK, it looks clean. I referred to:
config ASPEED_LPC_CTRL
depends on (ARCH_ASPEED || COMPILE_TEST) && REGMAP && MFD_SYSCON
Quoted 7 lines
Review
Quoted 6 lines
drivers/misc/Kconfig +3
-0+ tristate "Aspeed ast2500 eSPI slave device driver" + ---help--- + Control Aspeed ast2500 eSPI slave controller to handleevent
(quoted diff) +2
-0+ which needs the firmware's processing. +#include <linux/of.h>What exactly requires this header?
Oh, I ctrl+c / ctrl+v from other device tree usage module. :-( Remove it now. Thanks for making the code more clean.
Quoted 10 lines
drivers/misc/aspeed-espi-slave.c line 219
Quoted 9 lines
drivers/misc/aspeed-espi-slave.c new file +9
-0+static int aspeed_espi_slave_probe (struct platform_device *pdev) +{ +struct aspeed_espi_slave_data *priv; +struct device *dev = &pdev->dev; +struct resource *res; +void __iomem *regs; +int rc; + +dev_set_name (dev,DEVICE_NAME );Do this after checks and memory allocations.
Fixed!
Quoted 13 lines
Quoted 13 lines
drivers/misc/aspeed-espi-slave.c new file +11
-0+ + res =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); + regs =devm_ioremap_resource (dev, res); +if (IS_ERR (regs)) +return PTR_ERR (regs); + + priv =devm_kzalloc (dev,sizeof (*priv),GFP_KERNEL ); +if (!priv) +return -ENOMEM ; + + priv->map = devm_regmap_init_mmio(dev, regs,
&espi_slave_regmap_cfg);drivers/misc/aspeed-espi-slave.c new file +1
-0+if (IS_ERR (priv->map))
Quoted 8 lines
drivers/misc/aspeed-espi-slave.c line 260
Quoted 7 lines
drivers/misc/aspeed-espi-slave.c new file +7
-0+return PTR_ERR (priv->map); + +static const struct of_device_id of_espi_slave_match_table[] = { + { .compatible ="aspeed,ast2500-espi-slave" }, + { } +}; +MODULE_DEVICE_TABLE (of, of_espi_slave_match_table);This one should be closer to the struct
of_device_id.
Fixed.