Re: [PATCH v4 01/16] pci/shpc: set attention led to OFF on reset

[Vladimir Sementsov-Ogievskiy]

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

--
Best regards,
Vladimir