From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:55256) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SyJne-0006kd-Th for qemu-devel@nongnu.org; Mon, 06 Aug 2012 05:40:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SyJna-00050a-Fw for qemu-devel@nongnu.org; Mon, 06 Aug 2012 05:40:34 -0400 Received: from mailout2.w1.samsung.com ([210.118.77.12]:32528) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SyJna-00050I-7S for qemu-devel@nongnu.org; Mon, 06 Aug 2012 05:40:30 -0400 Received: from eusync2.samsung.com (mailout2.w1.samsung.com [210.118.77.12]) by mailout2.w1.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0M8B00G2DUW2G320@mailout2.w1.samsung.com> for qemu-devel@nongnu.org; Mon, 06 Aug 2012 10:40:50 +0100 (BST) Received: from [106.109.9.232] by eusync2.samsung.com (Oracle Communications Messaging Server 7u4-23.01(7.0.4.23.0) 64bit (built Aug 10 2011)) with ESMTPA id <0M8B00JQ6UVAQJ50@eusync2.samsung.com> for qemu-devel@nongnu.org; Mon, 06 Aug 2012 10:40:23 +0100 (BST) Message-id: <501F9106.9080903@samsung.com> Date: Mon, 06 Aug 2012 13:40:22 +0400 From: Igor Mitsyanko MIME-version: 1.0 References: <81b9e406e0b0292ccd3168385ab6a73a78a0ada5.1344218410.git.peter.crosthwaite@petalogix.com> In-reply-to: <81b9e406e0b0292ccd3168385ab6a73a78a0ada5.1344218410.git.peter.crosthwaite@petalogix.com> Content-type: text/plain; charset=ISO-8859-1; format=flowed Content-transfer-encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 09/15] hw: Added generic FIFO API. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Peter A. G. Crosthwaite" Cc: peter.maydell@linaro.org, stefanha@gmail.com, qemu-devel@nongnu.org, paul@codesourcery.com, edgar.iglesias@gmail.com, john.williams@petalogix.com On 08/06/2012 06:16 AM, Peter A. G. Crosthwaite wrote: > Added a FIFO API that can be used to create and operate byte FIFOs. > > Signed-off-by: Peter A. G. Crosthwaite > --- > hw/Makefile.objs | 1 + > hw/fifo.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > hw/fifo.h | 47 ++++++++++++++++++++++++++++++++ > 3 files changed, 127 insertions(+), 0 deletions(-) > create mode 100644 hw/fifo.c > create mode 100644 hw/fifo.h > > diff --git a/hw/Makefile.objs b/hw/Makefile.objs > index 8327e55..6ba570e 100644 > --- a/hw/Makefile.objs > +++ b/hw/Makefile.objs > @@ -15,6 +15,7 @@ hw-obj-$(CONFIG_ECC) += ecc.o > hw-obj-$(CONFIG_NAND) += nand.o > hw-obj-$(CONFIG_PFLASH_CFI01) += pflash_cfi01.o > hw-obj-$(CONFIG_PFLASH_CFI02) += pflash_cfi02.o > +hw-obj-y += fifo.o Perhaps it'd be better to make it common object and put it into root directory, like its done for bitops.c and bitmap.c > > hw-obj-$(CONFIG_M48T59) += m48t59.o > hw-obj-$(CONFIG_ESCC) += escc.o > diff --git a/hw/fifo.c b/hw/fifo.c > new file mode 100644 > index 0000000..5e14e1e > --- /dev/null > +++ b/hw/fifo.c > @@ -0,0 +1,79 @@ > +/* > + * Generic FIFO component, implemented as a circular buffer. > + * > + * Copyright (c) 2012 Peter A. G. Crosthwaite > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * as published by the Free Software Foundation; either version > + * 2 of the License, or (at your option) any later version. > + * > + * You should have received a copy of the GNU General Public License along > + * with this program; if not, see . > + */ > + > +#include "fifo.h" > + > +void fifo8_create(Fifo8 *fifo, uint32_t capacity) > +{ > + fifo->data = g_new(uint8_t, capacity); > + fifo->capacity = capacity; > + fifo->head = 0; > + fifo->num = 0; > +} > + > +void fifo8_destroy(Fifo8 *fifo) > +{ > + g_free(fifo->data); > +} > + > +void fifo8_push(Fifo8 *fifo, uint8_t data) > +{ > + if (fifo->num == fifo->capacity) { > + abort(); > + } I think its too harsh to abort here (and in pop() too), fifo overrun/underrun condition is absolutely normal for most of the devices, usually it would just trigger an interrupt. I suggest return a error code instead and let a caller decide what should happen in this situation. > + fifo->data[(fifo->head + fifo->num) % fifo->capacity] = data; > + fifo->num++; > +} > + > +uint8_t fifo8_pop(Fifo8 *fifo) > +{ > + uint8_t ret; > + > + if (fifo->num == 0) { > + abort(); > + } > + ret = fifo->data[fifo->head++]; > + fifo->head %= fifo->capacity; > + fifo->num--; > + return ret; > +} > + > +void fifo8_reset(Fifo8 *fifo) > +{ > + fifo->num = 0; > +} > + > +bool fifo8_is_empty(Fifo8 *fifo) > +{ > + return (fifo->num == 0); > +} > + > +bool fifo8_is_full(Fifo8 *fifo) > +{ > + return (fifo->num == fifo->capacity); > +} > + > +const VMStateDescription vmstate_fifo8 = { > + .name = "SSISlave", thats not a good name for a generic fifo) > + .version_id = 1, > + .minimum_version_id = 1, > + .minimum_version_id_old = 1, > + .fields = (VMStateField[]) { too much spaces here > + VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity), > + VMSTATE_UINT32(head, Fifo8), > + VMSTATE_UINT32(num, Fifo8), > + VMSTATE_END_OF_LIST() > + } > +}; > + > diff --git a/hw/fifo.h b/hw/fifo.h > new file mode 100644 > index 0000000..3fb09ff > --- /dev/null > +++ b/hw/fifo.h > @@ -0,0 +1,47 @@ > +#ifndef FIFO_H > +#define FIFO_H > + > +#include "hw.h" > + > +typedef struct { > + /* All fields are private */ > + uint8_t *data; > + uint32_t capacity; > + uint32_t head; > + uint32_t num; > +} Fifo8; > + > +/* create a fifo of the specified size */ > + > +void fifo8_create(Fifo8 *, uint32_t); > + > +/* cleanup a fifo */ > + > +void fifo8_destroy(Fifo8 *); > + > +/* push a data byte to the fifo. Behaviour is undefined if the fifo is full */ > + > +void fifo8_push(Fifo8 *, uint8_t); > + > +/* pop a data byte from the fifo. Behviour is undefined if the fifo is empty */ > + > +uint8_t fifo8_pop(Fifo8 *); > + > +/* reset (empty) the fifo */ > + > +void fifo8_reset(Fifo8 *); > + > +bool fifo8_is_empty(Fifo8 *); > +bool fifo8_is_full(Fifo8 *); > + > +extern const VMStateDescription vmstate_fifo8; > + > +#define VMSTATE_FIFO8(_field, _state) { \ > + .name = (stringify(_field)), \ > + .size = sizeof(Fifo8), \ > + .vmsd = &vmstate_fifo8, \ > + .flags = VMS_STRUCT, \ > + .offset = vmstate_offset_value(_state, _field, Fifo8), \ > +} > + > +#endif /* FIFO_H */