Re: [U-Boot] [PATCH v2] smc911x driver frame alignment patch

2010-04-25 Thread Ben Warren
  Valentin,
On 4/22/2010 10:40 PM, Valentin Yakovenkov wrote:
 SMSC911x chips have 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. 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 at the beginig. So calculated data length
 is always bigger by 1 word.

 It seems that 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.

 It seems that fifo underflow occurs only when using byte packing on
 32-bit blackfin bus (may be because of very small delay between reads).



 Signed-off-by: Valentin Yakovenkovyakoven...@niistt.ru
 diff -r 7dc8ff189175 a/drivers/net/smc911x.c
 --- a/drivers/net/smc911x.c   Mon Mar 29 11:08:55 2010 +0400
 +++ b/drivers/net/smc911x.c   Mon Apr 19 10:46:02 2010 +0400
 @@ -220,7 +220,7 @@

   smc911x_reg_write(dev, RX_CFG, 0);

 - tmplen = (pktlen + 2+ 3) / 4;
 + tmplen = (pktlen + 3) / 4;
   while (tmplen--)
   *data++ = pkt_data_pull(dev, RX_DATA_FIFO);

 --
WBR, Valentin
CJSC NII STT, Russia, Smolensk
http://www.niistt.ru


Applied to net repo.

thanks,
Ben
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2] smc911x driver frame alignment patch

2010-04-23 Thread Mike Frysinger
for future reference, you shouldnt put patch in the subject name ... i'm not 
referring to the leading [PATCH], but the trailing patch.

also, i'm not sure if you're using `git send-email` because the trail of your 
patch is missing the --- marker:
=
It seems that fifo underflow occurs only when using byte packing on
32-bit blackfin bus (may be because of very small delay between reads).



Signed-off-by: Valentin Yakovenkov yakoven...@niistt.ru
diff -r 7dc8ff189175 a/drivers/net/smc911x.c
--- a/drivers/net/smc911x.c Mon Mar 29 11:08:55 2010 +0400
=

it should have been like:
=
It seems that fifo underflow occurs only when using byte packing on
32-bit blackfin bus (may be because of very small delay between reads).

Signed-off-by: Valentin Yakovenkov yakoven...@niistt.ru
---
diff -r 7dc8ff189175 a/drivers/net/smc911x.c
--- a/drivers/net/smc911x.c Mon Mar 29 11:08:55 2010 +0400
=
-mike


signature.asc
Description: This is a digitally signed message part.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2] smc911x driver frame alignment patch

2010-04-23 Thread Mike Frysinger
On Friday 23 April 2010 12:48:55 Valentin Yakovenkov wrote:
 I don't use git at all. We have mercurial master repo which is pulled
 from local SVN synced repo which is synchronized via svn with
 blackfin.uclinux.org.

b.u.o offers git as well if that makes things easier to sync:
http://blackfin.uclinux.org/git/

 Is it so critical to make this patch via git?

no, but it is important the patch format matches what git expects.  so if you 
add the --- marker yourself after the s-o-b tag, that's fine.
-mike


signature.asc
Description: This is a digitally signed message part.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2] smc911x driver frame alignment patch

2010-04-23 Thread Valentin Yakovenkov

23.04.2010 19:11, Mike Frysinger wrote:


for future reference, you shouldnt put patch in the subject name ... i'm not
referring to the leading [PATCH], but the trailing patch.

ok, thanx


also, i'm not sure if you're using `git send-email` because the trail of your
patch is missing the --- marker:
I don't use git at all. We have mercurial master repo which is pulled 
from local SVN synced repo which is synchronized via svn with 
blackfin.uclinux.org.


This patch is a result of the hg diff smc911.x command with manually 
added signed-off-by line.


Is it so critical to make this patch via git?

--
  WBR, Valentin
  CJSC NII STT, Russia, Smolensk
  http://www.niistt.ru



smime.p7s
Description: S/MIME Cryptographic Signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v2] smc911x driver frame alignment patch

2010-04-22 Thread Valentin Yakovenkov
SMSC911x chips have 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. 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 at the beginig. So calculated data length
is always bigger by 1 word.

It seems that 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.

It seems that fifo underflow occurs only when using byte packing on
32-bit blackfin bus (may be because of very small delay between reads).



Signed-off-by: Valentin Yakovenkov yakoven...@niistt.ru
diff -r 7dc8ff189175 a/drivers/net/smc911x.c
--- a/drivers/net/smc911x.c Mon Mar 29 11:08:55 2010 +0400
+++ b/drivers/net/smc911x.c Mon Apr 19 10:46:02 2010 +0400
@@ -220,7 +220,7 @@

smc911x_reg_write(dev, RX_CFG, 0);

-   tmplen = (pktlen + 2+ 3) / 4;
+   tmplen = (pktlen + 3) / 4;
while (tmplen--)
*data++ = pkt_data_pull(dev, RX_DATA_FIFO);

--
  WBR, Valentin
  CJSC NII STT, Russia, Smolensk
  http://www.niistt.ru



smime.p7s
Description: S/MIME Cryptographic Signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot