coreboot-gerrit March 2020

coreboot-gerrit@coreboot.org

1 participants
3326 discussions

Change in coreboot[master]: drivers/spi/acpi: Support power resource as parent in device tree
by Tim Wawrzynczak (Code Review) 11 Mar '20

11 Mar '20

Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35076 ) Change subject: drivers/spi/acpi: Support power resource as parent in device tree ...................................................................... drivers/spi/acpi: Support power resource as parent in device tree This allows supporting an independent power resource for SPI ACPI devices, so that the power resource configuration can be refactored out of individual drivers and coalesced into a separate one. Change-Id: Ib1e52a0f31d430637c9d819e231f9c4af6c68672 Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/drivers/spi/acpi/acpi.c 1 file changed, 16 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/35076/1 diff --git a/src/drivers/spi/acpi/acpi.c b/src/drivers/spi/acpi/acpi.c index 5ae010f..07f1a4b 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,8 +74,19 @@ static void spi_acpi_fill_ssdt_generator(struct device *dev) { + const int parent_is_power_resource = + CONFIG(DRIVERS_GENERIC_POWER_RESOURCE) && + (dev->bus && dev->bus->children == dev && + dev->bus->dev->chip_ops == &drivers_generic_power_resource_ops); + /* + * Because the power resource is a "pseudo-device", we don't want it + * to be included in the ACPI path + */ + 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 *scope = (pr_scope == NULL) ? acpi_device_scope(dev) : pr_scope; const char *path = acpi_device_path(dev); struct acpi_spi spi = { .device_select = dev->path.spi.cs, @@ -184,6 +196,9 @@ acpi_device_path(dev)); acpi_add_device_power_resource("PRIC"); + } else if (parent_is_power_resource) { + /* Parent device is a power resource, so set up _PR0/_PR3 */ + acpi_add_device_power_resource(acpi_device_path(dev->bus->dev)); } acpigen_pop_len(); /* Device */ -- To view, visit https://review.coreboot.org/c/coreboot/+/35076 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib1e52a0f31d430637c9d819e231f9c4af6c68672 Gerrit-Change-Number: 35076 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newchange

1 3

Change in coreboot[master]: drivers/i2c/generic: Support power resource as parent in device tree
by Tim Wawrzynczak (Code Review) 11 Mar '20

11 Mar '20

Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35075 ) Change subject: drivers/i2c/generic: Support power resource as parent in device tree ...................................................................... drivers/i2c/generic: Support power resource as parent in device tree This allows supporting an independent power resource for I2C generic devices, so that the power resource configuration can be refactored out of individual drivers and coalesced into a separate one. Change-Id: I7554ab4fef88ffea0d3423ea27bb18a56b7f4de5 Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/drivers/i2c/generic/generic.c 1 file changed, 16 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/35075/1 diff --git a/src/drivers/i2c/generic/generic.c b/src/drivers/i2c/generic/generic.c index 27ff9ff..a3411b0 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,18 @@ void (*callback)(struct device *dev), struct drivers_i2c_generic_config *config) { - const char *scope = acpi_device_scope(dev); + const int parent_is_power_resource = + CONFIG(DRIVERS_GENERIC_POWER_RESOURCE) && + (dev->bus && dev->bus->children == dev && + dev->bus->dev->chip_ops == &drivers_generic_power_resource_ops); + /* + * Because the power resource is a "pseudo-device", we don't want it + * to be included in the ACPI path + */ + 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, @@ -163,7 +175,9 @@ acpi_device_path(dev)); acpi_add_device_power_resource("PRIC"); - + } else if (parent_is_power_resource) { + /* Parent device is a power resource, so set up _PR0/_PR3 */ + acpi_add_device_power_resource(acpi_device_path(dev->bus->dev)); } /* Callback if any. */ -- To view, visit https://review.coreboot.org/c/coreboot/+/35075 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7554ab4fef88ffea0d3423ea27bb18a56b7f4de5 Gerrit-Change-Number: 35075 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newchange

2 6

Change in coreboot[master]: device: Allow scan_generic_bus to recurse on pseudo-devices
by Tim Wawrzynczak (Code Review) 11 Mar '20

11 Mar '20

Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35074 ) Change subject: device: Allow scan_generic_bus to recurse on pseudo-devices ...................................................................... device: Allow scan_generic_bus to recurse on pseudo-devices To handle cases of pseudo-devices containing a child device, e.g., power resources, recurse onto them, treating them like a bus. Change-Id: Ia8442939ef1e840c112ad4dd23e324afd99ff7d3 Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/device/root_device.c 1 file changed, 7 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/35074/1 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); } } -- To view, visit https://review.coreboot.org/c/coreboot/+/35074 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia8442939ef1e840c112ad4dd23e324afd99ff7d3 Gerrit-Change-Number: 35074 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newchange

4 10

Change in coreboot[master]: drivers/generic: Add new driver for power resource pseudo-device
by Tim Wawrzynczak (Code Review) 11 Mar '20

11 Mar '20

Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35073 ) 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). Change-Id: Id9d66fedd342eb5359d90d9778c515e7aa2de1d7 Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- A src/drivers/generic/power_resource/Kconfig A src/drivers/generic/power_resource/Makefile.inc A src/drivers/generic/power_resource/chip.h A src/drivers/generic/power_resource/power_resource.c 4 files changed, 128 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/35073/1 diff --git a/src/drivers/generic/power_resource/Kconfig b/src/drivers/generic/power_resource/Kconfig new file mode 100644 index 0000000..447edb9 --- /dev/null +++ b/src/drivers/generic/power_resource/Kconfig @@ -0,0 +1,5 @@ +# TODO: once everything is moved to the new power resource structure, +# those drivers that use it should auto-select this config option +config DRIVERS_GENERIC_POWER_RESOURCE + bool + depends on HAVE_ACPI_TABLES diff --git a/src/drivers/generic/power_resource/Makefile.inc b/src/drivers/generic/power_resource/Makefile.inc new file mode 100644 index 0000000..2120ef1 --- /dev/null +++ b/src/drivers/generic/power_resource/Makefile.inc @@ -0,0 +1 @@ +ramstage-$(CONFIG_DRIVERS_GENERIC_POWER_RESOURCE) += power_resource.c diff --git a/src/drivers/generic/power_resource/chip.h b/src/drivers/generic/power_resource/chip.h new file mode 100644 index 0000000..e698296 --- /dev/null +++ b/src/drivers/generic/power_resource/chip.h @@ -0,0 +1,47 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2019 Google LLC. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef __GENERIC_POWER_RESOURCE_CHIP_H__ +#define __GENERIC_POWER_RESOURCE_CHIP_H__ + +#include <device/device.h> +#include <arch/acpi_device.h> + +extern struct chip_operations drivers_generic_power_resource_ops; + +struct drivers_generic_power_resource_config { + /* GPIO used to take device out of reset or to put it into reset. */ + struct acpi_gpio reset_gpio; + /* Delay to be inserted after device is taken out of reset. */ + unsigned int reset_delay_ms; + /* Delay to be inserted after device is put into reset. */ + unsigned int reset_off_delay_ms; + /* GPIO used to enable device. */ + struct acpi_gpio enable_gpio; + /* Delay to be inserted after device is enabled. */ + unsigned int enable_delay_ms; + /* Delay to be inserted after device is disabled. */ + unsigned int enable_off_delay_ms; + /* GPIO used to stop operation of device. */ + struct acpi_gpio stop_gpio; + /* Delay to be inserted after disabling stop. */ + unsigned int stop_delay_ms; + /* Delay to be inserted after enabling stop. */ + unsigned int stop_off_delay_ms; + /* ACPI device name */ + char acpi_name[5]; +}; + +#endif /* __GENERIC_POWER_RESOURCE_CHIP_H__ */ diff --git a/src/drivers/generic/power_resource/power_resource.c b/src/drivers/generic/power_resource/power_resource.c new file mode 100644 index 0000000..d08c1a2 --- /dev/null +++ b/src/drivers/generic/power_resource/power_resource.c @@ -0,0 +1,75 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2019 Google LLC. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <device/device.h> +#include <device/path.h> +#include <console/console.h> +#include <string.h> +#include "chip.h" + +static void power_resource_fill_ssdt(struct device *dev) +{ + bool ret; + struct drivers_generic_power_resource_config *config = dev->chip_info; + const struct acpi_power_res_params power_res_params = { + &config->reset_gpio, + config->reset_delay_ms, + config->reset_off_delay_ms, + &config->enable_gpio, + config->enable_delay_ms, + config->enable_off_delay_ms, + &config->stop_gpio, + config->stop_delay_ms, + config->stop_off_delay_ms + }; + + ret = acpi_declare_scoped_power_resource(acpi_device_scope(dev), + acpi_device_name(dev), + &power_res_params); + if (!ret) + printk(BIOS_ERR, "Failed to add power resource %s\n", + acpi_device_name(dev)); + +} + +static const char *power_resource_acpi_name(const struct device *dev) +{ + static unsigned int count; + struct drivers_generic_power_resource_config *config = dev->chip_info; + + if (config->acpi_name[0] == '\0') + snprintf(config->acpi_name, sizeof(config->acpi_name), "PR%02X", count++); + + return config->acpi_name; +} + +static struct device_operations power_resource_ops = { + .read_resources = DEVICE_NOOP, + .set_resources = DEVICE_NOOP, + .enable_resources = DEVICE_NOOP, + .scan_bus = scan_generic_bus, + .acpi_name = power_resource_acpi_name, + .acpi_fill_ssdt_generator = power_resource_fill_ssdt +}; + +static void power_resource_enable(struct device *dev) +{ + dev->ops = &power_resource_ops; +} + +struct chip_operations drivers_generic_power_resource_ops = { + CHIP_NAME("Power Resource") + .enable_dev = power_resource_enable +}; -- To view, visit https://review.coreboot.org/c/coreboot/+/35073 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id9d66fedd342eb5359d90d9778c515e7aa2de1d7 Gerrit-Change-Number: 35073 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newchange

3 10

Change in coreboot[master]: drivers/generic: Add new driver for power resource pseudo device
by Tim Wawrzynczak (Code Review) 11 Mar '20

11 Mar '20

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(a)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 */ -- To view, visit https://review.coreboot.org/c/coreboot/+/35036 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I358c48f4d4e4c4ef591ecf09ea9e9488c2652c96 Gerrit-Change-Number: 35036 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newchange

3 16

Change in coreboot[master]: nb/intel/i945: Programm CxODT value for each channel
by HAOUAS Elyes (Code Review) 11 Mar '20

11 Mar '20

Hello build bot (Jenkins), Nico Huber, Paul Menzel, Angel Pons, Arthur Heymans, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/18548 to look at the new patch set (#28). Change subject: nb/intel/i945: Programm CxODT value for each channel ...................................................................... nb/intel/i945: Programm CxODT value for each channel There is no raison to programm both C0ODT and C1ODT with the same value when for channel 0 and 1 are not eually populated. Change-Id: I7aec35f45250da554ddc5a68f5add157c313399c Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr> --- M src/northbridge/intel/i945/raminit.c 1 file changed, 8 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/18548/28 -- To view, visit https://review.coreboot.org/c/coreboot/+/18548 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7aec35f45250da554ddc5a68f5add157c313399c Gerrit-Change-Number: 18548 Gerrit-PatchSet: 28 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: nb/intel/i945: Programm CxODT value for each channel
by HAOUAS Elyes (Code Review) 11 Mar '20

11 Mar '20

Hello build bot (Jenkins), Nico Huber, Paul Menzel, Angel Pons, Arthur Heymans, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/18548 to look at the new patch set (#27). Change subject: nb/intel/i945: Programm CxODT value for each channel ...................................................................... nb/intel/i945: Programm CxODT value for each channel There is no raison to programm both C0ODT and C1ODT with the same value when for channel 0 and 1 are not eually populated. Change-Id: I7aec35f45250da554ddc5a68f5add157c313399c Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr> --- M src/northbridge/intel/i945/raminit.c 1 file changed, 8 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/18548/27 -- To view, visit https://review.coreboot.org/c/coreboot/+/18548 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7aec35f45250da554ddc5a68f5add157c313399c Gerrit-Change-Number: 18548 Gerrit-PatchSet: 27 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/google/dedede: Add ACPI configuration for USB ports
by Karthik Ramasubramanian (Code Review) 11 Mar '20

11 Mar '20

Karthik Ramasubramanian has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39422 ) Change subject: mb/google/dedede: Add ACPI configuration for USB ports ...................................................................... mb/google/dedede: Add ACPI configuration for USB ports Enable USB ACPI driver. Add ACPI configuration for all the USB ports. Since one of the USB ports is used for Bluetooth configure the reset_gpio used by that port. TEST=Build the mainboard. Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> Change-Id: I3e7b8f00102c96dcc295601359d3ecfbcd1bea00 --- M src/mainboard/google/dedede/Kconfig M src/mainboard/google/dedede/variants/baseboard/devicetree.cb M src/mainboard/google/dedede/variants/baseboard/gpio.c 3 files changed, 89 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/39422/1 diff --git a/src/mainboard/google/dedede/Kconfig b/src/mainboard/google/dedede/Kconfig index 2606e57..36b42bf 100644 --- a/src/mainboard/google/dedede/Kconfig +++ b/src/mainboard/google/dedede/Kconfig @@ -3,6 +3,7 @@ select DRIVERS_I2C_GENERIC select DRIVERS_I2C_HID select DRIVERS_SPI_ACPI + select DRIVERS_USB_ACPI select EC_GOOGLE_CHROMEEC select EC_GOOGLE_CHROMEEC_BOARDID select EC_GOOGLE_CHROMEEC_ESPI diff --git a/src/mainboard/google/dedede/variants/baseboard/devicetree.cb b/src/mainboard/google/dedede/variants/baseboard/devicetree.cb index 0efb76d..c232538 100644 --- a/src/mainboard/google/dedede/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/dedede/variants/baseboard/devicetree.cb @@ -158,7 +158,93 @@ device pci 05.0 off end # IPU device pci 09.0 off end # Intel Trace Hub device pci 12.6 off end # GSPI 2 - device pci 14.0 on end # USB xHCI + device pci 14.0 on + chip drivers/usb/acpi + register "desc" = ""Root Hub"" + register "type" = "UPC_TYPE_HUB" + device usb 0.0 on + chip drivers/usb/acpi + register "desc" = ""Left Type-C Port"" + register "type" = "UPC_TYPE_C_USB2_SS_SWITCH" + register "group" = "ACPI_PLD_GROUP(1, 1)" + device usb 2.0 on end + end + chip drivers/usb/acpi + register "desc" = ""Right Type-C Port"" + register "type" = "UPC_TYPE_C_USB2_SS_SWITCH" + register "group" = "ACPI_PLD_GROUP(2, 1)" + device usb 2.1 on end + end + chip drivers/usb/acpi + register "desc" = ""Left Type-A Port"" + register "type" = "UPC_TYPE_A" + register "group" = "ACPI_PLD_GROUP(1, 2)" + device usb 2.2 on end + end + chip drivers/usb/acpi + register "desc" = ""Right Type-A Port"" + register "type" = "UPC_TYPE_A" + register "group" = "ACPI_PLD_GROUP(2, 2)" + device usb 2.3 on end + end + chip drivers/usb/acpi + register "desc" = ""Bluetooth"" + register "type" = "UPC_TYPE_INTERNAL" + register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_H19)" + device usb 2.4 on end + end + chip drivers/usb/acpi + register "desc" = ""USB2 Port Unused 1"" + register "type" = "UPC_TYPE_UNUSED" + device usb 2.5 on end + end + chip drivers/usb/acpi + register "desc" = ""USB2 Port Unused 2"" + register "type" = "UPC_TYPE_UNUSED" + device usb 2.6 on end + end + chip drivers/usb/acpi + register "desc" = ""USB2 Port Unused 3"" + register "type" = "UPC_TYPE_UNUSED" + device usb 2.7 on end + end + chip drivers/usb/acpi + register "desc" = ""Left Type-C Port"" + register "type" = "UPC_TYPE_C_USB2_SS_SWITCH" + register "group" = "ACPI_PLD_GROUP(1, 1)" + device usb 3.0 on end + end + chip drivers/usb/acpi + register "desc" = ""Right Type-C Port"" + register "type" = "UPC_TYPE_C_USB2_SS_SWITCH" + register "group" = "ACPI_PLD_GROUP(2, 1)" + device usb 3.1 on end + end + chip drivers/usb/acpi + register "desc" = ""Left Type-A Port"" + register "type" = "UPC_TYPE_USB3_A" + register "group" = "ACPI_PLD_GROUP(1, 2)" + device usb 3.2 on end + end + chip drivers/usb/acpi + register "desc" = ""Right Type-A Port"" + register "type" = "UPC_TYPE_USB3_A" + register "group" = "ACPI_PLD_GROUP(2, 2)" + device usb 3.3 on end + end + chip drivers/usb/acpi + register "desc" = ""USB3 Port Unused 1"" + register "type" = "UPC_TYPE_UNUSED" + device usb 3.4 on end + end + chip drivers/usb/acpi + register "desc" = ""USB3 Port Unused 2"" + register "type" = "UPC_TYPE_UNUSED" + device usb 3.5 on end + end + end + end + end # USB xHCI device pci 14.1 off end # USB xDCI (OTG) device pci 14.2 off end # PMC SRAM device pci 14.3 off end # CNVi wifi diff --git a/src/mainboard/google/dedede/variants/baseboard/gpio.c b/src/mainboard/google/dedede/variants/baseboard/gpio.c index af95f6a..49af7fe 100644 --- a/src/mainboard/google/dedede/variants/baseboard/gpio.c +++ b/src/mainboard/google/dedede/variants/baseboard/gpio.c @@ -328,7 +328,7 @@ /* H18 : WLAN_DISABLE_L */ PAD_NC(GPP_H18, NONE), /* H19 : BT_DISABLE_L */ - PAD_NC(GPP_H19, NONE), + PAD_CFG_GPO(GPP_H19, 1, DEEP), /* R0 : I2S_HP_BCLK */ PAD_NC(GPP_R0, NONE), -- To view, visit https://review.coreboot.org/c/coreboot/+/39422 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3e7b8f00102c96dcc295601359d3ecfbcd1bea00 Gerrit-Change-Number: 39422 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: newchange

6 11

Change in coreboot[master]: mb/intel/tglrvp: Enable Hybride storage mode
by Wonkyu Kim (Code Review) 11 Mar '20

11 Mar '20

Wonkyu Kim has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39233 ) Change subject: mb/intel/tglrvp: Enable Hybride storage mode ...................................................................... mb/intel/tglrvp: Enable Hybride storage mode BUG=b:148604250 BRANCH=none TEST=Build and test booting TGLRVP form NVMe and Optane Check Pcie lane configuration Show all the NVMe devices lspci -d ::0108 Show all the NVMe devices and be really verbose lspci -vvvd ::0108 Print PCIe lane capabilities and configurations for all the NVMe devices. lspci -vvvd ::0108 | grep -e x[124] Print all the PCIe information of the device ae:00.0 lspci -vvvs ae: Signed-off-by: Wonkyu Kim <wonkyu.kim(a)intel.com> Change-Id: I5fc8fa0897ad006de9ebe20115bf3033e1e1b499 --- M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/39233/1 diff --git a/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb b/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb index d4b5a39..0a1e6f6 100644 --- a/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb +++ b/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb @@ -37,6 +37,9 @@ register "PcieRpEnable[8]" = "1" register "PcieRpEnable[10]" = "1" + # Hybride storage mode + register "HybridStorageMode" = "1" + register "PcieClkSrcClkReq[1]" = "1" register "PcieClkSrcClkReq[2]" = "2" register "PcieClkSrcClkReq[3]" = "3" -- To view, visit https://review.coreboot.org/c/coreboot/+/39233 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5fc8fa0897ad006de9ebe20115bf3033e1e1b499 Gerrit-Change-Number: 39233 Gerrit-PatchSet: 1 Gerrit-Owner: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-MessageType: newchange

7 21

Change in coreboot[master]: nb/intel/i945: Programm CxODT value for each channel
by Paul Menzel (Code Review) 11 Mar '20

11 Mar '20

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/18548 ) Change subject: nb/intel/i945: Programm CxODT value for each channel ...................................................................... Patch Set 26: (2 comments) https://review.coreboot.org/c/coreboot/+/18548/26//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/18548/26//COMMIT_MSG@10 PS26, Line 10: > Done The commit message should be more elaborate. https://review.coreboot.org/c/coreboot/+/18548/26/src/northbridge/intel/i94… File src/northbridge/intel/i945/raminit.c: https://review.coreboot.org/c/coreboot/+/18548/26/src/northbridge/intel/i94… PS26, Line 2445: if (!(sysinfo->dimm[0] != SYSINFO_DIMM_NOT_POPULATED && > From Nico's comment in PS22: It can be written even clearer by pulling the outer ! in: […] Could be a clean-up patch before this one. -- To view, visit https://review.coreboot.org/c/coreboot/+/18548 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7aec35f45250da554ddc5a68f5add157c313399c Gerrit-Change-Number: 18548 Gerrit-PatchSet: 26 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Wed, 11 Mar 2020 19:09:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Georgi <pgeorgi(a)google.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net> Comment-In-Reply-To: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2020