From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48440) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WV2lF-0004j3-EC for qemu-devel@nongnu.org; Tue, 01 Apr 2014 13:46:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WV2l8-00047Y-OY for qemu-devel@nongnu.org; Tue, 01 Apr 2014 13:46:09 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39211) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WV2l8-00047Q-Gs for qemu-devel@nongnu.org; Tue, 01 Apr 2014 13:46:02 -0400 Date: Tue, 1 Apr 2014 18:45:56 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20140401174555.GV2411@work-vm> References: <1396371187-8567-1-git-send-email-peter.maydell@linaro.org> <1396371187-8567-4-git-send-email-peter.maydell@linaro.org> <20140401172650.GS2411@work-vm> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH 3/5] hw/net/stellaris_enet: Rewrite tx fifo handling code List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Patch Tracking , QEMU Developers , "Michael S. Tsirkin" (resend reply - the mail gru got some of the 1st one) * Peter Maydell (peter.maydell@linaro.org) wrote: > On 1 April 2014 18:26, Dr. David Alan Gilbert wrote: > > * Peter Maydell (peter.maydell@linaro.org) wrote: > >> The datasheet is clear that the frame length written to the DATA > >> register is actually stored in the TX FIFO; this means we don't > >> need to keep both tx_frame_len and tx_fifo_len state separately. > >> > >> Signed-off-by: Peter Maydell > >> --- > >> hw/net/stellaris_enet.c | 119 +++++++++++++++++++++++++++++++----------------- > >> 1 file changed, 77 insertions(+), 42 deletions(-) > >> > >> diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c > >> index d0da819..b99f93e 100644 > >> --- a/hw/net/stellaris_enet.c > >> +++ b/hw/net/stellaris_enet.c > >> @@ -59,7 +59,6 @@ typedef struct { > >> uint32_t mtxd; > >> uint32_t mrxd; > >> uint32_t np; > >> - int tx_frame_len; > >> int tx_fifo_len; > >> uint8_t tx_fifo[2048]; > >> /* Real hardware has a 2k fifo, which works out to be at most 31 packets. > >> @@ -82,6 +81,62 @@ static void stellaris_enet_update(stellaris_enet_state *s) > >> qemu_set_irq(s->irq, (s->ris & s->im) != 0); > >> } > >> > >> +/* Return the data length of the packet currently being assembled > >> + * in the TX fifo. > >> + */ > >> +static inline int stellaris_txpacket_datalen(stellaris_enet_state *s) > >> +{ > >> + return s->tx_fifo[0] | (s->tx_fifo[1] << 8); > >> +} > >> + > >> +/* Return true if the packet currently in the TX FIFO is complete, > >> +* ie the FIFO holds enough bytes for the data length, ethernet header, > >> +* payload and optionally CRC. > >> +*/ > >> +static inline bool stellaris_txpacket_complete(stellaris_enet_state *s) > >> +{ > >> + int framelen = stellaris_txpacket_datalen(s); > >> + framelen += 16; > > > > What's the magical 16? (It doesn't jump out from the data sheet). > > See table 15-3: 2 bytes of data length, 6 bytes dest addr, > 6 bytes source addr, 2 bytes len/type. Ah yes or the text in 15.3.1.2 para 4 'referes to the Etehernet frame data payload, as shown in the 5th to nth FIFO positions' (1,2,3,4 each 4 bytes). > > You should probably increment the migration state version number to 2. > > Oops, yes. Fix if you need to reroll, but not too important since i doubt people are migrating it. Reviewed-by: Dr. David Alan Gilbert -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK