Re: [patch 3/3] Add tsi108 On Chip Ethernet device driver support
On Mon, 2006-10-23 at 10:09, Zang Roy-r61911 wrote: On Thu, 2006-09-21 at 12:46, Jeff Garzik wrote: you should have a chip structure, that contains two structs (one for each interface/port) Jeff, I updated the code according to all your feedback and post it here http://www.spinics.net/lists/netdev/msg17120.html Any comment? Roy Jeff How about it? What's your comment? Roy - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/3] Add tsi108 On Chip Ethernet device driver support
On Thu, 2006-09-21 at 12:46, Jeff Garzik wrote: you should have a chip structure, that contains two structs (one for each interface/port) Jeff, I updated the code according to all your feedback and post it here http://www.spinics.net/lists/netdev/msg17120.html Any comment? Roy - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/3] Add tsi108 On Chip Ethernet device driver support
On Thu, 2006-09-21 at 12:46, Jeff Garzik wrote:
+
+/* Synchronization is needed between the thread and up/down events.
+ * Note that the PHY is accessed through the same registers for
both
+ * interfaces, so this can't be made interface-specific.
+ */
+
+static DEFINE_SPINLOCK(phy_lock);
you should have a chip structure, that contains two structs (one for
each interface/port)
Do you mean you want to see this spinlock within dynamically allocated
data structure, and not as a global variable?
I am not sure :-).
All your other comments have been addressed. The following is the
updated tsi108 Ethernet port driver.
Any comment?
Signed-off-by: Alexandre Bounine[EMAIL PROTECTED]
Signed-off-by: Roy Zang [EMAIL PROTECTED]
diff --git a/drivers/net/tsi108_eth.c b/drivers/net/tsi108_eth.c
new file mode 100644
index 000..6c3146d
--- /dev/null
+++ b/drivers/net/tsi108_eth.c
@@ -0,0 +1,1709 @@
+/***
+
+ Copyright(c) 2006 Tundra Semiconductor Corporation.
+
+ This program is free software; you can redistribute it and/or modify it
+ under the terms of the GNU General Public License as published by the Free
+ Software Foundation; either version 2 of the License, or (at your option)
+ any later version.
+
+ This program is distributed in the hope that it will be useful, but WITHOUT
+ ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ more details.
+
+ You should have received a copy of the GNU General Public License along with
+ this program; if not, write to the Free Software Foundation, Inc., 59
+ Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+
+***/
+
+/* This driver is based on the driver code originally developed
+ * for the Intel IOC80314 (ForestLake) Gigabit Ethernet by
+ * [EMAIL PROTECTED] * Copyright (C) 2003 TimeSys Corporation
+ *
+ * Currently changes from original version are:
+ * - porting to Tsi108-based platform and kernel 2.6 ([EMAIL PROTECTED])
+ * - modifications to handle two ports independently and support for
+ * additional PHY devices ([EMAIL PROTECTED])
+ * - Get hardware information from platform device. ([EMAIL PROTECTED])
+ *
+ */
+
+#include linux/module.h
+#include linux/types.h
+#include linux/init.h
+#include linux/net.h
+#include linux/netdevice.h
+#include linux/etherdevice.h
+#include linux/skbuff.h
+#include linux/slab.h
+#include linux/sched.h
+#include linux/spinlock.h
+#include linux/delay.h
+#include linux/crc32.h
+#include linux/mii.h
+#include linux/device.h
+#include linux/pci.h
+#include linux/rtnetlink.h
+#include linux/timer.h
+#include linux/platform_device.h
+#include linux/etherdevice.h
+
+#include asm/system.h
+#include asm/io.h
+#include asm/tsi108.h
+
+#include tsi108_eth.h
+
+#define MII_READ_DELAY 1 /* max link wait time in msec */
+
+#define TSI108_RXRING_LEN 256
+
+/* NOTE: The driver currently does not support receiving packets
+ * larger than the buffer size, so don't decrease this (unless you
+ * want to add such support).
+ */
+#define TSI108_RXBUF_SIZE 1536
+
+#define TSI108_TXRING_LEN 256
+
+#define TSI108_TX_INT_FREQ64
+
+/* Check the phy status every half a second. */
+#define CHECK_PHY_INTERVAL (HZ/2)
+
+static int tsi108_init_one(struct platform_device *pdev);
+static int tsi108_ether_remove(struct platform_device *pdev);
+
+struct tsi108_prv_data {
+ void __iomem *regs;/* Base of normal regs */
+ void __iomem *phyregs; /* Base of register bank used for PHY access */
+
+ unsigned int phy; /* Index of PHY for this interface */
+ unsigned int irq_num;
+ unsigned int id;
+
+ struct timer_list timer;/* Timer that triggers the check phy function */
+ unsigned int rxtail;/* Next entry in rxring to read */
+ unsigned int rxhead;/* Next entry in rxring to give a new buffer */
+ unsigned int rxfree;/* Number of free, allocated RX buffers */
+
+ unsigned int rxpending; /* Non-zero if there are still descriptors
+* to be processed from a previous descriptor
+* interrupt condition that has been cleared */
+
+ unsigned int txtail;/* Next TX descriptor to check status on */
+ unsigned int txhead;/* Next TX descriptor to use */
+
+ /* Number of free TX descriptors. This could be calculated from
+* rxhead and rxtail if one descriptor were left unused to disambiguate
+* full and empty conditions, but it's simpler to just keep track
+* explicitly. */
+
+ unsigned int txfree;
+
+ unsigned int phy_ok;/* The PHY is currently powered on. */
+
+ /* PHY status (duplex is 1 for half, 2 for full,
+
Re: [patch 3/3] Add tsi108 On Chip Ethernet device driver support
On Thu, 2006-09-21 at 12:46, Jeff Garzik wrote:
Zang Roy-r61911 wrote:
+struct tsi108_prv_data {
+ void __iomem *regs;/* Base of normal regs */
+ void __iomem *phyregs; /* Base of register bank used for PHY
access */
+
+ int phy;/* Index of PHY for this interface */
+ int irq_num;
+ int id;
+
+ struct timer_list timer;/* Timer that triggers the check phy
function */
+ int rxtail; /* Next entry in rxring to read */
+ int rxhead; /* Next entry in rxring to give a new
buffer */
+ int rxfree; /* Number of free, allocated RX
buffers */
+
+ int rxpending; /* Non-zero if there are still
descriptors
+ * to be processed from a previous
descriptor
+ * interrupt condition that has been
cleared */
+
+ int txtail; /* Next TX descriptor to check status
on */
+ int txhead; /* Next TX descriptor to use */
most of these should be unsigned, to prevent bugs.
+ /* Number of free TX descriptors. This could be calculated
from
+ * rxhead and rxtail if one descriptor were left unused to
disambiguate
+ * full and empty conditions, but it's simpler to just keep
track
+ * explicitly. */
+
+ int txfree;
+
+ int phy_ok; /* The PHY is currently powered on. */
+
+ /* PHY status (duplex is 1 for half, 2 for full,
+ * so that the default 0 indicates that neither has
+ * yet been configured). */
+
+ int link_up;
+ int speed;
+ int duplex;
+
+ tx_desc *txring;
+ rx_desc *rxring;
+ struct sk_buff *txskbs[TSI108_TXRING_LEN];
+ struct sk_buff *rxskbs[TSI108_RXRING_LEN];
+
+ dma_addr_t txdma, rxdma;
+
+ /* txlock nests in misclock and phy_lock */
+
+ spinlock_t txlock, misclock;
+
+ /* stats is used to hold the upper bits of each hardware
counter,
+ * and tmpstats is used to hold the full values for returning
+ * to the caller of get_stats(). They must be separate in
case
+ * an overflow interrupt occurs before the stats are consumed.
+ */
+
+ struct net_device_stats stats;
+ struct net_device_stats tmpstats;
+
+ /* These stats are kept separate in hardware, thus require
individual
+ * fields for handling carry. They are combined in get_stats.
+ */
+
+ unsigned long rx_fcs; /* Add to rx_frame_errors */
+ unsigned long rx_short_fcs; /* Add to rx_frame_errors */
+ unsigned long rx_long_fcs; /* Add to rx_frame_errors */
+ unsigned long rx_underruns; /* Add to rx_length_errors */
+ unsigned long rx_overruns; /* Add to rx_length_errors */
+
+ unsigned long tx_coll_abort;/* Add to
tx_aborted_errors/collisions */
+ unsigned long tx_pause_drop;/* Add to tx_aborted_errors */
+
+ unsigned long mc_hash[16];
+};
+
+/* Structure for a device driver */
+
+static struct platform_driver tsi_eth_driver = {
+ .probe = tsi108_init_one,
+ .remove = tsi108_ether_remove,
+ .driver = {
+ .name = tsi-ethernet,
+ },
+};
+
+static void tsi108_timed_checker(unsigned long dev_ptr);
+
+static void dump_eth_one(struct net_device *dev)
+{
+ struct tsi108_prv_data *data = netdev_priv(dev);
+
+ printk(Dumping %s...\n, dev-name);
+ printk(intstat %x intmask %x phy_ok %d
+ link %d speed %d duplex %d\n,
+TSI108_ETH_READ_REG(TSI108_EC_INTSTAT),
+TSI108_ETH_READ_REG(TSI108_EC_INTMASK), data-phy_ok,
+data-link_up, data-speed, data-duplex);
+
+ printk(TX: head %d, tail %d, free %d, stat %x, estat %x, err
%x\n,
+data-txhead, data-txtail, data-txfree,
+TSI108_ETH_READ_REG(TSI108_EC_TXSTAT),
+TSI108_ETH_READ_REG(TSI108_EC_TXESTAT),
+TSI108_ETH_READ_REG(TSI108_EC_TXERR));
+
+ printk(RX: head %d, tail %d, free %d, stat %x,
+ estat %x, err %x, pending %d\n\n,
+data-rxhead, data-rxtail, data-rxfree,
+TSI108_ETH_READ_REG(TSI108_EC_RXSTAT),
+TSI108_ETH_READ_REG(TSI108_EC_RXESTAT),
+TSI108_ETH_READ_REG(TSI108_EC_RXERR), data-rxpending);
+}
+
+/* Synchronization is needed between the thread and up/down events.
+ * Note that the PHY is accessed through the same registers for
both
+ * interfaces, so this can't be made interface-specific.
+ */
+
+static DEFINE_SPINLOCK(phy_lock);
you should have a chip structure, that contains two structs (one for
each interface/port)
Could you interpret the chip structure in more detail?
Need I create two net_device struct for each port?
Thanks.
Roy
-
To unsubscribe from this list: send the line
Re: [patch 3/3] Add tsi108 On Chip Ethernet device driver support
Zang Roy-r61911 wrote: Could you interpret the chip structure in more detail? Need I create two net_device struct for each port? No. One net_device per port. And one container structure for the entire device. Jeff - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/3] Add tsi108 On Chip Ethernet device driver support
Zang Roy-r61911 wrote: On Thu, 2006-09-21 at 12:26, Jeff Garzik wrote: Zang Roy-r61911 wrote: +#define TSI108_ETH_WRITE_REG(offset, val) \ + writel(le32_to_cpu(val),data-regs + (offset)) + +#define TSI108_ETH_READ_REG(offset) \ + le32_to_cpu(readl(data-regs + (offset))) + +#define TSI108_ETH_WRITE_PHYREG(offset, val) \ + writel(le32_to_cpu(val), data-phyregs + (offset)) + +#define TSI108_ETH_READ_PHYREG(offset) \ + le32_to_cpu(readl(data-phyregs + (offset))) NAK: 1) writel() and readl() are defined to be little endian. If your platform is different, then your platform should have its own foobus_writel() and foobus_readl(). Tsi108 bridge is designed for powerpc platform. Originally, I use out_be32() and in_be32(). While there is no obvious reason to object using this bridge in a little endian system. Maybe some extra hardware logic needed for the bus interface. le32_to_cpu() can be aware the endian difference. To restate, readl() should read a little endian value, and return a CPU-endian value. writel() should receive a CPU-endian value, and write a little endian value. If your platform's readl/writel doesn't do that, it's broken. That's why normal PCI drivers can use readl() and writel() on either big-endian or little-endian machines, without needing to use le32_to_cpu(). Jeff - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [patch 3/3] Add tsi108 On Chip Ethernet device driver support
On Tue, 2006-09-19 at 15:39 +0800, Zang Roy-r61911 wrote: + spin_unlock_irq(phy_lock); + msleep(10); + spin_lock_irq(phy_lock); + } hmm some places take phy_lock with disabling interrupts, while others don't. I sort of fear the others may be buggy are you sure those are ok? Could you interpret your comments in detail? Roy Hi, sorry for being unclear/too short in the review. The phy_lock lock is sometimes taken as spin_lock() and sometimes as spin_lock_irq(). It looks likes it can be used in interrupt context, in which case the spin_lock_irq() version is correct and the places where spin_lock() is used would be a deadlock bug (just think what happens if the interrupt happens while spin_lock(phy_lock) is helt, and the spinlock then again tries to take the lock!) If there is no way this lock is used in interrupt context, then the spin_lock_irq() version is doing something which is not needed and also a bit expensive; so could be optimized. But my impression is that the _irq() is needed. Also, please consider switching from spin_lock_irq() to spin_lock_irqsave() version instead; spin_unlock_irq() has some side effects (interrupts get enabled unconditionally) so it is generally safer to use spin_lock_irqsave()/spin_unlock_irqrestore() API. If you have more questions please do not hesitate to ask! Greetings, Arjan van de Ven -- if you want to mail me at work (you don't), use arjan (at) linux.intel.com - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/3] Add tsi108 On Chip Ethernet device driver support
Zang Roy-r61911 wrote: +#define TSI108_ETH_WRITE_REG(offset, val) \ + writel(le32_to_cpu(val),data-regs + (offset)) + +#define TSI108_ETH_READ_REG(offset) \ + le32_to_cpu(readl(data-regs + (offset))) + +#define TSI108_ETH_WRITE_PHYREG(offset, val) \ + writel(le32_to_cpu(val), data-phyregs + (offset)) + +#define TSI108_ETH_READ_PHYREG(offset) \ + le32_to_cpu(readl(data-phyregs + (offset))) NAK: 1) writel() and readl() are defined to be little endian. If your platform is different, then your platform should have its own foobus_writel() and foobus_readl(). 2) TSI108_ETH_WRITE_REG() is just way too long. TSI_READ(), TSI_WRITE(), TSI_READ_PHY() and TSI_WRITE_PHY() would be far more readable. More in next email. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/3] Add tsi108 On Chip Ethernet device driver support
Zang Roy-r61911 wrote:
+struct tsi108_prv_data {
+ void __iomem *regs;/* Base of normal regs */
+ void __iomem *phyregs; /* Base of register bank used for PHY access */
+
+ int phy;/* Index of PHY for this interface */
+ int irq_num;
+ int id;
+
+ struct timer_list timer;/* Timer that triggers the check phy function */
+ int rxtail; /* Next entry in rxring to read */
+ int rxhead; /* Next entry in rxring to give a new buffer */
+ int rxfree; /* Number of free, allocated RX buffers */
+
+ int rxpending; /* Non-zero if there are still descriptors
+* to be processed from a previous descriptor
+* interrupt condition that has been cleared */
+
+ int txtail; /* Next TX descriptor to check status on */
+ int txhead; /* Next TX descriptor to use */
most of these should be unsigned, to prevent bugs.
+ /* Number of free TX descriptors. This could be calculated from
+* rxhead and rxtail if one descriptor were left unused to disambiguate
+* full and empty conditions, but it's simpler to just keep track
+* explicitly. */
+
+ int txfree;
+
+ int phy_ok; /* The PHY is currently powered on. */
+
+ /* PHY status (duplex is 1 for half, 2 for full,
+* so that the default 0 indicates that neither has
+* yet been configured). */
+
+ int link_up;
+ int speed;
+ int duplex;
+
+ tx_desc *txring;
+ rx_desc *rxring;
+ struct sk_buff *txskbs[TSI108_TXRING_LEN];
+ struct sk_buff *rxskbs[TSI108_RXRING_LEN];
+
+ dma_addr_t txdma, rxdma;
+
+ /* txlock nests in misclock and phy_lock */
+
+ spinlock_t txlock, misclock;
+
+ /* stats is used to hold the upper bits of each hardware counter,
+* and tmpstats is used to hold the full values for returning
+* to the caller of get_stats(). They must be separate in case
+* an overflow interrupt occurs before the stats are consumed.
+*/
+
+ struct net_device_stats stats;
+ struct net_device_stats tmpstats;
+
+ /* These stats are kept separate in hardware, thus require individual
+* fields for handling carry. They are combined in get_stats.
+*/
+
+ unsigned long rx_fcs; /* Add to rx_frame_errors */
+ unsigned long rx_short_fcs; /* Add to rx_frame_errors */
+ unsigned long rx_long_fcs; /* Add to rx_frame_errors */
+ unsigned long rx_underruns; /* Add to rx_length_errors */
+ unsigned long rx_overruns; /* Add to rx_length_errors */
+
+ unsigned long tx_coll_abort;/* Add to tx_aborted_errors/collisions
*/
+ unsigned long tx_pause_drop;/* Add to tx_aborted_errors */
+
+ unsigned long mc_hash[16];
+};
+
+/* Structure for a device driver */
+
+static struct platform_driver tsi_eth_driver = {
+ .probe = tsi108_init_one,
+ .remove = tsi108_ether_remove,
+ .driver = {
+ .name = tsi-ethernet,
+ },
+};
+
+static void tsi108_timed_checker(unsigned long dev_ptr);
+
+static void dump_eth_one(struct net_device *dev)
+{
+ struct tsi108_prv_data *data = netdev_priv(dev);
+
+ printk(Dumping %s...\n, dev-name);
+ printk(intstat %x intmask %x phy_ok %d
+ link %d speed %d duplex %d\n,
+ TSI108_ETH_READ_REG(TSI108_EC_INTSTAT),
+ TSI108_ETH_READ_REG(TSI108_EC_INTMASK), data-phy_ok,
+ data-link_up, data-speed, data-duplex);
+
+ printk(TX: head %d, tail %d, free %d, stat %x, estat %x, err %x\n,
+ data-txhead, data-txtail, data-txfree,
+ TSI108_ETH_READ_REG(TSI108_EC_TXSTAT),
+ TSI108_ETH_READ_REG(TSI108_EC_TXESTAT),
+ TSI108_ETH_READ_REG(TSI108_EC_TXERR));
+
+ printk(RX: head %d, tail %d, free %d, stat %x,
+ estat %x, err %x, pending %d\n\n,
+ data-rxhead, data-rxtail, data-rxfree,
+ TSI108_ETH_READ_REG(TSI108_EC_RXSTAT),
+ TSI108_ETH_READ_REG(TSI108_EC_RXESTAT),
+ TSI108_ETH_READ_REG(TSI108_EC_RXERR), data-rxpending);
+}
+
+/* Synchronization is needed between the thread and up/down events.
+ * Note that the PHY is accessed through the same registers for both
+ * interfaces, so this can't be made interface-specific.
+ */
+
+static DEFINE_SPINLOCK(phy_lock);
you should have a chip structure, that contains two structs (one for
each interface/port)
+static u16 tsi108_read_mii(struct tsi108_prv_data *data, int reg, int *status)
+{
+ int i;
+ u16 ret;
+
+ TSI108_ETH_WRITE_PHYREG(TSI108_MAC_MII_ADDR,
+ (data-phy TSI108_MAC_MII_ADDR_PHY) |
+ (reg TSI108_MAC_MII_ADDR_REG));
+
Re: [patch 3/3] Add tsi108 On Chip Ethernet device driver support
On Thu, 2006-09-21 at 12:26, Jeff Garzik wrote: Zang Roy-r61911 wrote: +#define TSI108_ETH_WRITE_REG(offset, val) \ + writel(le32_to_cpu(val),data-regs + (offset)) + +#define TSI108_ETH_READ_REG(offset) \ + le32_to_cpu(readl(data-regs + (offset))) + +#define TSI108_ETH_WRITE_PHYREG(offset, val) \ + writel(le32_to_cpu(val), data-phyregs + (offset)) + +#define TSI108_ETH_READ_PHYREG(offset) \ + le32_to_cpu(readl(data-phyregs + (offset))) NAK: 1) writel() and readl() are defined to be little endian. If your platform is different, then your platform should have its own foobus_writel() and foobus_readl(). Tsi108 bridge is designed for powerpc platform. Originally, I use out_be32() and in_be32(). While there is no obvious reason to object using this bridge in a little endian system. Maybe some extra hardware logic needed for the bus interface. le32_to_cpu() can be aware the endian difference. Any comment? 2) TSI108_ETH_WRITE_REG() is just way too long. TSI_READ(), TSI_WRITE(), TSI_READ_PHY() and TSI_WRITE_PHY() would be far more readable. More in next email. I will modify the name. Roy - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [patch 3/3] Add tsi108 On Chip Ethernet device driver support
I have some review comments about your driver; please consider them for fixing Thanks. + spin_unlock_irq(phy_lock); + msleep(10); + spin_lock_irq(phy_lock); + } hmm some places take phy_lock with disabling interrupts, while others don't. I sort of fear the others may be buggy are you sure those are ok? Could you interpret your comments in detail? Roy - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/3] Add tsi108 On Chip Ethernet device driver support
On Tue, 2006-09-12 at 22:43, Jeff Garzik wrote:
Roland Dreier wrote:
+struct tsi108_prv_data {
+ volatile u32 regs; /* Base of normal regs */
+ volatile u32 phyregs; /* Base of register bank used for PHY
access */
Why volatile? This looks really wrong here.
Indeed.
I will remove it.
+ data-regs = (u32)ioremap(einfo-regs, 0x400); /*FIX ME */
+ data-phyregs = (u32)ioremap(einfo-phyregs, 0x400);/*FIX
ME */
What needs to be fixed here? And why are you casting the result of
ioremap to u32? Shouldn't you keep the normal return value?
Oh, that's very, very wrong.
I will find method to avoid this :-).
Roy
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/3] Add tsi108 On Chip Ethernet device driver support
+struct tsi108_prv_data {
+volatile u32 regs; /* Base of normal regs */
+volatile u32 phyregs; /* Base of register bank used for PHY access */
Why volatile? This looks really wrong here.
+data-regs = (u32)ioremap(einfo-regs, 0x400); /*FIX ME */
+data-phyregs = (u32)ioremap(einfo-phyregs, 0x400);/*FIX ME */
What needs to be fixed here? And why are you casting the result of
ioremap to u32? Shouldn't you keep the normal return value?
- R.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 3/3] Add tsi108 On Chip Ethernet device driver support
The driver for tsi108/9 on chip Ethernet port
Signed-off-by: Alexandre Bounine[EMAIL PROTECTED]
Signed-off-by: Roy Zang [EMAIL PROTECTED]
---
drivers/net/tsi108_eth.c | 1752 ++
1 files changed, 1752 insertions(+), 0 deletions(-)
diff --git a/drivers/net/tsi108_eth.c b/drivers/net/tsi108_eth.c
new file mode 100644
index 000..eece2bb
--- /dev/null
+++ b/drivers/net/tsi108_eth.c
@@ -0,0 +1,1752 @@
+/***
+
+ Copyright(c) 2005 Tundra Semiconductor Corporation.
+
+ This program is free software; you can redistribute it and/or modify it
+ under the terms of the GNU General Public License as published by the Free
+ Software Foundation; either version 2 of the License, or (at your option)
+ any later version.
+
+ This program is distributed in the hope that it will be useful, but WITHOUT
+ ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ more details.
+
+ You should have received a copy of the GNU General Public License along with
+ this program; if not, write to the Free Software Foundation, Inc., 59
+ Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+
+***/
+
+/* This driver is based on the driver code originally developed
+ * for the Intel IOC80314 (ForestLake) Gigabit Ethernet by
+ * [EMAIL PROTECTED] * Copyright (C) 2003 TimeSys Corporation
+ *
+ * Currently changes from original version are:
+ * - portig to Tsi108-based platform and kernel 2.6 ([EMAIL PROTECTED])
+ * - modifications to handle two ports independently and support for
+ * additional PHY devices ([EMAIL PROTECTED])
+ *
+ */
+
+#include linux/config.h
+#include linux/module.h
+#include linux/types.h
+#include linux/init.h
+#include linux/net.h
+#include linux/netdevice.h
+#include linux/etherdevice.h
+#include linux/skbuff.h
+#include linux/slab.h
+#include linux/sched.h
+#include linux/spinlock.h
+#include linux/delay.h
+#include linux/crc32.h
+#include linux/mii.h
+#include linux/device.h
+#include linux/pci.h
+#include linux/rtnetlink.h
+#include linux/timer.h
+#include linux/platform_device.h
+
+#include asm/system.h
+#include asm/io.h
+#include asm/tsi108.h
+
+#include tsi108_eth.h
+
+#undef DEBUG
+#ifdef DEBUG
+#define DBG(fmt...) do { printk(fmt); } while(0)
+#else
+#define DBG(fmt...) do { } while(0)
+#endif
+
+typedef struct net_device net_device;
+typedef struct sk_buff sk_buff;
+
+#define MII_READ_DELAY 1 /* max link wait time in msec */
+
+#define TSI108_RXRING_LEN 256
+
+/* NOTE: The driver currently does not support receiving packets
+ * larger than the buffer size, so don't decrease this (unless you
+ * want to add such support).
+ */
+#define TSI108_RXBUF_SIZE 1536
+
+#define TSI108_TXRING_LEN 256
+
+#define TSI108_TX_INT_FREQ64
+
+/* Check the phy status every half a second. */
+#define CHECK_PHY_INTERVAL (HZ/2)
+
+static int tsi108_init_one(struct platform_device *pdev);
+static int tsi108_ether_remove(struct platform_device *pdev);
+
+struct tsi108_prv_data {
+ volatile u32 regs; /* Base of normal regs */
+ volatile u32 phyregs; /* Base of register bank used for PHY access */
+ int phy;/* Index of PHY for this interface */
+ int irq_num;
+ int id;
+
+ struct timer_list timer;/* Timer that triggers the check phy function */
+ int rxtail; /* Next entry in rxring to read */
+ int rxhead; /* Next entry in rxring to give a new buffer */
+ int rxfree; /* Number of free, allocated RX buffers */
+
+ int rxpending; /* Non-zero if there are still descriptors
+* to be processed from a previous descriptor
+* interrupt condition that has been cleared */
+
+ int txtail; /* Next TX descriptor to check status on */
+ int txhead; /* Next TX descriptor to use */
+
+ /* Number of free TX descriptors. This could be calculated from
+* rxhead and rxtail if one descriptor were left unused to disambiguate
+* full and empty conditions, but it's simpler to just keep track
+* explicitly. */
+
+ int txfree;
+
+ int phy_ok; /* The PHY is currently powered on. */
+
+ /* PHY status (duplex is 1 for half, 2 for full,
+* so that the default 0 indicates that neither has
+* yet been configured). */
+
+ int link_up;
+ int speed;
+ int duplex;
+
+ tx_desc *txring;
+ rx_desc *rxring;
+ sk_buff *txskbs[TSI108_TXRING_LEN];
+ sk_buff *rxskbs[TSI108_RXRING_LEN];
+
+ dma_addr_t txdma, rxdma;
+
+ /* txlock nests in misclock and phy_lock */
+
+
Re: [patch 3/3] Add tsi108 On Chip Ethernet device driver support
On Tue, 2006-09-12 at 16:55 +0800, Zang Roy-r61911 wrote:
The driver for tsi108/9 on chip Ethernet port
Hi,
I have some review comments about your driver; please consider them for
fixing
+
+#undef DEBUG
+#ifdef DEBUG
+#define DBG(fmt...) do { printk(fmt); } while(0)
+#else
+#define DBG(fmt...) do { } while(0)
+#endif
please don't do this, there is pr_debug and dev_dbg() for a reason
already. No reason to add a copy.
+
+typedef struct net_device net_device;
+typedef struct sk_buff sk_buff;
Please don't do this...
+ TSI108_ETH_WRITE_PHYREG(TSI108_MAC_MII_ADDR,
+ (data-phy TSI108_MAC_MII_ADDR_PHY) |
+ (reg TSI108_MAC_MII_ADDR_REG));
+ mb();
+ TSI108_ETH_WRITE_PHYREG(TSI108_MAC_MII_CMD, 0);
+ mb();
+ TSI108_ETH_WRITE_PHYREG(TSI108_MAC_MII_CMD, TSI108_MAC_MII_CMD_READ);
+ mb();
what is this mb() for? do you want them to be an io memory barrier?
+ while (TSI108_ETH_READ_REG(TSI108_MAC_MII_IND)
+TSI108_MAC_MII_IND_BUSY) ;
do you want this to be an infinate loop? At minimum the ; needs to be
a cpu_relax() to avoid the cpu from burning up but it'd be a lot nicer
if this loop wasn't infinite
+
+static irqreturn_t tsi108_irq(int irq, void *dev_id, struct pt_regs *regs)
+{
+ if (irq == NO_IRQ)
+ return IRQ_NONE;/* Not our interrupt */
+
+ return tsi108_irq_one(dev_id);
+}
hi this IRQ_NONE just looks odd, was this really needed?
+ mb();
+ while (tsi108_read_mii(data, PHY_CTRL, NULL) PHY_CTRL_AUTONEG_START)
+ ;
another infinite loop that wants cpu_relax() for sure
+ spin_unlock_irq(phy_lock);
+ msleep(10);
+ spin_lock_irq(phy_lock);
+ }
hmm some places take phy_lock with disabling interrupts, while others
don't. I sort of fear the others may be buggy are you sure those
are ok?
Greetings,
Arjan van de Ven
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/3] Add tsi108 On Chip Ethernet device driver support
Roland Dreier wrote:
+struct tsi108_prv_data {
+ volatile u32 regs; /* Base of normal regs */
+ volatile u32 phyregs; /* Base of register bank used for PHY access */
Why volatile? This looks really wrong here.
Indeed.
+ data-regs = (u32)ioremap(einfo-regs, 0x400);/*FIX ME */
+ data-phyregs = (u32)ioremap(einfo-phyregs, 0x400); /*FIX ME */
What needs to be fixed here? And why are you casting the result of
ioremap to u32? Shouldn't you keep the normal return value?
Oh, that's very, very wrong.
Jeff
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
