[PATCH, REPOST] Fix/Rewrite of the mipsnet driver
Hello All,
currently the mipsnet driver fails after transmitting a number of
packages because SKBs are allocated but never freed. I fixed that
and coudn't refrain from removing the most egregious warts.
- mipsnet.h folded into mipsnet.c, as it doesn't provide any
useful external interface.
- Free SKB after transmission.
- Call free_irq in mipsnet_close, to balance the request_irq in
mipsnet_open.
- Removed duplicate read of rxDataCount.
- Some identifiers are now less verbose.
- Removed dead and/or unnecessarily complex code.
- Code formatting fixes.
Tested on Qemu's mipssim emulation, with this patch it can boot a
Debian NFSroot.
Thiemo
Signed-off-by: Thiemo Seufer [EMAIL PROTECTED]
---
b/drivers/net/mipsnet.c | 201
drivers/net/mipsnet.h | 112 --
2 files changed, 134 insertions(+), 179 deletions(-)
diff --git a/drivers/net/mipsnet.c b/drivers/net/mipsnet.c
index aafc3ce..6d343ef 100644
--- a/drivers/net/mipsnet.c
+++ b/drivers/net/mipsnet.c
@@ -4,8 +4,6 @@
* for more details.
*/
-#define DEBUG
-
#include linux/init.h
#include linux/io.h
#include linux/kernel.h
@@ -15,11 +13,93 @@
#include linux/platform_device.h
#include asm/mips-boards/simint.h
-#include mipsnet.h /* actual device IO mapping */
+#define MIPSNET_VERSION 2007-11-17
+
+/*
+ * Net status/control block as seen by sw in the core.
+ */
+struct mipsnet_regs {
+ /*
+* Device info for probing, reads as MIPSNET%d where %d is some
+* form of version.
+*/
+ u64 devId; /*0x00 */
-#define MIPSNET_VERSION 2005-06-20
+ /*
+* read only busy flag.
+* Set and cleared by the Net Device to indicate that an rx or a tx
+* is in progress.
+*/
+ u32 busy; /*0x08 */
-#define mipsnet_reg_address(dev, field) (dev-base_addr + field_offset(field))
+ /*
+* Set by the Net Device.
+* The device will set it once data has been received.
+* The value is the number of bytes that should be read from
+* rxDataBuffer. The value will decrease till 0 until all the data
+* from rxDataBuffer has been read.
+*/
+ u32 rxDataCount;/*0x0c */
+#define MIPSNET_MAX_RXTX_DATACOUNT (1 16)
+
+ /*
+* Settable from the MIPS core, cleared by the Net Device.
+* The core should set the number of bytes it wants to send,
+* then it should write those bytes of data to txDataBuffer.
+* The device will clear txDataCount has been processed (not
+* necessarily sent).
+*/
+ u32 txDataCount;/*0x10 */
+
+ /*
+* Interrupt control
+*
+* Used to clear the interrupted generated by this dev.
+* Write a 1 to clear the interrupt. (except bit31).
+*
+* Bit0 is set if it was a tx-done interrupt.
+* Bit1 is set when new rx-data is available.
+*Until this bit is cleared there will be no other RXs.
+*
+* Bit31 is used for testing, it clears after a read.
+*Writing 1 to this bit will cause an interrupt to be generated.
+*To clear the test interrupt, write 0 to this register.
+*/
+ u32 interruptControl; /*0x14 */
+#define MIPSNET_INTCTL_TXDONE (1u 0)
+#define MIPSNET_INTCTL_RXDONE (1u 1)
+#define MIPSNET_INTCTL_TESTBIT(1u 31)
+
+ /*
+* Readonly core-specific interrupt info for the device to signal
+* the core. The meaning of the contents of this field might change.
+*/
+ /* XXX: the whole memIntf interrupt scheme is messy: the device
+* should have no control what so ever of what VPE/register set is
+* being used.
+* The MemIntf should only expose interrupt lines, and something in
+* the config should be responsible for the line-core/vpe bindings.
+*/
+ u32 interruptInfo; /*0x18 */
+
+ /*
+* This is where the received data is read out.
+* There is more data to read until rxDataReady is 0.
+* Only 1 byte at this regs offset is used.
+*/
+ u32 rxDataBuffer; /*0x1c */
+
+ /*
+* This is where the data to transmit is written.
+* Data should be written for the amount specified in the
+* txDataCount register.
+* Only 1 byte at this regs offset is used.
+*/
+ u32 txDataBuffer; /*0x20 */
+};
+
+#define regaddr(dev, field) \
+ (dev-base_addr + offsetof(struct mipsnet_regs, field))
static char mipsnet_string[] = mipsnet;
@@ -29,32 +109,27 @@ static char mipsnet_string[] = mipsnet;
static int ioiocpy_frommipsnet(struct net_device *dev, unsigned char *kdata,
int len)
{
- uint32_t available_len = inl(mipsnet_reg_address(dev, rxDataCount));
-
- if (available_len len)
- return -EFAULT
Re: [PATCH] Fix/Rewrite of the mipsnet driver
Ralf Baechle wrote:
On Sun, Oct 28, 2007 at 04:38:46AM +, Thiemo Seufer wrote:
Hello All,
currently the mipsnet driver fails after transmitting a number of
packages because SKBs are allocated but never freed. I fixed that
and coudn't refrain from removing the most egregious warts.
- mipsnet.h folded into mipsnet.c, as it doesn't provide any
useful external interface.
- Free SKB after transmission.
- Call free_irq in mipsnet_close, to balance the request_irq in
mipsnet_open.
- Removed duplicate read of rxDataCount.
- Some identifiers are now less verbose.
- Removed dead and/or unnecessarily complex code.
- Code formatting fixes.
Tested on Qemu's mipssim emulation, with this patch it can boot a
Debian NFSroot.
The patch does no longer apply to a recent tree, can you respin it?
Updated against linux-mips.org head and appended. Only compile-tested
this time, the mipssim kernel currently fails to build.
Thiemo
Signed-off-by: Thiemo Seufer [EMAIL PROTECTED]
---
b/drivers/net/mipsnet.c | 201
drivers/net/mipsnet.h | 112 --
2 files changed, 134 insertions(+), 179 deletions(-)
diff --git a/drivers/net/mipsnet.c b/drivers/net/mipsnet.c
index aafc3ce..e9c0c79 100644
--- a/drivers/net/mipsnet.c
+++ b/drivers/net/mipsnet.c
@@ -4,8 +4,6 @@
* for more details.
*/
-#define DEBUG
-
#include linux/init.h
#include linux/io.h
#include linux/kernel.h
@@ -15,11 +13,93 @@
#include linux/platform_device.h
#include asm/mips-boards/simint.h
-#include mipsnet.h /* actual device IO mapping */
+#define MIPSNET_VERSION 2007-10-28
+
+/*
+ * Net status/control block as seen by sw in the core.
+ */
+struct mipsnet_regs {
+ /*
+* Device info for probing, reads as MIPSNET%d where %d is some
+* form of version.
+*/
+ uint64_t devId; /*0x00 */
-#define MIPSNET_VERSION 2005-06-20
+ /*
+* read only busy flag.
+* Set and cleared by the Net Device to indicate that an rx or a tx
+* is in progress.
+*/
+ uint32_t busy; /*0x08 */
-#define mipsnet_reg_address(dev, field) (dev-base_addr + field_offset(field))
+ /*
+* Set by the Net Device.
+* The device will set it once data has been received.
+* The value is the number of bytes that should be read from
+* rxDataBuffer. The value will decrease till 0 until all the data
+* from rxDataBuffer has been read.
+*/
+ uint32_t rxDataCount; /*0x0c */
+#define MIPSNET_MAX_RXTX_DATACOUNT (1 16)
+
+ /*
+* Settable from the MIPS core, cleared by the Net Device.
+* The core should set the number of bytes it wants to send,
+* then it should write those bytes of data to txDataBuffer.
+* The device will clear txDataCount has been processed (not
+* necessarily sent).
+*/
+ uint32_t txDataCount; /*0x10 */
+
+ /*
+* Interrupt control
+*
+* Used to clear the interrupted generated by this dev.
+* Write a 1 to clear the interrupt. (except bit31).
+*
+* Bit0 is set if it was a tx-done interrupt.
+* Bit1 is set when new rx-data is available.
+*Until this bit is cleared there will be no other RXs.
+*
+* Bit31 is used for testing, it clears after a read.
+*Writing 1 to this bit will cause an interrupt to be generated.
+*To clear the test interrupt, write 0 to this register.
+*/
+ uint32_t interruptControl; /*0x14 */
+#define MIPSNET_INTCTL_TXDONE ((uint32_t)(1 0))
+#define MIPSNET_INTCTL_RXDONE ((uint32_t)(1 1))
+#define MIPSNET_INTCTL_TESTBIT((uint32_t)(1 31))
+
+ /*
+* Readonly core-specific interrupt info for the device to signal
+* the core. The meaning of the contents of this field might change.
+*/
+ /* XXX: the whole memIntf interrupt scheme is messy: the device
+* should have no control what so ever of what VPE/register set is
+* being used.
+* The MemIntf should only expose interrupt lines, and something in
+* the config should be responsible for the line-core/vpe bindings.
+*/
+ uint32_t interruptInfo; /*0x18 */
+
+ /*
+* This is where the received data is read out.
+* There is more data to read until rxDataReady is 0.
+* Only 1 byte at this regs offset is used.
+*/
+ uint32_t rxDataBuffer; /*0x1c */
+
+ /*
+* This is where the data to transmit is written.
+* Data should be written for the amount specified in the
+* txDataCount register.
+* Only 1 byte at this regs offset is used.
+*/
+ uint32_t txDataBuffer; /*0x20 */
+};
+
+#define regaddr(dev, field) \
+ (dev-base_addr + offsetof(struct mipsnet_regs, field
Re: [PATCH] Fix/Rewrite of the mipsnet driver
Stephen Hemminger wrote:
On Sun, 28 Oct 2007 20:03:08 +
Thiemo Seufer [EMAIL PROTECTED] wrote:
Ralf Baechle wrote:
On Sun, Oct 28, 2007 at 04:38:46AM +, Thiemo Seufer wrote:
Hello All,
currently the mipsnet driver fails after transmitting a number of
packages because SKBs are allocated but never freed. I fixed that
and coudn't refrain from removing the most egregious warts.
- mipsnet.h folded into mipsnet.c, as it doesn't provide any
useful external interface.
- Free SKB after transmission.
- Call free_irq in mipsnet_close, to balance the request_irq in
mipsnet_open.
- Removed duplicate read of rxDataCount.
- Some identifiers are now less verbose.
- Removed dead and/or unnecessarily complex code.
- Code formatting fixes.
Tested on Qemu's mipssim emulation, with this patch it can boot a
Debian NFSroot.
The patch does no longer apply to a recent tree, can you respin it?
Updated against linux-mips.org head and appended. Only compile-tested
this time, the mipssim kernel currently fails to build.
[snip]
It is standard practice in the kernel to use u32 rather than uint32_t.
Also cast of shift is unneeded (1u 0) works fine.
A leftover from the old code. changes accordingly.
Thiemo
Signed-off-by: Thiemo Seufer [EMAIL PROTECTED]
---
b/drivers/net/mipsnet.c | 201
drivers/net/mipsnet.h | 112 --
2 files changed, 134 insertions(+), 179 deletions(-)
diff --git a/drivers/net/mipsnet.c b/drivers/net/mipsnet.c
index aafc3ce..6ad5ab3 100644
--- a/drivers/net/mipsnet.c
+++ b/drivers/net/mipsnet.c
@@ -4,8 +4,6 @@
* for more details.
*/
-#define DEBUG
-
#include linux/init.h
#include linux/io.h
#include linux/kernel.h
@@ -15,11 +13,93 @@
#include linux/platform_device.h
#include asm/mips-boards/simint.h
-#include mipsnet.h /* actual device IO mapping */
+#define MIPSNET_VERSION 2007-10-28
+
+/*
+ * Net status/control block as seen by sw in the core.
+ */
+struct mipsnet_regs {
+ /*
+* Device info for probing, reads as MIPSNET%d where %d is some
+* form of version.
+*/
+ u64 devId; /*0x00 */
-#define MIPSNET_VERSION 2005-06-20
+ /*
+* read only busy flag.
+* Set and cleared by the Net Device to indicate that an rx or a tx
+* is in progress.
+*/
+ u32 busy; /*0x08 */
-#define mipsnet_reg_address(dev, field) (dev-base_addr + field_offset(field))
+ /*
+* Set by the Net Device.
+* The device will set it once data has been received.
+* The value is the number of bytes that should be read from
+* rxDataBuffer. The value will decrease till 0 until all the data
+* from rxDataBuffer has been read.
+*/
+ u32 rxDataCount;/*0x0c */
+#define MIPSNET_MAX_RXTX_DATACOUNT (1 16)
+
+ /*
+* Settable from the MIPS core, cleared by the Net Device.
+* The core should set the number of bytes it wants to send,
+* then it should write those bytes of data to txDataBuffer.
+* The device will clear txDataCount has been processed (not
+* necessarily sent).
+*/
+ u32 txDataCount;/*0x10 */
+
+ /*
+* Interrupt control
+*
+* Used to clear the interrupted generated by this dev.
+* Write a 1 to clear the interrupt. (except bit31).
+*
+* Bit0 is set if it was a tx-done interrupt.
+* Bit1 is set when new rx-data is available.
+*Until this bit is cleared there will be no other RXs.
+*
+* Bit31 is used for testing, it clears after a read.
+*Writing 1 to this bit will cause an interrupt to be generated.
+*To clear the test interrupt, write 0 to this register.
+*/
+ u32 interruptControl; /*0x14 */
+#define MIPSNET_INTCTL_TXDONE (1u 0)
+#define MIPSNET_INTCTL_RXDONE (1u 1)
+#define MIPSNET_INTCTL_TESTBIT(1u 31)
+
+ /*
+* Readonly core-specific interrupt info for the device to signal
+* the core. The meaning of the contents of this field might change.
+*/
+ /* XXX: the whole memIntf interrupt scheme is messy: the device
+* should have no control what so ever of what VPE/register set is
+* being used.
+* The MemIntf should only expose interrupt lines, and something in
+* the config should be responsible for the line-core/vpe bindings.
+*/
+ u32 interruptInfo; /*0x18 */
+
+ /*
+* This is where the received data is read out.
+* There is more data to read until rxDataReady is 0.
+* Only 1 byte at this regs offset is used.
+*/
+ u32 rxDataBuffer; /*0x1c */
+
+ /*
+* This is where the data to transmit is written
[PATCH] Fix/Rewrite of the mipsnet driver
Hello All,
currently the mipsnet driver fails after transmitting a number of
packages because SKBs are allocated but never freed. I fixed that
and coudn't refrain from removing the most egregious warts.
- mipsnet.h folded into mipsnet.c, as it doesn't provide any
useful external interface.
- Free SKB after transmission.
- Call free_irq in mipsnet_close, to balance the request_irq in
mipsnet_open.
- Removed duplicate read of rxDataCount.
- Some identifiers are now less verbose.
- Removed dead and/or unnecessarily complex code.
- Code formatting fixes.
Tested on Qemu's mipssim emulation, with this patch it can boot a
Debian NFSroot.
Thiemo
Signed-off-by: Thiemo Seufer [EMAIL PROTECTED]
---
diff --git a/drivers/net/mipsnet.c b/drivers/net/mipsnet.c
index 9853c74..28c66c1 100644
--- a/drivers/net/mipsnet.c
+++ b/drivers/net/mipsnet.c
@@ -4,8 +4,6 @@
* for more details.
*/
-#define DEBUG
-
#include linux/init.h
#include linux/kernel.h
#include linux/module.h
@@ -16,11 +14,93 @@
#include asm/io.h
#include asm/mips-boards/simint.h
-#include mipsnet.h /* actual device IO mapping */
+#define MIPSNET_VERSION 2007-10-28
+
+/*
+ * Net status/control block as seen by sw in the core.
+ */
+struct MIPS_T_NetControl {
+ /*
+* Device info for probing, reads as MIPSNET%d where %d is some
+* form of version.
+*/
+ uint64_t devId; /*0x00 */
+
+ /*
+* read only busy flag.
+* Set and cleared by the Net Device to indicate that an rx or a tx
+* is in progress.
+*/
+ uint32_t busy; /*0x08 */
+
+ /*
+* Set by the Net Device.
+* The device will set it once data has been received.
+* The value is the number of bytes that should be read from
+* rxDataBuffer. The value will decrease till 0 until all the data
+* from rxDataBuffer has been read.
+*/
+ uint32_t rxDataCount; /*0x0c */
+#define MIPSNET_MAX_RXTX_DATACOUNT (116)
+
+ /*
+* Settable from the MIPS core, cleared by the Net Device.
+* The core should set the number of bytes it wants to send,
+* then it should write those bytes of data to txDataBuffer.
+* The device will clear txDataCount has been processed (not
+* necessarily sent).
+*/
+ uint32_t txDataCount; /*0x10 */
-#define MIPSNET_VERSION 2005-06-20
+ /*
+* Interrupt control
+*
+* Used to clear the interrupted generated by this dev.
+* Write a 1 to clear the interrupt. (except bit31).
+*
+* Bit0 is set if it was a tx-done interrupt.
+* Bit1 is set when new rx-data is available.
+*Until this bit is cleared there will be no other RXs.
+*
+* Bit31 is used for testing, it clears after a read.
+*Writing 1 to this bit will cause an interrupt to be generated.
+*To clear the test interrupt, write 0 to this register.
+*/
+ uint32_t interruptControl; /*0x14 */
+#define MIPSNET_INTCTL_TXDONE ((uint32_t)(1 0))
+#define MIPSNET_INTCTL_RXDONE ((uint32_t)(1 1))
+#define MIPSNET_INTCTL_TESTBIT((uint32_t)(1 31))
-#define mipsnet_reg_address(dev, field) (dev-base_addr + field_offset(field))
+ /*
+* Readonly core-specific interrupt info for the device to signal
+* the core. The meaning of the contents of this field might change.
+*/
+ /* XXX: the whole memIntf interrupt scheme is messy: the device
+* should have no control what so ever of what VPE/register set is
+* being used.
+* The MemIntf should only expose interrupt lines, and something in
+* the config should be responsible for the line-core/vpe bindings.
+*/
+ uint32_t interruptInfo; /*0x18 */
+
+ /*
+* This is where the received data is read out.
+* There is more data to read until rxDataReady is 0.
+* Only 1 byte at this regs offset is used.
+*/
+ uint32_t rxDataBuffer; /*0x1c */
+
+ /*
+* This is where the data to transmit is written.
+* Data should be written for the amount specified in the
+* txDataCount register.
+* Only 1 byte at this regs offset is used.
+*/
+ uint32_t txDataBuffer; /*0x20 */
+};
+
+#define regaddr(dev, field) \
+ (dev-base_addr + offsetof(MIPS_T_NetControl, field))
struct mipsnet_priv {
struct net_device_stats stats;
@@ -34,18 +114,13 @@ static char mipsnet_string[] = mipsnet;
static int ioiocpy_frommipsnet(struct net_device *dev, unsigned char *kdata,
int len)
{
- uint32_t available_len = inl(mipsnet_reg_address(dev, rxDataCount));
- if (available_len len)
- return -EFAULT;
+ for (; len 0; len--, kdata++)
+ *kdata = inb(regaddr(dev, rxDataBuffer));
- for (; len 0; len--, kdata
Re: [PATCH][MIPS][7/7] AR7: ethernet
Ralf Baechle wrote:
On Sat, Sep 08, 2007 at 02:23:00AM +0200, Matteo Croce wrote:
[snip]
+/* Register definitions */
+struct cpmac_control_regs {
+ u32 revision;
+ u32 control;
+ u32 teardown;
+ u32 unused;
+} __attribute__ ((packed));
+
+struct cpmac_int_regs {
+ u32 stat_raw;
+ u32 stat_masked;
+ u32 enable;
+ u32 clear;
+} __attribute__ ((packed));
+
+struct cpmac_stats {
+ u32 good;
+ u32 bcast;
+ u32 mcast;
+ u32 pause;
+ u32 crc_error;
+ u32 align_error;
+ u32 oversized;
+ u32 jabber;
+ u32 undersized;
+ u32 fragment;
+ u32 filtered;
+ u32 qos_filtered;
+ u32 octets;
+} __attribute__ ((packed));
All struct members here are sized such that there is no padding needed, so
the packed attribute doesn't buy you anything - unless of course the
entire structure is missaligned but I don't see how that would be possible
in this driver so the __attribute__ ((packed)) should go - it result in
somwhat larger and slower code.
FWIW, a modern gcc will warn about such superfluous packed attributes,
that's another reason to remove those.
Thiemo
-
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
