IP1000 gigabit nic driver

IP1000 gigabit nic driver

[David Gómez]

Hi all,

Does anybody in this list know why the IP1000 driver is not
included in the kernel? Has the author submitted it in the
past for inclusion?

The card in question is:

Sundance Technology Inc IC Plus IP1000

and the driver can be found in sundance web, sources 
included. I tried to contact the author but my email
bounced.

There's no LICENSE in the source, just copyrigth
sentences in the .c files, so i'm not sure under
which license it's distributed :-?.

regards,

-- 
David Gómez                                      Jabber ID: davidge@jabber.org

Re: IP1000 gigabit nic driver

[Francois Romieu]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=unknown-8bit, Size: 969 bytes --]

David Gómez <david@pleyades.net> :
[...]
> Does anybody in this list know why the IP1000 driver is not
> included in the kernel ?

Afaik the driver has never been submitted for inclusion.
At least not on netdev@vger.kernel.org (hint, hint).

[...]
> The card in question is:
> 
> Sundance Technology Inc IC Plus IP1000
> 
> and the driver can be found in sundance web, sources 

URL please ?

> included. I tried to contact the author but my email
> bounced.
> 
> There's no LICENSE in the source, just copyrigth
> sentences in the .c files, so i'm not sure under
> which license it's distributed :-?.

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

$ unzip -c IP1000A-Linux-driver-v2.09f.zip | grep MODULE_LICENSE
    MODULE_LICENSE("GPL");

It's a bit bloaty but it does not seem too bad (not mergeable "as
is" though). Do you volunteer to test random cra^W^W carefully
engineered code on your computer to help the rework/merging process ?

-- 
Ueimor

Re: IP1000 gigabit nic driver

[David Vrabel]

Francois Romieu wrote:
> David Gómez <david@pleyades.net> :
> [...]
>> Does anybody in this list know why the IP1000 driver is not
>> included in the kernel ?
> 
> Afaik the driver has never been submitted for inclusion.
> At least not on netdev@vger.kernel.org (hint, hint).
> 
> [...]
>> The card in question is:
>>
>> Sundance Technology Inc IC Plus IP1000
>>
>> and the driver can be found in sundance web, sources 
> 
> URL please ?
> 
>> included. I tried to contact the author but my email
>> bounced.
>>
>> There's no LICENSE in the source, just copyrigth
>> sentences in the .c files, so i'm not sure under
>> which license it's distributed :-?.
> 
> /me goes to http://www.icplus.com.tw/driver-pp-IP1000A.html
> 
> $ unzip -c IP1000A-Linux-driver-v2.09f.zip | grep MODULE_LICENSE
>     MODULE_LICENSE("GPL");
> 
> It's a bit bloaty but it does not seem too bad (not mergeable "as
> is" though). Do you volunteer to test random cra^W^W carefully
> engineered code on your computer to help the rework/merging process ?

I finally got around to putting a 2nd NIC in my box that has one of this 
chips and was going to start fixing the driver up and preparing it for 
submission this weekend.  Or I might try rewriting from scratch based on 
the datasheet depending on how horrific the code looks on closer inspection.

Not got a whole lot of time to do this so no timescale for completion...

David Vrabel

Re: IP1000 gigabit nic driver

[David Gómez]

Hi Francois,

On Apr 27 at 08:56:27, Francois Romieu wrote:
> /me goes to http://www.icplus.com.tw/driver-pp-IP1000A.html
> 
> $ unzip -c IP1000A-Linux-driver-v2.09f.zip | grep MODULE_LICENSE
>     MODULE_LICENSE("GPL");

Thanks, i didn't notice it ;-)

> It's a bit bloaty but it does not seem too bad (not mergeable "as
> is" though). Do you volunteer to test random cra^W^W carefully
> engineered code on your computer to help the rework/merging process ?

Yes, i am. Send me all the the cra\x17\x17\x17 ;-), i'm ready to
test it.

cheers,

-- 
David Gómez                                      Jabber ID: davidge@jabber.org

Re: IP1000 gigabit nic driver

[David Gómez]

Hi David,

On Apr 27 at 11:26:07, David Vrabel wrote:
> I finally got around to putting a 2nd NIC in my box that has one of this 
> chips and was going to start fixing the driver up and preparing it for 
> submission this weekend.

Great!

>  Or I might try rewriting from scratch based on 
> the datasheet depending on how horrific the code looks on closer inspection.

For what i've seen, the code seems quite readable.

> Not got a whole lot of time to do this so no timescale for completion...

I could help. What things do you think need to be fixed before
submitting the driver?

cheers,

-- 
David Gómez                                      Jabber ID: davidge@jabber.org

Re: IP1000 gigabit nic driver

[Pekka Enberg]

On 4/28/06, David Gómez <david@pleyades.net> wrote:
> I could help. What things do you think need to be fixed before
> submitting the driver?

Needs some serious coding style cleanup and conversion to proper 2.6
APIs for starters.

Re: IP1000 gigabit nic driver

[David Gómez]

Hi Pekka,

On Apr 28 at 01:58:04, Pekka Enberg wrote:
> Needs some serious coding style cleanup and conversion to proper 2.6
> APIs for starters.

Ok, i could take care of that, and it's a good way of getting my hands
dirty with kernel programming ;). David, if it's ok to you i'll do the
cleanup thing.

What about 2.4/2.2 code? It's supposed to stay for compatibility
or it should be removed before submitting?

cheers,

-- 
David Gómez                                      Jabber ID: davidge@jabber.org

Re: IP1000 gigabit nic driver

[Pekka J Enberg]

On Apr 28 at 01:58:04, Pekka Enberg wrote:
> > Needs some serious coding style cleanup and conversion to proper 2.6
> > APIs for starters.

On Fri, 28 Apr 2006, David Gómezz wrote:
> Ok, i could take care of that, and it's a good way of getting my hands
> dirty with kernel programming ;). David, if it's ok to you i'll do the
> cleanup thing.
> 
> What about 2.4/2.2 code? It's supposed to stay for compatibility
> or it should be removed before submitting?

It's preferred not to have compatability cruft for 2.6 patch submissions, 
so I'd say kill them.

					Pekka

Re: IP1000 gigabit nic driver

[Pekka J Enberg]

Hi David,

On Fri, 28 Apr 2006, David Gómezz wrote:
> Ok, i could take care of that, and it's a good way of getting my hands
> dirty with kernel programming ;). David, if it's ok to you i'll do the
> cleanup thing.

Here are some suggestions for coding style cleanups:

  - Use Lindet for initial formatting
  - Kill use of LINUX_VERSION_CODE macro for compatability
  - Kill obfuscating macros. For example, IPG_DEV_KFREE_SKB and 
    IPG_DEVICE_TYPE.
  - Move changelogs outside of source files
  - Convert kmalloc/memset to kzalloc and kcalloc
  - Remove redundant typecasts
  - Remove dead code
  - Use dev_{dbg,err,info,warn} for logging
  - Remove unnecessary curly braces
  - Use proper naming convention for things like Length and pPHYParam
  - Convert macro functions to static inline functions for type safety

				Pekka

Re: IP1000 gigabit nic driver

[Ingo Oeser]

Hi David,

David Gómez wrote:
> On Apr 28 at 01:58:04, Pekka Enberg wrote:
> > Needs some serious coding style cleanup and conversion to proper 2.6
> > APIs for starters.
> 
> Ok, i could take care of that, and it's a good way of getting my hands
> dirty with kernel programming ;). David, if it's ok to you i'll do the
> cleanup thing.

Have fun! Great that you do this.
 
> What about 2.4/2.2 code? It's supposed to stay for compatibility
> or it should be removed before submitting?

Usually it should be removed.

The way to remove 2.4/2.2. code is by reimplementation
of 2.6-APIs in seperate files and headers and not submitting
these into latest kernel. Keep these somewhere else 
(e.g. a project web site).

That way your drivers ALWAYS work with latest kernels
and you notice breakage of backward compatiblity quite easily.
If maintaining these parts becomes a pain with no gain, 
you can simply stop providing these yourself.

"#ifdef KERNEL_VERSON" stuff in submitted drivers
is generally not acceptable. Since it is hard to test these parts.

I ported some off-tree drivers from 2.2 to 2.4. using this technique
and it works good, reduces maintainence burden and keeps
your driver current to latest APIs automatically.


Regards

Ingo Oeser

Re: IP1000 gigabit nic driver

[David Gómez]

On Apr 28 at 02:59:01, Pekka J Enberg wrote:
> Here are some suggestions for coding style cleanups:

Thanks for the hints!

cheers,

-- 
David Gómez                                      Jabber ID: davidge@jabber.org

Re: IP1000 gigabit nic driver

[Stephen Hemminger]

Also consider consolidating into two files ip1000.c and ip1000.h, this allows
you to keep almost all the variables and functions static, and eliminates the
need for function prototypes for everything.

Re: IP1000 gigabit nic driver

[David Gómez]

Hi Pekka,

On Apr 28 at 02:59:01, Pekka J Enberg wrote:
> Here are some suggestions for coding style cleanups:

Couple of questions,

>   - Use dev_{dbg,err,info,warn} for logging

Cannot, i need a "struct device" and most of the
there's only access to a "struct net_device". Am i
missing something? 

>   - Use proper naming convention for things like Length and pPHYParam

What's the convention for these names?

Thanks,

-- 
David Gómez                                      Jabber ID: davidge@jabber.org

Re: IP1000 gigabit nic driver

[David Gómez]

Hi Stephen,

On Apr 28 at 03:42:51, Stephen Hemminger wrote:
> It is stored if you use SET_NETDEV_DEV() macro prior to registration.
> Perhaps we need some netdev_{dbg,err,info,warn} macros?

Maybe. A grep reveals that no network driver is using still the
dev_* loggin macros.

> static inline struct device * netdev_dev(struct net_device *netd)
> {
> 	return netd->class_dev.dev;
> }

ok. thanks

-- 
David Gómez                                      Jabber ID: davidge@jabber.org

Re: IP1000 gigabit nic driver

[Pekka Enberg]

On Fri, 28 Apr 2006, David Gómezz wrote:
> > Ok, i could take care of that, and it's a good way of getting my hands
> > dirty with kernel programming ;). David, if it's ok to you i'll do the
> > cleanup thing.

On Fri, 2006-04-28 at 14:59 +0300, Pekka J Enberg wrote:
> Here are some suggestions for coding style cleanups:

[snip]

I ended up doing most of them myself [1]. Sorry :-) Are the datasheets
public by the way?

			Pekka

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

Re: IP1000 gigabit nic driver

[Arjan van de Ven]

On Sat, 2006-04-29 at 13:29 +0300, Pekka Enberg wrote:
> On Fri, 28 Apr 2006, David Gómezz wrote:
> > > Ok, i could take care of that, and it's a good way of getting my hands
> > > dirty with kernel programming ;). David, if it's ok to you i'll do the
> > > cleanup thing.
> 
> On Fri, 2006-04-28 at 14:59 +0300, Pekka J Enberg wrote:
> > Here are some suggestions for coding style cleanups:
> 
> [snip]
> 
> I ended up doing most of them myself [1]. Sorry :-) Are the datasheets
> public by the way?


you want to nuke the ioctl btw and just use ethtool ioctl instead

Re: IP1000 gigabit nic driver

[David Gómez]

Hi Pekka,

On Apr 29 at 01:29:26, Pekka Enberg wrote:
> I ended up doing most of them myself [1]. Sorry :-) Are the datasheets
> public by the way?

I already had it modified, just needed to create the patch... Anyway,
have you submitted it to netdev?

About the datasheets, i don't think so. The icplus  web page seems broken
except for the driver section. But maybe if asked they'll made public
the datasheets.

cheers,

-- 
David Gómez                                      Jabber ID: davidge@jabber.org

Re: IP1000 gigabit nic driver

[David Vrabel]

Pekka Enberg wrote:
> 
> I ended up doing most of them myself [1]. Sorry :-) Are the datasheets
> public by the way?

http://www.icplus.com.tw/Data/Datasheet/IP1000A-DS-R08-07052005.pdf

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

Re: IP1000 gigabit nic driver

[Pekka Enberg]

Hi David,

On Sat, 2006-04-29 at 14:21 +0200, David Gómez wrote:
> I already had it modified, just needed to create the patch... Anyway,
> have you submitted it to netdev?

No, I haven't. I don't have the hardware, so I can't test the driver.
Furthermore, there's plenty of stuff to fix before it's in any shape for
submission. If someone wants to give this patch a spin, I would love to
hear the results.

					Pekka

Re: IP1000 gigabit nic driver

[Pekka Enberg]

On Sat, 2006-04-29 at 14:21 +0200, David Gómez wrote:
> > I already had it modified, just needed to create the patch... Anyway,
> > have you submitted it to netdev?

On Sat, 2006-04-29 at 23:35 +0300, Pekka Enberg wrote:
> No, I haven't. I don't have the hardware, so I can't test the driver.
> Furthermore, there's plenty of stuff to fix before it's in any shape for
> submission. If someone wants to give this patch a spin, I would love to
> hear the results.

I killed the I/O write/read macros and switched the driver to iomap.

			Pekka

Subject: [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 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.

I don't have the hardware, so I don't know if I broke anything.
The patch is 138 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: Pekka Enberg <penberg@cs.helsinki.fi>

Re: IP1000 gigabit nic driver

[David Vrabel]

[-- Attachment #1: Type: text/plain, Size: 1844 bytes --]

Pekka Enberg wrote:
> On Sat, 2006-04-29 at 14:21 +0200, David Gómez wrote:
>>> I already had it modified, just needed to create the patch... Anyway,
>>> have you submitted it to netdev?
> 
> On Sat, 2006-04-29 at 23:35 +0300, Pekka Enberg wrote:
>> No, I haven't. I don't have the hardware, so I can't test the driver.
>> Furthermore, there's plenty of stuff to fix before it's in any shape for
>> submission. If someone wants to give this patch a spin, I would love to
>> hear the results.

Thanks for doing this Pekka.  I've fixed up some stuff and given it some 
brief testing on a 100BaseT network and it seems to work now.

> Subject: [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 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.

Still pending.  Also:

     - something (PHY reset/auto negotiation?) takes 2-3 seconds and
       appears to be done with interrupts disabled.

>   - For multiple devices, the driver uses a global root_dev and
>     ipg_remove() play some tricks which look fishy.

Killed this.  It was broke and ugly as hell.

Attached is patch with some more changes:

- Remove changelogs
- 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 <dvrabel@cantab.net>

[-- Attachment #2: working --]
[-- Type: text/plain, Size: 22818 bytes --]

Index: linux-source-2.6.16/drivers/net/ipg.c
===================================================================
--- linux-source-2.6.16.orig/drivers/net/ipg.c	2006-04-30 22:26:05.788013667 +0100
+++ linux-source-2.6.16/drivers/net/ipg.c	2006-05-01 00:23:28.358641581 +0100
@@ -13,149 +13,14 @@
  * 408 873 4117
  * www.sundanceti.com
  * craig_rich@sundanceti.com
- *
- * Rev  Date     Description
- * --------------------------------------------------------------
- * 0.1  11/8/99  Initial revision work begins.
- *
- * 0.2  11/12/99  Basic operation achieved, continuing work.
- *
- * 0.3  11/19/99  MAC Loop Back for sync problem testing.
- *
- * 0.4  12/22/99  ioctl for diagnotic program 'hunter' support.
- *
- * 0.5  4/13/00   Updates to
- *
- * 0.6  6/14/00   Slight correction to handling TFDDONE, and
- *                preservation of PHYCTRL polarity bits.
- *
- * 0.7  7/27/00   Modifications to accomodate triple speed
- *                autonegotiation. Also change to ioctl routine
- *                to handle unknown PHY address.
- *
- * 0.8  8/11/00   Added change_mtu function.
- *
- * 0.9  8/15/00   Corrected autonegotiation resolution.
- *
- * 0.10 8/30/00   Changed constants to use IPG in place
- *                of RIO. Also, removed most of debug
- *                code in preparation for production release.
- *
- * 0.11 8/31/00   Utilize 64 bit data types where appropriate.
- *
- * 0.12 9/1/00    Move some constants to include file and utilize
- *                RxDMAInt register.
- *
- * 0.13 10/31/00  Several minor modifications to improve stability.
- *
- * 0.14 11/28/00  Added call to nic_tx_free if TFD not available.
- *
- * 0.15 12/5/00   Corrected problem with receive errors, always set
- *                receive buffer address to NULL. Release RX buffers
- *                on errors.
- *
- * 0.16 12/20/00  Corrected autoneg resolution issue, must detect
- *                speed via PHYCTRL register. Also, perform only 1
- *                loop in the nic_txcleanup routine.
- *
- * 0.17 2/7/01    Changed all references of ST2021 to IPG.
- *                When next TFD not available, return -ENOMEM instead
- *                of 0. Removed references to RUBICON.
- *
- * 0.18 2/14/01   Corrected problem when unexpected jumbo frames are
- *                received (now dropped properly.) Changed
- *                "DROP_ON_ERRORS" breaking out Ethernet errors and
- *                TCP/IP errors serparately. Corrected Gigabit
- *                copper PAUSE autonegotiation.
- *
- * 0.19 2/22/01   Changed interrupt handling of RFD_LIST_END,
- *                INT_REQUESTED, and RX_DMA_COMPLETE. Masked off
- *                RMON statistics and unused MIB statistics.
- *                Make sure *all* statistics are accounted for
- *                (either masked or read in get_stats) to avoid
- *                perpetual UpdateStats interrupt from causing
- *                driver to crash.
- *
- * 0.20 3/2/01    Corrected error in nic_stop. Need to set
- *                TxBuff[] = NULL after freeing TxBuff and
- *                RxBuff[] = NULL after freeing RxBuff.
- *
- * 0.21 3/5/01    Correct 10/100Mbit PAUSE autonegotiation.
- *
- * 0.22 3/16/01   Used TxDMAIndicate for 100/1000Mbps modes. Add
- *                "TFD unavailable" and "RFD list end" counters
- *                to assist with performance measurement. Added
- *                check for maxtfdcnt != 0 to while loop within
- *                txcleanup.
- *
- * 0.23 3/22/01   Set the CurrentTxFrameID to 1 upon detecting
- *                a TxDMAComplete to reduce the number of TxDMAComplete.
- *                Also, indicate IP/TCP/UDP checksum is unneseccary
- *                if IPG indicates checksum validates.
- *
- * 0.24 3/23/01   Changed the txfree routine, eliminating the margin
- *                between the last freed TFD and the current TFD.
- *
- * 0.25 4/3/01    Corrected errors in config_autoneg to deal with
- *                fiber flag properly.
- *
- * 0.26 5/1/01    Port for operation with Linux 2.2 or 2.4 kernel.
- *
- * 0.27 5/22/01   Cleaned up some extraneous comments.
- *
- * 0.28 6/20/01   Added auto IP, TCP, and UDP checksum addition
- *                on transmit based on compilation option.
- *
- * 0.29 7/26/01   Comment out #include <asm/spinlock.h> from ipg.h
- *                for compatibility with RedHat 7.1. Unkown reason.
- *
- * 0.30 8/10/01   Added debug message to each function, print function
- *                name when entered. Added DEBUGCTRL register bit 5 for
- *                Rx DMA Poll Now bug work around. Added ifdef IPG_DEBUG
- *                flags to IPG_TFDlistunabvail and IPG_RFDlistend
- *                counters. Removed clearing of sp->stat struct from
- *                nic_open and added check in get_stats to make sure
- *                NIC is initialized before reading statistic registers.
- *                Corrected erroneous MACCTRL setting for Fiber based
- *                10/100 boards. Corrected storage of phyctrlpolarity
- *                variable.
- *
- * 0.31 8/13/01   Incorporate STI or TMI fiber based NIC detection.
- *                Corrected problem with _pciremove_linux2_4 routine.
- *                Corrected setting of IP/TCP/UDP checksumming on receive
- *                and transmit.
- *
- * 0.32 8/15/01   Changed the tmi_fiber_detect routine.
- *
- * 0.33 8/16/01   Changed PHY reset method in nic_open routine. Added
- *                a chip reset in nic_stop to shut down the IPG.
- *
- * 0.34 9/5/01    Corrected some misuage of dev_kfree_skb.
- *
- * 0.35 10/30/01  Unmap register space (IO or memory) in the nic_stop
- *                routine instead of in the cleanup or remove routines.
- *                Corrects driver up/down/up problem when using IO
- *                register mapping.
- *
- * 0.36 10/31/01  Modify the constant IPG_FRAMESBETWEENTXDMACOMPLETES
- *		  from 0x10 to 1.
- * 0.37 11/05/03  Modify the IPG_PHY_1000BASETCONTROL
- *				  in IP1000A this register is without 1000BPS Ability by default
- *				  so enable 1000BPS ability before PHY RESET/RESTART_AN
- * 0.38 11/05/03  update ipg_config_autoneg routine
- * 0.39 11/05/03  add Vendor_ID=13F0/Device_ID=1023 into support_cards
- * 2.05 10/16/04  Remove IPG_IE_RFD_LIST_END for pass SmartBit test.
- *                (see 20041019Jesse_For_SmartBit.)
- * 2.06 10/27/04 Support for kernel 2.6.x
- * 2.06a 11/03/04 remove some compile warring message.
- * 2.09b 06/03/05 Support 4k jumbo  (IC Plus, Jesse)
- * 2.09d 06/22/05 Support 10k jumbo, more than 4k will using copy mode (IC Plus, Jesse)
  */
 #define JUMBO_FRAME_4k_ONLY
 enum {
 	netdev_io_size = 128
 };
 
+#include <linux/crc32.h>
+
 #include "ipg.h"
 #define DRV_NAME	"ipg"
 
@@ -214,14 +79,12 @@
 static int ipg_nic_hard_start_xmit(struct sk_buff *, struct net_device *);
 static struct net_device_stats *ipg_nic_get_stats(struct net_device *);
 static void ipg_nic_set_multicast_list(struct net_device *);
-static int ipg_nic_init(struct net_device *);
 static int ipg_nic_rx(struct net_device *);
 static int ipg_nic_rxrestore(struct net_device *);
 static int init_rfdlist(struct net_device *);
 static int init_tfdlist(struct net_device *);
 static int ipg_get_rxbuff(struct net_device *, int);
 static int ipg_config_autoneg(struct net_device *);
-static unsigned ether_crc_le(int, unsigned char *);
 static int ipg_nic_do_ioctl(struct net_device *, struct ifreq *, int);
 static int ipg_nic_change_mtu(struct net_device *, int);
 
@@ -1637,60 +1500,12 @@
 	 * by ifconfig.
 	 */
 
-	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);
 
 	IPG_DEBUG_MSG("_nic_open\n");
 
-	/* Reset all functions within the IPG. Do not assert
-	 * RST_OUT as not compatible with some PHYs.
-	 */
-	i = IPG_AC_GLOBAL_RESET | IPG_AC_RX_RESET |
-	    IPG_AC_TX_RESET | IPG_AC_DMA |
-	    IPG_AC_FIFO | IPG_AC_NETWORK | IPG_AC_HOST | IPG_AC_AUTO_INIT;
-	/* Read/Write and Reset EEPROM Value Jesse20040128EEPROM_VALUE */
-	/* Read LED Mode Configuration from EEPROM */
-	sp->LED_Mode = read_eeprom(dev, 6);
-
-	error = ipg_reset(dev, i);
-	if (error < 0) {
-		return error;
-	}
-
-	/* Reset PHY. */
-	phyaddr = ipg_find_phyaddr(dev);
-
-	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 |
-				   GMII_PHY_1000BASETCONTROL_PreferMaster);
-
-		mii_phyctrl = read_phy_register(dev, phyaddr, GMII_PHY_CONTROL);
-		/* 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));
-
-	}
-
-	spin_lock_init(&sp->lock);
-
 	/* Check for interrupt line conflicts, and request interrupt
 	 * line for IPG.
 	 *
@@ -1743,26 +1558,6 @@
 		return error;
 	}
 
-	/* Read MAC Address from EERPOM Jesse20040128EEPROM_VALUE */
-	sp->StationAddr0 = read_eeprom(dev, 16);
-	sp->StationAddr1 = read_eeprom(dev, 17);
-	sp->StationAddr2 = read_eeprom(dev, 18);
-	/* Write MAC Address to Station Address */
-	iowrite16(sp->StationAddr0, ioaddr + IPG_STATIONADDRESS0);
-	iowrite16(sp->StationAddr1, ioaddr + IPG_STATIONADDRESS1);
-	iowrite16(sp->StationAddr2, ioaddr + IPG_STATIONADDRESS2);
-
-	/* Set station address in ethernet_device structure. */
-	dev->dev_addr[0] = ioread16(ioaddr + IPG_STATIONADDRESS0) & 0x00FF;
-	dev->dev_addr[1] =
-	    (ioread16(ioaddr + IPG_STATIONADDRESS0) & 0xFF00) >> 8;
-	dev->dev_addr[2] = ioread16(ioaddr + IPG_STATIONADDRESS1) & 0x00FF;
-	dev->dev_addr[3] =
-	    (ioread16(ioaddr + IPG_STATIONADDRESS1) & 0xFF00) >> 8;
-	dev->dev_addr[4] = ioread16(ioaddr + IPG_STATIONADDRESS2) & 0x00FF;
-	dev->dev_addr[5] =
-	    (ioread16(ioaddr + IPG_STATIONADDRESS2) & 0xFF00) >> 8;
-
 	/* Configure IPG I/O registers. */
 	error = ipg_io_config(dev);
 	if (error < 0) {
@@ -2860,7 +2655,8 @@
 	for (mc_list_ptr = dev->mc_list;
 	     mc_list_ptr != NULL; mc_list_ptr = mc_list_ptr->next) {
 		/* Calculate CRC result for each multicast address. */
-		hashindex = ether_crc_le(ETH_ALEN, mc_list_ptr->dmi_addr);
+		hashindex = crc32_le(0xffffffff, mc_list_ptr->dmi_addr,
+				     ETH_ALEN);
 
 		/* Use only the least significant 6 bits. */
 		hashindex = hashindex & 0x3F;
@@ -2883,86 +2679,79 @@
 		      ioread8(ioaddr + IPG_RECEIVEMODE));
 }
 
-/*
- * The following code fragment was authored by Donald Becker.
- */
-
-/* The little-endian AUTODIN II ethernet CRC calculations.
-   A big-endian version is also available.
-   This is slow but compact code.  Do not use this routine for bulk data,
-   use a table-based routine instead.
-   This is common code and should be moved to net/core/crc.c.
-   Chips may use the upper or lower CRC bits, and may reverse and/or invert
-   them.  Select the endian-ness that results in minimal calculations.
-*/
-static unsigned const ethernet_polynomial_le = 0xedb88320U;
-static unsigned ether_crc_le(int length, unsigned char *data)
+static int ipg_hw_init(struct net_device *dev)
 {
-	unsigned int crc = 0xffffffff;	/* Initial value. */
-	while (--length >= 0) {
-		unsigned char current_octet = *data++;
-		int bit;
-		for (bit = 8; --bit >= 0; current_octet >>= 1) {
-			if ((crc ^ current_octet) & 1) {
-				crc >>= 1;
-				crc ^= ethernet_polynomial_le;
-			} else
-				crc >>= 1;
-		}
-	}
-	return crc;
-}
+	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);
 
-/*
- * End of code fragment authored by Donald Becker.
- */
+	/* Reset all functions within the IPG. Do not assert
+	 * RST_OUT as not compatible with some PHYs.
+	 */
+	i = IPG_AC_GLOBAL_RESET | IPG_AC_RX_RESET |
+	    IPG_AC_TX_RESET | IPG_AC_DMA |
+	    IPG_AC_FIFO | IPG_AC_NETWORK | IPG_AC_HOST | IPG_AC_AUTO_INIT;
+	/* Read/Write and Reset EEPROM Value Jesse20040128EEPROM_VALUE */
+	/* Read LED Mode Configuration from EEPROM */
+	sp->LED_Mode = read_eeprom(dev, 6);
 
-static int ipg_nic_init(struct net_device *dev)
-{
-	/* Initialize IPG NIC. */
+	error = ipg_reset(dev, i);
+	if (error < 0) {
+		return error;
+	}
 
-	struct ipg_nic_private *sp = NULL;
+	ioaddr = ipg_ioaddr(dev);
 
-	IPG_DEBUG_MSG("_nic_init\n");
+	/* Reset PHY. */
+	phyaddr = ipg_find_phyaddr(dev);
 
-	/* Register the IPG NIC in the list of Ethernet devices. */
-	dev = alloc_etherdev(sizeof(struct ipg_nic_private));
+	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 |
+				   GMII_PHY_1000BASETCONTROL_PreferMaster);
 
-	if (dev == NULL) {
-		printk(KERN_INFO "Could not initialize IP1000 based NIC.\n");
-		return -ENODEV;
-	}
+		mii_phyctrl = read_phy_register(dev, phyaddr, GMII_PHY_CONTROL);
+		/* Set default phyparam */
+		pci_read_config_byte(sp->pdev, PCI_REVISION_ID, &revisionid);
+		ipg_set_phy_default_param(revisionid, dev, phyaddr);
 
-	/* Reserve memory for ipg_nic_private structure. */
-	sp = kmalloc(sizeof(struct ipg_nic_private), GFP_KERNEL);
+		/* reset Phy */
+		write_phy_register(dev,
+				   phyaddr, GMII_PHY_CONTROL,
+				   (mii_phyctrl | GMII_PHY_CONTROL_RESET |
+				    MII_PHY_CONTROL_RESTARTAN));
 
-	if (sp == NULL) {
-		printk(KERN_INFO
-		       "%s: No memory available for IP1000 private strucutre.\n",
-		       dev->name);
-		return -ENOMEM;
-	} else {
-		/* Fill the allocated memory space with 0s.
-		 * Essentially sets all ipg_nic_private
-		 * structure fields to 0.
-		 */
-		memset(sp, 0, sizeof(*sp));
-		dev->priv = sp;
 	}
 
-	/* Assign the new device to the list of IPG Ethernet devices. */
-	sp->next_dev = root_dev;
-	root_dev = dev;
+	/* Read MAC Address from EERPOM Jesse20040128EEPROM_VALUE */
+	sp->StationAddr0 = read_eeprom(dev, 16);
+	sp->StationAddr1 = read_eeprom(dev, 17);
+	sp->StationAddr2 = read_eeprom(dev, 18);
+	/* Write MAC Address to Station Address */
+	iowrite16(sp->StationAddr0, ioaddr + IPG_STATIONADDRESS0);
+	iowrite16(sp->StationAddr1, ioaddr + IPG_STATIONADDRESS1);
+	iowrite16(sp->StationAddr2, ioaddr + IPG_STATIONADDRESS2);
 
-	/* Declare IPG NIC functions for Ethernet device methods.
-	 */
-	dev->open = &ipg_nic_open;
-	dev->stop = &ipg_nic_stop;
-	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->change_mtu = &ipg_nic_change_mtu;
+	/* Set station address in ethernet_device structure. */
+	dev->dev_addr[0] = ioread16(ioaddr + IPG_STATIONADDRESS0) & 0x00FF;
+	dev->dev_addr[1] =
+	    (ioread16(ioaddr + IPG_STATIONADDRESS0) & 0xFF00) >> 8;
+	dev->dev_addr[2] = ioread16(ioaddr + IPG_STATIONADDRESS1) & 0x00FF;
+	dev->dev_addr[3] =
+	    (ioread16(ioaddr + IPG_STATIONADDRESS1) & 0xFF00) >> 8;
+	dev->dev_addr[4] = ioread16(ioaddr + IPG_STATIONADDRESS2) & 0x00FF;
+	dev->dev_addr[5] =
+	    (ioread16(ioaddr + IPG_STATIONADDRESS2) & 0xFF00) >> 8;
 
 	return 0;
 }
@@ -3144,99 +2933,23 @@
 
 static void ipg_remove(struct pci_dev *pdev)
 {
-	/* Remove function called when a IPG device is
-	 * to be shut down.
-	 */
-
-	struct net_device *prev_dev = NULL;
-	struct net_device *dev = NULL;
-	struct net_device *dev_to_remove = NULL;
-	struct ipg_nic_private *prev_sp = NULL;
-	struct ipg_nic_private *sp = NULL;
-	struct ipg_nic_private *sp_to_remove = NULL;
-
-	IPG_DEBUG_MSG("_pciremove_linux2_4\n");
-
-	dev = root_dev;
-
-	/* Move through list of Ethernet devices looking for
-	 * a match.
-	 */
-	while (dev) {
-		sp = netdev_priv(dev);
-
-		if (sp->pdev == pdev) {
-			/* Save the pointer to the previous Ethernet
-			 * device.
-			 */
-			dev_to_remove = dev;
-
-			sp_to_remove = sp;
-
-			break;
-		}
-
-		/* Save the "previous" device in the list. */
-		prev_dev = dev;
-
-		/* Retrieve next Ethernet device to be
-		 * released.
-		 */
-		dev = sp->next_dev;
-	}
-
-	/* Check if there is a device to remove. */
-	if (dev_to_remove == NULL) {
-		/* There are no Ethernet devices to remove. */
-		printk(KERN_INFO
-		       "A device remove request does not match with any Ethernet devices.\n");
-
-		return;
-	}
-
-	/* Check to see if we are removing the root device in the list. */
-	if (root_dev == dev_to_remove) {
-		/* Change the root Ethernet device to the next device to be
-		 * released.
-		 */
-		root_dev = sp_to_remove->next_dev;
-	} else if (sp_to_remove->next_dev != NULL)
-		/* Check if we need to re-link the list of devices. */
-	{
-		/* If the "previous" Ethernet device is NULL,
-		 * the device is at the head of the list, and
-		 * no re-linking is needed.
-		 */
-		prev_sp = netdev_priv(prev_dev);
-
-		prev_sp->next_dev = sp_to_remove->next_dev;
-	}
-
-	/* Free memory associated with Ethernet device's
-	 * private data structure.
-	 */
-	if (sp_to_remove) {
-		kfree(sp_to_remove);
-	}
+	struct net_device *dev = pci_get_drvdata(pdev);
+	struct ipg_nic_private *sp = netdev_priv(dev);
 
-	printk(KERN_INFO "Un-registering Ethernet device %s\n",
-	       dev_to_remove->name);
+	IPG_DEBUG_MSG("_remove\n");
 
 	/* Un-register Ethernet device. */
-	unregister_netdev(dev_to_remove);
+	unregister_netdev(dev);
 
 #ifdef USE_IO_OPS
 	ioport_unmap(ioaddr);
 #else
 	iounmap(sp->ioaddr);
 #endif
-
 	pci_release_regions(pdev);
 
-	/* Free memory associated with Ethernet device. */
-	if (dev_to_remove) {
-		kfree(dev_to_remove);
-	}
+	free_netdev(dev);
+	pci_disable_device(pdev);
 	pci_set_drvdata(pdev, NULL);
 }
 
@@ -3269,13 +2982,31 @@
 		goto out;
 	}
 
-	err = ipg_nic_init(dev);
-	if (err) {
-		printk(KERN_INFO "Could not intialize IP1000 based NIC.\n");
+	/*
+	 * Initialize net device.
+	 */
+	dev = alloc_etherdev(sizeof(struct ipg_nic_private));
+	if (!dev) {
+		printk(KERN_ERR "ipg: alloc_etherdev failed\n");
+		err = -ENOMEM;
 		goto out;
 	}
-	dev = root_dev;
+
+	sp = netdev_priv(dev);
+	spin_lock_init(&sp->lock);
+
+	/* Declare IPG NIC functions for Ethernet device methods.
+	 */
+	dev->open = &ipg_nic_open;
+	dev->stop = &ipg_nic_stop;
+	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->change_mtu = &ipg_nic_change_mtu;
+
 	SET_MODULE_OWNER(dev);
+	SET_NETDEV_DEV(dev, &pdev->dev);
 
 	err = pci_request_regions(pdev, DRV_NAME);
 	if (err)
@@ -3300,13 +3031,17 @@
 		goto out;
 	}
 #endif
-	sp = netdev_priv(root_dev);
+	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;
+
 	err = register_netdev(dev);
 	if (err)
 		goto out;
Index: linux-source-2.6.16/drivers/net/ipg.h
===================================================================
--- linux-source-2.6.16.orig/drivers/net/ipg.h	2006-04-30 23:02:09.861345105 +0100
+++ linux-source-2.6.16/drivers/net/ipg.h	2006-05-01 00:11:14.974349454 +0100
@@ -15,85 +15,6 @@
  * 408 873 4117
  * www.sundanceti.com
  * craig_rich@sundanceti.com
- *
- * Rev  Date     Description
- * --------------------------------------------------------------
- * 0.1  11/8/99  Initial revision work begins
- *
- * 0.2  12/1/99  Minor update to modversions.h inclusion.
- *
- * 0.3  12/30/99 Updates to fully comply with IPG spec.
- *
- * 0.4  4/24/00  Updates to allow for removal of FCS generation
- *               and verification.
- * 0.5  8/15/00  Updates for MII PHY registers and fields.
- *
- * 0.6  8/31/00  Updates to change to using 64 bit data types
- *
- * 0.7  10/31/00 Added DDEBUG_MSG to allow for easy activation of
- *               individual DEBUG_MSGs.
- *
- * 0.8  11/06/00 Changed LastFreedRxBuff to LastRestoredRxBuff for
- *               clarity.
- *
- * 0.9  11/10/00 Changed Sundance DeviceID to 0x9020
- *
- * 0.10 2/14/01  Changed "DROP_ON_ERRORS", breaking out Ethernet from
- *               TCP/IP errors.
- *
- * 0.11 3/16/01  Changed "IPG_FRAMESBETWEENTXCOMPLETES" to
- *               "IPG_FRAMESBETWEENTXDMACOMPLETES" since will
- *               be using TxDMAIndicate instead of TxIndicate to
- *               improve performance. Added TFDunavailCount and
- *               RFDlistendCount to aid in performance improvement.
- *
- * 0.12 3/22/01  Removed IPG_DROP_ON_RX_TCPIP_ERRORS.
- *
- * 0.13 3/23/01  Removed IPG_TXQUEUE_MARGIN.
- *
- * 0.14 3/30/01  Broke out sections into multiple files and added
- *               OS version specific detection and settings.
- */
-
-/*
- * Linux header utilization:
- *
- * config.h     For PCI support, namely CONFIG_PCI macro.
- *
- * version.h	For Linux kernel version detection.
- *
- * module.h	For modularized driver support.
- *
- * kernel.h     For 'printk'.
- *
- * pci.h        PCI support, including ID, VENDOR, and CLASS
- *              standard definitions; PCI specific structures,
- *              including pci_dev struct.
- *
- * ioport.h     I/O ports, check_region, request_region,
- *              release_region.
- *
- * errno.h      Standard error numbers, e.g. ENODEV.
- *
- * asm/io.h     For reading/writing I/O ports, and for virt_to_bus
- *		function.
- *
- * delay.h      For milisecond delays.
- *
- * types.h      For specific typedefs (i.e. u32, u16, u8).
- *
- * netdevice.h  For device structure needed for network support.
- *
- * etherdevice.h	For ethernet device support.
- *
- * init.h       For __initfunc.
- *
- * skbuff.h	Socket buffer (skbuff) definition.
- *
- * asm/bitops.h	For test_and_set_bit, clear_bit functions.
- *
- * asm/spinlock.h For spin_lock_irqsave, spin_lock_irqrestore functions.
- *
  */
 #ifndef __LINUX_IPG_H
 #define __LINUX_IPG_H
@@ -1000,9 +921,6 @@
  * End miscellaneous macros.
  */
 
-/* IPG Ethernet device structure, used for removing module. */
-struct net_device *root_dev = NULL;
-
 /* Transmit Frame Descriptor. The IPG supports 15 fragments,
  * however Linux requires only a single fragment. Note, each
  * TFD field is 64 bits wide.
@@ -1054,7 +972,6 @@
 	int RxBuffNotReady;
 	struct pci_dev *pdev;
 	struct net_device_stats stats;
-	struct net_device *next_dev;
 	spinlock_t lock;
 	int tenmbpsmode;
 

Re: IP1000 gigabit nic driver

[Pekka Enberg]

On Mon, 2006-05-01 at 00:40 +0100, David Vrabel wrote:
> Thanks for doing this Pekka.  I've fixed up some stuff and given it some 
> brief testing on a 100BaseT network and it seems to work now.

Thanks! I merged your stuff and pushed out an updated 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.
    We should use ethtool ioctl instead as suggested by Arjan.
  - something (PHY reset/auto negotiation?) takes 2-3 seconds and
    appears to be done with interrupts disabled.

Changelog:

  - Kill 2.2 and 2.4 compatability macros
  - Use proper module API
  - Use proper PCI API
  - Use netdev_priv
  - Consolidate headers to one file
  - Use __iomem annotations
  - Use iomap instead of read/out for I/O
  - Remove obfuscating register access macros
  - Remove changelogs
  - 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.

I don't have the hardware, so I don't know if I broke anything.
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 <dvrabel@cantab.net>
Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>

Re: IP1000 gigabit nic driver

[Pekka Enberg]

On Mon, 2006-05-01 at 00:40 +0100, David Vrabel wrote:
> Still pending.  Also:
> 
>      - something (PHY reset/auto negotiation?) takes 2-3 seconds and
>        appears to be done with interrupts disabled.

Are you seeing this at module initialization? Does Sysrq-t show anything
useful?

					Pekka

Re: IP1000 gigabit nic driver

[David Vrabel]

[-- Attachment #1: Type: text/plain, Size: 441 bytes --]

David Vrabel wrote:
> 
>     - something (PHY reset/auto negotiation?) takes 2-3 seconds and
>       appears to be done with interrupts disabled.

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.

David Vrabel

[-- Attachment #2: drivers-net-ipg-speed-up-phy-access --]
[-- Type: text/plain, Size: 3542 bytes --]

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 <dvrabel@cantab.net>

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
 

Re: IP1000 gigabit nic driver

[Pekka Enberg]

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 <dvrabel@cantab.net>
Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>

Re: IP1000 gigabit nic driver

[David Gómez]

Hi Pekka,

On Apr 29 at 11:35:04, Pekka Enberg wrote:
> No, I haven't. I don't have the hardware, so I can't test the driver.
> Furthermore, there's plenty of stuff to fix before it's in any shape for
> submission. If someone wants to give this patch a spin, I would love to
> hear the results.

The latest version of the driver including David's improvements works
here. Nice work ;)

-- 
David Gómez                                      Jabber ID: davidge@jabber.org

Re: IP1000 gigabit nic driver

[Francois Romieu]

David Vrabel <dvrabel@cantab.net> :
[...]
> 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

Re: IP1000 gigabit nic driver

[Lennert Buytenhek]

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?

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

[Francois Romieu]

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

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>

---

 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

[PATCH 2/3] ipg: leaks in ipg_probe

[Francois Romieu]

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 <romieu@fr.zoreil.com>

---

 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

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

[Francois Romieu]

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

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>

---

 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

Re: IP1000 gigabit nic driver

[David Vrabel]

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

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

[Pekka J Enberg]

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

Applied. Thanks!

				Pekka

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

[Pekka J Enberg]

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 <romieu@fr.zoreil.com>

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

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

[Pekka J Enberg]

On Tue, 2 May 2006, Francois Romieu wrote:
> Added ipg_{rx/tx}_clear() to factor out some code.
> 
> Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>

Applied. Thanks!

			Pekka

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

[Francois Romieu]

Pekka J Enberg <penberg@cs.Helsinki.FI> :
[...]
> 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

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

[Pekka Enberg]

Pekka J Enberg <penberg@cs.Helsinki.FI> :
> [...]
> > 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 <penberg@cs.Helsinki.FI> :
> 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

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

[Francois Romieu]

- 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 <romieu@fr.zoreil.com>

---

 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

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

[Francois Romieu]

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

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>

---

 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 @@
  * craig_rich@sundanceti.com
  */
 #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

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

[Francois Romieu]

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 <romieu@fr.zoreil.com>
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 <romieu@fr.zoreil.com>

commit 291360d4000e0b93baf0fb97aa15af48677e46af
Author: Romieu Francois <romieu@fr.zoreil.com>
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 <romieu@fr.zoreil.com>

commit 8b534f66d6be247b9f0d341b0ae7acbf46f128fb
Author: Romieu Francois <romieu@fr.zoreil.com>
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 <romieu@fr.zoreil.com>

commit 87c8b13dceccc42439e7262f1b60c9cbb14d5440
Author: Romieu Francois <romieu@fr.zoreil.com>
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 <romieu@fr.zoreil.com>

commit befd7e543fcdfb2d2c75de1748ae751fefdff58a
Author: Romieu Francois <romieu@fr.zoreil.com>
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 <romieu@fr.zoreil.com>

commit 17a9ce93ba6b6744489aa9168d757e9457158952
Author: Romieu Francois <romieu@fr.zoreil.com>
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 <dvrabel@cantab.net>

commit e18c33d6fa62b735426b8fe5a0f1aa61c0b5d7f7
Author: David Vrabel <dvrabel@cantab.net>
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 <dvrabel@cantab.net>

commit e99fcd4253f231b2c7a96e3be5067341de45ac2e
Author: David Vrabel <dvrabel@cantab.net>
Date:   Mon May 1 13:20:49 2006 +0200

    ipg: remove changelogs
    
    Signed-off-by: David Vrabel <dvrabel@cantab.net>

commit 8fd59026a272d3132f096965985e907d655ee087
Author: Pekka Enberg <penberg@cs.helsinki.fi>
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 <penberg@cs.helsinki.fi>

[PATCH] ipg: removing more dead code

[David Gómez]

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)
 {

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

[Pekka J Enberg]

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

Re: IP1000 gigabit nic driver

[Andrew Morton]

On Mon, 01 May 2006 12:31:40 +0300
Pekka Enberg <penberg@cs.helsinki.fi> wrote:

> [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

Please remember that to merge this we'll need a signed-off-by from the
original developers.  (That's not very gplish, but such is life).

Re: IP1000 gigabit nic driver

[Pekka J Enberg]

On Wed, 3 May 2006, Andrew Morton wrote:
> Please remember that to merge this we'll need a signed-off-by from the
> original developers.  (That's not very gplish, but such is life).

OK. Lets see if we can track one of them developers down. I see Craig 
Rich's email (only email found in the original mail) is out of date so if 
anyone knows how to reach him, please let me know. Thanks!

				Pekka

Re: [PATCH] ipg: removing more dead code

[Pekka J Enberg]

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

Re: IP1000 gigabit nic driver

[David Vrabel]

Pekka J Enberg wrote:
> On Wed, 3 May 2006, Andrew Morton wrote:
>> Please remember that to merge this we'll need a signed-off-by from the
>> original developers.  (That's not very gplish, but such is life).
> 
> OK. Lets see if we can track one of them developers down. I see Craig 
> Rich's email (only email found in the original mail) is out of date so if 
> anyone knows how to reach him, please let me know. Thanks!

I think/guess that IC Plus bought out Sundance so you'd need to contact
someone at IC Plus.

David Vrabel

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

[David Gómez]

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: davidge@jabber.org

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

[Francois Romieu]

Pekka J Enberg <penberg@cs.Helsinki.FI> :
[...]
> 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 <romieu@fr.zoreil.com>
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 <romieu@fr.zoreil.com>

commit 65940e5e0ab88d92fbac0f96b5d46ddfbd5cfa93
Author: Francois Romieu <romieu@fr.zoreil.com>
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 <romieu@fr.zoreil.com>

commit ab87a106690d6eaba4b7426fb074270e2e503e40
Author: Francois Romieu <romieu@fr.zoreil.com>
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 <romieu@fr.zoreil.com>

commit ef7bfd886bc436d14649e962edb6f5189cc4dcac
Author: Francois Romieu <romieu@fr.zoreil.com>
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 <romieu@fr.zoreil.com>

-- 
Ueimor

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

[David Vrabel]

Francois Romieu wrote:
> 
>     ipg: remove forward declarations
>     
>     It makes no sense in a new driver.
>     
>     Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>

Ack.

>     ipg: replace #define with enum
>     
>     Added some underscores to improve readability.
>     
>     Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>

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.

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

[Pekka Enberg]

Pekka J Enberg <penberg@cs.Helsinki.FI> :
> [...]
> > 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

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

[Francois Romieu]

Pekka Enberg <penberg@cs.helsinki.fi> :
[...]
> 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 <romieu@fr.zoreil.com>
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 <romieu@fr.zoreil.com>

commit 997a02f4c4b3cf2ec0177ba33ffda6d550396eb7
Author: Francois Romieu <romieu@fr.zoreil.com>
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 <romieu@fr.zoreil.com>

commit b141c4ec3572f3151b2eb037ef5e7aba8d190445
Author: Francois Romieu <romieu@fr.zoreil.com>
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 <romieu@fr.zoreil.com>

commit 9d27e1ab51b2295d72ea254bbc8de5b5797a4139
Author: Francois Romieu <romieu@fr.zoreil.com>
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 <romieu@fr.zoreil.com>

commit 42ad0c39990c1897ee56dfc34bb4a7b2de76ace0
Author: Francois Romieu <romieu@fr.zoreil.com>
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 <romieu@fr.zoreil.com>

commit be97a643f5d955208c0be7aa9e16a05c7737a2be
Author: Romieu Francois <romieu@fr.zoreil.com>
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 <romieu@fr.zoreil.com>

commit 273a5a5f0815a35cf06faeafea6fc47503d364cb
Author: Romieu Francois <romieu@fr.zoreil.com>
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 <romieu@fr.zoreil.com>

commit e1e773d79369783d68fd7f66fffbe154929df837
Author: Romieu Francois <romieu@fr.zoreil.com>
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 <romieu@fr.zoreil.com>

commit 9cb655fddfd2d169bc7574e849fa5eb179fef12c
Author: Romieu Francois <romieu@fr.zoreil.com>
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 <romieu@fr.zoreil.com>

commit 2b5d40c1c4cf795c31d2683bcdff646a8984115d
Author: Romieu Francois <romieu@fr.zoreil.com>
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 <romieu@fr.zoreil.com>

commit 5b8bde4d4c02e3d54672d0c431941d9357c35417
Author: Romieu Francois <romieu@fr.zoreil.com>
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 <dvrabel@cantab.net>

commit d4a621ceadb0e4ee83a644be4ffef8bdef687249
Author: David Vrabel <dvrabel@cantab.net>
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 <dvrabel@cantab.net>

commit ae6f7b51bfb69a60062d390f10c83396c37f0301
Author: David Vrabel <dvrabel@cantab.net>
Date:   Mon May 1 13:20:49 2006 +0200

    ip1000: remove changelogs
    
    Signed-off-by: David Vrabel <dvrabel@cantab.net>

commit 7aeb86e2db5deac7d8357d6d794550999fd5e7c6
Author: Pekka Enberg <penberg@cs.helsinki.fi>
Date:   Sun Apr 30 12:45:16 2006 +0300

    ip1000: alloc_etherdev already allocates memory for private data
    
    Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>

commit 49b94ecdf8124c988160585d460b1cc643ee07e0
Author: Pekka Enberg <penberg@cs.helsinki.fi>
Date:   Sun Apr 30 12:42:02 2006 +0300

    ip1000: remove unused forward declarations
    
    Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>

commit e85531a2d8c23b6ff17b3a8cd6d14e6c09954240
Author: Pekka Enberg <penberg@cs.helsinki.fi>
Date:   Sun Apr 30 12:16:58 2006 +0300

    ip1000: interrupt handler code cleanups
    
    Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>

commit 822b9f528c47df857514e33d1ad98a9a551a3b00
Author: Pekka Enberg <penberg@cs.helsinki.fi>
Date:   Sun Apr 30 12:05:09 2006 +0300

    ip1000: rename baseaddr to ioaddr
    
    Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>

commit e8e5f8dece06429793074df6f956a8c4cb60c362
Author: Pekka Enberg <penberg@cs.helsinki.fi>
Date:   Sun Apr 30 12:02:51 2006 +0300

    ip1000: remove device register write macros
    
    Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>

commit 8e1a207d9e0d355aa2949ef4e5710e8c8987b01b
Author: Pekka Enberg <penberg@cs.helsinki.fi>
Date:   Sun Apr 30 11:41:01 2006 +0300

    ip1000: kill obfuscating register read macros
    
    Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>

commit fd3e0c5b3599375024e1e7fc3ddb00f7f5bc0c53
Author: Pekka Enberg <penberg@cs.helsinki.fi>
Date:   Sun Apr 30 00:03:23 2006 +0300

    ip1000: use iomap for pio/mmio
    
    Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>

commit 7ed56b01680978afc5db87f684de81398366bf84
Author: Pekka Enberg <penberg@cs.helsinki.fi>
Date:   Sat Apr 29 14:02:34 2006 +0300

    ip1000: use static for private symbols
    
    Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>

commit b83bac329befd061bb66377d33b1efd4dd39a931
Author: Pekka Enberg <penberg@cs.helsinki.fi>
Date:   Sat Apr 29 13:19:36 2006 +0300

    ip1000: sparse fixes
    
    Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>

commit 65229dd8a40d887364f3aa60277ac3f0e8c64041
Author: Pekka Enberg <penberg@cs.helsinki.fi>
Date:   Sat Apr 29 13:06:27 2006 +0300

    ip1000: iomem annotations
    
    Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>

commit d79a8a846188eee278f5b490b7f15c070f1245d4
Author: Pekka Enberg <penberg@cs.helsinki.fi>
Date:   Sat Apr 29 12:41:45 2006 +0300

    ip1000: new gigabit ethernet device driver
    
    Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>

----------------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 <romieu@fr.zoreil.com>

---

 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 @@
  * craig_rich@sundanceti.com
  */
 #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

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

[Francois Romieu]

David Vrabel <dvrabel@cantab.net> :
[...]
> >    ipg: replace #define with enum
> >    
> >    Added some underscores to improve readability.
> >    
> >    Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
> 
> 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

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

[David Vrabel]

[-- Attachment #1: Type: text/plain, Size: 299 bytes --]

Apologies for the (very) late response.

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.

Did anyone manage to get a response from IC Plus regarding the required 
Signed-off-by line?

David Vrabel

[-- Attachment #2: ipg-fix-init_rfdlist --]
[-- Type: text/plain, Size: 1063 bytes --]

ipg: initialize Rx buffers correctly

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

Signed-off-by: David Vrabel <dvrabel@cantab.net>

--- 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;
 

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

[Pekka Enberg]

Hi David,

On 5/21/06, David Vrabel <dvrabel@cantab.net> 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
jesse@icplus.com.tw.  I have no received confirmation if he can sign
off or not yet.

                                         Pekka

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

[Francois Romieu]

David Vrabel <dvrabel@cantab.net> :
[...]
> 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

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

[jesse\(建興\)]

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" <romieu@fr.zoreil.com>
To: "David Vrabel" <dvrabel@cantab.net>
Cc: "Pekka Enberg" <penberg@cs.helsinki.fi>; <linux-kernel@vger.kernel.org>;
<netdev@vger.kernel.org>; <david@pleyades.net>; <jesse@icplus.com.tw>
Sent: Sunday, May 21, 2006 6:16 PM
Subject: Re: [PATCH 2/2] ipg: redundancy with mii.h


David Vrabel <dvrabel@cantab.net> :
[...]
> 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

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

[jesse\(建興\)]

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. jesse@icplus.com.tw

Best Regards,
Jesse

----- Original Message ----- 
From: "Francois Romieu" <romieu@fr.zoreil.com>
To: "David Vrabel" <dvrabel@cantab.net>
Cc: "Pekka Enberg" <penberg@cs.helsinki.fi>; <linux-kernel@vger.kernel.org>;
<netdev@vger.kernel.org>; <david@pleyades.net>; <jesse@icplus.com.tw>
Sent: Sunday, May 21, 2006 6:16 PM
Subject: Re: [PATCH 2/2] ipg: redundancy with mii.h


David Vrabel <dvrabel@cantab.net> :
[...]
> 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