2006-05-01 11:32:46

by David Vrabel

[permalink] [raw]
Subject: Re: IP1000 gigabit nic driver

Reduce delays when reading/writing the PHY registers so we clock the
MII management interface at 2.5 MHz (the maximum according to the
datasheet) instead of 500 Hz.

Signed-off-by: David Vrabel <[email protected]>

Index: linux-source-2.6.16/drivers/net/ipg.c
===================================================================
--- linux-source-2.6.16.orig/drivers/net/ipg.c 2006-05-01 11:52:32.555800238 +0100
+++ linux-source-2.6.16/drivers/net/ipg.c 2006-05-01 12:08:45.316188064 +0100
@@ -176,13 +176,13 @@
(IPG_PC_MGMTCLK_LO | (IPG_PC_MGMTDATA & 0) | IPG_PC_MGMTDIR |
phyctrlpolarity), ioaddr + IPG_PHYCTRL);

- mdelay(IPG_PC_PHYCTRLWAIT);
+ ndelay(IPG_PC_PHYCTRLWAIT_NS);

iowrite8(IPG_PC_RSVD_MASK &
(IPG_PC_MGMTCLK_HI | (IPG_PC_MGMTDATA & 0) | IPG_PC_MGMTDIR |
phyctrlpolarity), ioaddr + IPG_PHYCTRL);

- mdelay(IPG_PC_PHYCTRLWAIT);
+ ndelay(IPG_PC_PHYCTRLWAIT_NS);
}

static void send_end(void __iomem * ioaddr, u8 phyctrlpolarity)
@@ -198,7 +198,7 @@
iowrite8(IPG_PC_RSVD_MASK & (IPG_PC_MGMTCLK_LO | phyctrlpolarity),
ioaddr + IPG_PHYCTRL);

- mdelay(IPG_PC_PHYCTRLWAIT);
+ ndelay(IPG_PC_PHYCTRLWAIT_NS);

bit_data =
((ioread8(ioaddr + IPG_PHYCTRL) & IPG_PC_MGMTDATA) >> 1) & 1;
@@ -206,7 +206,7 @@
iowrite8(IPG_PC_RSVD_MASK & (IPG_PC_MGMTCLK_HI | phyctrlpolarity),
ioaddr + IPG_PHYCTRL);

- mdelay(IPG_PC_PHYCTRLWAIT);
+ ndelay(IPG_PC_PHYCTRLWAIT_NS);
return bit_data;
}

@@ -290,14 +290,14 @@
(IPG_PC_MGMTDATA & databit) | IPG_PC_MGMTDIR |
phyctrlpolarity), ioaddr + IPG_PHYCTRL);

- mdelay(IPG_PC_PHYCTRLWAIT);
+ ndelay(IPG_PC_PHYCTRLWAIT_NS);

iowrite8(IPG_PC_RSVD_MASK &
(IPG_PC_MGMTCLK_HI |
(IPG_PC_MGMTDATA & databit) | IPG_PC_MGMTDIR |
phyctrlpolarity), ioaddr + IPG_PHYCTRL);

- mdelay(IPG_PC_PHYCTRLWAIT);
+ ndelay(IPG_PC_PHYCTRLWAIT_NS);
}

send_three_state(ioaddr, phyctrlpolarity);
@@ -403,14 +403,14 @@
(IPG_PC_MGMTDATA & databit) | IPG_PC_MGMTDIR |
phyctrlpolarity), ioaddr + IPG_PHYCTRL);

- mdelay(IPG_PC_PHYCTRLWAIT);
+ ndelay(IPG_PC_PHYCTRLWAIT_NS);

iowrite8(IPG_PC_RSVD_MASK &
(IPG_PC_MGMTCLK_HI |
(IPG_PC_MGMTDATA & databit) | IPG_PC_MGMTDIR |
phyctrlpolarity), ioaddr + IPG_PHYCTRL);

- mdelay(IPG_PC_PHYCTRLWAIT);
+ ndelay(IPG_PC_PHYCTRLWAIT_NS);
}

/* The last cycle is a tri-state, so read from the PHY.
@@ -421,7 +421,7 @@
(IPG_PC_MGMTCLK_LO | phyctrlpolarity),
ioaddr + IPG_PHYCTRL);

- mdelay(IPG_PC_PHYCTRLWAIT);
+ ndelay(IPG_PC_PHYCTRLWAIT_NS);

field[j] |= ((ioread8(ioaddr + IPG_PHYCTRL) &
IPG_PC_MGMTDATA) >> 1)
@@ -431,7 +431,7 @@
(IPG_PC_MGMTCLK_HI | phyctrlpolarity),
ioaddr + IPG_PHYCTRL);

- mdelay(IPG_PC_PHYCTRLWAIT);
+ ndelay(IPG_PC_PHYCTRLWAIT_NS);

}
}
Index: linux-source-2.6.16/drivers/net/ipg.h
===================================================================
--- linux-source-2.6.16.orig/drivers/net/ipg.h 2006-05-01 12:08:58.343035854 +0100
+++ linux-source-2.6.16/drivers/net/ipg.h 2006-05-01 12:09:37.282602113 +0100
@@ -672,10 +672,10 @@
/* Number of IPG_AC_RESETWAIT timeperiods before declaring timeout. */
#define IPG_AC_RESET_TIMEOUT 0x0A

-/* Minimum number of miliseconds used to toggle MDC clock during
+/* Minimum number of nanoseconds used to toggle MDC clock during
* MII/GMII register access.
*/
-#define IPG_PC_PHYCTRLWAIT 0x01
+#define IPG_PC_PHYCTRLWAIT_NS 200

#define IPG_TFDLIST_LENGTH 0x100


Attachments:
drivers-net-ipg-speed-up-phy-access (3.46 kB)

2006-05-01 18:09:04

by Pekka Enberg

[permalink] [raw]
Subject: Re: IP1000 gigabit nic driver

Hi David,

On Mon, 2006-05-01 at 12:32 +0100, David Vrabel wrote:
> It was clocking the MII management interface (MDC) at 500 Hz so each PHY
> register access took some 130 ms, and many registers accesses were being
> done on initialization. According to the datasheet, the maximum
> frequency for MDC is 2.5 MHz. Delays have been adjusted accordingly.

Thanks. Merged your patch.

Pekka

[PATCH] IP1000 Gigabit Ethernet device driver

This is a cleaned up fork of the IP1000A device driver:

http://www.icplus.com.tw/driver-pp-IP1000A.html

Open issues:

- ipg_probe() looks really fishy and doesn't handle all errors
(e.g. ioremap failing).
- ipg_nic_do_ioctl() is playing games with user-space pointer
and lets userspace do PCI access. I think we should nuke the
ioctl. Arjan suggested ethtool ioctl instead, but we don't
seem to have that kind of functionality now anyway.

Changelog:

- Kill 2.2 and 2.4 compatability macros (Pekka)
- Use proper module API (Pekka)
- Use proper PCI API (Pekka)
- Use netdev_priv (Pekka)
- Consolidate headers to one file (Pekka)
- Use __iomem annotations (Pekka)
- Use iomap instead of read/out for I/O (Pekka)
- Remove obfuscating register access macros (Pekka)
- Remove changelogs (David)
- Remove ether_crc_le() and use crc32_le() instead (David)
- No more nonsense with root_dev. ipg_remove() now works (David)
- Move PHY and MAC address initialization into the ipg_probe(). It was
previously filling in the MAC address on open which breaks some user
space. (David)
- Folded ipg_nic_init into ipg_probe since it was broke otherwise (David)
- Reduce delays when reading/writing the PHY registers so we clock the
MII management interface at 2.5 MHz (the maximum according to the
datasheet) instead of 500 Hz. (David)

The patch is 128 KB in size, so I am not including it in this
mail. You can find the patch here:

http://www.cs.helsinki.fi/u/penberg/linux/ip1000-driver.patch

Signed-off-by: David Vrabel <[email protected]>
Signed-off-by: Pekka Enberg <[email protected]>

2006-05-01 20:39:50

by Francois Romieu

[permalink] [raw]
Subject: Re: IP1000 gigabit nic driver

David Vrabel <[email protected]> :
[...]
> It was clocking the MII management interface (MDC) at 500 Hz so each PHY
> register access took some 130 ms, and many registers accesses were being
> done on initialization. According to the datasheet, the maximum
> frequency for MDC is 2.5 MHz. Delays have been adjusted accordingly.

[...]
> Index: linux-source-2.6.16/drivers/net/ipg.h
> ===================================================================
> --- linux-source-2.6.16.orig/drivers/net/ipg.h 2006-05-01 12:08:58.343035854 +0100
> +++ linux-source-2.6.16/drivers/net/ipg.h 2006-05-01 12:09:37.282602113 +0100
> @@ -672,10 +672,10 @@
> /* Number of IPG_AC_RESETWAIT timeperiods before declaring timeout. */
> #define IPG_AC_RESET_TIMEOUT 0x0A
>
> -/* Minimum number of miliseconds used to toggle MDC clock during
> +/* Minimum number of nanoseconds used to toggle MDC clock during
> * MII/GMII register access.
> */
> -#define IPG_PC_PHYCTRLWAIT 0x01
> +#define IPG_PC_PHYCTRLWAIT_NS 200

I would have expected a cycle of 400 ns (p.72/77 of the datasheet)
for a 2.5 MHz clock. Why is it cut by a two factor ?

--
Ueimor

2006-05-01 20:41:53

by Lennert Buytenhek

[permalink] [raw]
Subject: Re: IP1000 gigabit nic driver

On Mon, May 01, 2006 at 10:38:47PM +0200, Francois Romieu wrote:

> > -/* Minimum number of miliseconds used to toggle MDC clock during
> > +/* Minimum number of nanoseconds used to toggle MDC clock during
> > * MII/GMII register access.
> > */
> > -#define IPG_PC_PHYCTRLWAIT 0x01
> > +#define IPG_PC_PHYCTRLWAIT_NS 200
>
> I would have expected a cycle of 400 ns (p.72/77 of the datasheet)
> for a 2.5 MHz clock. Why is it cut by a two factor ?

200 ns high + 200 ns low = 400 ns clock period?

2006-05-01 23:11:51

by Francois Romieu

[permalink] [raw]
Subject: [PATCH 1/3] ipg: removal of unreachable code

map/unmap is done in ipg_{probe/remove}

Signed-off-by: Francois Romieu <[email protected]>

---

drivers/net/ipg.c | 18 ------------------
1 files changed, 0 insertions(+), 18 deletions(-)

2b14ddef0c29f43c07ffd33c3d1d9e652db3a571
diff --git a/drivers/net/ipg.c b/drivers/net/ipg.c
index 5d4d023..42396ca 100644
--- a/drivers/net/ipg.c
+++ b/drivers/net/ipg.c
@@ -1790,24 +1790,6 @@ static int ipg_nic_stop(struct net_devic
/* Release interrupt line. */
free_irq(dev->irq, dev);
return 0;
-
-#ifdef USE_IO_OPS
-
- /* Release I/O range reserved for IPG registers. */
- release_region(dev->base_addr, IPG_IO_REG_RANGE);
-
-#else /* Not using I/O space. */
-
- /* Unmap memory used for IPG registers. */
-
- /* The following line produces strange results unless
- * unregister_netdev precedes it.
- */
- iounmap(sp->ioaddr);
-
-#endif
-
- return 0;
}

static int ipg_nic_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
--
1.3.1

2006-05-01 23:11:54

by Francois Romieu

[permalink] [raw]
Subject: [PATCH 2/3] ipg: leaks in ipg_probe

The error paths are badly broken.

Bonus:
- remove duplicate initialization of sp;
- remove useless NULL initialization of dev;
- USE_IO_OPS is not used (and the driver does not seem to care about
posted writes, rejoice).

Signed-off-by: Francois Romieu <[email protected]>

---

drivers/net/ipg.c | 93 ++++++++++++++++++++++++-----------------------------
1 files changed, 42 insertions(+), 51 deletions(-)

5ea54e95a2319311aaae857ecf2adb7fbe068c92
diff --git a/drivers/net/ipg.c b/drivers/net/ipg.c
index 42396ca..da0fa22 100644
--- a/drivers/net/ipg.c
+++ b/drivers/net/ipg.c
@@ -2914,11 +2914,8 @@ static void ipg_remove(struct pci_dev *p
/* Un-register Ethernet device. */
unregister_netdev(dev);

-#ifdef USE_IO_OPS
- ioport_unmap(ioaddr);
-#else
- iounmap(sp->ioaddr);
-#endif
+ pci_iounmap(pdev, sp->ioaddr);
+
pci_release_regions(pdev);

free_netdev(dev);
@@ -2929,19 +2926,15 @@ #endif
static int __devinit ipg_probe(struct pci_dev *pdev,
const struct pci_device_id *id)
{
- int err, i;
- struct net_device *dev = NULL;
+ unsigned int i = id->driver_data;
struct ipg_nic_private *sp;
+ struct net_device *dev;
void __iomem *ioaddr;
- unsigned long pio_start, pio_len;
- unsigned long mmio_start;
+ int rc;

- err = pci_enable_device(pdev);
- if (err)
- return err;
-
- /* Get the index for the driver description string. */
- i = id->driver_data;
+ rc = pci_enable_device(pdev);
+ if (rc < 0)
+ goto out;

printk(KERN_INFO "%s found.\n", nics_supported[i].NICname);
printk(KERN_INFO "Bus %x Slot %x\n",
@@ -2949,10 +2942,14 @@ static int __devinit ipg_probe(struct pc

pci_set_master(pdev);

- if (!pci_dma_supported(pdev, 0xFFFFFFFF)) {
- printk(KERN_INFO "pci_dma_supported out.\n");
- err = -ENODEV;
- goto out;
+ rc = pci_set_dma_mask(pdev, DMA_40BIT_MASK);
+ if (rc < 0) {
+ rc = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
+ if (rc < 0) {
+ printk(KERN_ERR "%s: DMA config failed.\n",
+ pci_name(pdev));
+ goto err_disable_0;
+ }
}

/*
@@ -2960,9 +2957,9 @@ static int __devinit ipg_probe(struct pc
*/
dev = alloc_etherdev(sizeof(struct ipg_nic_private));
if (!dev) {
- printk(KERN_ERR "ipg: alloc_etherdev failed\n");
- err = -ENOMEM;
- goto out;
+ printk(KERN_ERR "%s: alloc_etherdev failed\n", pci_name(pdev));
+ rc = -ENOMEM;
+ goto err_disable_0;
}

sp = netdev_priv(dev);
@@ -2981,50 +2978,44 @@ static int __devinit ipg_probe(struct pc
SET_MODULE_OWNER(dev);
SET_NETDEV_DEV(dev, &pdev->dev);

- err = pci_request_regions(pdev, DRV_NAME);
- if (err)
- goto out;
-
- pio_start = pci_resource_start(pdev, 0) & 0xffffff80;
- pio_len = pci_resource_len(pdev, 0);
- mmio_start = pci_resource_start(pdev, 1) & 0xffffff80;
+ rc = pci_request_regions(pdev, DRV_NAME);
+ if (rc)
+ goto err_free_dev_1;

-#ifdef USE_IO_OPS
- ioaddr = ioport_map(pio_start, pio_len);
- if (!ioaddr) {
- printk(KERN_ERR "%s cannot map PIO\n", pci_name(pdev));
- err = -EIO;
- goto out;
- }
-#else
- ioaddr = ioremap(mmio_start, netdev_io_size);
+ ioaddr = pci_iomap(pdev, 1, pci_resource_len(pdev, 1));
if (!ioaddr) {
printk(KERN_ERR "%s cannot map MMIO\n", pci_name(pdev));
- err = -EIO;
- goto out;
+ rc = -EIO;
+ goto err_release_regions_2;
}
-#endif
- sp = netdev_priv(dev);
+
/* Save the pointer to the PCI device information. */
sp->ioaddr = ioaddr;
sp->pdev = pdev;

pci_set_drvdata(pdev, dev);

- err = ipg_hw_init(dev);
- if (err)
- goto out;
+ rc = ipg_hw_init(dev);
+ if (rc < 0)
+ goto err_unmap_3;

- err = register_netdev(dev);
- if (err)
- goto out;
+ rc = register_netdev(dev);
+ if (rc < 0)
+ goto err_unmap_3;

printk(KERN_INFO "Ethernet device registered as: %s\n", dev->name);
- return 0;
+out:
+ return rc;

- out:
+err_unmap_3:
+ pci_iounmap(pdev, ioaddr);
+err_release_regions_2:
+ pci_release_regions(pdev);
+err_free_dev_1:
+ free_netdev(dev);
+err_disable_0:
pci_disable_device(pdev);
- return err;
+ goto out;
}

static void ipg_set_phy_default_param(unsigned char rev,
--
1.3.1

2006-05-01 23:15:50

by Francois Romieu

[permalink] [raw]
Subject: [PATCH 3/3] ipg: plug leaks in the error path of ipg_nic_open

Added ipg_{rx/tx}_clear() to factor out some code.

Signed-off-by: Francois Romieu <[email protected]>

---

drivers/net/ipg.c | 161 +++++++++++++++++++++++++++++++----------------------
1 files changed, 93 insertions(+), 68 deletions(-)

96f29e9d57f503f1275757f4bbec76c0f7f421fc
diff --git a/drivers/net/ipg.c b/drivers/net/ipg.c
index da0fa22..393b617 100644
--- a/drivers/net/ipg.c
+++ b/drivers/net/ipg.c
@@ -16,6 +16,9 @@
*/
#include <linux/crc32.h>

+#define IPG_RX_RING_BYTES (sizeof(struct RFD) * IPG_RFDLIST_LENGTH)
+#define IPG_TX_RING_BYTES (sizeof(struct TFD) * IPG_TFDLIST_LENGTH)
+
#define JUMBO_FRAME_4k_ONLY
enum {
netdev_io_size = 128
@@ -1485,15 +1488,46 @@ static void ipg_nic_txfree(struct net_de
} while (maxtfdcount != 0);
}

-static int ipg_nic_open(struct net_device *dev)
+static void ipg_rx_clear(struct ipg_nic_private *sp)
{
- /* The IPG NIC Ethernet interface is opened when activated
- * by ifconfig.
- */
+ struct pci_dev *pdev = sp->pdev;
+ unsigned int i;

- int error = 0;
- void __iomem *ioaddr = ipg_ioaddr(dev);
+ for (i = 0; i < IPG_RFDLIST_LENGTH; i++) {
+ if (sp->RxBuff[i]) {
+ struct ipg_dmabuff *desc = sp->RxBuffDMAhandle + i;
+
+ IPG_DEV_KFREE_SKB(sp->RxBuff[i]);
+ sp->RxBuff[i] = NULL;
+ pci_unmap_single(pdev, desc->dmahandle, desc->len,
+ PCI_DMA_FROMDEVICE);
+ }
+ }
+}
+
+static void ipg_tx_clear(struct ipg_nic_private *sp)
+{
+ struct pci_dev *pdev = sp->pdev;
+ unsigned int i;
+
+ for (i = 0; i < IPG_TFDLIST_LENGTH; i++) {
+ if (sp->TxBuff[i]) {
+ struct ipg_dmabuff *desc = sp->TxBuffDMAhandle + i;
+
+ IPG_DEV_KFREE_SKB(sp->TxBuff[i]);
+ sp->TxBuff[i] = NULL;
+ pci_unmap_single(pdev, desc->dmahandle, desc->len,
+ PCI_DMA_TODEVICE);
+ }
+ }
+}
+
+static int ipg_nic_open(struct net_device *dev)
+{
struct ipg_nic_private *sp = netdev_priv(dev);
+ void __iomem *ioaddr = ipg_ioaddr(dev);
+ struct pci_dev *pdev = sp->pdev;
+ int rc;

IPG_DEBUG_MSG("_nic_open\n");

@@ -1508,59 +1542,53 @@ static int ipg_nic_open(struct net_devic
/* Register the interrupt line to be used by the IPG within
* the Linux system.
*/
- if ((error = request_irq(sp->pdev->irq,
- &ipg_interrupt_handler,
- SA_SHIRQ, dev->name, dev)) < 0) {
+ rc = request_irq(pdev->irq, &ipg_interrupt_handler, SA_SHIRQ,
+ dev->name, dev);
+ if (rc < 0) {
printk(KERN_INFO "%s: Error when requesting interrupt.\n",
dev->name);
- return error;
+ goto out;
}

- dev->irq = sp->pdev->irq;
+ dev->irq = pdev->irq;

- sp->RFDList = pci_alloc_consistent(sp->pdev,
- (sizeof(struct RFD) *
- IPG_RFDLIST_LENGTH),
+ rc = -ENOMEM;
+
+ sp->RFDList = pci_alloc_consistent(pdev, IPG_RX_RING_BYTES,
&sp->RFDListDMAhandle);
+ if (!sp->RFDList)
+ goto err_free_irq_0;

- sp->TFDList = pci_alloc_consistent(sp->pdev,
- (sizeof(struct TFD) *
- IPG_TFDLIST_LENGTH),
+ sp->TFDList = pci_alloc_consistent(pdev, IPG_TX_RING_BYTES,
&sp->TFDListDMAhandle);
+ if (!sp->TFDList)
+ goto err_free_rx_1;

- if ((sp->RFDList == NULL) || (sp->TFDList == NULL)) {
- printk(KERN_INFO
- "%s: No memory available for IP1000 RFD and/or TFD lists.\n",
- dev->name);
- return -ENOMEM;
- }
-
- error = init_rfdlist(dev);
- if (error < 0) {
+ rc = init_rfdlist(dev);
+ if (rc < 0) {
printk(KERN_INFO "%s: Error during configuration.\n",
dev->name);
- return error;
+ goto err_free_tx_2;
}

- error = init_tfdlist(dev);
- if (error < 0) {
+ rc = init_tfdlist(dev);
+ if (rc < 0) {
printk(KERN_INFO "%s: Error during configuration.\n",
dev->name);
- return error;
+ goto err_release_rfdlist_3;
}

- /* Configure IPG I/O registers. */
- error = ipg_io_config(dev);
- if (error < 0) {
+ rc = ipg_io_config(dev);
+ if (rc < 0) {
printk(KERN_INFO "%s: Error during configuration.\n",
dev->name);
- return error;
+ goto err_release_tfdlist_4;
}

/* Resolve autonegotiation. */
- if (ipg_config_autoneg(dev) < 0) {
+ if (ipg_config_autoneg(dev) < 0)
printk(KERN_INFO "%s: Auto-negotiation error.\n", dev->name);
- }
+
#ifdef JUMBO_FRAME
/* initialize JUMBO Frame control variable */
sp->Jumbo.FoundStart = 0;
@@ -1575,8 +1603,22 @@ #endif
IPG_MC_TX_ENABLE), ioaddr + IPG_MACCTRL);

netif_start_queue(dev);
+out:
+ return rc;

- return 0;
+err_release_tfdlist_4:
+ ipg_tx_clear(sp);
+err_release_rfdlist_3:
+ ipg_rx_clear(sp);
+err_free_tx_2:
+ pci_free_consistent(pdev, IPG_TX_RING_BYTES, sp->TFDList,
+ sp->TFDListDMAhandle);
+err_free_rx_1:
+ pci_free_consistent(pdev, IPG_RX_RING_BYTES, sp->RFDList,
+ sp->RFDListDMAhandle);
+err_free_irq_0:
+ free_irq(pdev->irq, dev);
+ goto out;
}

static int init_rfdlist(struct net_device *dev)
@@ -1593,13 +1635,16 @@ static int init_rfdlist(struct net_devic
sp->RxBuffNotReady = 0;

for (i = 0; i < IPG_RFDLIST_LENGTH; i++) {
+ if (!sp->RxBuff[i])
+ continue;
+
/* Free any allocated receive buffers. */
pci_unmap_single(sp->pdev,
sp->RxBuffDMAhandle[i].dmahandle,
sp->RxBuffDMAhandle[i].len,
PCI_DMA_FROMDEVICE);
- if (sp->RxBuff[i] != NULL)
- IPG_DEV_KFREE_SKB(sp->RxBuff[i]);
+
+ IPG_DEV_KFREE_SKB(sp->RxBuff[i]);
sp->RxBuff[i] = NULL;

/* Clear out the RFS field. */
@@ -1727,11 +1772,11 @@ static int ipg_get_rxbuff(struct net_dev

static int ipg_nic_stop(struct net_device *dev)
{
- /* Release resources requested by driver open function. */
+ struct ipg_nic_private *sp = netdev_priv(dev);
+ struct pci_dev *pdev = sp->pdev;

int i;
int error;
- struct ipg_nic_private *sp = netdev_priv(dev);

IPG_DEBUG_MSG("_nic_stop\n");

@@ -1752,40 +1797,20 @@ static int ipg_nic_stop(struct net_devic

error = ipg_reset(dev, i);
if (error < 0) {
+ // FIXME FIXME FIXME: giant leak alert
return error;
}

- /* Free all receive buffers. */
- for (i = 0; i < IPG_RFDLIST_LENGTH; i++) {
- pci_unmap_single(sp->pdev,
- sp->RxBuffDMAhandle[i].dmahandle,
- sp->RxBuffDMAhandle[i].len,
- PCI_DMA_FROMDEVICE);
- if (sp->RxBuff[i] != NULL)
- IPG_DEV_KFREE_SKB(sp->RxBuff[i]);
- sp->RxBuff[i] = NULL;
- }
+ ipg_rx_clear(sp);

- /* Free all transmit buffers. */
- for (i = 0; i < IPG_TFDLIST_LENGTH; i++) {
- if (sp->TxBuff[i] != NULL)
- IPG_DEV_KFREE_SKB(sp->TxBuff[i]);
- sp->TxBuff[i] = NULL;
- }
+ ipg_tx_clear(sp);

netif_stop_queue(dev);

- /* Free memory associated with the RFDList. */
- pci_free_consistent(sp->pdev,
- (sizeof(struct RFD) *
- IPG_RFDLIST_LENGTH),
- sp->RFDList, sp->RFDListDMAhandle);
-
- /* Free memory associated with the TFDList. */
- pci_free_consistent(sp->pdev,
- (sizeof(struct TFD) *
- IPG_TFDLIST_LENGTH),
- sp->TFDList, sp->TFDListDMAhandle);
+ pci_free_consistent(pdev, IPG_RX_RING_BYTES, sp->RFDList,
+ sp->RFDListDMAhandle);
+ pci_free_consistent(pdev, IPG_TX_RING_BYTES, sp->TFDList,
+ sp->TFDListDMAhandle);

/* Release interrupt line. */
free_irq(dev->irq, dev);
--
1.3.1

2006-05-02 00:36:44

by David Vrabel

[permalink] [raw]
Subject: Re: IP1000 gigabit nic driver

Lennert Buytenhek wrote:
> On Mon, May 01, 2006 at 10:38:47PM +0200, Francois Romieu wrote:
>
>
>>>-/* Minimum number of miliseconds used to toggle MDC clock during
>>>+/* Minimum number of nanoseconds used to toggle MDC clock during
>>> * MII/GMII register access.
>>> */
>>>-#define IPG_PC_PHYCTRLWAIT 0x01
>>>+#define IPG_PC_PHYCTRLWAIT_NS 200
>>
>>I would have expected a cycle of 400 ns (p.72/77 of the datasheet)
>>for a 2.5 MHz clock. Why is it cut by a two factor ?
>
>
> 200 ns high + 200 ns low = 400 ns clock period?

Yes.

David

2006-05-02 06:36:33

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH 1/3] ipg: removal of unreachable code

On Tue, 2 May 2006, Francois Romieu wrote:
> map/unmap is done in ipg_{probe/remove}

Applied. Thanks!

Pekka

2006-05-02 06:41:27

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH 2/3] ipg: leaks in ipg_probe

On Tue, 2 May 2006, Francois Romieu wrote:
> The error paths are badly broken.
>
> Bonus:
> - remove duplicate initialization of sp;
> - remove useless NULL initialization of dev;
> - USE_IO_OPS is not used (and the driver does not seem to care about
> posted writes, rejoice).
>
> Signed-off-by: Francois Romieu <[email protected]>

Applied. Thanks! One comment below.

On Tue, 2 May 2006, Francois Romieu wrote:
> - err = pci_request_regions(pdev, DRV_NAME);
> - if (err)
> - goto out;
> -
> - pio_start = pci_resource_start(pdev, 0) & 0xffffff80;
> - pio_len = pci_resource_len(pdev, 0);
> - mmio_start = pci_resource_start(pdev, 1) & 0xffffff80;
> + rc = pci_request_regions(pdev, DRV_NAME);
> + if (rc)
> + goto err_free_dev_1;

Is this tested with hardware? Alignment of the start address looks bogus
for sure, but any idea why they had it in the first place?

Pekka

2006-05-02 06:45:23

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH 3/3] ipg: plug leaks in the error path of ipg_nic_open

On Tue, 2 May 2006, Francois Romieu wrote:
> Added ipg_{rx/tx}_clear() to factor out some code.
>
> Signed-off-by: Francois Romieu <[email protected]>

Applied. Thanks!

Pekka

2006-05-02 18:36:06

by Francois Romieu

[permalink] [raw]
Subject: Re: [PATCH 2/3] ipg: leaks in ipg_probe

Pekka J Enberg <[email protected]> :
[...]
> Is this tested with hardware?

No.

> Alignment of the start address looks bogus for sure, but any idea
> why they had it in the first place?

No clear idea but it matches the significant part of the BAR register
for the 256 bytes of I/O space that the device uses.

--
Ueimor

2006-05-02 19:04:52

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH 2/3] ipg: leaks in ipg_probe

Pekka J Enberg <[email protected]> :
> [...]
> > Is this tested with hardware?

On Tue, 2006-05-02 at 20:33 +0200, Francois Romieu wrote:
> No.

On Tue, 2006-05-02 at 20:33 +0200, Francois Romieu wrote:
> > Alignment of the start address looks bogus for sure, but any idea
> > why they had it in the first place?

Pekka J Enberg <[email protected]> :
> No clear idea but it matches the significant part of the BAR register
> for the 256 bytes of I/O space that the device uses.

OK. David & David, would appreciate if either you could give the patch a
spin with Francois' changes. Thanks.

Pekka

2006-05-02 21:47:56

by Francois Romieu

[permalink] [raw]
Subject: [PATCH 2/2] ipg: redundancy with mii.h

Replace a bunch of #define with their counterpart from mii.h

Signed-off-by: Francois Romieu <[email protected]>

---

drivers/net/ipg.c | 82 ++++++++++++++++++++++-------------------------------
drivers/net/ipg.h | 29 -------------------
2 files changed, 34 insertions(+), 77 deletions(-)

ea93e36c70b16ab91340e320ed96df1df14ea608
diff --git a/drivers/net/ipg.c b/drivers/net/ipg.c
index ea7c1f8..5be2af1 100644
--- a/drivers/net/ipg.c
+++ b/drivers/net/ipg.c
@@ -15,6 +15,7 @@
* [email protected]
*/
#include <linux/crc32.h>
+#include <linux/mii.h>

#define IPG_RX_RING_BYTES (sizeof(struct RFD) * IPG_RFDLIST_LENGTH)
#define IPG_TX_RING_BYTES (sizeof(struct TFD) * IPG_TFDLIST_LENGTH)
@@ -465,7 +466,7 @@ static int ipg_tmi_fiber_detect(struct n

IPG_DEBUG_MSG("_tmi_fiber_detect\n");

- phyid = read_phy_register(dev, phyaddr, GMII_PHY_ID_1);
+ phyid = read_phy_register(dev, phyaddr, MII_PHYSID1);

IPG_DEBUG_MSG("PHY ID = %x\n", phyid);

@@ -492,7 +493,7 @@ static int ipg_find_phyaddr(struct net_d
GMII_PHY_ID1
*/

- status = read_phy_register(dev, phyaddr, GMII_PHY_STATUS);
+ status = read_phy_register(dev, phyaddr, MII_BMSR);

if ((status != 0xFFFF) && (status != 0))
return phyaddr;
@@ -600,20 +601,18 @@ #endif

IPG_DEBUG_MSG("GMII/MII PHY address = %x\n", phyaddr);

- status = read_phy_register(dev, phyaddr, GMII_PHY_STATUS);
+ status = read_phy_register(dev, phyaddr, MII_BMSR);

printk("PHYStatus = %x \n", status);
- if ((status & GMII_PHY_STATUS_AUTONEG_ABILITY) == 0) {
+ if ((status & BMSR_ANEGCAPABLE) == 0) {
printk(KERN_INFO
"%s: Error PHY unable to perform auto-negotiation.\n",
dev->name);
return -EILSEQ;
}

- advertisement = read_phy_register(dev, phyaddr,
- GMII_PHY_AUTONEGADVERTISEMENT);
- linkpartner_ability = read_phy_register(dev, phyaddr,
- GMII_PHY_AUTONEGLINKPARTABILITY);
+ advertisement = read_phy_register(dev, phyaddr, MII_ADVERTISE);
+ linkpartner_ability = read_phy_register(dev, phyaddr, MII_LPA);

printk("PHYadvertisement=%x LinkPartner=%x \n", advertisement,
linkpartner_ability);
@@ -634,20 +633,16 @@ #endif
* bits are logic 1 in both registers, configure the
* IPG for full duplex operation.
*/
- if ((advertisement & GMII_PHY_ADV_FULL_DUPLEX) ==
- (linkpartner_ability & GMII_PHY_ADV_FULL_DUPLEX)) {
+ if ((advertisement & ADVERTISE_1000XFULL) ==
+ (linkpartner_ability & ADVERTISE_1000XFULL)) {
fullduplex = 1;

/* In 1000BASE-X using IPG's internal PCS
* layer, so write to the GMII duplex bit.
*/
- write_phy_register(dev,
- phyaddr,
- GMII_PHY_CONTROL,
- read_phy_register
- (dev, phyaddr,
- GMII_PHY_CONTROL) |
- GMII_PHY_CONTROL_FULL_DUPLEX);
+ write_phy_register(dev, phyaddr, MII_BMCR,
+ read_phy_register(dev, phyaddr, MII_BMCR) |
+ ADVERTISE_1000HALF); // Typo ?

} else {
fullduplex = 0;
@@ -655,13 +650,9 @@ #endif
/* In 1000BASE-X using IPG's internal PCS
* layer, so write to the GMII duplex bit.
*/
- write_phy_register(dev,
- phyaddr,
- GMII_PHY_CONTROL,
- read_phy_register
- (dev, phyaddr,
- GMII_PHY_CONTROL) &
- ~GMII_PHY_CONTROL_FULL_DUPLEX);
+ write_phy_register(dev, phyaddr, MII_BMCR,
+ read_phy_register(dev, phyaddr, MII_BMCR) &
+ ~ADVERTISE_1000HALF); // Typo ?
}
}

@@ -672,21 +663,18 @@ #endif
* link partner abilities exchanged via next page
* transfers.
*/
- gigadvertisement = read_phy_register(dev,
- phyaddr,
- GMII_PHY_1000BASETCONTROL);
- giglinkpartner_ability = read_phy_register(dev,
- phyaddr,
- GMII_PHY_1000BASETSTATUS);
+ gigadvertisement =
+ read_phy_register(dev, phyaddr, MII_CTRL1000);
+ giglinkpartner_ability =
+ read_phy_register(dev, phyaddr, MII_STAT1000);

/* Compare the full duplex bits in the 1000BASE-T GMII
* registers for the local device, and the link partner.
* If these bits are logic 1 in both registers, configure
* the IPG for full duplex operation.
*/
- if ((gigadvertisement & GMII_PHY_1000BASETCONTROL_FULL_DUPLEX)
- && (giglinkpartner_ability &
- GMII_PHY_1000BASETSTATUS_FULL_DUPLEX)) {
+ if ((gigadvertisement & ADVERTISE_1000FULL) &&
+ (giglinkpartner_ability & ADVERTISE_1000FULL)) {
fullduplex = 1;
} else {
fullduplex = 0;
@@ -751,8 +739,12 @@ #endif
/* In full duplex mode, resolve PAUSE
* functionality.
*/
- switch (((advertisement & GMII_PHY_ADV_PAUSE) >> 5) |
- ((linkpartner_ability & GMII_PHY_ADV_PAUSE) >> 7)) {
+ u8 flow_ctl;
+#define LPA_PAUSE_ANY (LPA_1000XPAUSE_ASYM | LPA_1000XPAUSE)
+
+ flow_ctl = (advertisement & LPA_PAUSE_ANY) >> 5;
+ flow_ctl |= (linkpartner_ability & LPA_PAUSE_ANY) >> 7;
+ switch (flow_ctl) {
case 0x7:
txflowcontrol = 1;
rxflowcontrol = 0;
@@ -2682,26 +2674,20 @@ static int ipg_hw_init(struct net_device

if (phyaddr != -1) {
u16 mii_phyctrl, mii_1000cr;
- mii_1000cr = read_phy_register(dev,
- phyaddr,
- GMII_PHY_1000BASETCONTROL);
- write_phy_register(dev, phyaddr,
- GMII_PHY_1000BASETCONTROL,
- mii_1000cr |
- GMII_PHY_1000BASETCONTROL_FULL_DUPLEX |
- GMII_PHY_1000BASETCONTROL_HALF_DUPLEX |
+ mii_1000cr =
+ read_phy_register(dev, phyaddr, MII_CTRL1000);
+ write_phy_register(dev, phyaddr, MII_CTRL1000,
+ mii_1000cr | ADVERTISE_1000FULL | ADVERTISE_1000HALF |
GMII_PHY_1000BASETCONTROL_PreferMaster);

- mii_phyctrl = read_phy_register(dev, phyaddr, GMII_PHY_CONTROL);
+ mii_phyctrl = read_phy_register(dev, phyaddr, MII_BMCR);
/* Set default phyparam */
pci_read_config_byte(sp->pdev, PCI_REVISION_ID, &revisionid);
ipg_set_phy_default_param(revisionid, dev, phyaddr);

/* reset Phy */
- write_phy_register(dev,
- phyaddr, GMII_PHY_CONTROL,
- (mii_phyctrl | GMII_PHY_CONTROL_RESET |
- MII_PHY_CONTROL_RESTARTAN));
+ write_phy_register(dev, phyaddr, MII_BMCR,
+ (mii_phyctrl | BMCR_RESET | BMCR_ANRESTART));

}

diff --git a/drivers/net/ipg.h b/drivers/net/ipg.h
index 03bc6f1..cb51b2b 100644
--- a/drivers/net/ipg.h
+++ b/drivers/net/ipg.h
@@ -86,38 +86,9 @@ #define MII_PHY_TECHABILITY_ASM_
#define MII_PHY_TECHABILITY_RSVD2 0x1000
#define MII_PHY_STATUS_AUTONEG_ABILITY 0x0008

-/* NIC Physical Layer Device GMII register addresses. */
-#define GMII_PHY_CONTROL 0x00
-#define GMII_PHY_STATUS 0x01
-#define GMII_PHY_ID_1 0x02
-#define GMII_PHY_AUTONEGADVERTISEMENT 0x04
-#define GMII_PHY_AUTONEGLINKPARTABILITY 0x05
-#define GMII_PHY_AUTONEGEXPANSION 0x06
-#define GMII_PHY_AUTONEGNEXTPAGE 0x07
-#define GMII_PHY_AUTONEGLINKPARTNEXTPAGE 0x08
-#define GMII_PHY_EXTENDEDSTATUS 0x0F
-
-#define GMII_PHY_1000BASETCONTROL 0x09
-#define GMII_PHY_1000BASETSTATUS 0x0A
-
/* GMII_PHY_1000 need to set to prefer master */
#define GMII_PHY_1000BASETCONTROL_PreferMaster 0x0400

-#define GMII_PHY_1000BASETCONTROL_FULL_DUPLEX 0x0200
-#define GMII_PHY_1000BASETCONTROL_HALF_DUPLEX 0x0100
-#define GMII_PHY_1000BASETSTATUS_FULL_DUPLEX 0x0800
-#define GMII_PHY_1000BASETSTATUS_HALF_DUPLEX 0x0400
-
-/* NIC Physical Layer Device GMII register Fields. */
-#define GMII_PHY_CONTROL_RESET 0x8000
-#define GMII_PHY_CONTROL_FULL_DUPLEX 0x0100
-#define GMII_PHY_STATUS_AUTONEG_ABILITY 0x0008
-#define GMII_PHY_ADV_FULL_DUPLEX 0x0020
-#define GMII_PHY_ADV_HALF_DUPLEX 0x0040
-#define GMII_PHY_ADV_PAUSE 0x0180
-#define GMII_PHY_ADV_PAUSE_PS1 0x0080
-#define GMII_PHY_ADV_ASM_DIR_PS2 0x0100
-
/* NIC Physical Layer Device GMII constants. */
#define GMII_PREAMBLE 0xFFFFFFFF
#define GMII_ST 0x1
--
1.3.1

2006-05-02 21:47:59

by Francois Romieu

[permalink] [raw]
Subject: [PATCH 1/2] ipg: sanitize the pci device table

- vendor id belong to include/linux/pci_id.h ;
- the pci table does not include all the devices in nics_supported ;
- qualify the pci table as __devinitdata ;
- kill 50 LOC.

Signed-off-by: Francois Romieu <[email protected]>

---

drivers/net/ipg.c | 46 ++++++--------------------------
drivers/net/ipg.h | 67 +++++++++++------------------------------------
include/linux/pci_ids.h | 2 +
3 files changed, 27 insertions(+), 88 deletions(-)

aa5b322a6f91ca12a580dfa7022fca1a088b4abd
diff --git a/drivers/net/ipg.c b/drivers/net/ipg.c
index 393b617..ea7c1f8 100644
--- a/drivers/net/ipg.c
+++ b/drivers/net/ipg.c
@@ -32,40 +32,14 @@ MODULE_DESCRIPTION("IC Plus IP1000 Gigab
DrvVer);
MODULE_LICENSE("GPL");

-static struct pci_device_id ipg_pci_tbl[] = {
- {PCI_VENDOR_ID_ICPLUS,
- PCI_DEVICE_ID_IP1000,
- PCI_ANY_ID,
- PCI_ANY_ID,
- 0x020000,
- 0xFFFFFF,
- 0},
-
- {PCI_VENDOR_ID_SUNDANCE,
- PCI_DEVICE_ID_SUNDANCE_ST2021,
- PCI_ANY_ID,
- PCI_ANY_ID,
- 0x020000,
- 0xFFFFFF,
- 1},
-
- {PCI_VENDOR_ID_SUNDANCE,
- PCI_DEVICE_ID_TAMARACK_TC9020_9021,
- PCI_ANY_ID,
- PCI_ANY_ID,
- 0x020000,
- 0xFFFFFF,
- 2},
-
- {PCI_VENDOR_ID_DLINK,
- PCI_DEVICE_ID_DLINK_1002,
- PCI_ANY_ID,
- PCI_ANY_ID,
- 0x020000,
- 0xFFFFFF,
- 3},
-
- {0,}
+static struct pci_device_id ipg_pci_tbl[] __devinitdata = {
+ { PCI_DEVICE(PCI_VENDOR_ID_SUNDANCE, 0x1023), 0, 0, 0 },
+ { PCI_DEVICE(PCI_VENDOR_ID_SUNDANCE, 0x2021), 0, 0, 1 },
+ { PCI_DEVICE(PCI_VENDOR_ID_SUNDANCE, 0x1021), 0, 0, 2 },
+ { PCI_DEVICE(PCI_VENDOR_ID_DLINK, 0x9021), 0, 0, 3 },
+ { PCI_DEVICE(PCI_VENDOR_ID_DLINK, 0x4000), 0, 0, 4 },
+ { PCI_DEVICE(PCI_VENDOR_ID_DLINK, 0x4020), 0, 0, 5 },
+ { 0, }
};

MODULE_DEVICE_TABLE(pci, ipg_pci_tbl);
@@ -2961,9 +2935,7 @@ static int __devinit ipg_probe(struct pc
if (rc < 0)
goto out;

- printk(KERN_INFO "%s found.\n", nics_supported[i].NICname);
- printk(KERN_INFO "Bus %x Slot %x\n",
- pdev->bus->number, PCI_SLOT(pdev->devfn));
+ printk(KERN_INFO "%s: %s\n", pci_name(pdev), nics_supported[i].name);

pci_set_master(pdev);

diff --git a/drivers/net/ipg.h b/drivers/net/ipg.h
index ae3424a..03bc6f1 100644
--- a/drivers/net/ipg.h
+++ b/drivers/net/ipg.h
@@ -589,34 +589,6 @@ #define IPG_RI_RXDMAWAIT_TIME
* Tune
*/

-#ifndef PCI_VENDOR_ID_ICPLUS
-# define PCI_VENDOR_ID_ICPLUS 0x13F0
-#endif
-#ifndef PCI_DEVICE_ID_IP1000
-# define PCI_DEVICE_ID_IP1000 0x1023
-#endif
-#ifndef PCI_VENDOR_ID_SUNDANCE
-# define PCI_VENDOR_ID_SUNDANCE 0x13F0
-#endif
-#ifndef PCI_DEVICE_ID_SUNDANCE_IPG
-# define PCI_DEVICE_ID_SUNDANCE_ST2021 0x2021
-#endif
-#ifndef PCI_DEVICE_ID_TAMARACK_TC9020_9021_ALT
-# define PCI_DEVICE_ID_TAMARACK_TC9020_9021_ALT 0x9021
-#endif
-#ifndef PCI_DEVICE_ID_TAMARACK_TC9020_9021
-# define PCI_DEVICE_ID_TAMARACK_TC9020_9021 0x1021
-#endif
-#ifndef PCI_VENDOR_ID_DLINK
-# define PCI_VENDOR_ID_DLINK 0x1186
-#endif
-#ifndef PCI_DEVICE_ID_DLINK_1002
-# define PCI_DEVICE_ID_DLINK_1002 0x4000
-#endif
-#ifndef PCI_DEVICE_ID_DLINK_IP1000A
-# define PCI_DEVICE_ID_DLINK_IP1000A 0x4020
-#endif
-
/* Miscellaneous Constants. */
#define TRUE 1
#define FALSE 0
@@ -990,32 +962,25 @@ #endif
};

struct nic_id {
- char *NICname;
- int vendorid;
- int deviceid;
+ char *name;
+ u32 vendor;
+ u32 device;
};

struct nic_id nics_supported[] = {
- {"IC PLUS IP1000 1000/100/10 based NIC",
- PCI_VENDOR_ID_ICPLUS,
- PCI_DEVICE_ID_IP1000},
- {"Sundance Technology ST2021 based NIC",
- PCI_VENDOR_ID_SUNDANCE,
- PCI_DEVICE_ID_SUNDANCE_ST2021},
- {"Tamarack Microelectronics TC9020/9021 based NIC",
- PCI_VENDOR_ID_SUNDANCE,
- PCI_DEVICE_ID_TAMARACK_TC9020_9021},
- {"Tamarack Microelectronics TC9020/9021 based NIC",
- PCI_VENDOR_ID_SUNDANCE,
- PCI_DEVICE_ID_TAMARACK_TC9020_9021_ALT},
- {"D-Link NIC",
- PCI_VENDOR_ID_DLINK,
- PCI_DEVICE_ID_DLINK_1002},
- {"D-Link NIC IP1000A",
- PCI_VENDOR_ID_DLINK,
- PCI_DEVICE_ID_DLINK_IP1000A},
-
- {"N/A", 0xFFFF, 0}
+ { "IC PLUS IP1000 1000/100/10 based NIC",
+ PCI_VENDOR_ID_SUNDANCE, 0x1023 },
+ { "Sundance Technology ST2021 based NIC",
+ PCI_VENDOR_ID_SUNDANCE, 0x2021 },
+ { "Tamarack Microelectronics TC9020/9021 based NIC",
+ PCI_VENDOR_ID_SUNDANCE, 0x1021 },
+ { "Tamarack Microelectronics TC9020/9021 based NIC",
+ PCI_VENDOR_ID_SUNDANCE, 0x9021 },
+ { "D-Link NIC",
+ PCI_VENDOR_ID_DLINK, 0x4000 },
+ { "D-Link NIC IP1000A",
+ PCI_VENDOR_ID_DLINK, 0x4020 },
+ { NULL, 0xffff, 0 }
};

//variable record -- index by leading revision/length
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index d6fe048..b772f2b 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -1788,6 +1788,8 @@ #define PCI_DEVICE_ID_IOMEGA_BUZ 0x4231
#define PCI_VENDOR_ID_ABOCOM 0x13D1
#define PCI_DEVICE_ID_ABOCOM_2BD1 0x2BD1

+#define PCI_VENDOR_ID_SUNDANCE 0x13f0
+
#define PCI_VENDOR_ID_CMEDIA 0x13f6
#define PCI_DEVICE_ID_CMEDIA_CM8338A 0x0100
#define PCI_DEVICE_ID_CMEDIA_CM8338B 0x0101
--
1.3.1

2006-05-02 21:59:55

by Francois Romieu

[permalink] [raw]
Subject: Re: [PATCH 2/2] ipg: redundancy with mii.h

Btw the whole serie is available in branch 'netdev-ipg' at:
git://electric-eye.fr.zoreil.com/home/romieu/linux-2.6.git

The interim steps may be useful if testing reveals something wrong
(especially if it happens in a few weeks/months).

$ git rev-list --pretty ebf34c9b6fcd22338ef764b039b3ac55ed0e297b..HEAD

commit 8a98963033425729158d48066a3380f811c711b3
Author: Romieu Francois <[email protected]>
Date: Tue May 2 23:25:44 2006 +0200

ipg: redundancy with mii.h

Replace a bunch of #define with their counterpart from mii.h

Signed-off-by: Francois Romieu <[email protected]>

commit 291360d4000e0b93baf0fb97aa15af48677e46af
Author: Romieu Francois <[email protected]>
Date: Tue May 2 22:15:34 2006 +0200

ipg: sanitize the pci device table

- vendor id belong to include/linux/pci_id.h ;
- the pci table does not include all the devices in nics_supported ;
- qualify the pci table as __devinitdata ;
- kill 50 LOC.

Signed-off-by: Francois Romieu <[email protected]>

commit 8b534f66d6be247b9f0d341b0ae7acbf46f128fb
Author: Romieu Francois <[email protected]>
Date: Tue May 2 01:07:48 2006 +0200

ipg: plug leaks in the error path of ipg_nic_open

Added ipg_{rx/tx}_clear() to factor out some code.

Signed-off-by: Francois Romieu <[email protected]>

commit 87c8b13dceccc42439e7262f1b60c9cbb14d5440
Author: Romieu Francois <[email protected]>
Date: Tue May 2 00:15:54 2006 +0200

ipg: leaks in ipg_probe

The error paths are badly broken.

Bonus:
- remove duplicate initialization of sp;
- remove useless NULL initialization of dev;
- USE_IO_OPS is not used (and the driver does not seem to care about
posted writes, rejoice).

Signed-off-by: Francois Romieu <[email protected]>

commit befd7e543fcdfb2d2c75de1748ae751fefdff58a
Author: Romieu Francois <[email protected]>
Date: Mon May 1 23:52:37 2006 +0200

ipg: removal of unreachable code

map/unmap is done in ipg_{probe/remove}

Signed-off-by: Francois Romieu <[email protected]>

commit 17a9ce93ba6b6744489aa9168d757e9457158952
Author: Romieu Francois <[email protected]>
Date: Mon May 1 22:40:29 2006 +0200

ipg: speed-up access to the PHY registers

Reduce delays when reading/writing the PHY registers so we clock the
MII management interface at 2.5 MHz (the maximum according to the
datasheet) instead of 500 Hz.

Signed-off-by: David Vrabel <[email protected]>

commit e18c33d6fa62b735426b8fe5a0f1aa61c0b5d7f7
Author: David Vrabel <[email protected]>
Date: Mon May 1 21:34:19 2006 +0200

ipg: root_dev removal and PHY initialization

- Remove ether_crc_le() -- use crc32_le() instead.
- No more nonsense with root_dev -- ipg_remove() now works.
- Move PHY and MAC address initialization into the ipg_probe().
It was previously filling in the MAC address on open which breaks
some user space.
- Folded ipg_nic_init into ipg_probe since it was broke otherwise.

Signed-off-by: David Vrabel <[email protected]>

commit e99fcd4253f231b2c7a96e3be5067341de45ac2e
Author: David Vrabel <[email protected]>
Date: Mon May 1 13:20:49 2006 +0200

ipg: remove changelogs

Signed-off-by: David Vrabel <[email protected]>

commit 8fd59026a272d3132f096965985e907d655ee087
Author: Pekka Enberg <[email protected]>
Date: Mon May 1 12:53:43 2006 +0200

ipg: initial inclusion of IC Plus IP1000 driver

This is a cleaned up fork of the IP1000A device driver:

<http://www.icplus.com.tw/driver-pp-IP1000A.html>

Open issues include but are not limited to:

- ipg_probe() looks really fishy and doesn't handle all errors
(e.g. ioremap failing).
- ipg_nic_do_ioctl() is playing games with user-space pointer.
We should use ethtool ioctl instead as suggested by Arjan.
- For multiple devices, the driver uses a global root_dev and
ipg_remove() play some tricks which look fishy.

Signed-off-by: Pekka Enberg <[email protected]>

2006-05-02 22:29:19

by David Gómez

[permalink] [raw]
Subject: [PATCH] ipg: removing more dead code

Hi Pekka,

On May 02 at 10:04:47, Pekka Enberg wrote:
> OK. David & David, would appreciate if either you could give the patch a
> spin with Francois' changes. Thanks.

I'll test it tomorrow ASAP. For now, here is another patch removing
more dead code. This code is never reached (NOTGRACE is not defined)
and the *fiber_detect functions are subsequently never used.

--- drivers/net/ipg.c.old 2006-05-03 00:16:47.000000000 +0200
+++ drivers/net/ipg.c 2006-05-03 00:28:02.000000000 +0200
@@ -461,43 +461,6 @@
return 0;
}

-#ifdef IPG_TMI_FIBER_DETECT
-static int ipg_sti_fiber_detect(struct net_device *dev)
-{
- /* Determine if NIC is fiber based by reading the PhyMedia
- * bit in the AsicCtrl register.
- */
-
- u32 asicctrl;
- void __iomem *ioaddr = ipg_ioaddr(dev);
-
- IPG_DEBUG_MSG("_sti_fiber_detect\n");
-
- asicctrl = ioread32(ioaddr + IPG_ASICCTRL);
-
- return asicctrl & IPG_AC_PHY_MEDIA;
-}
-
-static int ipg_tmi_fiber_detect(struct net_device *dev, int phyaddr)
-{
- /* Determine if NIC is fiber based by reading the ID register
- * of the PHY and the GMII address.
- */
-
- u16 phyid;
-
- IPG_DEBUG_MSG("_tmi_fiber_detect\n");
-
- phyid = read_phy_register(dev, phyaddr, GMII_PHY_ID_1);
-
- IPG_DEBUG_MSG("PHY ID = %x\n", phyid);
-
- /* We conclude the mode is fiber if the GMII address
- * is 0x1 and the PHY ID is 0x0000.
- */
- return (phyaddr == 0x1) && (phyid == 0x0000);
-}
-#endif

static int ipg_find_phyaddr(struct net_device *dev)
{
@@ -524,434 +487,6 @@
return -1;
}

-#ifdef NOTGRACE
-static int ipg_config_autoneg(struct net_device *dev)
-{
- /* Configure IPG based on result of IEEE 802.3 PHY
- * auto-negotiation.
- */
-
- int phyaddr = 0;
- u8 phyctrl;
- u32 asicctrl;
- void __iomem *ioaddr = ipg_ioaddr(dev);
- u16 status = 0;
- u16 advertisement;
- u16 linkpartner_ability;
- u16 gigadvertisement;
- u16 giglinkpartner_ability;
- u16 techabilities;
- int fiber;
- int gig;
- int fullduplex;
- int txflowcontrol;
- int rxflowcontrol;
- struct ipg_nic_private *sp = netdev_priv(dev);
-
- IPG_DEBUG_MSG("_config_autoneg\n");
-
- asicctrl = ioread32(ioaddr + IPG_ASICCTRL);
- phyctrl = ioread8(ioaddr + IPG_PHYCTRL);
-
- /* Set flags for use in resolving auto-negotation, assuming
- * non-1000Mbps, half duplex, no flow control.
- */
- fiber = 0;
- fullduplex = 0;
- txflowcontrol = 0;
- rxflowcontrol = 0;
- gig = 0;
-
- /* To accomodate a problem in 10Mbps operation,
- * set a global flag if PHY running in 10Mbps mode.
- */
- sp->tenmbpsmode = 0;
-
- printk("Link speed = ");
-
- /* Determine actual speed of operation. */
- switch (phyctrl & IPG_PC_LINK_SPEED) {
- case IPG_PC_LINK_SPEED_10MBPS:
- printk("10Mbps.\n");
- printk(KERN_INFO "%s: 10Mbps operational mode enabled.\n",
- dev->name);
- sp->tenmbpsmode = 1;
- break;
- case IPG_PC_LINK_SPEED_100MBPS:
- printk("100Mbps.\n");
- break;
- case IPG_PC_LINK_SPEED_1000MBPS:
- printk("1000Mbps.\n");
- gig = 1;
- break;
- default:
- printk("undefined!\n");
- }
-
-#ifndef IPG_TMI_FIBER_DETECT
- fiber = ipg_sti_fiber_detect(dev);
-
- /* Determine if auto-negotiation resolution is necessary.
- * First check for fiber based media 10/100 media.
- */
- if ((fiber == 1) && (asicctrl &
- (IPG_AC_PHY_SPEED10 | IPG_AC_PHY_SPEED100))) {
- printk(KERN_INFO
- "%s: Fiber based PHY, setting full duplex, no flow control.\n",
- dev->name);
- return -EILSEQ;
- iowrite32(IPG_MC_RSVD_MASK &
- ((ioread32(ioaddr + IPG_MACCTRL) |
- IPG_MC_DUPLEX_SELECT_FD) &
- ~IPG_MC_TX_FLOW_CONTROL_ENABLE &
- ~IPG_MC_RX_FLOW_CONTROL_ENABLE),
- ioaddr + IPG_MACCTRL);
-
- return 0;
- }
-#endif
-
- /* Determine if PHY is auto-negotiation capable. */
- phyaddr = ipg_find_phyaddr(dev);
-
- if (phyaddr == -1) {
- printk(KERN_INFO
- "%s: Error on read to GMII/MII Status register.\n",
- dev->name);
- return -EILSEQ;
- }
-
- IPG_DEBUG_MSG("GMII/MII PHY address = %x\n", phyaddr);
-
- status = read_phy_register(dev, phyaddr, GMII_PHY_STATUS);
-
- printk("PHYStatus = %x \n", status);
- if ((status & GMII_PHY_STATUS_AUTONEG_ABILITY) == 0) {
- printk(KERN_INFO
- "%s: Error PHY unable to perform auto-negotiation.\n",
- dev->name);
- return -EILSEQ;
- }
-
- advertisement = read_phy_register(dev, phyaddr,
- GMII_PHY_AUTONEGADVERTISEMENT);
- linkpartner_ability = read_phy_register(dev, phyaddr,
- GMII_PHY_AUTONEGLINKPARTABILITY);
-
- printk("PHYadvertisement=%x LinkPartner=%x \n", advertisement,
- linkpartner_ability);
- if ((advertisement == 0xFFFF) || (linkpartner_ability == 0xFFFF)) {
- printk(KERN_INFO
- "%s: Error on read to GMII/MII registers 4 and/or 5.\n",
- dev->name);
- return -EILSEQ;
- }
-#ifdef IPG_TMI_FIBER_DETECT
- fiber = ipg_tmi_fiber_detect(dev, phyaddr);
-#endif
-
- /* Resolve full/half duplex if 1000BASE-X. */
- if ((gig == 1) && (fiber == 1)) {
- /* Compare the full duplex bits in the GMII registers
- * for the local device, and the link partner. If these
- * bits are logic 1 in both registers, configure the
- * IPG for full duplex operation.
- */
- if ((advertisement & GMII_PHY_ADV_FULL_DUPLEX) ==
- (linkpartner_ability & GMII_PHY_ADV_FULL_DUPLEX)) {
- fullduplex = 1;
-
- /* In 1000BASE-X using IPG's internal PCS
- * layer, so write to the GMII duplex bit.
- */
- write_phy_register(dev,
- phyaddr,
- GMII_PHY_CONTROL,
- read_phy_register
- (dev, phyaddr,
- GMII_PHY_CONTROL) |
- GMII_PHY_CONTROL_FULL_DUPLEX);
-
- } else {
- fullduplex = 0;
-
- /* In 1000BASE-X using IPG's internal PCS
- * layer, so write to the GMII duplex bit.
- */
- write_phy_register(dev,
- phyaddr,
- GMII_PHY_CONTROL,
- read_phy_register
- (dev, phyaddr,
- GMII_PHY_CONTROL) &
- ~GMII_PHY_CONTROL_FULL_DUPLEX);
- }
- }
-
- /* Resolve full/half duplex if 1000BASE-T. */
- if ((gig == 1) && (fiber == 0)) {
- /* Read the 1000BASE-T "Control" and "Status"
- * registers which represent the advertised and
- * link partner abilities exchanged via next page
- * transfers.
- */
- gigadvertisement = read_phy_register(dev,
- phyaddr,
- GMII_PHY_1000BASETCONTROL);
- giglinkpartner_ability = read_phy_register(dev,
- phyaddr,
- GMII_PHY_1000BASETSTATUS);
-
- /* Compare the full duplex bits in the 1000BASE-T GMII
- * registers for the local device, and the link partner.
- * If these bits are logic 1 in both registers, configure
- * the IPG for full duplex operation.
- */
- if ((gigadvertisement & GMII_PHY_1000BASETCONTROL_FULL_DUPLEX)
- && (giglinkpartner_ability &
- GMII_PHY_1000BASETSTATUS_FULL_DUPLEX)) {
- fullduplex = 1;
- } else {
- fullduplex = 0;
- }
- }
-
- /* Resolve full/half duplex for 10/100BASE-T. */
- if (gig == 0) {
- /* Autonegotiation Priority Resolution algorithm, as defined in
- * IEEE 802.3 Annex 28B.
- */
- if (((advertisement & MII_PHY_SELECTORFIELD) ==
- MII_PHY_SELECTOR_IEEE8023) &&
- ((linkpartner_ability & MII_PHY_SELECTORFIELD) ==
- MII_PHY_SELECTOR_IEEE8023)) {
- techabilities = (advertisement & linkpartner_ability &
- MII_PHY_TECHABILITYFIELD);
-
- fullduplex = 0;
-
- /* 10BASE-TX half duplex is lowest priority. */
- if (techabilities & MII_PHY_TECHABILITY_10BT) {
- fullduplex = 0;
- }
-
- if (techabilities & MII_PHY_TECHABILITY_10BTFD) {
- fullduplex = 1;
- }
-
- if (techabilities & MII_PHY_TECHABILITY_100BTX) {
- fullduplex = 0;
- }
-
- if (techabilities & MII_PHY_TECHABILITY_100BT4) {
- fullduplex = 0;
- }
-
- /* 100BASE-TX half duplex is highest priority. *///Sorbica full duplex ?
- if (techabilities & MII_PHY_TECHABILITY_100BTXFD) {
- fullduplex = 1;
- }
-
- if (fullduplex == 1) {
- /* If in full duplex mode, determine if PAUSE
- * functionality is supported by the local
- * device, and the link partner.
- */
- if (techabilities & MII_PHY_TECHABILITY_PAUSE) {
- txflowcontrol = 1;
- rxflowcontrol = 1;
- } else {
- txflowcontrol = 0;
- rxflowcontrol = 0;
- }
- }
- }
- }
-
- /* If in 1000Mbps, fiber, and full duplex mode, resolve
- * 1000BASE-X PAUSE capabilities. */
- if ((fullduplex == 1) && (fiber == 1) && (gig == 1)) {
- /* In full duplex mode, resolve PAUSE
- * functionality.
- */
- switch (((advertisement & GMII_PHY_ADV_PAUSE) >> 5) |
- ((linkpartner_ability & GMII_PHY_ADV_PAUSE) >> 7)) {
- case 0x7:
- txflowcontrol = 1;
- rxflowcontrol = 0;
- break;
-
- case 0xA:
- case 0xB:
- case 0xE:
- case 0xF:
- txflowcontrol = 1;
- rxflowcontrol = 1;
- break;
-
- case 0xD:
- txflowcontrol = 0;
- rxflowcontrol = 1;
- break;
-
- default:
- txflowcontrol = 0;
- rxflowcontrol = 0;
- }
- }
-
- /* If in 1000Mbps, non-fiber, full duplex mode, resolve
- * 1000BASE-T PAUSE capabilities. */
- if ((fullduplex == 1) && (fiber == 0) && (gig == 1)) {
- /* Make sure the PHY is advertising we are PAUSE
- * capable.
- */
- if (!(advertisement & (MII_PHY_TECHABILITY_PAUSE |
- MII_PHY_TECHABILITY_ASM_DIR))) {
- /* PAUSE is not being advertised. Advertise
- * PAUSE and restart auto-negotiation.
- */
- write_phy_register(dev,
- phyaddr,
- MII_PHY_AUTONEGADVERTISEMENT,
- (advertisement |
- MII_PHY_TECHABILITY_PAUSE |
- MII_PHY_TECHABILITY_ASM_DIR));
- write_phy_register(dev,
- phyaddr,
- MII_PHY_CONTROL,
- MII_PHY_CONTROL_RESTARTAN);
-
- return -EAGAIN;
- }
-
- /* In full duplex mode, resolve PAUSE
- * functionality.
- */
- switch (((advertisement &
- MII_PHY_TECHABILITY_PAUSE_FIELDS) >> 0x8) |
- ((linkpartner_ability &
- MII_PHY_TECHABILITY_PAUSE_FIELDS) >> 0xA)) {
- case 0x7:
- txflowcontrol = 1;
- rxflowcontrol = 0;
- break;
-
- case 0xA:
- case 0xB:
- case 0xE:
- case 0xF:
- txflowcontrol = 1;
- rxflowcontrol = 1;
- break;
-
- case 0xD:
- txflowcontrol = 0;
- rxflowcontrol = 1;
- break;
-
- default:
- txflowcontrol = 0;
- rxflowcontrol = 0;
- }
- }
-
- /* If in 10/100Mbps, non-fiber, full duplex mode, assure
- * 10/100BASE-T PAUSE capabilities are advertised. */
- if ((fullduplex == 1) && (fiber == 0) && (gig == 0)) {
- /* Make sure the PHY is advertising we are PAUSE
- * capable.
- */
- if (!(advertisement & (MII_PHY_TECHABILITY_PAUSE))) {
- /* PAUSE is not being advertised. Advertise
- * PAUSE and restart auto-negotiation.
- */
- write_phy_register(dev,
- phyaddr,
- MII_PHY_AUTONEGADVERTISEMENT,
- (advertisement |
- MII_PHY_TECHABILITY_PAUSE));
- write_phy_register(dev,
- phyaddr,
- MII_PHY_CONTROL,
- MII_PHY_CONTROL_RESTARTAN);
-
- return -EAGAIN;
- }
-
- }
-
- if (fiber == 1) {
- printk(KERN_INFO "%s: Fiber based PHY, ", dev->name);
- } else {
- printk(KERN_INFO "%s: Copper based PHY, ", dev->name);
- }
-
- /* Configure full duplex, and flow control. */
- if (fullduplex == 1) {
- /* Configure IPG for full duplex operation. */
- printk("setting full duplex, ");
-
- iowrite32(IPG_MC_RSVD_MASK &
- (ioread32(ioaddr + IPG_MACCTRL) |
- IPG_MC_DUPLEX_SELECT_FD), ioaddr + IPG_MACCTRL);
-
- if (txflowcontrol == 1) {
- printk("TX flow control");
- iowrite32(IPG_MC_RSVD_MASK &
- (ioread32(ioaddr + IPG_MACCTRL) |
- IPG_MC_TX_FLOW_CONTROL_ENABLE),
- ioaddr + IPG_MACCTRL);
- } else {
- printk("no TX flow control");
- iowrite32(IPG_MC_RSVD_MASK &
- (ioread32(ioaddr + IPG_MACCTRL) &
- ~IPG_MC_TX_FLOW_CONTROL_ENABLE),
- ioaddr + IPG_MACCTRL);
- }
-
- if (rxflowcontrol == 1) {
- printk(", RX flow control.");
- iowrite32(IPG_MC_RSVD_MASK &
- (ioread32(ioaddr + IPG_MACCTRL) |
- IPG_MC_RX_FLOW_CONTROL_ENABLE),
- ioaddr + IPG_MACCTRL);
- } else {
- printk(", no RX flow control.");
- iowrite32(IPG_MC_RSVD_MASK &
- (ioread32(ioaddr + IPG_MACCTRL) &
- ~IPG_MC_RX_FLOW_CONTROL_ENABLE),
- ioaddr + IPG_MACCTRL);
- }
-
- printk("\n");
- } else {
- /* Configure IPG for half duplex operation. */
- printk
- ("setting half duplex, no TX flow control, no RX flow control.\n");
-
- iowrite32(IPG_MC_RSVD_MASK &
- (ioread32(ioaddr + IPG_MACCTRL) &
- ~IPG_MC_DUPLEX_SELECT_FD &
- ~IPG_MC_TX_FLOW_CONTROL_ENABLE &
- ~IPG_MC_RX_FLOW_CONTROL_ENABLE),
- ioaddr + IPG_MACCTRL);
- }
-
- IPG_DEBUG_MSG("G/MII reg 4 (advertisement) = %4.4x\n", advertisement);
- IPG_DEBUG_MSG("G/MII reg 5 (link partner) = %4.4x\n",
- linkpartner_ability);
- IPG_DEBUG_MSG("G/MII reg 9 (1000BASE-T control) = %4.4x\n",
- advertisement);
- IPG_DEBUG_MSG("G/MII reg 10 (1000BASE-T status) = %4.4x\n",
- linkpartner_ability);
-
- IPG_DEBUG_MSG("Auto-neg complete, MACCTRL = %8.8x\n",
- ioread32(ioaddr + IPG_MACCTRL));
-
- return 0;
-}
-#else
static int ipg_config_autoneg(struct net_device *dev)
{
/* Configure IPG based on result of IEEE 802.3 PHY
@@ -1072,7 +607,6 @@
return 0;
}

-#endif

static int ipg_io_config(struct net_device *dev)
{

2006-05-03 06:16:40

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH 2/2] ipg: redundancy with mii.h

Hi Francois,

On Tue, 2 May 2006, Francois Romieu wrote:
> Btw the whole serie is available in branch 'netdev-ipg' at:
> git://electric-eye.fr.zoreil.com/home/romieu/linux-2.6.git
>
> The interim steps may be useful if testing reveals something wrong
> (especially if it happens in a few weeks/months).

I have all the interim steps in a private git tree also. I asked for a
kernel.org account so I could publish the tree. However, if you wish to
maintain the tree, I can send you my patches so you can recreate the full
history. The first steps were produced by quilt on the original
out-of-tree driver, though, so they're probably not helpful.

Pekka

2006-05-03 13:12:34

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH] ipg: removing more dead code

Hi David,

On Wed, 3 May 2006, David G?mezz wrote:
> I'll test it tomorrow ASAP. For now, here is another patch removing
> more dead code. This code is never reached (NOTGRACE is not defined)
> and the *fiber_detect functions are subsequently never used.

No need to resend this one, but in future, please follow the proper patch
format:

http://www.zipworld.com.au/~akpm/linux/patches/stuff/tpp.txt

Pekka

2006-05-03 20:58:47

by David Gómez

[permalink] [raw]
Subject: Re: [PATCH 2/3] ipg: leaks in ipg_probe

Hi Pekka,

On May 02 at 10:04:47, Pekka Enberg wrote:
> > No clear idea but it matches the significant part of the BAR register
> > for the 256 bytes of I/O space that the device uses.
>
> OK. David & David, would appreciate if either you could give the patch a
> spin with Francois' changes. Thanks.

I applied latest Francois patch and the changes (and specially the dma
changes) seems to be ok.

On the other hand i'm having some problems. Nothing related to the
patches, because it happens with the original driver too: After some
time using it, the driver becomes unresponsive and i need to restart
the interface and/or unload-load the ipg module. I'd need to compile
it with debug enabled when i have some time to see what it's going
on...

cheers,

--
David Gómez Jabber ID: [email protected]

2006-05-03 23:39:17

by Francois Romieu

[permalink] [raw]
Subject: Re: [PATCH 2/2] ipg: redundancy with mii.h

Pekka J Enberg <[email protected]> :
[...]
> maintain the tree, I can send you my patches so you can recreate the full
> history. The first steps were produced by quilt on the original
> out-of-tree driver, though, so they're probably not helpful.

It will be welcome.

I have added a few little things (changelog below). Next step will
probably be some mii/ethtool stuff.

The branch 'netdev-ipg' is available at:
git://electric-eye.fr.zoreil.com/home/romieu/linux-2.6.git.

Serie of patches (ala 'git format-patch'):
http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.17-rc3/ip1000a/

All-in-one patch:
http://www.fr.zoreil.com/people/francois/misc/20060504-2.6.17-rc3-git-ip1000-test.patch

ChangeLog from yesterday version:

commit 8b0a8db32d1ac6e9bc23300a6a0533b4d7e251e3
Author: Francois Romieu <[email protected]>
Date: Thu May 4 00:29:59 2006 +0200

ipg: remove forward declarations

It makes no sense in a new driver.

Signed-off-by: Francois Romieu <[email protected]>

commit 65940e5e0ab88d92fbac0f96b5d46ddfbd5cfa93
Author: Francois Romieu <[email protected]>
Date: Thu May 4 00:04:57 2006 +0200

ipg: replace #define with enum

Added some underscores to improve readability.

Signed-off-by: Francois Romieu <[email protected]>

commit ab87a106690d6eaba4b7426fb074270e2e503e40
Author: Francois Romieu <[email protected]>
Date: Wed May 3 22:51:16 2006 +0200

ipg: removal of useless #defines

IPG_TX_NOTBUSY apart (one occurence in ipg.c), the #defines appear
nowhere in the sources.

Signed-off-by: Francois Romieu <[email protected]>

commit ef7bfd886bc436d14649e962edb6f5189cc4dcac
Author: Francois Romieu <[email protected]>
Date: Wed May 3 22:44:47 2006 +0200

ipg: redundancy with mii.h - take II

Replace a bunch of #define with their counterpart from mii.h

It is applied to the usual MII registers this time.

Signed-off-by: Francois Romieu <[email protected]>

--
Ueimor

2006-05-04 06:52:24

by David Vrabel

[permalink] [raw]
Subject: Re: [PATCH 2/2] ipg: redundancy with mii.h

Francois Romieu wrote:
>
> ipg: remove forward declarations
>
> It makes no sense in a new driver.
>
> Signed-off-by: Francois Romieu <[email protected]>

Ack.

> ipg: replace #define with enum
>
> Added some underscores to improve readability.
>
> Signed-off-by: Francois Romieu <[email protected]>

Nack. Register names in code should match those used in the
documentation (even if they are a bit unreadable). Though I will
conceed that the available datasheet doesn't actually describe the
majority of the registers.

> ipg: removal of useless #defines
>
> IPG_TX_NOTBUSY apart (one occurence in ipg.c), the #defines appear
> nowhere in the sources.

Ack.

> ipg: redundancy with mii.h - take II
>
> Replace a bunch of #define with their counterpart from mii.h
>
> It is applied to the usual MII registers this time.

Ack.

2006-05-04 13:45:10

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH 2/2] ipg: redundancy with mii.h

Pekka J Enberg <[email protected]> :
> [...]
> > maintain the tree, I can send you my patches so you can recreate the full
> > history. The first steps were produced by quilt on the original
> > out-of-tree driver, though, so they're probably not helpful.

On Thu, 2006-05-04 at 01:35 +0200, Francois Romieu wrote:
> It will be welcome.

My initial patches are here:

http://www.cs.helsinki.fi/u/penberg/linux/ip1000/

I consider your tree the master now. Please let me know when you have
recreated the history, so I can clone your tree. Thanks.

Pekka

2006-05-04 23:59:33

by Francois Romieu

[permalink] [raw]
Subject: Re: [PATCH 2/2] ipg: redundancy with mii.h

Pekka Enberg <[email protected]> :
[...]
> My initial patches are here:
>
> http://www.cs.helsinki.fi/u/penberg/linux/ip1000/

Nice. Thanks.

> I consider your tree the master now. Please let me know when you have
> recreated the history, so I can clone your tree. Thanks.

The new serie is available in branch 'netdev-ipg' at
git://electric-eye.fr.zoreil.com/home/romieu/linux-2.6.git

Former branch 'netdev-ipg' was moved to 'netdev-ipg.log'.

I have added a new patch (see (way) below).

$ git log master..

commit 17adeb85054a693224a2ab7787a224c722bdd4eb
Author: Francois Romieu <[email protected]>
Date: Fri May 5 01:42:12 2006 +0200

ip1000: ethtool {get/set}_settings support

The patch shoehorns the driver into the usual mii/ethtool framework.
mii_mutex will prove useful when the link management tasks issued in
irq context are moved to user/workqueue context. Next step.

At least the current code should not be _too_ broken.

Signed-off-by: Francois Romieu <[email protected]>

commit 997a02f4c4b3cf2ec0177ba33ffda6d550396eb7
Author: Francois Romieu <[email protected]>
Date: Thu May 4 00:29:59 2006 +0200

ip1000: remove forward declarations

It makes no sense in a new driver.

Signed-off-by: Francois Romieu <[email protected]>

commit b141c4ec3572f3151b2eb037ef5e7aba8d190445
Author: Francois Romieu <[email protected]>
Date: Thu May 4 00:04:57 2006 +0200

ip1000: replace #define with enum

Added some underscores to improve readability.

Signed-off-by: Francois Romieu <[email protected]>

commit 9d27e1ab51b2295d72ea254bbc8de5b5797a4139
Author: Francois Romieu <[email protected]>
Date: Wed May 3 22:51:16 2006 +0200

ip1000: removal of useless #defines

IPG_TX_NOTBUSY apart (one occurence in ipg.c), the #defines appear
nowhere in the sources.

Signed-off-by: Francois Romieu <[email protected]>

commit 42ad0c39990c1897ee56dfc34bb4a7b2de76ace0
Author: Francois Romieu <[email protected]>
Date: Wed May 3 22:44:47 2006 +0200

ip1000: redundancy with mii.h - take II

Replace a bunch of #define with their counterpart from mii.h

It is applied to the usual MII registers this time.

Signed-off-by: Francois Romieu <[email protected]>

commit be97a643f5d955208c0be7aa9e16a05c7737a2be
Author: Romieu Francois <[email protected]>
Date: Tue May 2 23:25:44 2006 +0200

ip1000: redundancy with mii.h

Replace a bunch of #define with their counterpart from mii.h

Signed-off-by: Francois Romieu <[email protected]>

commit 273a5a5f0815a35cf06faeafea6fc47503d364cb
Author: Romieu Francois <[email protected]>
Date: Tue May 2 22:15:34 2006 +0200

ip1000: sanitize the pci device table

- vendor id belong to include/linux/pci_id.h ;
- the pci table does not include all the devices in nics_supported ;
- qualify the pci table as __devinitdata ;
- kill 50 LOC.

Signed-off-by: Francois Romieu <[email protected]>

commit e1e773d79369783d68fd7f66fffbe154929df837
Author: Romieu Francois <[email protected]>
Date: Tue May 2 01:07:48 2006 +0200

ip1000: plug leaks in the error path of ipg_nic_open

Added ipg_{rx/tx}_clear() to factor out some code.

Signed-off-by: Francois Romieu <[email protected]>

commit 9cb655fddfd2d169bc7574e849fa5eb179fef12c
Author: Romieu Francois <[email protected]>
Date: Tue May 2 00:15:54 2006 +0200

ip1000: leaks in ipg_probe

The error paths are badly broken.

Bonus:
- remove duplicate initialization of sp;
- remove useless NULL initialization of dev;
- USE_IO_OPS is not used (and the driver does not seem to care about
posted writes, rejoice).

Signed-off-by: Francois Romieu <[email protected]>

commit 2b5d40c1c4cf795c31d2683bcdff646a8984115d
Author: Romieu Francois <[email protected]>
Date: Mon May 1 23:52:37 2006 +0200

ip1000: removal of unreachable code

map/unmap is done in ipg_{probe/remove}

Signed-off-by: Francois Romieu <[email protected]>

commit 5b8bde4d4c02e3d54672d0c431941d9357c35417
Author: Romieu Francois <[email protected]>
Date: Mon May 1 22:40:29 2006 +0200

ip1000: speed-up access to the PHY registers

Reduce delays when reading/writing the PHY registers so we clock the
MII management interface at 2.5 MHz (the maximum according to the
datasheet) instead of 500 Hz.

Signed-off-by: David Vrabel <[email protected]>

commit d4a621ceadb0e4ee83a644be4ffef8bdef687249
Author: David Vrabel <[email protected]>
Date: Mon May 1 21:34:19 2006 +0200

ip1000: root_dev removal and PHY initialization

- Remove ether_crc_le() -- use crc32_le() instead.
- No more nonsense with root_dev -- ipg_remove() now works.
- Move PHY and MAC address initialization into the ipg_probe().
It was previously filling in the MAC address on open which breaks
some user space.
- Folded ipg_nic_init into ipg_probe since it was broke otherwise.

Signed-off-by: David Vrabel <[email protected]>

commit ae6f7b51bfb69a60062d390f10c83396c37f0301
Author: David Vrabel <[email protected]>
Date: Mon May 1 13:20:49 2006 +0200

ip1000: remove changelogs

Signed-off-by: David Vrabel <[email protected]>

commit 7aeb86e2db5deac7d8357d6d794550999fd5e7c6
Author: Pekka Enberg <[email protected]>
Date: Sun Apr 30 12:45:16 2006 +0300

ip1000: alloc_etherdev already allocates memory for private data

Signed-off-by: Pekka Enberg <[email protected]>

commit 49b94ecdf8124c988160585d460b1cc643ee07e0
Author: Pekka Enberg <[email protected]>
Date: Sun Apr 30 12:42:02 2006 +0300

ip1000: remove unused forward declarations

Signed-off-by: Pekka Enberg <[email protected]>

commit e85531a2d8c23b6ff17b3a8cd6d14e6c09954240
Author: Pekka Enberg <[email protected]>
Date: Sun Apr 30 12:16:58 2006 +0300

ip1000: interrupt handler code cleanups

Signed-off-by: Pekka Enberg <[email protected]>

commit 822b9f528c47df857514e33d1ad98a9a551a3b00
Author: Pekka Enberg <[email protected]>
Date: Sun Apr 30 12:05:09 2006 +0300

ip1000: rename baseaddr to ioaddr

Signed-off-by: Pekka Enberg <[email protected]>

commit e8e5f8dece06429793074df6f956a8c4cb60c362
Author: Pekka Enberg <[email protected]>
Date: Sun Apr 30 12:02:51 2006 +0300

ip1000: remove device register write macros

Signed-off-by: Pekka Enberg <[email protected]>

commit 8e1a207d9e0d355aa2949ef4e5710e8c8987b01b
Author: Pekka Enberg <[email protected]>
Date: Sun Apr 30 11:41:01 2006 +0300

ip1000: kill obfuscating register read macros

Signed-off-by: Pekka Enberg <[email protected]>

commit fd3e0c5b3599375024e1e7fc3ddb00f7f5bc0c53
Author: Pekka Enberg <[email protected]>
Date: Sun Apr 30 00:03:23 2006 +0300

ip1000: use iomap for pio/mmio

Signed-off-by: Pekka Enberg <[email protected]>

commit 7ed56b01680978afc5db87f684de81398366bf84
Author: Pekka Enberg <[email protected]>
Date: Sat Apr 29 14:02:34 2006 +0300

ip1000: use static for private symbols

Signed-off-by: Pekka Enberg <[email protected]>

commit b83bac329befd061bb66377d33b1efd4dd39a931
Author: Pekka Enberg <[email protected]>
Date: Sat Apr 29 13:19:36 2006 +0300

ip1000: sparse fixes

Signed-off-by: Pekka Enberg <[email protected]>

commit 65229dd8a40d887364f3aa60277ac3f0e8c64041
Author: Pekka Enberg <[email protected]>
Date: Sat Apr 29 13:06:27 2006 +0300

ip1000: iomem annotations

Signed-off-by: Pekka Enberg <[email protected]>

commit d79a8a846188eee278f5b490b7f15c070f1245d4
Author: Pekka Enberg <[email protected]>
Date: Sat Apr 29 12:41:45 2006 +0300

ip1000: new gigabit ethernet device driver

Signed-off-by: Pekka Enberg <[email protected]>

----------------8<-----------------------------------------------------------

Patch of the day:

The patch shoehorns the driver into the usual mii/ethtool framework.
mii_mutex will prove useful when the link management tasks issued in
irq context are moved to user/workqueue context. Next step.

At least the current code should not be _too_ broken.

Signed-off-by: Francois Romieu <[email protected]>

---

drivers/net/ipg.c | 344 ++++++++++++++++++++---------------------------------
drivers/net/ipg.h | 3
2 files changed, 130 insertions(+), 217 deletions(-)

8e15d782fa4a0f483a71faec7ba17697d9397e76
diff --git a/drivers/net/ipg.c b/drivers/net/ipg.c
index 31cde6c..5d19269 100644
--- a/drivers/net/ipg.c
+++ b/drivers/net/ipg.c
@@ -15,7 +15,9 @@
* [email protected]
*/
#include <linux/crc32.h>
+#include <linux/ethtool.h>
#include <linux/mii.h>
+#include <linux/mutex.h>

#define IPG_RX_RING_BYTES (sizeof(struct RFD) * IPG_RFDLIST_LENGTH)
#define IPG_TX_RING_BYTES (sizeof(struct TFD) * IPG_TFDLIST_LENGTH)
@@ -161,8 +163,7 @@ static u16 read_phy_bit(void __iomem * i
return bit_data;
}

-u16 read_phy_register(struct net_device * dev,
- int phy_address, int phy_register)
+static int mdio_read(struct net_device * dev, int phy_address, int phy_register)
{
/* Read a register from the Physical Layer device located
* on the IPG NIC, using the IPG PHYCTRL register.
@@ -176,7 +177,7 @@ u16 read_phy_register(struct net_device
u8 databit;
u8 phyctrlpolarity;

- IPG_DEBUG_MSG("read_phy_register\n");
+ IPG_DEBUG_MSG("mdio_read\n");

/* The GMII mangement frame structure for a read is as follows:
*
@@ -274,8 +275,8 @@ u16 read_phy_register(struct net_device
return field[6];
}

-static void write_phy_register(struct net_device *dev,
- int phy_address, int phy_register, u16 writeval)
+static void mdio_write(struct net_device *dev,
+ int phy_address, int phy_register, int val)
{
/* Write to a register from the Physical Layer device located
* on the IPG NIC, using the IPG PHYCTRL register.
@@ -289,7 +290,7 @@ static void write_phy_register(struct ne
u8 databit;
u8 phyctrlpolarity;

- IPG_DEBUG_MSG("write_phy_register\n");
+ IPG_DEBUG_MSG("mdio_write\n");

/* The GMII mangement frame structure for a read is as follows:
*
@@ -318,7 +319,7 @@ static void write_phy_register(struct ne
fieldlen[4] = 5; /* REGAD */
field[5] = 0x0002;
fieldlen[5] = 2; /* TA */
- field[6] = writeval;
+ field[6] = val & 0xffff;
fieldlen[6] = 16; /* DATA */
field[7] = 0x0000;
fieldlen[7] = 1; /* IDLE */
@@ -486,7 +487,7 @@ static int ipg_tmi_fiber_detect(struct n

IPG_DEBUG_MSG("_tmi_fiber_detect\n");

- phyid = read_phy_register(dev, phyaddr, MII_PHYSID1);
+ phyid = mdio_read(dev, phyaddr, MII_PHYSID1);

IPG_DEBUG_MSG("PHY ID = %x\n", phyid);

@@ -497,15 +498,14 @@ static int ipg_tmi_fiber_detect(struct n
}
#endif

+/* Find the GMII PHY address. */
static int ipg_find_phyaddr(struct net_device *dev)
{
- /* Find the GMII PHY address. */
-
- int i;
- int phyaddr;
- u32 status;
+ int phyaddr, i;

for (i = 0; i < 32; i++) {
+ u32 status;
+
/* Search for the correct PHY address among 32 possible. */
phyaddr = (IPG_NIC_PHY_ADDRESS + i) % 32;

@@ -513,13 +513,13 @@ static int ipg_find_phyaddr(struct net_d
GMII_PHY_ID1
*/

- status = read_phy_register(dev, phyaddr, MII_BMSR);
+ status = mdio_read(dev, phyaddr, MII_BMSR);

if ((status != 0xFFFF) && (status != 0))
return phyaddr;
}

- return -1;
+ return 0x1f;
}

#ifdef NOTGRACE
@@ -618,10 +618,11 @@ #endif
dev->name);
return -EILSEQ;
}
+ sp->mii_if.phy_id = phyaddr;

IPG_DEBUG_MSG("GMII/MII PHY address = %x\n", phyaddr);

- status = read_phy_register(dev, phyaddr, MII_BMSR);
+ status = mdio_read(dev, phyaddr, MII_BMSR);

printk("PHYStatus = %x \n", status);
if ((status & BMSR_ANEGCAPABLE) == 0) {
@@ -631,8 +632,8 @@ #endif
return -EILSEQ;
}

- advertisement = read_phy_register(dev, phyaddr, MII_ADVERTISE);
- linkpartner_ability = read_phy_register(dev, phyaddr, MII_LPA);
+ advertisement = mdio_read(dev, phyaddr, MII_ADVERTISE);
+ linkpartner_ability = mdio_read(dev, phyaddr, MII_LPA);

printk("PHYadvertisement=%x LinkPartner=%x \n", advertisement,
linkpartner_ability);
@@ -660,8 +661,8 @@ #endif
/* In 1000BASE-X using IPG's internal PCS
* layer, so write to the GMII duplex bit.
*/
- write_phy_register(dev, phyaddr, MII_BMCR,
- read_phy_register(dev, phyaddr, MII_BMCR) |
+ mdio_write(dev, phyaddr, MII_BMCR,
+ mdio_read(dev, phyaddr, MII_BMCR) |
ADVERTISE_1000HALF); // Typo ?

} else {
@@ -670,8 +671,8 @@ #endif
/* In 1000BASE-X using IPG's internal PCS
* layer, so write to the GMII duplex bit.
*/
- write_phy_register(dev, phyaddr, MII_BMCR,
- read_phy_register(dev, phyaddr, MII_BMCR) &
+ mdio_write(dev, phyaddr, MII_BMCR,
+ mdio_read(dev, phyaddr, MII_BMCR) &
~ADVERTISE_1000HALF); // Typo ?
}
}
@@ -683,10 +684,8 @@ #endif
* link partner abilities exchanged via next page
* transfers.
*/
- gigadvertisement =
- read_phy_register(dev, phyaddr, MII_CTRL1000);
- giglinkpartner_ability =
- read_phy_register(dev, phyaddr, MII_STAT1000);
+ gigadvertisement = mdio_read(dev, phyaddr, MII_CTRL1000);
+ giglinkpartner_ability = mdio_read(dev, phyaddr, MII_STAT1000);

/* Compare the full duplex bits in the 1000BASE-T GMII
* registers for the local device, and the link partner.
@@ -795,11 +794,10 @@ #define LPA_PAUSE_ANY (LPA_1000XPAUSE_AS
/* PAUSE is not being advertised. Advertise
* PAUSE and restart auto-negotiation.
*/
- write_phy_register(dev, phyaddr, MII_ADVERTISE,
+ mdio_write(dev, phyaddr, MII_ADVERTISE,
(advertisement |
ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_ASYM));
- write_phy_register(dev, phyaddr, MII_BMCR,
- BMCR_ANRESTART);
+ mdio_write(dev, phyaddr, MII_BMCR, BMCR_ANRESTART);

return -EAGAIN;
}
@@ -845,10 +843,9 @@ #define LPA_PAUSE_ANY (LPA_1000XPAUSE_AS
/* PAUSE is not being advertised. Advertise
* PAUSE and restart auto-negotiation.
*/
- write_phy_register(dev, phyaddr, MII_ADVERTISE,
- advertisement | ADVERTISE_PAUSE_CAP);
- write_phy_register(dev, phyaddr, MII_BMCR,
- BMCR_ANRESTART);
+ mdio_write(dev, phyaddr, MII_ADVERTISE,
+ advertisement | ADVERTISE_PAUSE_CAP);
+ mdio_write(dev, phyaddr, MII_BMCR, BMCR_ANRESTART);
return -EAGAIN;
}

@@ -2671,8 +2668,7 @@ static void ipg_set_phy_default_param(un
address = *phy_param;
value = *(phy_param + 1);
phy_param += 2;
- write_phy_register(dev,
- phy_address, address, value);
+ mdio_write(dev, phy_address, address, value);
length -= 4;
}

@@ -2710,14 +2706,47 @@ static int read_eeprom(struct net_device
return ret;
}

+static void ipg_init_mii(struct net_device *dev)
+{
+ struct ipg_nic_private *sp = netdev_priv(dev);
+ struct mii_if_info *mii_if = &sp->mii_if;
+ int phyaddr;
+
+ mii_if->dev = dev;
+ mii_if->mdio_read = mdio_read;
+ mii_if->mdio_write = mdio_write;
+ mii_if->phy_id_mask = 0x1f;
+ mii_if->reg_num_mask = 0x1f;
+
+ mii_if->phy_id = phyaddr = ipg_find_phyaddr(dev);
+
+ if (phyaddr != 0x1f) {
+ u16 mii_phyctrl, mii_1000cr;
+ u8 revisionid = 0;
+
+ mii_1000cr = mdio_read(dev, phyaddr, MII_CTRL1000);
+ mii_1000cr |= ADVERTISE_1000FULL | ADVERTISE_1000HALF |
+ GMII_PHY_1000BASETCONTROL_PreferMaster;
+ mdio_write(dev, phyaddr, MII_CTRL1000, mii_1000cr);
+
+ mii_phyctrl = mdio_read(dev, phyaddr, MII_BMCR);
+
+ /* Set default phyparam */
+ pci_read_config_byte(sp->pdev, PCI_REVISION_ID, &revisionid);
+ ipg_set_phy_default_param(revisionid, dev, phyaddr);
+
+ /* Reset PHY */
+ mii_phyctrl |= BMCR_RESET | BMCR_ANRESTART;
+ mdio_write(dev, phyaddr, MII_BMCR, mii_phyctrl);
+
+ }
+}
+
static int ipg_hw_init(struct net_device *dev)
{
- int phyaddr = 0;
- int error = 0;
- int i;
- void __iomem *ioaddr = ipg_ioaddr(dev);
- u8 revisionid = 0;
struct ipg_nic_private *sp = netdev_priv(dev);
+ void __iomem *ioaddr = sp->ioaddr;
+ int i, rc;

/* Reset all functions within the IPG. Do not assert
* RST_OUT as not compatible with some PHYs.
@@ -2729,34 +2758,11 @@ static int ipg_hw_init(struct net_device
/* Read LED Mode Configuration from EEPROM */
sp->LED_Mode = read_eeprom(dev, 6);

- error = ipg_reset(dev, i);
- if (error < 0) {
- return error;
- }
-
- ioaddr = ipg_ioaddr(dev);
-
- /* Reset PHY. */
- phyaddr = ipg_find_phyaddr(dev);
-
- if (phyaddr != -1) {
- u16 mii_phyctrl, mii_1000cr;
- mii_1000cr =
- read_phy_register(dev, phyaddr, MII_CTRL1000);
- write_phy_register(dev, phyaddr, MII_CTRL1000,
- mii_1000cr | ADVERTISE_1000FULL | ADVERTISE_1000HALF |
- GMII_PHY_1000BASETCONTROL_PreferMaster);
-
- mii_phyctrl = read_phy_register(dev, phyaddr, MII_BMCR);
- /* Set default phyparam */
- pci_read_config_byte(sp->pdev, PCI_REVISION_ID, &revisionid);
- ipg_set_phy_default_param(revisionid, dev, phyaddr);
-
- /* reset Phy */
- write_phy_register(dev, phyaddr, MII_BMCR,
- (mii_phyctrl | BMCR_RESET | BMCR_ANRESTART));
+ rc = ipg_reset(dev, i);
+ if (rc < 0)
+ goto out;

- }
+ ipg_init_mii(dev);

/* Read MAC Address from EERPOM Jesse20040128EEPROM_VALUE */
sp->StationAddr0 = read_eeprom(dev, 16);
@@ -2774,161 +2780,21 @@ static int ipg_hw_init(struct net_device
dev->dev_addr[3] = (ioread16(ioaddr + STATION_ADDRESS_1) & 0xff00) >> 8;
dev->dev_addr[4] = ioread16(ioaddr + STATION_ADDRESS_2) & 0x00ff;
dev->dev_addr[5] = (ioread16(ioaddr + STATION_ADDRESS_2) & 0xff00) >> 8;
-
- return 0;
+out:
+ return rc;
}

-static int ipg_nic_do_ioctl(struct net_device *dev, struct ifreq *req, int cmd)
+static int ipg_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
{
- /* IOCTL commands for IPG NIC.
- *
- * SIOCDEVPRIVATE nothing
- * SIOCDEVPRIVATE+1 register read
- * ifr_data[0] = 0x08, 0x10, 0x20
- * ifr_data[1] = register offset
- * ifr_data[2] = value read
- * SIOCDEVPRIVATE+2 register write
- * ifr_data[0] = 0x08, 0x10, 0x20
- * ifr_data[1] = register offset
- * ifr_data[2] = value to write
- * SIOCDEVPRIVATE+3 GMII register read
- * ifr_data[1] = register offset
- * SIOCDEVPRIVATE+4 GMII register write
- * ifr_data[1] = register offset
- * ifr_data[2] = value to write
- * SIOCDEVPRIVATE+5 PCI register read
- * ifr_data[0] = 0x08, 0x10, 0x20
- * ifr_data[1] = register offset
- * ifr_data[2] = value read
- * SIOCDEVPRIVATE+6 PCI register write
- * ifr_data[0] = 0x08, 0x10, 0x20
- * ifr_data[1] = register offset
- * ifr_data[2] = value to write
- *
- */
-
- u8 val8;
- u16 val16;
- u32 val32;
- unsigned int *data;
- int phyaddr = 0;
- void __iomem *ioaddr = ipg_ioaddr(dev);
struct ipg_nic_private *sp = netdev_priv(dev);
+ struct mii_ioctl_data *data = if_mii(ifr);
+ int rc;

- IPG_DEBUG_MSG("_nic_do_ioctl\n");
-
- data = (unsigned int *)&req->ifr_data;
-
- switch (cmd) {
- case SIOCDEVPRIVATE:
- return 0;
-
- case SIOCDEVPRIVATE + 1:
- switch (data[0]) {
- case 0x08:
- data[2] = ioread8(ioaddr + data[1]);
- return 0;
-
- case 0x10:
- data[2] = ioread16(ioaddr + data[1]);
- return 0;
-
- case 0x20:
- data[2] = ioread32(ioaddr + data[1]);
- return 0;
-
- default:
- data[2] = 0x00;
- return -EINVAL;
- }
-
- case SIOCDEVPRIVATE + 2:
- switch (data[0]) {
- case 0x08:
- iowrite8(data[2], ioaddr + data[1]);
- return 0;
-
- case 0x10:
- iowrite16(data[2], ioaddr + data[1]);
- return 0;
-
- case 0x20:
- iowrite32(data[2], ioaddr + data[1]);
- return 0;
-
- default:
- return -EINVAL;
- }
-
- case SIOCDEVPRIVATE + 3:
- phyaddr = ipg_find_phyaddr(dev);
-
- if (phyaddr == -1)
- return -EINVAL;
-
- data[2] = read_phy_register(dev, phyaddr, data[1]);
-
- return 0;
-
- case SIOCDEVPRIVATE + 4:
- phyaddr = ipg_find_phyaddr(dev);
-
- if (phyaddr == -1)
- return -EINVAL;
-
- write_phy_register(dev, phyaddr, data[1], (u16) data[2]);
-
- return 0;
-
- case SIOCDEVPRIVATE + 5:
- switch (data[0]) {
- case 0x08:
- pci_read_config_byte(sp->pdev, data[1], &val8);
- data[2] = (unsigned int)val8;
- return 0;
-
- case 0x10:
- pci_read_config_word(sp->pdev, data[1], &val16);
- data[2] = (unsigned int)val16;
- return 0;
-
- case 0x20:
- pci_read_config_dword(sp->pdev, data[1], &val32);
- data[2] = (unsigned int)val32;
- return 0;
-
- default:
- data[2] = 0x00;
- return -EINVAL;
- }
-
- case SIOCDEVPRIVATE + 6:
- switch (data[0]) {
- case 0x08:
- pci_write_config_byte(sp->pdev, data[1], (u8) data[2]);
- return 0;
-
- case 0x10:
- pci_write_config_word(sp->pdev, data[1], (u16) data[2]);
- return 0;
-
- case 0x20:
- pci_write_config_dword(sp->pdev, data[1],
- (u32) data[2]);
- return 0;
+ mutex_lock(&sp->mii_mutex);
+ rc = generic_mii_ioctl(&sp->mii_if, data, cmd, NULL);
+ mutex_unlock(&sp->mii_mutex);

- default:
- return -EINVAL;
- }
-
- case SIOCSIFMTU:
- {
- return 0;
- }
-
- default:
- return -EOPNOTSUPP;
- }
+ return rc;
}

static int ipg_nic_change_mtu(struct net_device *dev, int new_mtu)
@@ -2953,6 +2819,48 @@ static int ipg_nic_change_mtu(struct net
return 0;
}

+static int ipg_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+{
+ struct ipg_nic_private *sp = netdev_priv(dev);
+ int rc;
+
+ mutex_lock(&sp->mii_mutex);
+ rc = mii_ethtool_gset(&sp->mii_if, cmd);
+ mutex_unlock(&sp->mii_mutex);
+
+ return rc;
+}
+
+static int ipg_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+{
+ struct ipg_nic_private *sp = netdev_priv(dev);
+ int rc;
+
+ mutex_lock(&sp->mii_mutex);
+ rc = mii_ethtool_sset(&sp->mii_if, cmd);
+ mutex_unlock(&sp->mii_mutex);
+
+ return rc;
+}
+
+static int ipg_nway_reset(struct net_device *dev)
+{
+ struct ipg_nic_private *sp = netdev_priv(dev);
+ int rc;
+
+ mutex_lock(&sp->mii_mutex);
+ rc = mii_nway_restart(&sp->mii_if);
+ mutex_unlock(&sp->mii_mutex);
+
+ return rc;
+}
+
+static struct ethtool_ops ipg_ethtool_ops = {
+ .get_settings = ipg_get_settings,
+ .set_settings = ipg_set_settings,
+ .nway_reset = ipg_nway_reset,
+};
+
static void ipg_remove(struct pci_dev *pdev)
{
struct net_device *dev = pci_get_drvdata(pdev);
@@ -3011,6 +2919,7 @@ static int __devinit ipg_probe(struct pc

sp = netdev_priv(dev);
spin_lock_init(&sp->lock);
+ mutex_init(&sp->mii_mutex);

/* Declare IPG NIC functions for Ethernet device methods.
*/
@@ -3019,11 +2928,12 @@ static int __devinit ipg_probe(struct pc
dev->hard_start_xmit = &ipg_nic_hard_start_xmit;
dev->get_stats = &ipg_nic_get_stats;
dev->set_multicast_list = &ipg_nic_set_multicast_list;
- dev->do_ioctl = &ipg_nic_do_ioctl;
+ dev->do_ioctl = ipg_ioctl;
dev->change_mtu = &ipg_nic_change_mtu;

SET_MODULE_OWNER(dev);
SET_NETDEV_DEV(dev, &pdev->dev);
+ SET_ETHTOOL_OPS(dev, &ipg_ethtool_ops);

rc = pci_request_regions(pdev, DRV_NAME);
if (rc)
diff --git a/drivers/net/ipg.h b/drivers/net/ipg.h
index 6c55b0b..58b1417 100644
--- a/drivers/net/ipg.h
+++ b/drivers/net/ipg.h
@@ -870,6 +870,9 @@ #endif
u16 StationAddr1; /* Station Address in EEPROM Reg 0x11 */
u16 StationAddr2; /* Station Address in EEPROM Reg 0x12 */

+ struct mutex mii_mutex;
+ struct mii_if_info mii_if;
+
#ifdef IPG_DEBUG
int TFDunavailCount;
int RFDlistendCount;
--
1.3.1

2006-05-05 00:27:08

by Francois Romieu

[permalink] [raw]
Subject: Re: [PATCH 2/2] ipg: redundancy with mii.h

David Vrabel <[email protected]> :
[...]
> > ipg: replace #define with enum
> >
> > Added some underscores to improve readability.
> >
> > Signed-off-by: Francois Romieu <[email protected]>
>
> Nack. Register names in code should match those used in the
> documentation (even if they are a bit unreadable). Though I will
> conceed that the available datasheet doesn't actually describe the
> majority of the registers.

We will have to agree to disagree. Feel free to open a different
branch.

--
Ueimor

2006-05-20 21:03:24

by David Vrabel

[permalink] [raw]
Subject: Re: [PATCH 2/2] ipg: redundancy with mii.h

ipg: initialize Rx buffers correctly

A previous patch resulted in the initialization of the Rx buffers
being skipped.

Signed-off-by: David Vrabel <[email protected]>

--- linux-source-2.6.16.orig/drivers/net/ipg.c 2006-05-20 21:38:44.604788258 +0100
+++ linux-source-2.6.16/drivers/net/ipg.c 2006-05-20 21:39:14.298898552 +0100
@@ -1298,18 +1298,16 @@
sp->RxBuffNotReady = 0;

for (i = 0; i < IPG_RFDLIST_LENGTH; i++) {
- if (!sp->RxBuff[i])
- continue;
-
- /* Free any allocated receive buffers. */
- pci_unmap_single(sp->pdev,
- sp->RxBuffDMAhandle[i].dmahandle,
- sp->RxBuffDMAhandle[i].len,
- PCI_DMA_FROMDEVICE);
-
- IPG_DEV_KFREE_SKB(sp->RxBuff[i]);
- sp->RxBuff[i] = NULL;
+ if (sp->RxBuff[i]) {
+ /* Free any allocated receive buffers. */
+ pci_unmap_single(sp->pdev,
+ sp->RxBuffDMAhandle[i].dmahandle,
+ sp->RxBuffDMAhandle[i].len,
+ PCI_DMA_FROMDEVICE);

+ IPG_DEV_KFREE_SKB(sp->RxBuff[i]);
+ sp->RxBuff[i] = NULL;
+ }
/* Clear out the RFS field. */
sp->RFDList[i].RFS = 0x0000000000000000;


Attachments:
ipg-fix-init_rfdlist (1.04 kB)

2006-05-21 07:23:39

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH 2/2] ipg: redundancy with mii.h

Hi David,

On 5/21/06, David Vrabel <[email protected]> wrote:
> Did anyone manage to get a response from IC Plus regarding the required
> Signed-off-by line?

The current IP1000A driver maintainer at IC Plus is
[email protected]. I have no received confirmation if he can sign
off or not yet.

Pekka

2006-05-21 10:20:29

by Francois Romieu

[permalink] [raw]
Subject: Re: [PATCH 2/2] ipg: redundancy with mii.h

David Vrabel <[email protected]> :
[...]
> 0007-ipg-plug-leaks-in-the-error-path-of-ipg_nic_open.txt broke receive
> since it was skipping the initialization of the Rx buffers. Patch attached.

Oops. Applied to branch netdev-ipg of
git://electric-eye.fr.zoreil.com/home/romieu/linux-2.6.git

(please include the '-p' option in future invocation of diff)

I have put an updated patch against 2.6.17-rc4 for the whole driver at
http://www.fr.zoreil.com/people/francois/misc/20060521-2.6.17-rc4-git-ip1000-test.patch

--
Ueimor

2006-05-22 03:23:00

by Jesse Huang

[permalink] [raw]
Subject: Re: [PATCH 2/2] ipg: redundancy with mii.h

Hi Francois:

Thanks for your help.
I will try this patch and test it.
http://www.fr.zoreil.com/people/francois/misc/20060521-2.6.17-rc4-git-ip1000-test.patch

Jesse

----- Original Message -----
From: "Francois Romieu" <[email protected]>
To: "David Vrabel" <[email protected]>
Cc: "Pekka Enberg" <[email protected]>; <[email protected]>;
<[email protected]>; <[email protected]>; <[email protected]>
Sent: Sunday, May 21, 2006 6:16 PM
Subject: Re: [PATCH 2/2] ipg: redundancy with mii.h


David Vrabel <[email protected]> :
[...]
> 0007-ipg-plug-leaks-in-the-error-path-of-ipg_nic_open.txt broke receive
> since it was skipping the initialization of the Rx buffers. Patch
attached.

Oops. Applied to branch netdev-ipg of
git://electric-eye.fr.zoreil.com/home/romieu/linux-2.6.git

(please include the '-p' option in future invocation of diff)

I have put an updated patch against 2.6.17-rc4 for the whole driver at
http://www.fr.zoreil.com/people/francois/misc/20060521-2.6.17-rc4-git-ip1000-test.patch

--
Ueimor

2006-05-23 06:51:30

by Jesse Huang

[permalink] [raw]
Subject: Re: [PATCH 2/2] ipg: redundancy with mii.h

Dear All:

I had tested the following patch of IP1000A
http://www.fr.zoreil.com/people/francois/misc/20060521-2.6.17-rc4-git-ip1000-test.patch

It works fine. We will discuss this driver with our members see if there are
anything need to add or modify.
Thanks for everybody's effort for our IP1000A.

Would anybody need some piece of IP1000A, please give me your address, we
will send some of it to you.
Thanks!

If any problem, please feel free to contact me. [email protected]

Best Regards,
Jesse

----- Original Message -----
From: "Francois Romieu" <[email protected]>
To: "David Vrabel" <[email protected]>
Cc: "Pekka Enberg" <[email protected]>; <[email protected]>;
<[email protected]>; <[email protected]>; <[email protected]>
Sent: Sunday, May 21, 2006 6:16 PM
Subject: Re: [PATCH 2/2] ipg: redundancy with mii.h


David Vrabel <[email protected]> :
[...]
> 0007-ipg-plug-leaks-in-the-error-path-of-ipg_nic_open.txt broke receive
> since it was skipping the initialization of the Rx buffers. Patch
attached.

Oops. Applied to branch netdev-ipg of
git://electric-eye.fr.zoreil.com/home/romieu/linux-2.6.git

(please include the '-p' option in future invocation of diff)

I have put an updated patch against 2.6.17-rc4 for the whole driver at
http://www.fr.zoreil.com/people/francois/misc/20060521-2.6.17-rc4-git-ip1000-test.patch

--
Ueimor