Re: [PATCH net-next 1/1] net: fec: clear receive interrupts before processing a packet

2015-09-03 Thread Philippe De Muyter 
Hi Andy,

can you resubmit it, adding also my

Reported-by: Philippe De Muyter <p...@macqel.be>

and explaining that it also prevents a complete rx blockage failure ?

Philippe

On Wed, Sep 02, 2015 at 11:40:15AM +0200, Philippe De Muyter wrote:
> On Wed, Sep 02, 2015 at 05:24:14PM +0800, Fugang Duan wrote:
> > From: Russell King <rmk+ker...@arm.linux.org.uk>
> > 
> > The patch just to re-submit the patch "db3421c114cfa6326" because the
> > patch "4d494cdc92b3b9a0" remove the change.
> 
> I think you should mention also the titles of the commits.
> 
> And maybe send it also to stable.
> > 
> > Clear any pending receive interrupt before we process a pending packet.
> > This helps to avoid any spurious interrupts being raised after we have
> > fully cleaned the receive ring, while still allowing an interrupt to be
> > raised if we receive another packet.
> > 
> > The position of this is critical: we must do this prior to reading the
> > next packet status to avoid potentially dropping an interrupt when a
> > packet is still pending.
> > 
> > Acked-by: Fugang Duan <b38...@freescale.com>
> > Signed-off-by: Russell King <rmk+ker...@arm.linux.org.uk>
> > ---
> >  drivers/net/ethernet/freescale/fec_main.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/net/ethernet/freescale/fec_main.c 
> > b/drivers/net/ethernet/freescale/fec_main.c
> > index 1f89c59..6bed0ff 100644
> > --- a/drivers/net/ethernet/freescale/fec_main.c
> > +++ b/drivers/net/ethernet/freescale/fec_main.c
> > @@ -1400,6 +1400,7 @@ fec_enet_rx_queue(struct net_device *ndev, int 
> > budget, u16 queue_id)
> > if ((status & BD_ENET_RX_LAST) == 0)
> > netdev_err(ndev, "rcv is not +last\n");
> >  
> Could a comment be added here to avoid another future removal ?
> 
> > +   writel(FEC_ENET_RXF, fep->hwp + FEC_IEVENT);
> >  
> > /* Check for errors. */
> > if (status & (BD_ENET_RX_LG | BD_ENET_RX_SH | BD_ENET_RX_NO |
> > -- 
> > 1.9.1
> 
> Philippe

-- 
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next 1/1] net: fec: clear receive interrupts before processing a packet

2015-09-02 Thread Philippe De Muyter 
On Wed, Sep 02, 2015 at 05:24:14PM +0800, Fugang Duan wrote:
> From: Russell King 
> 
> The patch just to re-submit the patch "db3421c114cfa6326" because the
> patch "4d494cdc92b3b9a0" remove the change.

I think you should mention also the titles of the commits.

And maybe send it also to stable.
> 
> Clear any pending receive interrupt before we process a pending packet.
> This helps to avoid any spurious interrupts being raised after we have
> fully cleaned the receive ring, while still allowing an interrupt to be
> raised if we receive another packet.
> 
> The position of this is critical: we must do this prior to reading the
> next packet status to avoid potentially dropping an interrupt when a
> packet is still pending.
> 
> Acked-by: Fugang Duan 
> Signed-off-by: Russell King 
> ---
>  drivers/net/ethernet/freescale/fec_main.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c 
> b/drivers/net/ethernet/freescale/fec_main.c
> index 1f89c59..6bed0ff 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -1400,6 +1400,7 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, 
> u16 queue_id)
>   if ((status & BD_ENET_RX_LAST) == 0)
>   netdev_err(ndev, "rcv is not +last\n");
>  
Could a comment be added here to avoid another future removal ?

> + writel(FEC_ENET_RXF, fep->hwp + FEC_IEVENT);
>  
>   /* Check for errors. */
>   if (status & (BD_ENET_RX_LG | BD_ENET_RX_SH | BD_ENET_RX_NO |
> -- 
> 1.9.1

Philippe
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] 7990 : Various fixes and cleanups

2007-07-13 Thread Philippe De Muyter 
On Tue, Jul 10, 2007 at 12:38:45PM -0400, Jeff Garzik wrote:
 Philippe De Muyter wrote:
 This patch
 - avoids 7990 blocking when no tx buffer is available,
 [...]
 diff -r 6c0a10cc415a drivers/net/7990.c
 --- a/drivers/net/7990.c Thu Jul  5 16:10:16 2007 -0700
 +++ b/drivers/net/7990.c Fri Jul  6 11:27:20 2007 +0200
 [...]
 @@ -541,9 +546,6 @@ int lance_start_xmit (struct sk_buff *sk
  static int outs;
  unsigned long flags;
  
 -if (!TX_BUFFS_AVAIL)
 -return -1;
 -
  netif_stop_queue (dev);
  
  skblen = skb-len;
 
 
 NAK
 
 It avoids by removing an overrun check in hard_start_xmit that should 
 not be removed.

Yup, sorry.

The real fact is still that this prevents/fixes lance/driver blocking on my
board, while the tx_timeout mechanism does not succeed at that, and that
on my board the driver is blocked when we return -1 on !TX_BUFFS_AVAIL.

I'll investigate why.

Philippe

PS : did you apply the rest of the patch ?

-- 
Philippe De Muyter  phdm at macqel dot be  Tel +32 27029044
Macq Electronique SA  rue de l'Aeronef 2  B-1140 Bruxelles  Fax +32 27029077
-
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] 7990 : Various fixes and cleanups

2007-07-06 Thread Philippe De Muyter 
Hi all,

This patch
- avoids 7990 blocking when no tx buffer is available,
- implements tx_bytes statistic that was missing
- sets skb-dev before calling netix_rx()
- improves readability and code efficiency in buffer rings initialisation
- avoids useless memset part for tx packets smaller than ETH_ZLEN

Signed-off-by: Philippe De Muyter [EMAIL PROTECTED]

diff -r 6c0a10cc415a drivers/net/7990.c
--- a/drivers/net/7990.cThu Jul  5 16:10:16 2007 -0700
+++ b/drivers/net/7990.cFri Jul  6 11:27:20 2007 +0200
@@ -179,12 +179,14 @@ static void lance_init_ring (struct net_
lp-tx_full = 0;
 /* Setup the Tx ring entries */
 for (i = 0; i  (1lp-lance_log_tx_bufs); i++) {
+   volatile struct lance_tx_desc *td;
+td = ib-btx_ring [i];
 leptr = LANCE_ADDR(aib-tx_buf[i][0]);
-ib-btx_ring [i].tmd0  = leptr;
-ib-btx_ring [i].tmd1_hadr = leptr  16;
-ib-btx_ring [i].tmd1_bits = 0;
-ib-btx_ring [i].length= 0xf000; /* The ones required by 
tmd2 */
-ib-btx_ring [i].misc  = 0;
+td-tmd0  = leptr;
+td-tmd1_hadr = leptr  16;
+td-tmd1_bits = 0;
+td-length= 0xf000; /* The ones required by tmd2 */
+td-misc  = 0;
 if (DEBUG_IRING)
printk (%d: 0x%8.8x\n, i, leptr);
 }
@@ -193,14 +195,16 @@ static void lance_init_ring (struct net_
 if (DEBUG_IRING)
 printk (RX rings:\n);
 for (i = 0; i  (1lp-lance_log_rx_bufs); i++) {
+   volatile struct lance_rx_desc *rd;
+rd = ib-brx_ring [i];
 leptr = LANCE_ADDR(aib-rx_buf[i][0]);
 
-ib-brx_ring [i].rmd0  = leptr;
-ib-brx_ring [i].rmd1_hadr = leptr  16;
-ib-brx_ring [i].rmd1_bits = LE_R1_OWN;
+rd-rmd0  = leptr;
+rd-rmd1_hadr = leptr  16;
+rd-rmd1_bits = LE_R1_OWN;
 /* 0xf000 == bits that must be one (reserved, presumably) */
-ib-brx_ring [i].length= -RX_BUFF_SIZE | 0xf000;
-ib-brx_ring [i].mblength  = 0;
+rd-length= -RX_BUFF_SIZE | 0xf000;
+rd-mblength  = 0;
 if (DEBUG_IRING)
 printk (%d: 0x%8.8x\n, i, leptr);
 }
@@ -331,6 +335,7 @@ static int lance_rx (struct net_device *
 return 0;
 }
 
+skb-dev = dev;
 skb_reserve (skb, 2);   /* 16 byte align */
 skb_put (skb, len); /* make room */
 eth_copy_and_sum(skb,
@@ -541,9 +546,6 @@ int lance_start_xmit (struct sk_buff *sk
 static int outs;
unsigned long flags;
 
-if (!TX_BUFFS_AVAIL)
-return -1;
-
netif_stop_queue (dev);
 
 skblen = skb-len;
@@ -565,9 +567,11 @@ int lance_start_xmit (struct sk_buff *sk
 ib-btx_ring [entry].length = (-len) | 0xf000;
 ib-btx_ring [entry].misc = 0;
 
-   if (skb-len  ETH_ZLEN)
-   memset((void *)ib-tx_buf[entry][0], 0, ETH_ZLEN);
 skb_copy_from_linear_data(skb, (void *)ib-tx_buf[entry][0], skblen);
+   if (skblen  ETH_ZLEN)
+   memset((char *)ib-tx_buf[entry][0] + skblen, 0, ETH_ZLEN - 
skblen);
+
+   lp-stats.tx_bytes += skblen;
 
 /* Now, give the packet to the lance */
 ib-btx_ring [entry].tmd1_bits = (LE_T1_POK|LE_T1_OWN);
-
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] net : make netlink_seq_ops const

2007-07-06 Thread Philippe De Muyter 
On Fri, Jul 06, 2007 at 02:52:11PM +0200, Patrick McHardy wrote:
 [please send networking patches to netdev]
 
 Philippe De Muyter wrote:
  Hi all,
  
  Make netlink_seq_ops const
 
 
 Might make more sense to do a big patch for all occurences of this
 in net/:
 
 # grep ^static struct seq_operations -r net/ | wc -l
 76

OK

Philippe
-
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] make all initialized struct seq_operations const

2007-07-06 Thread Philippe De Muyter 
Hi all

Make all initialized struct seq_operations in net/ const

Signed-off-by: Philippe De Muyter [EMAIL PROTECTED]

diff -r 6c0a10cc415a net/802/tr.c
--- a/net/802/tr.c  Thu Jul  5 16:10:16 2007 -0700
+++ b/net/802/tr.c  Fri Jul  6 15:17:53 2007 +0200
@@ -567,7 +567,7 @@ static int rif_seq_show(struct seq_file 
 }
 
 
-static struct seq_operations rif_seq_ops = {
+static const struct seq_operations rif_seq_ops = {
.start = rif_seq_start,
.next  = rif_seq_next,
.stop  = rif_seq_stop,
diff -r 6c0a10cc415a net/8021q/vlanproc.c
--- a/net/8021q/vlanproc.c  Thu Jul  5 16:10:16 2007 -0700
+++ b/net/8021q/vlanproc.c  Fri Jul  6 15:17:53 2007 +0200
@@ -69,7 +69,7 @@ static const char name_conf[]  = config
  * Generic /proc/net/vlan/file file and inode operations
  */
 
-static struct seq_operations vlan_seq_ops = {
+static const struct seq_operations vlan_seq_ops = {
.start = vlan_seq_start,
.next = vlan_seq_next,
.stop = vlan_seq_stop,
diff -r 6c0a10cc415a net/appletalk/aarp.c
--- a/net/appletalk/aarp.c  Thu Jul  5 16:10:16 2007 -0700
+++ b/net/appletalk/aarp.c  Fri Jul  6 15:17:53 2007 +0200
@@ -1024,7 +1024,7 @@ static int aarp_seq_show(struct seq_file
return 0;
 }
 
-static struct seq_operations aarp_seq_ops = {
+static const struct seq_operations aarp_seq_ops = {
.start  = aarp_seq_start,
.next   = aarp_seq_next,
.stop   = aarp_seq_stop,
diff -r 6c0a10cc415a net/appletalk/atalk_proc.c
--- a/net/appletalk/atalk_proc.cThu Jul  5 16:10:16 2007 -0700
+++ b/net/appletalk/atalk_proc.cFri Jul  6 15:17:53 2007 +0200
@@ -204,21 +204,21 @@ out:
return 0;
 }
 
-static struct seq_operations atalk_seq_interface_ops = {
+static const struct seq_operations atalk_seq_interface_ops = {
.start  = atalk_seq_interface_start,
.next   = atalk_seq_interface_next,
.stop   = atalk_seq_interface_stop,
.show   = atalk_seq_interface_show,
 };
 
-static struct seq_operations atalk_seq_route_ops = {
+static const struct seq_operations atalk_seq_route_ops = {
.start  = atalk_seq_route_start,
.next   = atalk_seq_route_next,
.stop   = atalk_seq_route_stop,
.show   = atalk_seq_route_show,
 };
 
-static struct seq_operations atalk_seq_socket_ops = {
+static const struct seq_operations atalk_seq_socket_ops = {
.start  = atalk_seq_socket_start,
.next   = atalk_seq_socket_next,
.stop   = atalk_seq_socket_stop,
diff -r 6c0a10cc415a net/atm/br2684.c
--- a/net/atm/br2684.c  Thu Jul  5 16:10:16 2007 -0700
+++ b/net/atm/br2684.c  Fri Jul  6 15:17:53 2007 +0200
@@ -772,7 +772,7 @@ static int br2684_seq_show(struct seq_fi
return 0;
 }
 
-static struct seq_operations br2684_seq_ops = {
+static const struct seq_operations br2684_seq_ops = {
.start = br2684_seq_start,
.next  = br2684_seq_next,
.stop  = br2684_seq_stop,
diff -r 6c0a10cc415a net/atm/clip.c
--- a/net/atm/clip.cThu Jul  5 16:10:16 2007 -0700
+++ b/net/atm/clip.cFri Jul  6 15:17:53 2007 +0200
@@ -928,7 +928,7 @@ static int clip_seq_show(struct seq_file
return 0;
 }
 
-static struct seq_operations arp_seq_ops = {
+static const struct seq_operations arp_seq_ops = {
.start  = clip_seq_start,
.next   = neigh_seq_next,
.stop   = neigh_seq_stop,
diff -r 6c0a10cc415a net/atm/lec.c
--- a/net/atm/lec.c Thu Jul  5 16:10:16 2007 -0700
+++ b/net/atm/lec.c Fri Jul  6 15:17:53 2007 +0200
@@ -1174,7 +1174,7 @@ static int lec_seq_show(struct seq_file 
return 0;
 }
 
-static struct seq_operations lec_seq_ops = {
+static const struct seq_operations lec_seq_ops = {
.start = lec_seq_start,
.next = lec_seq_next,
.stop = lec_seq_stop,
diff -r 6c0a10cc415a net/atm/mpoa_proc.c
--- a/net/atm/mpoa_proc.c   Thu Jul  5 16:10:16 2007 -0700
+++ b/net/atm/mpoa_proc.c   Fri Jul  6 15:17:53 2007 +0200
@@ -177,7 +177,7 @@ static int mpc_show(struct seq_file *m, 
return 0;
 }
 
-static struct seq_operations mpc_op = {
+static const struct seq_operations mpc_op = {
.start =mpc_start,
.next = mpc_next,
.stop = mpc_stop,
diff -r 6c0a10cc415a net/atm/proc.c
--- a/net/atm/proc.cThu Jul  5 16:10:16 2007 -0700
+++ b/net/atm/proc.cFri Jul  6 15:17:53 2007 +0200
@@ -260,7 +260,7 @@ static int atm_dev_seq_show(struct seq_f
return 0;
 }
 
-static struct seq_operations atm_dev_seq_ops = {
+static const struct seq_operations atm_dev_seq_ops = {
.start  = atm_dev_seq_start,
.next   = atm_dev_seq_next,
.stop   = atm_dev_seq_stop,
@@ -295,7 +295,7 @@ static int pvc_seq_show(struct seq_file 
return 0;
 }
 
-static struct seq_operations pvc_seq_ops = {
+static const struct seq_operations pvc_seq_ops = {
.start  = vcc_seq_start,
.next   = vcc_seq_next,
.stop   = vcc_seq_stop,
@@ -329,7

Re: [patch 3/8] sundance: remove TxStartThresh and RxEarlyThresh

2006-10-22 Thread Philippe De Muyter 
On Fri, Oct 20, 2006 at 02:42:05PM -0700, [EMAIL PROTECTED] wrote:
 From: Jesse Huang [EMAIL PROTECTED]
 For patent issue need to remove TxStartThresh and RxEarlyThresh.  This patent
 is cut-through patent.  If use this function, Tx will start to transmit after
 few data be move in to Tx FIFO.  We are not allow to use those function in
 DFE530/DFE550/DFE580/DL10050/IP100/IP100A.  It will decrease a little
 performance.
[...]
 @@ -,6 +1109,7 @@ static irqreturn_t intr_handler(int irq,
   int tx_cnt;
   int tx_status;
   int handled = 0;
 + int i;

What the use here of adding this variable ? Is that actually a part of another
patch ?

  
  
   do {
 @@ -1153,17 +1152,14 @@ static irqreturn_t intr_handler(int irq,
   np-stats.tx_fifo_errors++;
   if (tx_status  0x02)
   np-stats.tx_window_errors++;
 +
   /*
   ** This reset has been verified on
   ** DFE-580TX boards ! [EMAIL PROTECTED]
   */
   if (tx_status  0x10) { /* TxUnderrun */
 - unsigned short txthreshold;
 -
 - txthreshold = ioread16 (ioaddr 
 + TxStartThresh);
   /* Restart Tx FIFO and 
 transmitter */
   sundance_reset(dev, 
 (NetworkReset|FIFOReset|TxReset)  16);
 - iowrite16 (txthreshold, ioaddr 
 + TxStartThresh);

I must be dense, but I do not understand how merely preserving the value of
a register across a reset can infringe on any patent

Philippe
-
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 1/4] IP100A: Fix TX Pause bug (reset_tx, intr_handler)

2006-09-18 Thread Philippe De Muyter 
On Mon, Sep 18, 2006 at 11:41:09AM +0800, Jesse Huang wrote:
 Dear Philippe:
 
 (1) We are not allow to support register TxStartThresh and, RxEarlyThresh,
 so
 we remove it.

Could you develop ?
- What do you mean by `We are not allow'
- Is it specific to the IP100A chip ?

Those register are documented in the Sundance Technology ST201 Data Sheet
and when modified with fine-tuned values, they can have a real positive
effect on the overall throughput on a loaded system.

 
 (2) Your consideration is right. But reset_tx is workaround for customer's
 embedded system, I don't have this
 enviroment now. I can't sure it will work fine if I removed this.

On DFE-580TX boards, the reset_tx way did not work.  The ports remained
blocked until a power-cycle.  I do not know if the TxUnderrun problem ever
happened with earlier (one port) boards, so I doubt that the reset_tx way
ever worked.  Is was even commented as not being tested.  On DFE-580TX
boards, the current way has been verified by me and others to work, so
please do not break it.

Best regards

Philippe

 
 Thanks you very mutch.
 
 Best Regards,
 Jesse Huang.
 
 - Original Message - 
 From: Philippe De Muyter [EMAIL PROTECTED]
 To: Jesse Huang [EMAIL PROTECTED]
 Cc: netdev@vger.kernel.org
 Sent: Friday, September 15, 2006 7:44 PM
 Subject: Re: [PATCH 1/4] IP100A: Fix TX Pause bug (reset_tx, intr_handler)
 
 
 On Thu, Sep 14, 2006 at 12:58:30AM +, Jesse Huang wrote:
 [...]
  @@ -262,8 +262,6 @@ enum alta_offsets {
   ASICCtrl = 0x30,
   EEData = 0x34,
   EECtrl = 0x36,
  - TxStartThresh = 0x3c,
  - RxEarlyThresh = 0x3e,
 
 Why ?
 
   FlashAddr = 0x40,
   FlashData = 0x44,
   TxStatus = 0x46,
 [...]
  @@ -1156,29 +1160,29 @@ static irqreturn_t intr_handler(int irq,
   np-stats.tx_fifo_errors++;
   if (tx_status  0x02)
   np-stats.tx_window_errors++;
  - /*
  - ** This reset has been verified on
  - ** DFE-580TX boards ! [EMAIL PROTECTED]
  - */
  - if (tx_status  0x10) { /* TxUnderrun */
  - unsigned short txthreshold;
  -
  - txthreshold = ioread16 (ioaddr + TxStartThresh);
  - /* Restart Tx FIFO and transmitter */
  - sundance_reset(dev, (NetworkReset|FIFOReset|TxReset)  16);
  - iowrite16 (txthreshold, ioaddr + TxStartThresh);
  - /* No need to reset the Tx pointer here */
  +
  + /* FIFO ERROR need to be reset tx */
  + if (tx_status  0x10) { /* Reset the Tx. */
  + spin_lock(np-lock);
  + reset_tx(dev);
  + spin_unlock(np-lock);
  + }
 
 Just as the comments say, on DFE-580TX 4 port boards, where it is easy to
 reproduce TxUnderrun problems, just resetting on the chip the Tx FIFO and
 transmitter is enough.
 There is no need to call reset_tx, which discards all pending messages and
 frees all the skb's.  It is also not necessary to reload the Tx pointer.
 
 Is it different with newer versions of the chip ?
 
 Philippe
 

-- 
-
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 1/4] IP100A: Fix TX Pause bug (reset_tx, intr_handler)

2006-09-18 Thread Philippe De Muyter 
Hi Jesse,

On Mon, Sep 18, 2006 at 07:11:29PM +0800, Jesse Huang wrote:
 Dear Philippe:
 (1)Because this is a patent issue, we are not allow to use it again, even it
 is in Data Sheet.

I surmise this is only a concern for icplus as a hardware company.

The sundance driver in Linux is meant to work also with the previous versions
of the chip (Sundance, Kendin, D-Link).  If you wish you can make it clear
that those registers have disappeared or have no effect in the icplus 100A
version.

 
 (2)Ok, sorry for this, I will add it back.

Thanks

 
 Should I resent those 4 patches? Or generate this as a new patch?

I do not know about the other patches, but for this one of course you should

Philippe

 
 Thanks very much!
 
 Best Regards,
 Jesse Huang.
 
 - Original Message - 
 From: Philippe De Muyter [EMAIL PROTECTED]
 To: Jesse Huang [EMAIL PROTECTED]
 Cc: netdev@vger.kernel.org
 Sent: Monday, September 18, 2006 5:41 PM
 Subject: Re: [PATCH 1/4] IP100A: Fix TX Pause bug (reset_tx, intr_handler)
 
 
 On Mon, Sep 18, 2006 at 11:41:09AM +0800, Jesse Huang wrote:
  Dear Philippe:
 
  (1) We are not allow to support register TxStartThresh and, RxEarlyThresh,
  so
  we remove it.
 
 Could you develop ?
 - What do you mean by `We are not allow'
 - Is it specific to the IP100A chip ?
 
 Those register are documented in the Sundance Technology ST201 Data Sheet
 and when modified with fine-tuned values, they can have a real positive
 effect on the overall throughput on a loaded system.
 
 
  (2) Your consideration is right. But reset_tx is workaround for customer's
  embedded system, I don't have this
  enviroment now. I can't sure it will work fine if I removed this.
 
 On DFE-580TX boards, the reset_tx way did not work.  The ports remained
 blocked until a power-cycle.  I do not know if the TxUnderrun problem ever
 happened with earlier (one port) boards, so I doubt that the reset_tx way
 ever worked.  Is was even commented as not being tested.  On DFE-580TX
 boards, the current way has been verified by me and others to work, so
 please do not break it.
 
 Best regards
 
 Philippe
 
 
  Thanks you very mutch.
 
  Best Regards,
  Jesse Huang.
 
  - Original Message - 
  From: Philippe De Muyter [EMAIL PROTECTED]
  To: Jesse Huang [EMAIL PROTECTED]
  Cc: netdev@vger.kernel.org
  Sent: Friday, September 15, 2006 7:44 PM
  Subject: Re: [PATCH 1/4] IP100A: Fix TX Pause bug (reset_tx, intr_handler)
 
 
  On Thu, Sep 14, 2006 at 12:58:30AM +, Jesse Huang wrote:
  [...]
   @@ -262,8 +262,6 @@ enum alta_offsets {
ASICCtrl = 0x30,
EEData = 0x34,
EECtrl = 0x36,
   - TxStartThresh = 0x3c,
   - RxEarlyThresh = 0x3e,
 
  Why ?
 
FlashAddr = 0x40,
FlashData = 0x44,
TxStatus = 0x46,
  [...]
   @@ -1156,29 +1160,29 @@ static irqreturn_t intr_handler(int irq,
np-stats.tx_fifo_errors++;
if (tx_status  0x02)
np-stats.tx_window_errors++;
   - /*
   - ** This reset has been verified on
   - ** DFE-580TX boards ! [EMAIL PROTECTED]
   - */
   - if (tx_status  0x10) { /* TxUnderrun */
   - unsigned short txthreshold;
   -
   - txthreshold = ioread16 (ioaddr + TxStartThresh);
   - /* Restart Tx FIFO and transmitter */
   - sundance_reset(dev, (NetworkReset|FIFOReset|TxReset)  16);
   - iowrite16 (txthreshold, ioaddr + TxStartThresh);
   - /* No need to reset the Tx pointer here */
   +
   + /* FIFO ERROR need to be reset tx */
   + if (tx_status  0x10) { /* Reset the Tx. */
   + spin_lock(np-lock);
   + reset_tx(dev);
   + spin_unlock(np-lock);
   + }
 
  Just as the comments say, on DFE-580TX 4 port boards, where it is easy to
  reproduce TxUnderrun problems, just resetting on the chip the Tx FIFO and
  transmitter is enough.
  There is no need to call reset_tx, which discards all pending messages and
  frees all the skb's.  It is also not necessary to reload the Tx pointer.
 
  Is it different with newer versions of the chip ?
 
  Philippe
 
 
 --
 

-- 
-
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 1/4] IP100A: Fix TX Pause bug (reset_tx, intr_handler)

2006-09-15 Thread Philippe De Muyter 
On Thu, Sep 14, 2006 at 12:58:30AM +, Jesse Huang wrote:
[...]
 @@ -262,8 +262,6 @@ enum alta_offsets {
   ASICCtrl = 0x30,
   EEData = 0x34,
   EECtrl = 0x36,
 - TxStartThresh = 0x3c,
 - RxEarlyThresh = 0x3e,

Why ?

   FlashAddr = 0x40,
   FlashData = 0x44,
   TxStatus = 0x46,
[...]
 @@ -1156,29 +1160,29 @@ static irqreturn_t intr_handler(int irq,
   np-stats.tx_fifo_errors++;
   if (tx_status  0x02)
   np-stats.tx_window_errors++;
 - /*
 - ** This reset has been verified on
 - ** DFE-580TX boards ! [EMAIL PROTECTED]
 - */
 - if (tx_status  0x10) { /* TxUnderrun */
 - unsigned short txthreshold;
 -
 - txthreshold = ioread16 (ioaddr 
 + TxStartThresh);
 - /* Restart Tx FIFO and 
 transmitter */
 - sundance_reset(dev, 
 (NetworkReset|FIFOReset|TxReset)  16);
 - iowrite16 (txthreshold, ioaddr 
 + TxStartThresh);
 - /* No need to reset the Tx 
 pointer here */
 +
 + /* FIFO ERROR need to be reset tx */
 + if (tx_status  0x10) { /* Reset the 
 Tx. */
 + spin_lock(np-lock);
 + reset_tx(dev);
 + spin_unlock(np-lock);
 + }

Just as the comments say, on DFE-580TX 4 port boards, where it is easy to
reproduce TxUnderrun problems, just resetting on the chip the Tx FIFO and
transmitter is enough.
There is no need to call reset_tx, which discards all pending messages and
frees all the skb's.  It is also not necessary to reload the Tx pointer.

Is it different with newer versions of the chip ?

Philippe
-
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] sundance: small cleanup

2006-08-03 Thread Philippe De Muyter 
This patch uses the sundance_reset function everywhere a reset is done,
and adds a link to the archives of the original mailing list.

Signed-off-by: Philippe De Muyter [EMAIL PROTECTED]

diff -r 9062402c439c drivers/net/sundance.c
--- a/drivers/net/sundance.cThu Aug  3 12:35:26 2006 +0700
+++ b/drivers/net/sundance.cThu Aug  3 18:17:31 2006 +0200
@@ -17,6 +17,8 @@
Support and updates available at
http://www.scyld.com/network/sundance.html
[link no longer provides useful info -jgarzik]
+   Archives of the mailing list are still available at
+   http://www.beowulf.org/pipermail/netdrivers/
 
 */
 
@@ -646,7 +648,7 @@ static int __devinit sundance_probe1 (st
/* Reset the chip to erase previous misconfiguration. */
if (netif_msg_hw(np))
printk(ASIC Control is %x.\n, ioread32(ioaddr + ASICCtrl));
-   iowrite16(0x00ff, ioaddr + ASICCtrl + 2);
+   sundance_reset(dev, 0x00ff  16);
if (netif_msg_hw(np))
printk(ASIC Control is now %x.\n, ioread32(ioaddr + 
ASICCtrl));
 
@@ -1075,13 +1077,8 @@ reset_tx (struct net_device *dev)

/* Reset tx logic, TxListPtr will be cleaned */
iowrite16 (TxDisable, ioaddr + MACCtrl1);
-   iowrite16 (TxReset | DMAReset | FIFOReset | NetworkReset,
-   ioaddr + ASICCtrl + 2);
-   for (i=50; i  0; i--) {
-   if ((ioread16(ioaddr + ASICCtrl + 2)  ResetBusy) == 0)
-   break;
-   mdelay(1);
-   }
+   sundance_reset(dev, (NetworkReset|FIFOReset|DMAReset|TxReset)  16);
+
/* free all tx skbuff */
for (i = 0; i  TX_RING_SIZE; i++) {
skb = np-tx_skbuff[i];
-
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

Q: link up/down, netif_carrier_on/off tx_timeout

2006-01-26 Thread Philippe De Muyter 
Hi all,

I work with the fec driver (used with embedded linuxes), and I am not very
satisfied of the behaviour regarding link down / link up.

So here are my questions :

1. When/where should netif_carrierr_on/off or another similar routine
   be called ?
2. If the above are called properly, will tx_timeout be called, and when ?

Thanks for listening

Philippe
-
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: [Fwd: [patch 11/15] ppp: handle misaligned accesses]

2005-08-11 Thread Philippe De Muyter 
I wrote :
 I just noticed something at the end of process_input_packet :
 In the normal case, skb is given to the next stage and ap-rpkt is reset,
 but in the error case, skb is kept, ap-rpkt is not reset, so we keep
 the skb with skb-data aligned for one message and we put another one
 into it :)
 
 Could that not be the culprit ?

Based on my previous observation, here is a revised patch, that replaces
the previous one.

This patch avoids ppp-generated kernel crashes on machines where
unaligned accesses are forbidden, by fixing ppp alignment setting
for reused skb's.

Signed-off-by: Philippe De Muyter [EMAIL PROTECTED]

--- drivers/net/ppp_async.c 2004/05/07 08:38:32 1.1.1.1
+++ drivers/net/ppp_async.c 2005/08/11 11:21:33
@@ -30,6 +30,7 @@
 #include linux/spinlock.h
 #include linux/init.h
 #include asm/uaccess.h
+#include asm/string.h
 
 #define PPP_VERSION2.4.2
 
@@ -846,7 +847,11 @@ process_input_packet(struct asyncppp *ap
/* frame had an error, remember that, reset SC_TOSS  SC_ESCAPE */
ap-state = SC_PREV_ERROR;
if (skb)
+   {
+   /* make skb appear as freshly allocated */
skb_trim(skb, 0);
+   skb_reserve(skb, - skb_headroom(skb));
+   }
 }
 
 /* called when the tty driver has data for us. */
@@ -897,10 +902,18 @@ ppp_async_input(struct asyncppp *ap, con
skb = dev_alloc_skb(ap-mru + PPP_HDRLEN + 2);
if (skb == 0)
goto nomem;
+   ap-rpkt = skb;
+   }
+   if (skb-len == 0) {
/* Try to get the payload 4-byte aligned */
+   /* This should match the
+   ** PPP_ALLSTATIONS/PPP_UI/compressed tests
+   ** in process_input_packet,
+   ** but we do not have enough chars here to
+   ** test buf[1] and buf[2].
+   */
if (buf[0] != PPP_ALLSTATIONS)
skb_reserve(skb, 2 + (buf[0]  1));
-   ap-rpkt = skb;
}
if (n  skb_tailroom(skb)) {
/* packet overflowed MRU */
-
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: [Fwd: [patch 11/15] ppp: handle misaligned accesses]

2005-08-10 Thread Philippe De Muyter 
Paul Mackerras wrote :
 Philippe De Muyter writes:
 
  Actually, that's probably the case I had, but my fix gets the ip adresses
  4byte aligned in my case : I had verified the address of the saddr field,
  and I needed to shift the buffer by 3, not 1, to get it 4byte aligned.
 
 Please outline the code flow that leads to that situation.  AFAICS we
 would only need to shift the buffer by 3 if we had a compressed
 (1-byte) protocol number at the original skb-data, implying that the
 protocol byte was first.  But if the protocol byte was first, then
 this code:
 
   if (buf[0] != PPP_ALLSTATIONS)
   skb_reserve(skb, 2 + (buf[0]  1));
 
 at line 893 should have moved skb-data up by 3 bytes already, since a
 1-byte protocol number is always odd.
 
 If that is not the case then there is something else going on beyond
 the data getting misaligned, and I would like to know what it is.

I just noticed something at the end of process_input_packet :

/* queue the frame to be processed */
skb-cb[0] = ap-state;
skb_queue_tail(ap-rqueue, skb);
ap-rpkt = 0;
ap-state = 0;
return;

 err:
/* frame had an error, remember that, reset SC_TOSS  SC_ESCAPE */
ap-state = SC_PREV_ERROR;
if (skb)
skb_trim(skb, 0);
}

In the normal case, skb is given to the next stage and ap-rpkt is reset,
but in the error case, skb is kept, ap-rpkt is not reset, so we keep
the skb with skb-data aligned for one message and we put another one
into it :)

Could that not be the culprit ?

Philippe
 
 Paul.
 

-
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: [Fwd: [patch 11/15] ppp: handle misaligned accesses]

2005-08-09 Thread Philippe De Muyter 
Paul Mackerras wrote :
 Philippe De Muyter writes:
 
   This patch seems a bit strange and/or incomplete.  Are we trying to
   get 2-byte alignment or 4-byte alignment of the payload?  It seems
  
  Actually, we try to get a 4n+2 alignment for skb-data, to get the 
  ip-addresses
  field 4bytes aligned.
  I think the only thing wrong is the old comment that said :
  /* Try to get the payload 4-byte aligned */
  and that I did not change.
 
 Yes, the payload is the part after the protocol field, so the comment
 is still correct.
 
   that if the protocol field is uncompressed, we don't do anything to
   the alignment, but if it is compressed, we do this:
   
/* protocol is compressed */
-   skb_push(skb, 1)[0] = 0;
+   if ((unsigned long)skb-data  1)
+   skb_push(skb, 1)[0] = 0;
+   else { /* Ditto, but realign the payload to 4-byte 
boundary */
+   short len = skb-len;
+
+   skb_put(skb, 3);
+   memmove(skb-data + 3, skb-data, len);
+   skb_pull(skb, 2)[0] = 0;
   
   I'm puzzled that we are not testing ((unsigned long)skb-data  2) if
   we are really trying to achieve 4-byte alignment.  In fact, if the
   skb-data that we get from dev_alloc_skb is 4-byte aligned to start
   with, this will end up with the payload starting at the original
   skb-data + 6, i.e. 2-byte aligned but not 4-byte aligned AFAICS.
   
   Can we assume that dev_alloc_skb will give us a 4-byte aligned
   skb-data?  If we can then I suggest we change 3 to 1 in the skb_put
  
  Are you not forgetting that the alignment of skb-data is changed (by the 
  existing code ! ) :
  if (buf[0] != PPP_ALLSTATIONS)
  skb_reserve(skb, 2 + (buf[0]  1));
 
 No, I'm not forgetting.  If we assume that skb-data starts out 4-byte
 aligned, then the only case in which we will have
 
   ((unsigned long)skb-data  1) == 0
 
 is if we have protocol field compression (and a compressible protocol
 number, i.e. less than 0x100) but not address/control compression
 (which would be a weird combination, but legal).  In that case, with
 your patch we will move the protocol byte to the original skb-data+5
 and have the payload at +6.

Actually, that's probably the case I had, but my fix gets the ip adresses
4byte aligned in my case : I had verified the address of the saddr field,
and I needed to shift the buffer by 3, not 1, to get it 4byte aligned.

 
 If there is any possibility that skb-data is not 4-byte aligned when

No, that's not the problem I had.  skb-data was always 4-byte aligned
at allocation time.

 the skb is first allocated, I think that we should do something like
 
   if ((unsigned long)skb-data  3)
   skb_reserve(skb, 4 - ((unsigned long)skb-data  3));
 
 immediately after allocating it, and then just memmove the stuff up
 one byte (rather than 3) if it isn't aligned as we want.
 
 Paul.
 

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