Re: [PATCH 3/3] util/fifo8: Introduce fifo8_change_capacity()

[Peter Maydell]

On Mon, 22 Jul 2024 at 11:56, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 19/7/24 22:21, Mark Cave-Ayland wrote:
> > On 19/07/2024 16:16, Philippe Mathieu-Daudé wrote:
> >
> >> FIFOs can be resized at runtime. Introduce the
> >> fifo8_change_capacity() method to do that.
> >> When capacity is changed, the FIFO must be reset.
> >>
> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >> ---
> >>   include/qemu/fifo8.h | 10 ++++++++++
> >>   util/fifo8.c         |  7 +++++++
> >>   2 files changed, 17 insertions(+)
> >>
> >> diff --git a/include/qemu/fifo8.h b/include/qemu/fifo8.h
> >> index c6295c6ff0..9fe0555a24 100644
> >> --- a/include/qemu/fifo8.h
> >> +++ b/include/qemu/fifo8.h
> >> @@ -31,6 +31,16 @@ void fifo8_create(Fifo8 *fifo, uint32_t capacity);
> >>   void fifo8_destroy(Fifo8 *fifo);
> >> +/**
> >> + * fifo8_change_capacity:
> >> + * @fifo: struct Fifo8 to change the capacity
> >> + * @capacity: new capacity of the FIFO
> >> + *
> >> + * Change a FIFO capacity to the specified size. The FIFO is reset.
> >> + */
> >> +
> >> +void fifo8_change_capacity(Fifo8 *fifo, uint32_t capacity);
> >> +
> >>   /**
> >>    * fifo8_push:
> >>    * @fifo: FIFO to push to
> >> diff --git a/util/fifo8.c b/util/fifo8.c
> >> index 2925fe5611..c453afd774 100644
> >> --- a/util/fifo8.c
> >> +++ b/util/fifo8.c
> >> @@ -34,6 +34,13 @@ void fifo8_destroy(Fifo8 *fifo)
> >>       g_free(fifo->data);
> >>   }
> >> +void fifo8_change_capacity(Fifo8 *fifo, uint32_t capacity)
> >> +{
> >> +    fifo->data = g_renew(uint8_t, fifo->data, capacity);
> >> +    fifo->capacity = capacity;
> >> +    fifo8_reset(fifo);
> >> +}
> >> +
> >>   void fifo8_push(Fifo8 *fifo, uint8_t data)
> >>   {
> >>       assert(fifo->num < fifo->capacity);
> >
> > The changes look okay, however I'm a little confused as to why this is
> > needed as generally hardware FIFOs are a fixed size? Presumably this is
> > related to the PL011 series?
>
> Indeed, this is to model trying to stay as close as possible to
> the datasheet, which states:
>
>    2.4.3 UART operation
>
>      Disabling the FIFOs
>
>        Additionally, you can disable the FIFOs. In this case,
>        the transmit and receive sides of the UART have 1-byte
>        holding registers (the bottom entry of the FIFOs). The
>        overrun bit is set when a word has been received, and
>        the previous one was not yet read. In this implementation,
>        the FIFOs are not physically disabled, but the flags are
>        manipulated to give the illusion of a 1-byte register.

Notice that in the hardware we don't actually resize the FIFO,
though -- we just only ever keep one element in it...

thanks
-- PMM