diff mbox

[v5,1/1] Driver for Beckhoff CX5020 EtherCAT master module.

Message ID 20140503114029.GA19315@electric-eye.fr.zoreil.com
State Superseded, archived
Delegated to: David Miller
Headers show

Commit Message

Francois Romieu May 3, 2014, 11:40 a.m. UTC 
Darek Marcinkiewicz <reksio@newterm.pl> :
[...]
> This driver adds support for EtherCAT master module located on CCAT
> FPGA found on Beckhoff CX series industrial PCs. The driver exposes
> EtherCAT master as an ethernet interface.
> 
> EtherCAT is a filedbus protocol defined on top of ethernet and Beckhoff

s/filedbus/fieldbus/ :o)

> CX5020 PCs come with built-in EtherCAT master module located on a FPGA,
> which in turn is connected to a PCI bus.
> 
> Signed-off-by: Dariusz Marcinkiewicz <reksio@newterm.pl>

I have attached four patches. Patch #3 / 4 contains a bugfix. Remaining
patches are small nits.

The points below are not showstoppers. Your answer will be welcome though.

If the tx_desc_free list can turn empty, I don't see much point for not
using an array as descriptor ring. It's quite common for linux ethernet
drivers.

Be it a list or an array, sizing it as the max count of descriptors that
could fit in the memory window through which packet descriptors _and_
ethernet data are copied seems overkill. It may be an upper bound though.
If you are not sure about a sane default value, ethtool API allows to
change the descriptor ring size.

Given the amount of debug messages, ethtool registers dump support could
be of some value.

I guess that the ASIC and the host CPU need some external polling to
communicate (no IRQ, really ?), whence the hrtimer. It should naturally
fit with NAPI. hrtimer could thus be disabled as soon as some ASIC event
is detected. It should save some hrtimer work when load increases and
the usual Tx lock avoidance patterns would kick in (current locking is
gross).

Did Beckhoff document their FPGA PCI interface ?
diff mbox

Patch

diff --git a/drivers/net/ethernet/ec_bhf.c b/drivers/net/ethernet/ec_bhf.c
index 8e78b8d..9e1636b 100644
--- a/drivers/net/ethernet/ec_bhf.c
+++ b/drivers/net/ethernet/ec_bhf.c
@@ -205,8 +205,8 @@  static void ec_bhf_reset(struct ec_bhf_priv *priv)
 
 static void ec_bhf_send_packet(struct ec_bhf_priv *priv, struct tx_desc *desc)
 {
-	u32 addr = (u8 *)desc - priv->tx_buf.buf;
 	u32 len = le16_to_cpu(desc->header.len) + sizeof(desc->header);
+	u32 addr = (u8 *)desc - priv->tx_buf.buf;
 
 	iowrite32((ALIGN(len, 8) << 24) | addr, priv->fifo_io + FIFO_TX_REG);
 
@@ -598,11 +598,11 @@  static const struct net_device_ops ec_bhf_netdev_ops = {
 
 static int ec_bhf_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
-	int err = 0;
-	void * __iomem io;
-	void * __iomem dma_io;
 	struct net_device *net_dev;
 	struct ec_bhf_priv *priv;
+	void * __iomem dma_io;
+	void * __iomem io;
+	int err = 0;
 
 	err = pci_enable_device(dev);
 	if (err)