On 13/02/2023 16:00, Vladimir Sementsov-Ogievskiy wrote:
> ENABLED -> PWRONLY transition is not allowed and we handle it by
> shpc_invalid_command(). But PWRONLY -> ENABLED transition is silently
> ignored, which seems wrong. Let's handle it as correct.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
> hw/pci/shpc.c | 24 +++++++++---------------
> 1 file changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
> index 5d71569b13..25e4172382 100644
> --- a/hw/pci/shpc.c
> +++ b/hw/pci/shpc.c
> @@ -273,28 +273,22 @@ static void shpc_slot_command(SHPCDevice *shpc, uint8_t target,
> return;
> }
>
> - switch (power) {
> - case SHPC_LED_NO:
> - break;
> - default:
> + if (power != SHPC_LED_NO) {
> /* TODO: send event to monitor */
> shpc_set_status(shpc, slot, power, SHPC_SLOT_PWR_LED_MASK);
> }
> - switch (attn) {
> - case SHPC_LED_NO:
> - break;
> - default:
> + if (attn != SHPC_LED_NO) {
> /* TODO: send event to monitor */
> shpc_set_status(shpc, slot, attn, SHPC_SLOT_ATTN_LED_MASK);
> }
> -
> - if ((current_state == SHPC_STATE_DISABLED && state == SHPC_STATE_PWRONLY) ||
> - (current_state == SHPC_STATE_DISABLED && state == SHPC_STATE_ENABLED)) {
> - shpc_set_status(shpc, slot, state, SHPC_SLOT_STATE_MASK);
> - } else if ((current_state == SHPC_STATE_ENABLED ||
> - current_state == SHPC_STATE_PWRONLY) &&
> - state == SHPC_STATE_DISABLED) {
> + if (state != SHPC_STATE_NO) {
> shpc_set_status(shpc, slot, state, SHPC_SLOT_STATE_MASK);
> + }
> +
> + if ((current_state == SHPC_STATE_ENABLED ||
> + current_state == SHPC_STATE_PWRONLY) &&
> + state == SHPC_STATE_DISABLED)
> + {
> power = shpc_get_status(shpc, slot, SHPC_SLOT_PWR_LED_MASK);
> /* TODO: track what monitor requested. */
> /* Look at LED to figure out whether it's ok to remove the device. */
Reviewed-by: Anton Kuchin <antonkuchin@yandex-team.ru>