diff options
| -rw-r--r-- | zephyr/subsys/ap_pwrseq/include/power_signals.h | 16 | ||||
| -rw-r--r-- | zephyr/subsys/ap_pwrseq/power_signals.c | 82 | ||||
| -rw-r--r-- | zephyr/subsys/ap_pwrseq/signal_adc.c | 6 | ||||
| -rw-r--r-- | zephyr/subsys/ap_pwrseq/signal_gpio.c | 7 | ||||
| -rw-r--r-- | zephyr/subsys/ap_pwrseq/signal_vw.c | 29 | ||||
| -rw-r--r-- | zephyr/subsys/ap_pwrseq/x86_non_dsx_common_pwrseq_sm_handler.c | 4 | ||||
| -rw-r--r-- | zephyr/test/ap_power/src/signals.c | 18 |
7 files changed, 107 insertions, 55 deletions
diff --git a/zephyr/subsys/ap_pwrseq/include/power_signals.h b/zephyr/subsys/ap_pwrseq/include/power_signals.h index 8c168077f7..cdeb9ec783 100644 --- a/zephyr/subsys/ap_pwrseq/include/power_signals.h +++ b/zephyr/subsys/ap_pwrseq/include/power_signals.h @@ -203,20 +203,22 @@ void power_signal_init(void); * @brief Power signal interrupt handler * * Called when an input signal causes an interrupt. + * + * @param signal The power_signal that has changed. + * @param value The new value of the signal */ -void power_signal_interrupt(void); - -typedef uint32_t power_signal_mask_t; +void power_signal_interrupt(enum power_signal signal, int value); /** - * @brief Update the stored mask of power signals. + * Interrupt based signals update a bitfield mask, which can be + * used to wait for signal changes. */ -void power_update_signals(void); +typedef uint32_t power_signal_mask_t; /** - * @brief Get the current power signals as a mask. + * @brief Get the current interrupt bitfield * - * @return Power signals as a mask. + * @return Interrupt power signals as a mask. */ power_signal_mask_t power_get_signals(void); diff --git a/zephyr/subsys/ap_pwrseq/power_signals.c b/zephyr/subsys/ap_pwrseq/power_signals.c index 30961acd38..bbcd7ebcae 100644 --- a/zephyr/subsys/ap_pwrseq/power_signals.c +++ b/zephyr/subsys/ap_pwrseq/power_signals.c @@ -6,6 +6,7 @@ #include <kernel.h> #include <toolchain.h> #include <logging/log.h> +#include <sys/atomic.h> #include <power_signals.h> @@ -75,25 +76,22 @@ DT_FOREACH_STATUS_OKAY_VARGS(intel_ap_pwrseq_adc, GEN_PS_ENTRY, PWR_SIG_SRC_ADC, PWR_SIG_TAG_ADC) }; -static power_signal_mask_t power_signals; -static power_signal_mask_t debug_signals; +#define PWR_SIGNAL_POLLED(id) PWR_SIGNAL_ENUM(id), -void power_update_signals(void) -{ - power_signal_mask_t n = 0; +/* + * List of power signals that need to be polled. + */ +static const uint8_t polled_signals[] = { +DT_FOREACH_STATUS_OKAY(intel_ap_pwrseq_external, PWR_SIGNAL_POLLED) +}; - for (int i = 0; i < POWER_SIGNAL_COUNT; i++) { - if (power_signal_get(i)) { - n |= BIT(i); - } - } - /* Check if any signals flagged for debug have changed. */ - if ((n ^ power_signals) & debug_signals) { - LOG_INF("power update (0x%04x -> 0x%04x, 0x%04x changed)", - power_signals, n, n ^ power_signals); - } - power_signals = n; -} +/* + * Bitmask of power signals updated via interrupt. + */ +static atomic_t interrupt_power_signals; + +static power_signal_mask_t output_signals; +static power_signal_mask_t debug_signals; void power_set_debug(power_signal_mask_t debug) { @@ -105,14 +103,32 @@ power_signal_mask_t power_get_debug(void) return debug_signals; } +static inline void check_debug(power_signal_mask_t mask, + enum power_signal signal, + int value) +{ + if (debug_signals & mask) { + LOG_INF("%s -> %d", power_signal_name(signal), value); + } +} + power_signal_mask_t power_get_signals(void) { - return power_signals; + power_signal_mask_t mask = 0; + + for (int i = 0; i < ARRAY_SIZE(polled_signals); i++) { + if (power_signal_get(polled_signals[i])) { + mask |= POWER_SIGNAL_MASK(polled_signals[i]); + } + } + return mask | output_signals | + atomic_get(&interrupt_power_signals); } -void power_signal_interrupt(void) +void power_signal_interrupt(enum power_signal signal, int value) { - power_update_signals(); + atomic_set_bit_to(&interrupt_power_signals, signal, value); + check_debug(POWER_SIGNAL_MASK(signal), signal, value); } int power_wait_mask_signals_timeout(power_signal_mask_t mask, @@ -124,12 +140,11 @@ int power_wait_mask_signals_timeout(power_signal_mask_t mask, } want &= mask; while (timeout-- > 0) { - if ((power_signals & mask) == want) { + if ((power_get_signals() & mask) == want) { return 0; } k_msleep(1); } - power_update_signals(); return -ETIMEDOUT; } @@ -166,6 +181,7 @@ int power_signal_get(enum power_signal signal) int power_signal_set(enum power_signal signal, int value) { const struct ps_config *cp = &sig_config[signal]; + int ret; LOG_DBG("Set %s to %d", power_signal_name(signal), value); switch (cp->source) { @@ -174,14 +190,32 @@ int power_signal_set(enum power_signal signal, int value) #if HAS_GPIO_SIGNALS case PWR_SIG_SRC_GPIO: - return power_signal_gpio_set(cp->src_enum, value); + ret = power_signal_gpio_set(cp->src_enum, value); + break; #endif #if HAS_EXT_SIGNALS case PWR_SIG_SRC_EXT: - return board_power_signal_set(signal, value); + ret = board_power_signal_set(signal, value); + break; #endif } + /* + * Output succeeded, update output mask. + */ + if (ret == 0) { + power_signal_mask_t mask = POWER_SIGNAL_MASK(signal); + power_signal_mask_t old = output_signals; + + if (value) + output_signals |= mask; + else + output_signals &= ~mask; + if (old != output_signals) { + check_debug(mask, signal, value); + } + } + return ret; } int power_signal_enable_interrupt(enum power_signal signal) diff --git a/zephyr/subsys/ap_pwrseq/signal_adc.c b/zephyr/subsys/ap_pwrseq/signal_adc.c index 39f1d51a15..9f07b73964 100644 --- a/zephyr/subsys/ap_pwrseq/signal_adc.c +++ b/zephyr/subsys/ap_pwrseq/signal_adc.c @@ -19,12 +19,14 @@ LOG_MODULE_DECLARE(ap_pwrseq, CONFIG_AP_PWRSEQ_LOG_LEVEL); struct adc_config { const struct device *dev_trig_high; const struct device *dev_trig_low; + enum power_signal signal; }; #define INIT_ADC_CONFIG(id) \ { \ .dev_trig_high = DEVICE_DT_GET(DT_PHANDLE(id, trigger_high)), \ .dev_trig_low = DEVICE_DT_GET(DT_PHANDLE(id, trigger_low)), \ + .signal = PWR_SIGNAL_ENUM(id), \ }, static const struct adc_config config[] = { @@ -49,7 +51,7 @@ static void trigger_high(enum pwr_sig_adc adc) SENSOR_ATTR_ALERT, &val); LOG_DBG("power signal adc%d is HIGH", adc); - power_update_signals(); + power_signal_interrupt(config[adc].signal, 1); } static void trigger_low(enum pwr_sig_adc adc) @@ -68,7 +70,7 @@ static void trigger_low(enum pwr_sig_adc adc) SENSOR_ATTR_ALERT, &val); LOG_DBG("power signal adc%d is LOW", adc); - power_update_signals(); + power_signal_interrupt(config[adc].signal, 0); } int power_signal_adc_get(enum pwr_sig_adc adc) diff --git a/zephyr/subsys/ap_pwrseq/signal_gpio.c b/zephyr/subsys/ap_pwrseq/signal_gpio.c index a130bc8abf..c2df5e56c5 100644 --- a/zephyr/subsys/ap_pwrseq/signal_gpio.c +++ b/zephyr/subsys/ap_pwrseq/signal_gpio.c @@ -23,6 +23,7 @@ DT_FOREACH_STATUS_OKAY(MY_COMPAT, INIT_GPIO_SPEC) */ struct ps_gpio_int { gpio_flags_t flags; + uint8_t signal; unsigned output : 1; unsigned no_enable : 1; }; @@ -30,6 +31,7 @@ struct ps_gpio_int { #define INIT_GPIO_CONFIG(id) \ { \ .flags = DT_PROP_OR(id, interrupt_flags, 0), \ + .signal = PWR_SIGNAL_ENUM(id), \ .no_enable = DT_PROP(id, no_enable), \ .output = DT_PROP(id, output), \ }, @@ -89,7 +91,10 @@ void power_signal_gpio_interrupt(const struct device *port, struct gpio_callback *cb, gpio_port_pins_t pins) { - power_signal_interrupt(); + int index = cb - int_cb; + + power_signal_interrupt(gpio_config[index].signal, + gpio_pin_get_dt(&spec[index])); } int power_signal_gpio_get(enum pwr_sig_gpio index) diff --git a/zephyr/subsys/ap_pwrseq/signal_vw.c b/zephyr/subsys/ap_pwrseq/signal_vw.c index 6177637eba..b0abe4fe13 100644 --- a/zephyr/subsys/ap_pwrseq/signal_vw.c +++ b/zephyr/subsys/ap_pwrseq/signal_vw.c @@ -17,6 +17,7 @@ LOG_MODULE_DECLARE(ap_pwrseq, CONFIG_AP_PWRSEQ_LOG_LEVEL); #define INIT_ESPI_SIGNAL(id) \ { \ .espi_signal = DT_STRING_UPPER_TOKEN(id, virtual_wire), \ + .signal = PWR_SIGNAL_ENUM(id), \ .invert = DT_PROP(id, vw_invert), \ }, @@ -25,6 +26,7 @@ LOG_MODULE_DECLARE(ap_pwrseq, CONFIG_AP_PWRSEQ_LOG_LEVEL); */ struct vw_config { uint8_t espi_signal; /* associated VW signal */ + uint8_t signal; /* power signal */ bool invert; /* Invert the signal value */ }; @@ -41,7 +43,8 @@ static bool signal_data[ARRAY_SIZE(vw_config)]; * and it is only when all the signals have been updated that * notification is sent that the signals are ready. */ -static uint8_t espi_ready; +static uint8_t espi_mask; +static bool espi_not_valid; BUILD_ASSERT(ARRAY_SIZE(vw_config) <= 8); static void espi_handler(const struct device *dev, @@ -62,21 +65,32 @@ static void espi_handler(const struct device *dev, * the signal mask. */ notify_espi_ready(false); - espi_ready = 0; + espi_mask = 0; + espi_not_valid = true; break; case ESPI_BUS_EVENT_VWIRE_RECEIVED: for (int i = 0; i < ARRAY_SIZE(vw_config); i++) { if (event.evt_details == vw_config[i].espi_signal) { - signal_data[i] = !!event.evt_data; - espi_ready |= BIT(i); + int value = vw_config[i].invert + ? !event.evt_data + : !!event.evt_data; + + signal_data[i] = value; + if (espi_not_valid) { + espi_mask |= BIT(i); + } + power_signal_interrupt(vw_config[i].signal, + value); } } /* * When all the signals have been updated, notify that * the ESPI signals are valid. */ - if (espi_ready == BIT_MASK(ARRAY_SIZE(vw_config))) { + if (espi_not_valid && + espi_mask == BIT_MASK(ARRAY_SIZE(vw_config))) { + espi_not_valid = false; LOG_DBG("ESPI signals valid"); /* * TODO(b/222946923): Convert to generalised @@ -90,13 +104,10 @@ static void espi_handler(const struct device *dev, int power_signal_vw_get(enum pwr_sig_vw vw) { - int value; - if (vw < 0 || vw >= ARRAY_SIZE(vw_config)) { return -EINVAL; } - value = signal_data[vw]; - return vw_config[vw].invert ? !value : value; + return signal_data[vw]; } void power_signal_vw_init(void) diff --git a/zephyr/subsys/ap_pwrseq/x86_non_dsx_common_pwrseq_sm_handler.c b/zephyr/subsys/ap_pwrseq/x86_non_dsx_common_pwrseq_sm_handler.c index 47f22f73dc..d712bb008a 100644 --- a/zephyr/subsys/ap_pwrseq/x86_non_dsx_common_pwrseq_sm_handler.c +++ b/zephyr/subsys/ap_pwrseq/x86_non_dsx_common_pwrseq_sm_handler.c @@ -41,9 +41,6 @@ const char pwrsm_dbg[][25] = { void notify_espi_ready(bool ready) { pwrseq_ctx.espi_ready = ready; - if (ready) { - power_update_signals(); - } } #endif @@ -344,6 +341,7 @@ static void pwrseq_loop_thread(void *p1, void *p2, void *p3) * comes back by the time we update our signals. */ this_in_signals = power_get_signals(); + if (this_in_signals != last_in_signals || curr_state != last_state) { LOG_INF("power state %d = %s, in 0x%04x", diff --git a/zephyr/test/ap_power/src/signals.c b/zephyr/test/ap_power/src/signals.c index 0498a7a6da..8bbf88c2a9 100644 --- a/zephyr/test/ap_power/src/signals.c +++ b/zephyr/test/ap_power/src/signals.c @@ -262,9 +262,16 @@ ZTEST(signals, test_gpio_output) ZTEST(signals, test_signal_mask) { power_signal_mask_t vm = POWER_SIGNAL_MASK(PWR_IMVP9_VRRDY); + power_signal_mask_t bm = POWER_SIGNAL_MASK(PWR_ALL_SYS_PWRGD); power_signal_mask_t m; /* + * Set board level (polled) signal. + */ + power_signal_set(PWR_ALL_SYS_PWRGD, 1); + zassert_equal(bm, (power_get_signals() & bm), + "Expected PWR_ALL_SYS_PWRGD signal to be present in mask"); + /* * Use GPIO that does not interrupt to confirm that a pin change * will not update the power signal mask. */ @@ -273,16 +280,9 @@ ZTEST(signals, test_signal_mask) zassert_equal(0, (power_get_signals() & vm), "Expected mask to be 0"); emul_set(PWR_IMVP9_VRRDY, 1); zassert_equal(0, (power_get_signals() & vm), "Expected mask to be 0"); - /* - * Calling power_update_signals will read all the signals and - * incorporate them into the mask, even the non-interrupt signals. - */ - power_update_signals(); - zassert_equal(vm, (power_get_signals() & vm), - "Expected signals to be present in mask"); - zassert_equal(true, power_signals_match(vm, vm), + zassert_equal(true, power_signals_match(bm, bm), "Expected match of mask to signal match"); - zassert_equal(-ETIMEDOUT, power_wait_mask_signals_timeout(vm, 0, 5), + zassert_equal(-ETIMEDOUT, power_wait_mask_signals_timeout(bm, 0, 5), "Expected timeout waiting for mask to be 0"); zassert_ok(power_wait_mask_signals_timeout(0, vm, 5), "expected match with a 0 mask (always true)"); |