Re: [PATCH 5/6] IP100A correct init and close step
Hi Jeff: (3)Yes, This is a bug, I will correct it. Thanks. (4)This will halt TxDMA and RxDMA, after that will let reseting safely. Should I add description in source code or in change log? Thanks! Jesse - Original Message - From: Jeff Garzik [EMAIL PROTECTED] To: Jesse Huang [EMAIL PROTECTED] Cc: linux-kernel@vger.kernel.org; netdev@vger.kernel.org; [EMAIL PROTECTED] Sent: Friday, August 18, 2006 7:12 PM Subject: Re: [PATCH 5/6] IP100A correct init and close step Jesse Huang wrote: Hi Jeff: (1)Should I change to : spin_lock_irqsave(np-lock,flags); reset_tx(dev); spin_lock_irqrestore(np-lock,flags); (2)I will remove date and author information out of source code comment. Correct. Also: (3) Use iowrite16(), not writew(). I just noticed this bug. iowrite16() will work for both MMIO and IO cycles, writew() only works for MMIO. (4) We need a description of why this change is needed. What does writing 0x500 to DMACtrl actually do? Why do we need to do this? 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 5/6] IP100A correct init and close step
Hi Jeff: (1)Should I change to : spin_lock_irqsave(np-lock,flags); reset_tx(dev); spin_lock_irqrestore(np-lock,flags); (2)I will remove date and author information out of source code comment. - Original Message - From: Jeff Garzik [EMAIL PROTECTED] To: Jesse Huang [EMAIL PROTECTED] Cc: linux-kernel@vger.kernel.org; netdev@vger.kernel.org; [EMAIL PROTECTED] Sent: Thursday, August 17, 2006 10:51 PM Subject: Re: [PATCH 5/6] IP100A correct init and close step Jesse Huang wrote: From: Jesse Huang [EMAIL PROTECTED] correct init and close step Change Logs: correct init and close step --- drivers/net/sundance.c | 10 +- 1 files changed, 9 insertions(+), 1 deletions(-) b5e343a17f5d70d1cc9a4ba20d366bab355f64a6 diff --git a/drivers/net/sundance.c b/drivers/net/sundance.c index f63871a..c7c22f0 100755 --- a/drivers/net/sundance.c +++ b/drivers/net/sundance.c @@ -830,6 +830,11 @@ #endif iowrite8(0x01, ioaddr + DebugCtrl1); netif_start_queue(dev); + // 04/19/2005 Jesse fix for complete initial step + spin_lock(np-lock); + reset_tx(dev); + spin_unlock(np-lock); + NAK -- ineffective locking. If you need locking here, you must use spin_lock_irqsave() @@ -1654,7 +1659,10 @@ static int netdev_close(struct net_devic /* Disable interrupts by clearing the interrupt mask. */ iowrite16(0x, ioaddr + IntrEnable); - + + // 04/19/2005 Jesse fix for complete initial step + writew(0x500, ioaddr + DMACtrl); NAK: 1) date and author information is already present in the kernel changelog. It should not be present in the source code. 2) The comment (or patch description) fails to describe -why- this change is needed. It only described what is being changes, which is already obvious from reading the source code. - 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 5/6] IP100A correct init and close step
Jesse Huang wrote: Hi Jeff: (1)Should I change to : spin_lock_irqsave(np-lock,flags); reset_tx(dev); spin_lock_irqrestore(np-lock,flags); (2)I will remove date and author information out of source code comment. Correct. Also: (3) Use iowrite16(), not writew(). I just noticed this bug. iowrite16() will work for both MMIO and IO cycles, writew() only works for MMIO. (4) We need a description of why this change is needed. What does writing 0x500 to DMACtrl actually do? Why do we need to do this? 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
[PATCH 5/6] IP100A correct init and close step
From: Jesse Huang [EMAIL PROTECTED] correct init and close step Change Logs: correct init and close step --- drivers/net/sundance.c | 10 +- 1 files changed, 9 insertions(+), 1 deletions(-) b5e343a17f5d70d1cc9a4ba20d366bab355f64a6 diff --git a/drivers/net/sundance.c b/drivers/net/sundance.c index f63871a..c7c22f0 100755 --- a/drivers/net/sundance.c +++ b/drivers/net/sundance.c @@ -830,6 +830,11 @@ #endif iowrite8(0x01, ioaddr + DebugCtrl1); netif_start_queue(dev); + // 04/19/2005 Jesse fix for complete initial step + spin_lock(np-lock); + reset_tx(dev); + spin_unlock(np-lock); + iowrite16 (StatsEnable | RxEnable | TxEnable, ioaddr + MACCtrl1); if (netif_msg_ifup(np)) @@ -1654,7 +1659,10 @@ static int netdev_close(struct net_devic /* Disable interrupts by clearing the interrupt mask. */ iowrite16(0x, ioaddr + IntrEnable); - + + // 04/19/2005 Jesse fix for complete initial step + writew(0x500, ioaddr + DMACtrl); + /* Stop the chip's Tx and Rx processes. */ iowrite16(TxDisable | RxDisable | StatsDisable, ioaddr + MACCtrl1); -- 1.3.GIT - 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 5/6] IP100A correct init and close step
Jesse Huang wrote: From: Jesse Huang [EMAIL PROTECTED] correct init and close step Change Logs: correct init and close step --- drivers/net/sundance.c | 10 +- 1 files changed, 9 insertions(+), 1 deletions(-) b5e343a17f5d70d1cc9a4ba20d366bab355f64a6 diff --git a/drivers/net/sundance.c b/drivers/net/sundance.c index f63871a..c7c22f0 100755 --- a/drivers/net/sundance.c +++ b/drivers/net/sundance.c @@ -830,6 +830,11 @@ #endif iowrite8(0x01, ioaddr + DebugCtrl1); netif_start_queue(dev); + // 04/19/2005 Jesse fix for complete initial step + spin_lock(np-lock); + reset_tx(dev); + spin_unlock(np-lock); + NAK -- ineffective locking. If you need locking here, you must use spin_lock_irqsave() @@ -1654,7 +1659,10 @@ static int netdev_close(struct net_devic /* Disable interrupts by clearing the interrupt mask. */ iowrite16(0x, ioaddr + IntrEnable); - + + // 04/19/2005 Jesse fix for complete initial step + writew(0x500, ioaddr + DMACtrl); NAK: 1) date and author information is already present in the kernel changelog. It should not be present in the source code. 2) The comment (or patch description) fails to describe -why- this change is needed. It only described what is being changes, which is already obvious from reading the source code. - 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
