| Message ID | 20230213140103.1518173-4-vsementsov@yandex-team.ru |
|---|---|
| State | New |
| Headers | show |
| Series | pci hotplug tracking | expand |
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>
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. */
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(-)