Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35036 )
Change subject: drivers/generic: Add new driver for power resource pseudo device ......................................................................
drivers/generic: Add new driver for power resource pseudo device
There is some duplication of fields dealing with ACPI power resources (see drivers/i2c/generic.c, also used by drivers/i2c/hid, as well as drivers/spi/acpi). This CL creates a new driver for this pseudo-device.
This patch allows a new style of dealing with power resources in the device tree:
device pci 00.0 chip drivers/generic/power_resource register "enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_A1)" device generic 0 on chip drivers/i2c/generic register ... end end end end
and then the power resource driver does most of the work. This should make it easier to support other drivers requiring ACPI power resources in the future (see the diff in the two changed here).
I tested this patch on a device using ACPI power resources, and it was successfully able to invoke the _ON and _OFF routines for the device, both with the new-style and old-style way of creating power resources.
My hope would be to update device trees in later patches to accomodate this new style, and then deprecate the old style.
Change-Id: I358c48f4d4e4c4ef591ecf09ea9e9488c2652c96 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/arch/x86/acpi_device.c M src/arch/x86/acpigen.c M src/arch/x86/include/arch/acpi_device.h M src/arch/x86/include/arch/acpigen.h M src/device/root_device.c M src/drivers/i2c/generic/generic.c M src/drivers/spi/acpi/acpi.c 7 files changed, 122 insertions(+), 52 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/35036/1
diff --git a/src/arch/x86/acpi_device.c b/src/arch/x86/acpi_device.c index 47bcc52..40e9677 100644 --- a/src/arch/x86/acpi_device.c +++ b/src/arch/x86/acpi_device.c @@ -494,20 +494,12 @@ acpi_device_fill_len(desc_length); }
-/* PowerResource() with Enable and/or Reset control */ -void acpi_device_add_power_res(const struct acpi_power_res_params *params) +/* Adds _STA, _ON, and _OFF methods to a power resource, based on config */ +static void acpi_add_power_res_methods(const struct acpi_power_res_params *config) { - static const char *power_res_dev_states[] = { "_PR0", "_PR3" }; - unsigned int reset_gpio = params->reset_gpio->pins[0]; - unsigned int enable_gpio = params->enable_gpio->pins[0]; - unsigned int stop_gpio = params->stop_gpio->pins[0]; - - if (!reset_gpio && !enable_gpio && !stop_gpio) - return; - - /* PowerResource (PRIC, 0, 0) */ - acpigen_write_power_res("PRIC", 0, 0, power_res_dev_states, - ARRAY_SIZE(power_res_dev_states)); + const unsigned int reset_gpio = config->reset_gpio->pins[0]; + const unsigned int enable_gpio = config->enable_gpio->pins[0]; + const unsigned int stop_gpio = config->stop_gpio->pins[0];
/* Method (_STA, 0, NotSerialized) { Return (0x1) } */ acpigen_write_STA(0x1); @@ -515,44 +507,75 @@ /* Method (_ON, 0, Serialized) */ acpigen_write_method_serialized("_ON", 0); if (reset_gpio) - acpigen_enable_tx_gpio(params->reset_gpio); + acpigen_enable_tx_gpio(config->reset_gpio); if (enable_gpio) { - acpigen_enable_tx_gpio(params->enable_gpio); - if (params->enable_delay_ms) - acpigen_write_sleep(params->enable_delay_ms); + acpigen_enable_tx_gpio(config->enable_gpio); + if (config->enable_delay_ms) + acpigen_write_sleep(config->enable_delay_ms); } if (reset_gpio) { - acpigen_disable_tx_gpio(params->reset_gpio); - if (params->reset_delay_ms) - acpigen_write_sleep(params->reset_delay_ms); + acpigen_disable_tx_gpio(config->reset_gpio); + if (config->reset_delay_ms) + acpigen_write_sleep(config->reset_delay_ms); } if (stop_gpio) { - acpigen_disable_tx_gpio(params->stop_gpio); - if (params->stop_delay_ms) - acpigen_write_sleep(params->stop_delay_ms); + acpigen_disable_tx_gpio(config->stop_gpio); + if (config->stop_delay_ms) + acpigen_write_sleep(config->stop_delay_ms); } - acpigen_pop_len(); /* _ON method */ + + acpigen_pop_len(); /* _ON method */
/* Method (_OFF, 0, Serialized) */ acpigen_write_method_serialized("_OFF", 0); if (stop_gpio) { - acpigen_enable_tx_gpio(params->stop_gpio); - if (params->stop_off_delay_ms) - acpigen_write_sleep(params->stop_off_delay_ms); + acpigen_enable_tx_gpio(config->stop_gpio); + if (config->stop_off_delay_ms) + acpigen_write_sleep(config->stop_off_delay_ms); } if (reset_gpio) { - acpigen_enable_tx_gpio(params->reset_gpio); - if (params->reset_off_delay_ms) - acpigen_write_sleep(params->reset_off_delay_ms); + acpigen_enable_tx_gpio(config->reset_gpio); + if (config->reset_off_delay_ms) + acpigen_write_sleep(config->reset_off_delay_ms); } if (enable_gpio) { - acpigen_disable_tx_gpio(params->enable_gpio); - if (params->enable_off_delay_ms) - acpigen_write_sleep(params->enable_off_delay_ms); + acpigen_disable_tx_gpio(config->enable_gpio); + if (config->enable_off_delay_ms) + acpigen_write_sleep(config->enable_off_delay_ms); } - acpigen_pop_len(); /* _OFF method */
- acpigen_pop_len(); /* PowerResource PRIC */ + acpigen_pop_len(); /* _OFF method */ +} + +void acpi_add_power_res(const char *power_resource_name, + const struct acpi_power_res_params *config) +{ + const unsigned int reset_gpio = config->reset_gpio->pins[0]; + const unsigned int enable_gpio = config->enable_gpio->pins[0]; + const unsigned int stop_gpio = config->stop_gpio->pins[0]; + + if (!reset_gpio && !enable_gpio && !stop_gpio) + return; + + /* PowerResource (name, 0, 0) */ + acpigen_write_power_res(power_resource_name, 0, 0); + acpi_add_power_res_methods(config); + acpigen_pop_len(); /* PowerResource */ +} + +void acpi_add_power_states(const char *power_resource_name) +{ + static const char *dev_states[] = { "_PR0", "_PR3" }; + acpigen_write_power_states(power_resource_name, dev_states, 2); +} + +void acpi_add_scoped_power_res(const char *scope, + const char *name, + const struct acpi_power_res_params *config) +{ + acpigen_write_scope(scope); + acpi_add_power_res(name, config); + acpigen_pop_len(); /* Scope */ }
static void acpi_dp_write_array(const struct acpi_dp *array); diff --git a/src/arch/x86/acpigen.c b/src/arch/x86/acpigen.c index f9af102..553c4ff 100644 --- a/src/arch/x86/acpigen.c +++ b/src/arch/x86/acpigen.c @@ -1107,22 +1107,26 @@ acpigen_pop_len(); }
+void acpigen_write_power_states(const char *power_resource_name, + const char *power_states[], + size_t power_states_count) +{ + size_t i; + for (i = 0; i < power_states_count; i++) { + acpigen_write_name(power_states[i]); + acpigen_write_package(1); + acpigen_emit_namestring(power_resource_name); + acpigen_pop_len(); /* Package */ + } +} + /* * Name (_PRx, Package(One) { name }) * ... * PowerResource (name, level, order) */ -void acpigen_write_power_res(const char *name, uint8_t level, uint16_t order, - const char *dev_states[], size_t dev_states_count) +void acpigen_write_power_res(const char *name, uint8_t level, uint16_t order) { - size_t i; - for (i = 0; i < dev_states_count; i++) { - acpigen_write_name(dev_states[i]); - acpigen_write_package(1); - acpigen_emit_simple_namestring(name); - acpigen_pop_len(); /* Package */ - } - acpigen_emit_ext_op(POWER_RES_OP);
acpigen_write_len_f(); diff --git a/src/arch/x86/include/arch/acpi_device.h b/src/arch/x86/include/arch/acpi_device.h index d74af9d..6c7993f 100644 --- a/src/arch/x86/include/arch/acpi_device.h +++ b/src/arch/x86/include/arch/acpi_device.h @@ -403,7 +403,19 @@ * Enable - Enable / disable power to device. * Stop - Stop / start operation of device. */ -void acpi_device_add_power_res(const struct acpi_power_res_params *params); +void acpi_add_power_res(const char *power_resource_name, + const struct acpi_power_res_params *config); + +/* Adds a PowerResource (as above), but in its own scope */ +void acpi_add_scoped_power_res(const char *scope, + const char *name, + const struct acpi_power_res_params *config); + +/* + * Write out _PR0 and _PR3 methods for a device, using power_resource_name + * as the PowerResource to reference +*/ +void acpi_add_power_states(const char *power_resource_name);
/* * Writing Device Properties objects via _DSD diff --git a/src/arch/x86/include/arch/acpigen.h b/src/arch/x86/include/arch/acpigen.h index db003fe..242b1e1 100644 --- a/src/arch/x86/include/arch/acpigen.h +++ b/src/arch/x86/include/arch/acpigen.h @@ -359,8 +359,10 @@ void acpigen_write_mainboard_resources(const char *scope, const char *name); void acpigen_write_irq(u16 mask); void acpigen_write_uuid(const char *uuid); -void acpigen_write_power_res(const char *name, uint8_t level, uint16_t order, - const char *dev_states[], size_t dev_states_count); +void acpigen_write_power_res(const char *name, uint8_t level, uint16_t order); +void acpigen_write_power_states(const char *power_resource_name, + const char *power_states[], + size_t power_states_count); void acpigen_write_sleep(uint64_t sleep_ms); void acpigen_write_store(void); void acpigen_write_store_ops(uint8_t src, uint8_t dst); diff --git a/src/device/root_device.c b/src/device/root_device.c index f8e2907..ec24ff5 100644 --- a/src/device/root_device.c +++ b/src/device/root_device.c @@ -112,6 +112,13 @@
printk(BIOS_DEBUG, "%s %s\n", dev_path(child), child->enabled ? "enabled" : "disabled"); + + /* + * To handle cases of pseudo-devices containing a child + * device when it's not explicitly a bus, e.g., power + * resources, we recurse into them. + */ + scan_generic_bus(child); } }
diff --git a/src/drivers/i2c/generic/generic.c b/src/drivers/i2c/generic/generic.c index 9b4f00d..308f200 100644 --- a/src/drivers/i2c/generic/generic.c +++ b/src/drivers/i2c/generic/generic.c @@ -23,6 +23,7 @@ #include <stdint.h> #include <string.h> #include "chip.h" +#include "drivers/generic/power_resource/chip.h"
#if CONFIG(HAVE_ACPI_TABLES)
@@ -59,7 +60,13 @@ void (*callback)(struct device *dev), struct drivers_i2c_generic_config *config) { - const char *scope = acpi_device_scope(dev); + const int parent_is_power_resource = + (dev->bus && dev->bus->children == dev && + dev->bus->dev->chip_ops == &drivers_generic_power_resource_ops); + const char *pr_scope = parent_is_power_resource ? + acpi_device_scope(dev->bus->dev) : + NULL; + const char *scope = (pr_scope == NULL) ? acpi_device_scope(dev) : pr_scope; struct acpi_i2c i2c = { .address = dev->path.i2c.device, .mode_10bit = dev->path.i2c.mode_10bit, @@ -157,7 +164,11 @@ config->stop_delay_ms, config->stop_off_delay_ms }; - acpi_device_add_power_res(&power_res_params); + acpi_add_power_res("PRIC", &power_res_params); + acpi_add_power_states("PRIC"); + } else if (parent_is_power_resource) { + /* Parent device is a power resource, so set up _PR0/_PR3 */ + acpi_add_power_states(acpi_device_path(dev->bus->dev)); }
/* Callback if any. */ diff --git a/src/drivers/spi/acpi/acpi.c b/src/drivers/spi/acpi/acpi.c index 7c8a887..f7cb2e1 100644 --- a/src/drivers/spi/acpi/acpi.c +++ b/src/drivers/spi/acpi/acpi.c @@ -23,6 +23,7 @@ #include <stdint.h> #include <string.h> #include "chip.h" +#include "drivers/generic/power_resource/chip.h"
static int spi_acpi_get_bus(const struct device *dev) { @@ -73,13 +74,19 @@
static void spi_acpi_fill_ssdt_generator(struct device *dev) { + const int parent_is_power_resource = + (dev->bus && dev->bus->children == dev && + dev->bus->dev->chip_ops == &drivers_generic_power_resource_ops); + const char *pr_scope = parent_is_power_resource ? + acpi_device_scope(dev->bus->dev) : + NULL; struct drivers_spi_acpi_config *config = dev->chip_info; const char *scope = acpi_device_scope(dev); const char *path = acpi_device_path(dev); struct acpi_spi spi = { .device_select = dev->path.spi.cs, .speed = config->speed ? : 1 * MHz, - .resource = scope, + .resource = (pr_scope != NULL) ? pr_scope : scope, .device_select_polarity = SPI_POLARITY_LOW, .wire_mode = SPI_4_WIRE_MODE, .data_bit_length = 8, @@ -178,7 +185,11 @@ config->stop_delay_ms, config->stop_off_delay_ms }; - acpi_device_add_power_res(&power_res_params); + acpi_add_power_res("PRIC", &power_res_params); + acpi_add_power_states("PRIC"); + } else if (parent_is_power_resource) { + /* Parent device is a power resource, so set up _PR0/_PR3 */ + acpi_add_power_states(acpi_device_path(dev->bus->dev)); }
acpigen_pop_len(); /* Device */