Re: [PATCH v3] net: Add support for the OpenCores 10/100 Mbps Ethernet MAC.

Previous thread: [PATCH 3/3] smsc911x: add flag SMSC911X_USE_INTERPHY by Richard Zhao on Monday, March 23, 2009 - 11:47 pm. (5 messages)

Next thread: [PATCH] macb: fix warning "warning: unused variable `dev' " by Haavard Skinnemoen on Tuesday, March 24, 2009 - 3:26 am. (2 messages)
From: Thierry Reding
Date: Tuesday, March 24, 2009 - 3:18 am

This patch adds a platform device driver that supports the OpenCores 10/100
Mbps Ethernet MAC.

The driver expects three resources: one IORESOURCE_MEM resource defines the
memory region for the core's memory-mapped registers while a second
IORESOURCE_MEM resource defines the network packet buffer space. The third
resource, of type IORESOURCE_IRQ, associates an interrupt with the driver.

Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>

---
 drivers/net/Kconfig  |    7 +
 drivers/net/Makefile |    1 +
 drivers/net/ethoc.c  | 1153 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/net/ethoc.h  |   22 +
 4 files changed, 1183 insertions(+), 0 deletions(-)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 62d732a..fab21ca 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -963,6 +963,13 @@ config ENC28J60_WRITEVERIFY
 	  Enable the verify after the buffer write useful for debugging purpose.
 	  If unsure, say N.
 
+config ETHOC
+	tristate "OpenCores 10/100 Mbps Ethernet MAC support"
+	depends on NET_ETHERNET
+	select PHYLIB
+	help
+	  Say Y here if you want to use the OpenCores 10/100 Mbps Ethernet MAC.
+
 config SMC911X
 	tristate "SMSC LAN911[5678] support"
 	select CRC32
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 471baaf..ac3d6f7 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -229,6 +229,7 @@ obj-$(CONFIG_PASEMI_MAC) += pasemi_mac_driver.o
 pasemi_mac_driver-objs := pasemi_mac.o pasemi_mac_ethtool.o
 obj-$(CONFIG_MLX4_CORE) += mlx4/
 obj-$(CONFIG_ENC28J60) += enc28j60.o
+obj-$(CONFIG_ETHOC) += ethoc.o
 
 obj-$(CONFIG_XTENSA_XT2000_SONIC) += xtsonic.o
 
diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
new file mode 100644
index 0000000..05e6bb3
--- /dev/null
+++ b/drivers/net/ethoc.c
@@ -0,0 +1,1153 @@
+/*
+ * linux/drivers/net/ethoc.c
+ *
+ * Copyright (C) 2007-2008 Avionic Design Development GmbH
+ * Copyright (C) 2008-2009 Avionic Design GmbH
+ *
+ * This ...
From: Florian Fainelli
Date: Tuesday, March 24, 2009 - 3:32 am

Hi Thierry,


I am glad someone updated and submitted this driver, excellent work !. Few 

I think there are some other authors like Simon Srot and Tensilica, unless you 
did wrote this completely from scratch and did not look at the uClincu 
open_eth driver at all ?


Why do you need these declarations ? Are not your functions properly ordered 

That's what uClinux driver calls SRAM right ?


Please use netdev_ops.


What about allowing platform configuration of the RX/TX buffers size and 
number of them ?

Otherwise

Acked-by: Florian Fainelli <florian@openwrt.org>
-- 
Best regards, Florian Fainelli
Email : florian@openwrt.org
http://openwrt.org
-------------------------------
--

From: Thierry Reding
Date: Wednesday, March 25, 2009 - 7:43 am

I did look at the uClinux driver at some point, but decided to rewrite it-
from scratch. But to be honest it's been quite some time since I started work
on this and I'm not a 100% certain that one part or another may not be
borrowed from it.





I think this is a good idea, but I'm not quite sure about how this should be
implemented. The total number of buffers is dependent on the total buffer size
as defined by the second IORESOURCE_MEM resource. That really only leaves the
option for allowing the individual buffer size to be defined by the platform
configuration. Furthermore the network controller can only handle fixed-sized
buffers (at least for reception), so perhaps defining some kind of RX/TX
buffer number ratio would be useful. Or perhaps defining a minimum or maximum
number of TX buffers and leaving the rest up for RX for instance.

Any suggestions?

Cheers,
Thierry

--

From: Florian Fainelli
Date: Wednesday, March 25, 2009 - 2:00 pm

Hello Thierry,


Sure my concern was more about the original authors somehow claiming copyright 

I like the idea of defining ratios but for now let's just stick to what you 
proposed and we can later agree on getting more parameters being passed to 
the driver using platform_data I am not sure yet how I will tune these 
parameters on my design.
-- 
Best regards, Florian Fainelli
Email : florian@openwrt.org
http://openwrt.org
-------------------------------
--

From: Andrey Panin
Date: Tuesday, March 24, 2009 - 4:13 am

From: David Miller
Date: Tuesday, March 24, 2009 - 4:36 pm

From: Andrey Panin <pazke@donpac.ru>

Agreed, this stuff must be fixed.

You could use skb_copy_and_csum_dev() directly into your buffer, then
pad out the end of the buffer with a memset() call, if necessary.

Actually, no you can't...

You should not be using memcpy() to store things into I/O memory.
That's what memcpy_io() is for.

I think because of all of these special padding cases and the use
of I/O memory instead of DMA, there is no real gain by using
skb_copy_and_csum_dev() in this driver.  Just let the network
stack checksum the packet, and use memcpy_io() here which is
the currect interface for copying data into I/O memory.
--

From: Thierry Reding
Date: Wednesday, March 25, 2009 - 7:52 am

Actually these special padding cases are only necessary for the FPGA interface
we use and have nothing to do with the OpenCores Ethernet MAC itself. The ARM
memcpy_{to,from}io() functions don't work correctly with my setup because our
VLIO bridge implementation apparently doesn't handle byte-wise accesses very
well.

I can work around that by implementing the memcpy_{to,from}io() functions
differently, though. That may however break anything else using those

See follow-up patch in reply to the first patch.

Cheers,
Thierry

--

From: Andrey Panin
Date: Tuesday, March 24, 2009 - 5:01 am

From: Thierry Reding
Date: Wednesday, March 25, 2009 - 7:55 am

[snip]

Yes, ether_crc() does the trick. See follow-up patch. Thanks.

Cheers,
Thierry

--

From: Thierry Reding
Date: Wednesday, March 25, 2009 - 7:57 am

This patch adds a platform device driver that supports the OpenCores 10/100
Mbps Ethernet MAC.

The driver expects three resources: one IORESOURCE_MEM resource defines the
memory region for the core's memory-mapped registers while a second
IORESOURCE_MEM resource defines the network packet buffer space. The third
resource, of type IORESOURCE_IRQ, associates an interrupt with the driver.

Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Florian Fainelli <florian@openwrt.org>

---
 drivers/net/Kconfig  |    7 +
 drivers/net/Makefile |    1 +
 drivers/net/ethoc.c  | 1110 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/net/ethoc.h  |   22 +
 4 files changed, 1140 insertions(+), 0 deletions(-)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 62d732a..fab21ca 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -963,6 +963,13 @@ config ENC28J60_WRITEVERIFY
 	  Enable the verify after the buffer write useful for debugging purpose.
 	  If unsure, say N.
 
+config ETHOC
+	tristate "OpenCores 10/100 Mbps Ethernet MAC support"
+	depends on NET_ETHERNET
+	select PHYLIB
+	help
+	  Say Y here if you want to use the OpenCores 10/100 Mbps Ethernet MAC.
+
 config SMC911X
 	tristate "SMSC LAN911[5678] support"
 	select CRC32
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 471baaf..ac3d6f7 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -229,6 +229,7 @@ obj-$(CONFIG_PASEMI_MAC) += pasemi_mac_driver.o
 pasemi_mac_driver-objs := pasemi_mac.o pasemi_mac_ethtool.o
 obj-$(CONFIG_MLX4_CORE) += mlx4/
 obj-$(CONFIG_ENC28J60) += enc28j60.o
+obj-$(CONFIG_ETHOC) += ethoc.o
 
 obj-$(CONFIG_XTENSA_XT2000_SONIC) += xtsonic.o
 
diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
new file mode 100644
index 0000000..38370ee
--- /dev/null
+++ b/drivers/net/ethoc.c
@@ -0,0 +1,1110 @@
+/*
+ * linux/drivers/net/ethoc.c
+ *
+ * Copyright (C) 2007-2008 Avionic Design Development GmbH
+ * Copyright (C) ...
From: David Miller
Date: Wednesday, March 25, 2009 - 5:18 pm

From: Thierry Reding <thierry.reding@avionic-design.de>

I have some issues with the CONFIG_MII tests.

It seems better to just "select MII" in the Kconfig entry for this
driver.  So could you please do that instead?  I don't see how this
chip could possible work and obtain a link without the MII interface
being setup properly.

Besides, the ifdefs are ugly :-)

Other than this, the driver looks great.  Please fix up this MII
issue and I will add the driver to my tree.

Thanks!
--

From: Thierry Reding
Date: Thursday, March 26, 2009 - 12:42 am

This patch adds a platform device driver that supports the OpenCores 10/100
Mbps Ethernet MAC.

The driver expects three resources: one IORESOURCE_MEM resource defines the
memory region for the core's memory-mapped registers while a second
IORESOURCE_MEM resource defines the network packet buffer space. The third
resource, of type IORESOURCE_IRQ, associates an interrupt with the driver.

Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Florian Fainelli <florian@openwrt.org>

---
 drivers/net/Kconfig  |    8 +
 drivers/net/Makefile |    1 +
 drivers/net/ethoc.c  | 1112 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/net/ethoc.h  |   22 +
 4 files changed, 1143 insertions(+), 0 deletions(-)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 62d732a..1067240 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -963,6 +963,14 @@ config ENC28J60_WRITEVERIFY
 	  Enable the verify after the buffer write useful for debugging purpose.
 	  If unsure, say N.
 
+config ETHOC
+	tristate "OpenCores 10/100 Mbps Ethernet MAC support"
+	depends on NET_ETHERNET
+	select MII
+	select PHYLIB
+	help
+	  Say Y here if you want to use the OpenCores 10/100 Mbps Ethernet MAC.
+
 config SMC911X
 	tristate "SMSC LAN911[5678] support"
 	select CRC32
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 471baaf..ac3d6f7 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -229,6 +229,7 @@ obj-$(CONFIG_PASEMI_MAC) += pasemi_mac_driver.o
 pasemi_mac_driver-objs := pasemi_mac.o pasemi_mac_ethtool.o
 obj-$(CONFIG_MLX4_CORE) += mlx4/
 obj-$(CONFIG_ENC28J60) += enc28j60.o
+obj-$(CONFIG_ETHOC) += ethoc.o
 
 obj-$(CONFIG_XTENSA_XT2000_SONIC) += xtsonic.o
 
diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
new file mode 100644
index 0000000..1ccd512
--- /dev/null
+++ b/drivers/net/ethoc.c
@@ -0,0 +1,1112 @@
+/*
+ * linux/drivers/net/ethoc.c
+ *
+ * Copyright (C) 2007-2008 Avionic Design Development GmbH
+ * ...
From: David Miller
Date: Thursday, March 26, 2009 - 12:55 am

From: Thierry Reding <thierry.reding@avionic-design.de>

This doesn't build:

drivers/net/ethoc.c: In function ‘ethoc_interrupt’:
drivers/net/ethoc.c:510: error: implicit declaration of function ‘netif_rx_schedule_prep’
drivers/net/ethoc.c:511: error: implicit declaration of function ‘__netif_rx_schedule’
drivers/net/ethoc.c: In function ‘ethoc_poll’:
drivers/net/ethoc.c:550: error: implicit declaration of function ‘netif_rx_complete’

You are missing some header includes.
From: David Miller
Date: Thursday, March 26, 2009 - 12:58 am

From: David Miller <davem@davemloft.net>

Actually the problem is that you're using deprecated interfaces which
have been removed in net-next-2.6, please use napi_*() instead of
netif_rx_*().
From: Thierry Reding
Date: Thursday, March 26, 2009 - 1:09 am

Oh, okay. I've only been building against the vanilla tree. I'll grab a copy
of net-next-2.6 and fix the driver up against that. Thanks,

Thierry

--

From: Thierry Reding
Date: Thursday, March 26, 2009 - 3:16 am

This patch adds a platform device driver that supports the OpenCores 10/100
Mbps Ethernet MAC.

The driver expects three resources: one IORESOURCE_MEM resource defines the
memory region for the core's memory-mapped registers while a second
IORESOURCE_MEM resource defines the network packet buffer space. The third
resource, of type IORESOURCE_IRQ, associates an interrupt with the driver.

Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Florian Fainelli <florian@openwrt.org>

---
 drivers/net/Kconfig  |    8 +
 drivers/net/Makefile |    1 +
 drivers/net/ethoc.c  | 1112 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/net/ethoc.h  |   22 +
 4 files changed, 1143 insertions(+), 0 deletions(-)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index e5ffc1c..f062b42 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -972,6 +972,14 @@ config ENC28J60_WRITEVERIFY
 	  Enable the verify after the buffer write useful for debugging purpose.
 	  If unsure, say N.
 
+config ETHOC
+	tristate "OpenCores 10/100 Mbps Ethernet MAC support"
+	depends on NET_ETHERNET
+	select MII
+	select PHYLIB
+	help
+	  Say Y here if you want to use the OpenCores 10/100 Mbps Ethernet MAC.
+
 config SMC911X
 	tristate "SMSC LAN911[5678] support"
 	select CRC32
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 758ecdf..98409c9 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -230,6 +230,7 @@ obj-$(CONFIG_PASEMI_MAC) += pasemi_mac_driver.o
 pasemi_mac_driver-objs := pasemi_mac.o pasemi_mac_ethtool.o
 obj-$(CONFIG_MLX4_CORE) += mlx4/
 obj-$(CONFIG_ENC28J60) += enc28j60.o
+obj-$(CONFIG_ETHOC) += ethoc.o
 
 obj-$(CONFIG_XTENSA_XT2000_SONIC) += xtsonic.o
 
diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
new file mode 100644
index 0000000..c737bd6
--- /dev/null
+++ b/drivers/net/ethoc.c
@@ -0,0 +1,1112 @@
+/*
+ * linux/drivers/net/ethoc.c
+ *
+ * Copyright (C) 2007-2008 Avionic Design Development GmbH
+ * ...
From: David Miller
Date: Friday, March 27, 2009 - 12:17 am

From: Thierry Reding <thierry.reding@avionic-design.de>

Applied, thanks.
--

Previous thread: [PATCH 3/3] smsc911x: add flag SMSC911X_USE_INTERPHY by Richard Zhao on Monday, March 23, 2009 - 11:47 pm. (5 messages)

Next thread: [PATCH] macb: fix warning "warning: unused variable `dev' " by Haavard Skinnemoen on Tuesday, March 24, 2009 - 3:26 am. (2 messages)