From: Mike Frysinger Date: Thu, 22 Apr 2010 15:43:05 -0400 Subject: [U-Boot] [PATCH] smc911x driver frame alignment patch In-Reply-To: <4BD09F93.3010905@niistt.ru> References: <4BCEAFA6.8010605@niistt.ru> <201004211552.08874.vapier@gentoo.org> <4BD09F93.3010905@niistt.ru> Message-ID: <201004221543.06534.vapier@gentoo.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Thursday 22 April 2010 15:12:19 Valentin Yakovenkov wrote: > 21.04.2010 23:52, Mike Frysinger wrote: > >>> Wrong alignment in smc911x driver when reading a frame from fifo. > >>> Neither smc911x chip nor U-Boot doesn't use IP-alignment, so we don't > >>> need to add anything here. > >> > >> I know you use this driver a lot. Please comment on this patch. > > > > i really havent a clue what this change is doing, and the changelog > > doesnt make much sense to me. too many negatives perhaps ... > > SMSC911x chips has alignment function to allow frame payload data (which > comes after 14-bytes ethernet header) to be aligned at some boundary > when reading it from fifo (usually - 4 bytes boundary). > This is done by inserting fake zeros bytes BEFORE actual frame data when > reading from SMSC's fifo. > This function controlled by RX_CFG register. There are bits that > represents amount of fake bytes to be inserted. > > Linux uses alignment of 4 bytes. Ethernet frame header is 14 bytes long, > so we need to add 2 fake bytes to get payload data aligned at 4-bytes > boundary. > Linux driver does this by adding IP_ALIGNMENT constant (defined at > skb.h) when calculating fifo data length. But all network subsystem of > Linux uses this constant too when calculating different offsets. > > But u-boot does not use any packet data alignment, so we don't need to > add anything when calculating fifo data length. > Moreover, driver zeros the RX_CFG register just one line up, so chip > does not insert any fake data. After calculating data length we always > got 1 more word to read. > > So, at almost every packet read we get an underflow condition at fifo > and possible corruption of data. Especially at continuous transfers, > such as tftp. > > Just after removing this magic addition, I've got tftp transfer speed as > it aught to be at 100Mbps. It was really slow before. i would send the patch again with this info in the changelog. however, at least on my board, i see no speed difference with this patch. i get about 2.8MiB/s on my bf548-ezkit with and without your change. so, it doesnt break anything that i can see, nor have i been experiencing any problems before, so i dont have a problem with the patch being merged (once a better changelog is added). -mike -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part. Url : http://lists.denx.de/pipermail/u-boot/attachments/20100422/315ce0d6/attachment.pgp