Re: [PATCH] Fix/Rewrite of the mipsnet driver
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? Ralf - 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] 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
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.
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 */
+};
+
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.
+
Re: [PATCH] Fix/Rewrite of the mipsnet driver
On Sun, Oct 28, 2007 at 01:22:04PM -0700, Stephen Hemminger wrote: -#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_INTCTL_ALLSOURCES (MIPSNET_INTCTL_TXDONE | \ -MIPSNET_INTCTL_RXDONE | \ -MIPSNET_INTCTL_TESTBIT) It is standard practice in the kernel to use u32 rather than uint32_t. uint32_t has widely leaked in and as long as it's not used in headers exported to userland is perfectly fine. But if we want to achieve consistence throughout the kernel it'll take a little witch hunt for uint32_t and co. Also cast of shift is unneeded (1u 0) works fine. Old sins of mipsnet.h which was just copied into mipsnet.c. Or toothing pains of a driver on its way to sanity. Ralf - 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] 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++) {
-
