| Message ID | 20230213140103.1518173-2-vsementsov@yandex-team.ru |
|---|---|
| State | New |
| Headers | show |
| Series | pci hotplug tracking | expand |
On 13/2/23 15:00, Vladimir Sementsov-Ogievskiy wrote: > 0 is not a valid state for the led. Let's start with OFF. "0 is not valid" so we should abort(value != 0) in shpc_set_status(), > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > --- > hw/pci/shpc.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c > index fca7f6691a..1b3f619dc9 100644 > --- a/hw/pci/shpc.c > +++ b/hw/pci/shpc.c > @@ -223,6 +223,7 @@ void shpc_reset(PCIDevice *d) > SHPC_SLOT_STATUS_PRSNT_MASK); > shpc_set_status(shpc, i, SHPC_LED_OFF, SHPC_SLOT_PWR_LED_MASK); > } > + shpc_set_status(shpc, i, SHPC_LED_OFF, SHPC_SLOT_ATTN_LED_MASK); > shpc_set_status(shpc, i, 0, SHPC_SLOT_STATUS_66); ... however value=0 is used: hw/pci/shpc.c:215: shpc_set_status(shpc, i, 0, SHPC_SLOT_STATUS_MRL_OPEN); hw/pci/shpc.c:226: shpc_set_status(shpc, i, 0, SHPC_SLOT_STATUS_66); hw/pci/shpc.c:522: shpc_set_status(shpc, slot, 0, SHPC_SLOT_STATUS_MRL_OPEN); hw/pci/shpc.c:531: shpc_set_status(shpc, slot, 0, SHPC_SLOT_STATUS_MRL_OPEN); hw/pci/shpc.c:543: shpc_set_status(shpc, slot, 0, SHPC_SLOT_STATUS_66); hw/pci/shpc.c:589: shpc_set_status(shpc, slot, 0, SHPC_SLOT_STATUS_66); Is this fixable?
On 13.02.23 17:16, Philippe Mathieu-Daudé wrote: > On 13/2/23 15:00, Vladimir Sementsov-Ogievskiy wrote: >> 0 is not a valid state for the led. Let's start with OFF. > > "0 is not valid" so we should abort(value != 0) in shpc_set_status(), > >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> >> --- >> hw/pci/shpc.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c >> index fca7f6691a..1b3f619dc9 100644 >> --- a/hw/pci/shpc.c >> +++ b/hw/pci/shpc.c >> @@ -223,6 +223,7 @@ void shpc_reset(PCIDevice *d) >> SHPC_SLOT_STATUS_PRSNT_MASK); >> shpc_set_status(shpc, i, SHPC_LED_OFF, SHPC_SLOT_PWR_LED_MASK); >> } >> + shpc_set_status(shpc, i, SHPC_LED_OFF, SHPC_SLOT_ATTN_LED_MASK); >> shpc_set_status(shpc, i, 0, SHPC_SLOT_STATUS_66); > > ... however value=0 is used: > > hw/pci/shpc.c:215: shpc_set_status(shpc, i, 0, SHPC_SLOT_STATUS_MRL_OPEN); > hw/pci/shpc.c:226: shpc_set_status(shpc, i, 0, SHPC_SLOT_STATUS_66); > hw/pci/shpc.c:522: shpc_set_status(shpc, slot, 0, SHPC_SLOT_STATUS_MRL_OPEN); > hw/pci/shpc.c:531: shpc_set_status(shpc, slot, 0, SHPC_SLOT_STATUS_MRL_OPEN); > hw/pci/shpc.c:543: shpc_set_status(shpc, slot, 0, SHPC_SLOT_STATUS_66); > hw/pci/shpc.c:589: shpc_set_status(shpc, slot, 0, SHPC_SLOT_STATUS_66); > > Is this fixable? 0 is not valid only for _STATE and _LED_ masks. It's OK for other fields
On 13/02/2023 16:00, Vladimir Sementsov-Ogievskiy wrote: > 0 is not a valid state for the led. Let's start with OFF. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > --- > hw/pci/shpc.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c > index fca7f6691a..1b3f619dc9 100644 > --- a/hw/pci/shpc.c > +++ b/hw/pci/shpc.c > @@ -223,6 +223,7 @@ void shpc_reset(PCIDevice *d) > SHPC_SLOT_STATUS_PRSNT_MASK); > shpc_set_status(shpc, i, SHPC_LED_OFF, SHPC_SLOT_PWR_LED_MASK); > } > + shpc_set_status(shpc, i, SHPC_LED_OFF, SHPC_SLOT_ATTN_LED_MASK); > shpc_set_status(shpc, i, 0, SHPC_SLOT_STATUS_66); > } > shpc_set_sec_bus_speed(shpc, SHPC_SEC_BUS_33); Reviewed-by: Anton Kuchin <antonkuchin@yandex-team.ru>
diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c index fca7f6691a..1b3f619dc9 100644 --- a/hw/pci/shpc.c +++ b/hw/pci/shpc.c @@ -223,6 +223,7 @@ void shpc_reset(PCIDevice *d) SHPC_SLOT_STATUS_PRSNT_MASK); shpc_set_status(shpc, i, SHPC_LED_OFF, SHPC_SLOT_PWR_LED_MASK); } + shpc_set_status(shpc, i, SHPC_LED_OFF, SHPC_SLOT_ATTN_LED_MASK); shpc_set_status(shpc, i, 0, SHPC_SLOT_STATUS_66); } shpc_set_sec_bus_speed(shpc, SHPC_SEC_BUS_33);
0 is not a valid state for the led. Let's start with OFF. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> --- hw/pci/shpc.c | 1 + 1 file changed, 1 insertion(+)