[Pdbg] [PATCH v2 3/3] libsbefifo: Set long read timeout for chip-ops

[Joel Stanley]

On Wed, 23 Feb 2022 at 11:54, Nicholas Piggin <npiggin at gmail.com> wrote:
>
> Excerpts from Joel Stanley's message of February 23, 2022 3:34 pm:
> > From: Amitay Isaacs <amitay at ozlabs.org>
> >
> > SBE interface specification lists several sbe chip-ops that require long
> > read timeout as those chip-ops can take up to 30 seconds to complete.
> >
> > Reset the long timeout back to the default after performing the chip-op.
> >
> > The list of chip-ops from the spec (v2.0):
> >
> >  0xA1 0x01    Execute istep
> >  0xA2         0x05    Execute multi-SCOM
> >  0xA3 0x01    Get Ring
> >       0x02    Set Ring
> >  0xA4 0x01    Get Memory
> >       0x02    Put Memory
> >       0x03    Get SRAM
> >       0x04    Put SRAM
> >  0xA5 0x01    Get Architected Register
> >       0x02    Put Architected Register
> >  0xA6 0x01    Control Fast Array
> >       0x02    Control Trace Array
> >  0xA8 0x03    Quiesce SBE
> >  0xA9 0x01    Enter MPIPL
> >       0x02    Continue MPIPL
> >       0x04    TI Info
> >
> > Signed-off-by: Amitay Isaacs <amitay at ozlabs.org>
> > Signed-off-by: Joel Stanley <joel at jms.id.au>
> > ---
> > v2: Split out from second patch
> >     Add list of ops from spec to commit message
>
> That's useful ^ could add it in a comment or doc somewhere maybe?

It comes from the SBE spec, which I don't think is published.

>
>
> >
> >  libsbefifo/cmd_array.c    | 12 ++++++++++++
> >  libsbefifo/cmd_control.c  |  6 ++++++
> >  libsbefifo/cmd_generic.c  |  6 ++++++
> >  libsbefifo/cmd_memory.c   | 23 +++++++++++++++++++++++
> >  libsbefifo/cmd_mpipl.c    | 18 ++++++++++++++++++
> >  libsbefifo/cmd_register.c | 12 ++++++++++++
> >  libsbefifo/cmd_ring.c     | 18 ++++++++++++++++++
> >  7 files changed, 95 insertions(+)
> >
> > diff --git a/libsbefifo/cmd_array.c b/libsbefifo/cmd_array.c
> > index 1ff8c03d7278..d579eafdfb77 100644
> > --- a/libsbefifo/cmd_array.c
> > +++ b/libsbefifo/cmd_array.c
> > @@ -69,12 +69,18 @@ int sbefifo_control_fast_array(struct sbefifo_context *sctx, uint16_t target_typ
> >       if (rc)
> >               return rc;
> >
> > +     rc = sbefifo_set_long_timeout(sctx);
> > +     if (rc)
> > +             return rc;
>
> Leaks msg if we fail.

Ah tricky. The _push function allocates the memory, and it is freed in
the sbefifo_operation.

If we set the timeout before the _push call, but then we need to
unwind it if _push fails. I'll leave it where it is and do a free().

>
> > +
> >       out_len = 0;
> >       rc = sbefifo_operation(sctx, msg, msg_len, &out, &out_len);
> >       free(msg);
> >       if (rc)
> >               return rc;
> >
> > +     sbefifo_reset_timeout(sctx);
>
> This misses the reset if we fail. Same issues repeat a few times.

Good catch. We can do the reset straight after the sbefifo_operation.