Re: [PATCH 1/3] tg3: Limit minimum tx queue wakeup threshold

2014-08-19 Thread Michael Chan
On Tue, 2014-08-19 at 11:52 -0700, Benjamin Poirier wrote: 
 diff --git a/drivers/net/ethernet/broadcom/tg3.c 
 b/drivers/net/ethernet/broadcom/tg3.c
 index 3ac5d23..b11c0fd 100644
 --- a/drivers/net/ethernet/broadcom/tg3.c
 +++ b/drivers/net/ethernet/broadcom/tg3.c
 @@ -202,7 +202,8 @@ static inline void _tg3_flag_clear(enum TG3_FLAGS flag, 
 unsigned long *bits)
  #endif
  
  /* minimum number of free TX descriptors required to wake up TX process */
 -#define TG3_TX_WAKEUP_THRESH(tnapi)((tnapi)-tx_pending / 4)
 +#define TG3_TX_WAKEUP_THRESH(tnapi)max_t(u32, (tnapi)-tx_pending / 4, \
 + MAX_SKB_FRAGS + 1)

I think we should precompute this and store it in something like
tp-tx_wake_thresh.

  #define TG3_TX_BD_DMA_MAX_2K   2048
  #define TG3_TX_BD_DMA_MAX_4K   4096
   

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/3] tg3: Fix tx_pending checks for tg3_tso_bug

2014-08-19 Thread Michael Chan
On Tue, 2014-08-19 at 11:52 -0700, Benjamin Poirier wrote: 
 @@ -7838,11 +7838,14 @@ static int tg3_tso_bug(struct tg3 *tp, struct 
 tg3_napi *tnapi,
struct netdev_queue *txq, struct sk_buff *skb)
  {
 struct sk_buff *segs, *nskb;
 -   u32 frag_cnt_est = skb_shinfo(skb)-gso_segs * 3;
  
 -   /* Estimate the number of fragments in the worst case */
 -   if (unlikely(tg3_tx_avail(tnapi) = frag_cnt_est)) {
 +   if (unlikely(tg3_tx_avail(tnapi) = skb_shinfo(skb)-gso_segs)) {
 +   trace_printk(stopping queue, %d = %d\n,
 +tg3_tx_avail(tnapi), skb_shinfo(skb)-gso_segs);
 netif_tx_stop_queue(txq);
 +   trace_printk(stopped queue\n);
 +   tnapi-wakeup_thresh = skb_shinfo(skb)-gso_segs;
 +   BUG_ON(tnapi-wakeup_thresh = tnapi-tx_pending);
  
 /* netif_tx_stop_queue() must be done before checking
  * checking tx index in tg3_tx_avail() below, because in 

I don't quite understand this logic and I must be missing something.
gso_segs is the number of TCP segments the large packet will be broken
up into.  If it exceeds dev-gso_max_segs, it means it exceeds
hardware's capabilty and it will do GSO instead of TSO.  But in this
case in tg3_tso_bug(), we are doing GSO and we may not have enough DMA
descriptors to do GSO.  Each gso_seg typically requires 2 DMA
descriptors.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2] cnic: Replace rcu_dereference() with rcu_access_pointer()

2014-08-18 Thread Michael Chan
On Sun, 2014-08-17 at 13:12 +0300, Andreea-Cristina Bernat wrote: 
> The "rcu_dereference()" calls are used directly in conditions.
> Since their return values are never dereferenced it is recommended to use
> "rcu_access_pointer()" instead of "rcu_dereference()".
> Therefore, this patch makes the replacements.
> 
> The following Coccinelle semantic patch was used:
> @@
> @@
> 
> (
>  if(
>  (<+...
> - rcu_dereference
> + rcu_access_pointer
>   (...)
>   ...+>)) {...}
> |
>  while(
>  (<+...
> - rcu_dereference
> + rcu_access_pointer
>   (...)
>   ...+>)) {...}
> )
> 
> Signed-off-by: Andreea-Cristina Bernat 

Acked-by: Michael Chan 

> ---
> v2: Modified subject line from
>  "rcu: Replace rcu_dereference() with rcu_access_pointer()"
>  to
>  "cnic: Replace rcu_dereference() with rcu_access_pointer()",
>  noted by David Miller 
> 
>  drivers/net/ethernet/broadcom/cnic.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/cnic.c 
> b/drivers/net/ethernet/broadcom/cnic.c
> index 8244e2b..dad9cb7 100644
> --- a/drivers/net/ethernet/broadcom/cnic.c
> +++ b/drivers/net/ethernet/broadcom/cnic.c
> @@ -381,7 +381,7 @@ static int cnic_iscsi_nl_msg_recv(struct cnic_dev *dev, 
> u32 msg_type,
>   break;
>  
>   rcu_read_lock();
> - if (!rcu_dereference(cp->ulp_ops[CNIC_ULP_L4])) {
> + if (!rcu_access_pointer(cp->ulp_ops[CNIC_ULP_L4])) {
>   rc = -ENODEV;
>   rcu_read_unlock();
>   break;
> @@ -525,7 +525,7 @@ int cnic_unregister_driver(int ulp_type)
>   list_for_each_entry(dev, _dev_list, list) {
>   struct cnic_local *cp = dev->cnic_priv;
>  
> - if (rcu_dereference(cp->ulp_ops[ulp_type])) {
> + if (rcu_access_pointer(cp->ulp_ops[ulp_type])) {
>   pr_err("%s: Type %d still has devices registered\n",
>  __func__, ulp_type);
>   read_unlock(_dev_lock);
> @@ -573,7 +573,7 @@ static int cnic_register_device(struct cnic_dev *dev, int 
> ulp_type,
>   mutex_unlock(_lock);
>   return -EAGAIN;
>   }
> - if (rcu_dereference(cp->ulp_ops[ulp_type])) {
> + if (rcu_access_pointer(cp->ulp_ops[ulp_type])) {
>   pr_err("%s: Type %d has already been registered to this 
> device\n",
>  __func__, ulp_type);
>   mutex_unlock(_lock);


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2] cnic: Replace rcu_dereference() with rcu_access_pointer()

2014-08-18 Thread Michael Chan
On Sun, 2014-08-17 at 13:12 +0300, Andreea-Cristina Bernat wrote: 
 The rcu_dereference() calls are used directly in conditions.
 Since their return values are never dereferenced it is recommended to use
 rcu_access_pointer() instead of rcu_dereference().
 Therefore, this patch makes the replacements.
 
 The following Coccinelle semantic patch was used:
 @@
 @@
 
 (
  if(
  (+...
 - rcu_dereference
 + rcu_access_pointer
   (...)
   ...+)) {...}
 |
  while(
  (+...
 - rcu_dereference
 + rcu_access_pointer
   (...)
   ...+)) {...}
 )
 
 Signed-off-by: Andreea-Cristina Bernat bernat@gmail.com

Acked-by: Michael Chan mc...@broadcom.com

 ---
 v2: Modified subject line from
  rcu: Replace rcu_dereference() with rcu_access_pointer()
  to
  cnic: Replace rcu_dereference() with rcu_access_pointer(),
  noted by David Miller da...@davemloft.net
 
  drivers/net/ethernet/broadcom/cnic.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/net/ethernet/broadcom/cnic.c 
 b/drivers/net/ethernet/broadcom/cnic.c
 index 8244e2b..dad9cb7 100644
 --- a/drivers/net/ethernet/broadcom/cnic.c
 +++ b/drivers/net/ethernet/broadcom/cnic.c
 @@ -381,7 +381,7 @@ static int cnic_iscsi_nl_msg_recv(struct cnic_dev *dev, 
 u32 msg_type,
   break;
  
   rcu_read_lock();
 - if (!rcu_dereference(cp-ulp_ops[CNIC_ULP_L4])) {
 + if (!rcu_access_pointer(cp-ulp_ops[CNIC_ULP_L4])) {
   rc = -ENODEV;
   rcu_read_unlock();
   break;
 @@ -525,7 +525,7 @@ int cnic_unregister_driver(int ulp_type)
   list_for_each_entry(dev, cnic_dev_list, list) {
   struct cnic_local *cp = dev-cnic_priv;
  
 - if (rcu_dereference(cp-ulp_ops[ulp_type])) {
 + if (rcu_access_pointer(cp-ulp_ops[ulp_type])) {
   pr_err(%s: Type %d still has devices registered\n,
  __func__, ulp_type);
   read_unlock(cnic_dev_lock);
 @@ -573,7 +573,7 @@ static int cnic_register_device(struct cnic_dev *dev, int 
 ulp_type,
   mutex_unlock(cnic_lock);
   return -EAGAIN;
   }
 - if (rcu_dereference(cp-ulp_ops[ulp_type])) {
 + if (rcu_access_pointer(cp-ulp_ops[ulp_type])) {
   pr_err(%s: Type %d has already been registered to this 
 device\n,
  __func__, ulp_type);
   mutex_unlock(cnic_lock);


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: igb and bnx2: "NETDEV WATCHDOG: transmit queue timed out" when skb has huge linear buffer

2014-02-04 Thread Michael Chan
On Fri, 2014-01-31 at 14:29 +0100, Zoltan Kiss wrote: 
> [ 5417.275472] WARNING: at net/sched/sch_generic.c:255 
> dev_watchdog+0x156/0x1f0()
> [ 5417.275474] NETDEV WATCHDOG: eth1 (bnx2): transmit queue 2 timed out 

The dump shows an internal IRQ pending on MSIX vector 2 which matches
the the queue number that is timing out.  I don't know what happened to
the MSIX and why the driver is not seeing it.  Do you see an IRQ error
message from the kernel a few seconds before the tx timeout message?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: igb and bnx2: NETDEV WATCHDOG: transmit queue timed out when skb has huge linear buffer

2014-02-04 Thread Michael Chan
On Fri, 2014-01-31 at 14:29 +0100, Zoltan Kiss wrote: 
 [ 5417.275472] WARNING: at net/sched/sch_generic.c:255 
 dev_watchdog+0x156/0x1f0()
 [ 5417.275474] NETDEV WATCHDOG: eth1 (bnx2): transmit queue 2 timed out 

The dump shows an internal IRQ pending on MSIX vector 2 which matches
the the queue number that is timing out.  I don't know what happened to
the MSIX and why the driver is not seeing it.  Do you see an IRQ error
message from the kernel a few seconds before the tx timeout message?

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: igb and bnx2: "NETDEV WATCHDOG: transmit queue timed out" when skb has huge linear buffer

2014-01-30 Thread Michael Chan
On Thu, 2014-01-30 at 19:08 +, Zoltan Kiss wrote:
> I've experienced some queue timeout problems mentioned in the subject 
> with igb and bnx2 cards. 

Please provide the full tx timeout dmesg.  bnx2 dumps some diagnostic
information during tx timeout that may be useful.  Thanks.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: igb and bnx2: NETDEV WATCHDOG: transmit queue timed out when skb has huge linear buffer

2014-01-30 Thread Michael Chan
On Thu, 2014-01-30 at 19:08 +, Zoltan Kiss wrote:
 I've experienced some queue timeout problems mentioned in the subject 
 with igb and bnx2 cards. 

Please provide the full tx timeout dmesg.  bnx2 dumps some diagnostic
information during tx timeout that may be useful.  Thanks.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] net-tg3: Initialize REG_BASE_ADDR at PCI config offset 120 to 0

2013-12-11 Thread Michael Chan
On Tue, 2013-12-10 at 22:23 -0500, David Miller wrote:
> Ok, that may be true, but I'd like to consider the much larger issue
> at hand.
> 
> If the indirect mechanism is enabled, some of the offsets that may be
> in there might be value, but would be entirely undesirable to be read
> because the read has side effects.
> 
> What if the interrupt status register is what gets read if the user
> scans config space at just the wrong moment, and therefore an
> interrupt gets lost?
> 
I did a quick audit of the driver to see if we are seriously exposed to
this problem.  Only very old chips (5700 to 5703) use indirect register
access as workarounds for various issues.  I looked at the IRQ path
which uses status block to determine if there is work to do.  If status
block contains no new data, we check the PCI_STATE register and the bits
don't get cleared on read.  MAC status register which is used to
determine link status does not have bits that are cleared on read.  So
we should be ok.

We use the statistics block for counters on these older chips so there
is no conflict.  On newer chips, we read counters from registers which
get cleared on read, but we never use indirect methods to read these
counters.

On all new chips, indirect method is not used except to download
firmware (on 57766 only).  Userspace read will not interfere with
firmware download.

So it looks to me that we are in good shape.  Thanks.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] net-tg3: Initialize REG_BASE_ADDR at PCI config offset 120 to 0

2013-12-11 Thread Michael Chan
On Tue, 2013-12-10 at 22:23 -0500, David Miller wrote:
 Ok, that may be true, but I'd like to consider the much larger issue
 at hand.
 
 If the indirect mechanism is enabled, some of the offsets that may be
 in there might be value, but would be entirely undesirable to be read
 because the read has side effects.
 
 What if the interrupt status register is what gets read if the user
 scans config space at just the wrong moment, and therefore an
 interrupt gets lost?
 
I did a quick audit of the driver to see if we are seriously exposed to
this problem.  Only very old chips (5700 to 5703) use indirect register
access as workarounds for various issues.  I looked at the IRQ path
which uses status block to determine if there is work to do.  If status
block contains no new data, we check the PCI_STATE register and the bits
don't get cleared on read.  MAC status register which is used to
determine link status does not have bits that are cleared on read.  So
we should be ok.

We use the statistics block for counters on these older chips so there
is no conflict.  On newer chips, we read counters from registers which
get cleared on read, but we never use indirect methods to read these
counters.

On all new chips, indirect method is not used except to download
firmware (on 57766 only).  Userspace read will not interfere with
firmware download.

So it looks to me that we are in good shape.  Thanks.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] net-tg3: Initialize REG_BASE_ADDR at PCI config offset 120 to 0

2013-12-10 Thread Michael Chan
On Tue, 2013-12-10 at 13:43 -0500, David Miller wrote:

> What if the kernel is booted via kexec, and the driver in the kernel
> we are kexec'ing from left indirect access enabled in MISC_HOST_CTRL?

That should be ok.  The driver will only use valid register offsets in
indirect mode during run time, so register 0x78 should point to a valid
register.  If indirect mode is never used by the driver, it will point
to zero with this patch.  So register 0x78 will be valid (or zero) in
the kexec'ed kernel.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] net-tg3: Initialize REG_BASE_ADDR at PCI config offset 120 to 0

2013-12-10 Thread Michael Chan
On Tue, 2013-12-10 at 07:01 -0800, Natarajan Gurumoorthy wrote:
>  The only time I see crashes is after the tg3 driver has been
> loaded into the system. I our use case we are poking around
> /sys/devices/pci//config. 

David, please apply this first patch.

Acked-by: Michael Chan 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



Re: [PATCH] net-tg3: Initialize REG_BASE_ADDR at PCI config offset 120 to 0

2013-12-10 Thread Michael Chan
On Tue, 2013-12-10 at 13:43 -0500, David Miller wrote:

 What if the kernel is booted via kexec, and the driver in the kernel
 we are kexec'ing from left indirect access enabled in MISC_HOST_CTRL?

That should be ok.  The driver will only use valid register offsets in
indirect mode during run time, so register 0x78 should point to a valid
register.  If indirect mode is never used by the driver, it will point
to zero with this patch.  So register 0x78 will be valid (or zero) in
the kexec'ed kernel.


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] net-tg3: Initialize REG_BASE_ADDR at PCI config offset 120 to 0

2013-12-10 Thread Michael Chan
On Tue, 2013-12-10 at 07:01 -0800, Natarajan Gurumoorthy wrote:
  The only time I see crashes is after the tg3 driver has been
 loaded into the system. I our use case we are poking around
 /sys/devices/pci//config. 

David, please apply this first patch.

Acked-by: Michael Chan mc...@broadcom.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



Re: [PATCH] net-tg3: Initialize REG_BASE_ADDR at PCI config offset 120 to 0

2013-12-09 Thread Michael Chan
On Mon, 2013-12-09 at 13:07 -0800, Michael Chan wrote: 
> On Tue, 2013-12-10 at 00:18 +0300, Sergei Shtylyov wrote: 
> > >We had crashes when the PCI config space got scanned via
> > > /sys/devices/pci/../config.
> > 
> > > I agree that this fix will not help if the scan happens before the tg3
> > > driver gets loaded.
> > 
> > Then perhaps a better place for such fixup would be a PCI quirk?
> > 
> Yes, I agree.  Thanks.
> 

On second thought, I think your original patch should be sufficient and
we don't need to add the PCI quirk to cover so many devices.  The reason
is that indirect access needs to be explicitly enabled in the
MISC_HOST_CTRL (0x68) register.  The default value for register 0x68
should have indirect access disabled.

Nat, does this match what you're seeing?  Did you ever see any system
crash before tg3 loads?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] net-tg3: Initialize REG_BASE_ADDR at PCI config offset 120 to 0

2013-12-09 Thread Michael Chan
On Tue, 2013-12-10 at 00:18 +0300, Sergei Shtylyov wrote: 
> >We had crashes when the PCI config space got scanned via
> > /sys/devices/pci/../config.
> 
> > I agree that this fix will not help if the scan happens before the tg3
> > driver gets loaded.
> 
> Then perhaps a better place for such fixup would be a PCI quirk?
> 
Yes, I agree.  Thanks.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] net-tg3: Initialize REG_BASE_ADDR at PCI config offset 120 to 0

2013-12-09 Thread Michael Chan
On Mon, 2013-12-09 at 10:43 -0800, Nat Gurumoorthy wrote: 
> The new tg3 driver leaves REG_BASE_ADDR (PCI config offset 120)
> uninitialized. From power on reset this register may have garbage in it. The
> Register Base Address register defines the device local address of a
> register. The data pointed to by this location is read or written using
> the Register Data register (PCI config offset 128). When REG_BASE_ADDR has
> garbage any read or write of Register Data Register (PCI 128) will cause the
> PCI bus to lock up. The TCO watchdog will fire and bring down the system.
> 
> 
Is this to prevent problem from other software that may be scanning the
PCI config space?  It won't help if this happens before the tg3 driver
is loaded, right?

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] net-tg3: Initialize REG_BASE_ADDR at PCI config offset 120 to 0

2013-12-09 Thread Michael Chan
On Mon, 2013-12-09 at 10:43 -0800, Nat Gurumoorthy wrote: 
 The new tg3 driver leaves REG_BASE_ADDR (PCI config offset 120)
 uninitialized. From power on reset this register may have garbage in it. The
 Register Base Address register defines the device local address of a
 register. The data pointed to by this location is read or written using
 the Register Data register (PCI config offset 128). When REG_BASE_ADDR has
 garbage any read or write of Register Data Register (PCI 128) will cause the
 PCI bus to lock up. The TCO watchdog will fire and bring down the system.
 
 
Is this to prevent problem from other software that may be scanning the
PCI config space?  It won't help if this happens before the tg3 driver
is loaded, right?

Thanks.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] net-tg3: Initialize REG_BASE_ADDR at PCI config offset 120 to 0

2013-12-09 Thread Michael Chan
On Tue, 2013-12-10 at 00:18 +0300, Sergei Shtylyov wrote: 
 We had crashes when the PCI config space got scanned via
  /sys/devices/pci/../config.
 
  I agree that this fix will not help if the scan happens before the tg3
  driver gets loaded.
 
 Then perhaps a better place for such fixup would be a PCI quirk?
 
Yes, I agree.  Thanks.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] net-tg3: Initialize REG_BASE_ADDR at PCI config offset 120 to 0

2013-12-09 Thread Michael Chan
On Mon, 2013-12-09 at 13:07 -0800, Michael Chan wrote: 
 On Tue, 2013-12-10 at 00:18 +0300, Sergei Shtylyov wrote: 
  We had crashes when the PCI config space got scanned via
   /sys/devices/pci/../config.
  
   I agree that this fix will not help if the scan happens before the tg3
   driver gets loaded.
  
  Then perhaps a better place for such fixup would be a PCI quirk?
  
 Yes, I agree.  Thanks.
 

On second thought, I think your original patch should be sufficient and
we don't need to add the PCI quirk to cover so many devices.  The reason
is that indirect access needs to be explicitly enabled in the
MISC_HOST_CTRL (0x68) register.  The default value for register 0x68
should have indirect access disabled.

Nat, does this match what you're seeing?  Did you ever see any system
crash before tg3 loads?

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] PCI / tg3: Give up chip reset and carrier loss handling if PCI device is not present

2013-12-02 Thread Michael Chan
On Mon, 2013-12-02 at 13:48 -0500, David Miller wrote: 
> From: "Michael Chan" 
> Date: Sun, 1 Dec 2013 12:39:19 -0800
> 
> > I'm not familiar with the DS1.  Does the tg3 device get removed through
> > tg3_remove_one() in this case?  What happens when you reconnect the DS1?
> 
> Michael, is Rafael's explanation sufficient for you?
> 

Yes, thanks.

Acked-by: Michael Chan 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] PCI / tg3: Give up chip reset and carrier loss handling if PCI device is not present

2013-12-02 Thread Michael Chan
On Mon, 2013-12-02 at 13:48 -0500, David Miller wrote: 
 From: Michael Chan mc...@broadcom.com
 Date: Sun, 1 Dec 2013 12:39:19 -0800
 
  I'm not familiar with the DS1.  Does the tg3 device get removed through
  tg3_remove_one() in this case?  What happens when you reconnect the DS1?
 
 Michael, is Rafael's explanation sufficient for you?
 

Yes, thanks.

Acked-by: Michael Chan mc...@broadcom.com


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] PCI / tg3: Give up chip reset and carrier loss handling if PCI device is not present

2013-12-01 Thread Michael Chan
On Sun, 2013-12-01 at 02:34 +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki 
> 
> Modify tg3_chip_reset() and tg3_close() to check if the PCI network
> adapter device is accessible at all in order to skip poking it or
> trying to handle a carrier loss in vain when that's not the case.
> Introduce a special PCI helper function pci_device_is_present()
> for this purpose.
> 
> Of course, this uncovers the lack of the appropriate RTNL locking
> in tg3_suspend() and tg3_resume(), so add that locking in there
> too.
> 
> These changes prevent tg3 from burning a CPU at 100% load level for
> solid several seconds after the Thunderbolt link is disconnected from
> a Matrox DS1 docking station. 

I'm not familiar with the DS1.  Does the tg3 device get removed through
tg3_remove_one() in this case?  What happens when you reconnect the DS1?


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] PCI / tg3: Give up chip reset and carrier loss handling if PCI device is not present

2013-12-01 Thread Michael Chan
On Sun, 2013-12-01 at 02:34 +0100, Rafael J. Wysocki wrote:
 From: Rafael J. Wysocki rafael.j.wyso...@intel.com
 
 Modify tg3_chip_reset() and tg3_close() to check if the PCI network
 adapter device is accessible at all in order to skip poking it or
 trying to handle a carrier loss in vain when that's not the case.
 Introduce a special PCI helper function pci_device_is_present()
 for this purpose.
 
 Of course, this uncovers the lack of the appropriate RTNL locking
 in tg3_suspend() and tg3_resume(), so add that locking in there
 too.
 
 These changes prevent tg3 from burning a CPU at 100% load level for
 solid several seconds after the Thunderbolt link is disconnected from
 a Matrox DS1 docking station. 

I'm not familiar with the DS1.  Does the tg3 device get removed through
tg3_remove_one() in this case?  What happens when you reconnect the DS1?


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] tg3: remove redundant pm init code

2013-05-29 Thread Michael Chan
On Wed, 2013-05-29 at 17:00 +0800, Yijing Wang wrote: 
> Pci_enable_device() will set device pm state to D0, so
> it's no need to do it again in tg3_init_one().
> 
> Signed-off-by: Yijing Wang 

Acked-by: Michael Chan 

> ---
>  drivers/net/ethernet/broadcom/tg3.c |   20 +---
>  1 files changed, 1 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/tg3.c 
> b/drivers/net/ethernet/broadcom/tg3.c
> index 728d42a..1396ab3 100644
> --- a/drivers/net/ethernet/broadcom/tg3.c
> +++ b/drivers/net/ethernet/broadcom/tg3.c
> @@ -17042,25 +17042,10 @@ static int tg3_init_one(struct pci_dev *pdev,
>  
>   pci_set_master(pdev);
>  
> - /* Find power-management capability. */
> - pm_cap = pci_find_capability(pdev, PCI_CAP_ID_PM);
> - if (pm_cap == 0) {
> - dev_err(>dev,
> - "Cannot find Power Management capability, aborting\n");
> - err = -EIO;
> - goto err_out_free_res;
> - }
> -
> - err = pci_set_power_state(pdev, PCI_D0);
> - if (err) {
> - dev_err(>dev, "Transition to D0 failed, aborting\n");
> - goto err_out_free_res;
> - }
> -
>   dev = alloc_etherdev_mq(sizeof(*tp), TG3_IRQ_MAX_VECS);
>   if (!dev) {
>   err = -ENOMEM;
> - goto err_out_power_down;
> + goto err_out_free_res;
>   }
>  
>   SET_NETDEV_DEV(dev, >dev);
> @@ -17406,9 +17391,6 @@ err_out_iounmap:
>  err_out_free_dev:
>   free_netdev(dev);
>  
> -err_out_power_down:
> - pci_set_power_state(pdev, PCI_D3hot);
> -
>  err_out_free_res:
>   pci_release_regions(pdev);
>  



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] tg3: remove redundant pm init code

2013-05-29 Thread Michael Chan
On Wed, 2013-05-29 at 17:00 +0800, Yijing Wang wrote: 
 Pci_enable_device() will set device pm state to D0, so
 it's no need to do it again in tg3_init_one().
 
 Signed-off-by: Yijing Wang wangyij...@huawei.com

Acked-by: Michael Chan mc...@broadcom.com

 ---
  drivers/net/ethernet/broadcom/tg3.c |   20 +---
  1 files changed, 1 insertions(+), 19 deletions(-)
 
 diff --git a/drivers/net/ethernet/broadcom/tg3.c 
 b/drivers/net/ethernet/broadcom/tg3.c
 index 728d42a..1396ab3 100644
 --- a/drivers/net/ethernet/broadcom/tg3.c
 +++ b/drivers/net/ethernet/broadcom/tg3.c
 @@ -17042,25 +17042,10 @@ static int tg3_init_one(struct pci_dev *pdev,
  
   pci_set_master(pdev);
  
 - /* Find power-management capability. */
 - pm_cap = pci_find_capability(pdev, PCI_CAP_ID_PM);
 - if (pm_cap == 0) {
 - dev_err(pdev-dev,
 - Cannot find Power Management capability, aborting\n);
 - err = -EIO;
 - goto err_out_free_res;
 - }
 -
 - err = pci_set_power_state(pdev, PCI_D0);
 - if (err) {
 - dev_err(pdev-dev, Transition to D0 failed, aborting\n);
 - goto err_out_free_res;
 - }
 -
   dev = alloc_etherdev_mq(sizeof(*tp), TG3_IRQ_MAX_VECS);
   if (!dev) {
   err = -ENOMEM;
 - goto err_out_power_down;
 + goto err_out_free_res;
   }
  
   SET_NETDEV_DEV(dev, pdev-dev);
 @@ -17406,9 +17391,6 @@ err_out_iounmap:
  err_out_free_dev:
   free_netdev(dev);
  
 -err_out_power_down:
 - pci_set_power_state(pdev, PCI_D3hot);
 -
  err_out_free_res:
   pci_release_regions(pdev);
  



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] tg3: Use different macros for pci_chip_rev_id accesses

2013-02-16 Thread Michael Chan
On Sat, 2013-02-16 at 13:20 -0800, Joe Perches wrote:
> Upper case macros for various chip attributes are slightly
> difficult to read and are a bit out of characterto the other
> tg3_ attribute functions.
> 
> Convert:
> 
> GET_ASIC_REV(tp->pci_chip_rev_id)   -> tg3_asic_rev(tp)
> GET_CHIP_REV(tp->pci_chip_rev_id)   -> tg3_chip_rev(tp)
> 
> Remove:
> GET_METAL_REV(tp->pci_chip_rev_id)  -> tg3_metal_rev(tp) (unused)
> 
> Add:
> tg3_chip_rev_id(tp) for tp->pci_chip_rev_id so access styles
> are similar to tg3_asic_rev and tg3_chip_rev.
> 
> These macros are not converted to static inline functions
> because gcc (tested with 4.7.2) is currently unable to
> optimize the object code it produces the same way and code
> is otherwise larger.
> 
> Signed-off-by: Joe Perches  

Acked-by: Michael Chan 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] tg3: Remove define and single use of GET_CHIP_REV_ID

2013-02-16 Thread Michael Chan
On Sat, 2013-02-16 at 13:20 -0800, Joe Perches wrote:
> It's the same value as tp->pci_chip_rev_id so use that
> instead.  This makes all CHIPREV_ID_ tests the same.
> 
> Signed-off-by: Joe Perches  

Acked-by: Michael Chan 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] tg3: Remove define and single use of GET_CHIP_REV_ID

2013-02-16 Thread Michael Chan
On Sat, 2013-02-16 at 13:20 -0800, Joe Perches wrote:
 It's the same value as tp-pci_chip_rev_id so use that
 instead.  This makes all CHIPREV_ID_foo tests the same.
 
 Signed-off-by: Joe Perches j...@perches.com 

Acked-by: Michael Chan mc...@broadcom.com


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] tg3: Use different macros for pci_chip_rev_id accesses

2013-02-16 Thread Michael Chan
On Sat, 2013-02-16 at 13:20 -0800, Joe Perches wrote:
 Upper case macros for various chip attributes are slightly
 difficult to read and are a bit out of characterto the other
 tg3_foo attribute functions.
 
 Convert:
 
 GET_ASIC_REV(tp-pci_chip_rev_id)   - tg3_asic_rev(tp)
 GET_CHIP_REV(tp-pci_chip_rev_id)   - tg3_chip_rev(tp)
 
 Remove:
 GET_METAL_REV(tp-pci_chip_rev_id)  - tg3_metal_rev(tp) (unused)
 
 Add:
 tg3_chip_rev_id(tp) for tp-pci_chip_rev_id so access styles
 are similar to tg3_asic_rev and tg3_chip_rev.
 
 These macros are not converted to static inline functions
 because gcc (tested with 4.7.2) is currently unable to
 optimize the object code it produces the same way and code
 is otherwise larger.
 
 Signed-off-by: Joe Perches j...@perches.com 

Acked-by: Michael Chan mc...@broadcom.com


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] serial_core: Fix type definition for PORT_BRCM_TRUMANAGE.

2013-01-29 Thread Michael Chan
It was mistakenly defined to be 24 instead of the next higher number 25.

Reported-by: Alexander Shishkin 
Cc: Stephen Hurd 
Signed-off-by: Michael Chan 
---
 include/uapi/linux/serial_core.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index 08464ef..b6a23a4 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -50,7 +50,7 @@
 #define PORT_LPC3220   22  /* NXP LPC32xx SoC "Standard" UART */
 #define PORT_8250_CIR  23  /* CIR infrared port, has its own driver */
 #define PORT_XR17V35X  24  /* Exar XR17V35x UARTs */
-#define PORT_BRCM_TRUMANAGE24
+#define PORT_BRCM_TRUMANAGE25
 #define PORT_MAX_8250  25  /* max port ID */
 
 /*
-- 
1.7.1


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] serial_core: Fix type definition for PORT_BRCM_TRUMANAGE.

2013-01-29 Thread Michael Chan
It was mistakenly defined to be 24 instead of the next higher number 25.

Reported-by: Alexander Shishkin alexander.shish...@linux.intel.com
Cc: Stephen Hurd sh...@broadcom.com
Signed-off-by: Michael Chan mc...@broadcom.com
---
 include/uapi/linux/serial_core.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index 08464ef..b6a23a4 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -50,7 +50,7 @@
 #define PORT_LPC3220   22  /* NXP LPC32xx SoC Standard UART */
 #define PORT_8250_CIR  23  /* CIR infrared port, has its own driver */
 #define PORT_XR17V35X  24  /* Exar XR17V35x UARTs */
-#define PORT_BRCM_TRUMANAGE24
+#define PORT_BRCM_TRUMANAGE25
 #define PORT_MAX_8250  25  /* max port ID */
 
 /*
-- 
1.7.1


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] 8250/16?50: Add support for Broadcom TruManage redirected serial port

2013-01-17 Thread Michael Chan
From: Stephen Hurd 

Add support for the UART device present in Broadcom TruManage capable
NetXtreme chips (ie: 5761m 5762, and 5725).

This implementation has a hidden transmit FIFO, so running in single-byte
interrupt mode results in too many interrupts.  The UART_CAP_HFIFO
capability was added to track this.  It continues to reload the THR as long
as the THRE and TSRE bits are set in the LSR up to a specified limit (1024
is used here).

Signed-off-by: Stephen Hurd 
Signed-off-by: Michael Chan 
---
 drivers/tty/serial/8250/8250.c |   11 ++
 drivers/tty/serial/8250/8250.h |1 +
 drivers/tty/serial/8250/8250_pci.c |   38 
 include/uapi/linux/serial_core.h   |3 +-
 4 files changed, 52 insertions(+), 1 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
index d085e3a..f932043 100644
--- a/drivers/tty/serial/8250/8250.c
+++ b/drivers/tty/serial/8250/8250.c
@@ -300,6 +300,12 @@ static const struct serial8250_config uart_config[] = {
  UART_FCR_R_TRIG_00 | UART_FCR_T_TRIG_00,
.flags  = UART_CAP_FIFO,
},
+   [PORT_BRCM_TRUMANAGE] = {
+   .name   = "TruManage",
+   .fifo_size  = 1,
+   .tx_loadsz  = 1024,
+   .flags  = UART_CAP_HFIFO,
+   },
[PORT_8250_CIR] = {
.name   = "CIR port"
}
@@ -1490,6 +1496,11 @@ void serial8250_tx_chars(struct uart_8250_port *up)
port->icount.tx++;
if (uart_circ_empty(xmit))
break;
+   if (up->capabilities & UART_CAP_HFIFO) {
+   if ((serial_port_in(port, UART_LSR) & BOTH_EMPTY) !=
+   BOTH_EMPTY)
+   break;
+   }
} while (--count > 0);
 
if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index 3b4ea84..12caa12 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -40,6 +40,7 @@ struct serial8250_config {
 #define UART_CAP_AFE   (1 << 11)   /* MCR-based hw flow control */
 #define UART_CAP_UUE   (1 << 12)   /* UART needs IER bit 6 set (Xscale) */
 #define UART_CAP_RTOIE (1 << 13)   /* UART needs IER bit 4 set (Xscale, 
Tegra) */
+#define UART_CAP_HFIFO (1 << 14)   /* UART has a "hidden" FIFO */
 
 #define UART_BUG_QUOT  (1 << 0)/* UART has buggy quot LSB */
 #define UART_BUG_TXEN  (1 << 1)/* UART has buggy TX IIR status */
diff --git a/drivers/tty/serial/8250/8250_pci.c 
b/drivers/tty/serial/8250/8250_pci.c
index 26b9dc0..c6777b5 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1085,6 +1085,18 @@ pci_omegapci_setup(struct serial_private *priv,
return setup_port(priv, port, 2, idx * 8, 0);
 }
 
+static int
+pci_brcm_trumanage_setup(struct serial_private *priv,
+const struct pciserial_board *board,
+struct uart_8250_port *port, int idx)
+{
+   int ret = pci_default_setup(priv, board, port, idx);
+
+   port->port.type = PORT_BRCM_TRUMANAGE;
+   port->port.flags = (port->port.flags | UPF_FIXED_PORT | UPF_FIXED_TYPE);
+   return ret;
+}
+
 static int skip_tx_en_setup(struct serial_private *priv,
const struct pciserial_board *board,
struct uart_8250_port *port, int idx)
@@ -1304,6 +1316,7 @@ pci_wch_ch353_setup(struct serial_private *priv,
 #define PCI_DEVICE_ID_COMMTECH_4222PCIE 0x0019
 #define PCI_DEVICE_ID_COMMTECH_4224PCIE0x0020
 #define PCI_DEVICE_ID_COMMTECH_4228PCIE0x0021
+#define PCI_DEVICE_ID_BROADCOM_TRUMANAGE 0x160a
 
 
 /* Unknown vendors/cards - this should not be in linux/pci_ids.h */
@@ -1954,6 +1967,17 @@ static struct pci_serial_quirk pci_serial_quirks[] 
__refdata = {
.setup  = pci_xr17v35x_setup,
},
/*
+* Broadcom TruManage (NetXtreme)
+*/
+   {
+   .vendor = PCI_VENDOR_ID_BROADCOM,
+   .device = PCI_DEVICE_ID_BROADCOM_TRUMANAGE,
+   .subvendor  = PCI_ANY_ID,
+   .subdevice  = PCI_ANY_ID,
+   .setup  = pci_brcm_trumanage_setup,
+   },
+
+   /*
 * Default "match everything" terminator entry
 */
{
@@ -2148,6 +2172,7 @@ enum pci_board_num_t {
pbn_ce4100_1_115200,
pbn_omegapci,
pbn_NETMOS9900_2s_115200,
+   pbn_brcm_trumanage,
 };
 
 /*
@@ -2892,6 +2917,12 @@ static struct pciserial_board pci_boards[] = {
.num_ports  = 2,
.base_baud  = 115200,
},
+  

Re: tg3 v3.123 in 100Mbps Full-Duplex mode with autoneg off

2013-01-17 Thread Michael Chan
On Thu, 2013-01-17 at 17:59 +0100, Marcin Miotk wrote:
> root@XX ~ # cat miitool_non_working_3.4.11
> Using SIOCGMIIPHY=0x8947
> eth1: 100 Mbit, full duplex, no link
>   registers for MII PHY 1:
> 2100 7949 0020 6340 0041  0004 2001
>        3000
> 0001 2000      4000
> 7277 1000   2801   
>   product info: vendor 00:08:18, model 52 rev 0
>   basic mode:   100 Mbit, full duplex
>   basic status: no link
>   capabilities: 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
>   advertising:  10baseT-FD

Autoneg is disabled, so the advertising register that is advertising
only 10Mbps Full Duplex should not matter.  What matters is that
register 0 has set it to 100Mbps Full Duplex.
> 
> 
> What looks suspicious here is the last line of mii-tool output:
> 'advertising: 10baseT-FD', while only 100baseTx-FD is set on a switch.
> 'mii-tool -vvv eth1' on working 3.0.9 gives output like below:
> 
> root@XXX ~ # cat miitool_working_3.0.9
> Using SIOCGMIIPHY=0x8947
> eth1: 100 Mbit, full duplex, link ok
>   registers for MII PHY 1:
> 2100 794d 0020 6340 0501  0004 2001
>        3000
> 0001 0301      4000
> 7277 0504   2801   
>   product info: vendor 00:08:18, model 52 rev 0
>   basic mode:   100 Mbit, full duplex
>   basic status: link ok
>   capabilities: 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
>   advertising:  100baseTx-FD flow-control
> 
In the working case, register 0 where it matters is the same (autoneg
disabled and 100Mbps Full Duplex).  Register 4 happens to advertise the
same 100 Mbps Full duplex forced speed, and apparently your link partner
cares about that, which is very strange because autoneg is disabled.

There apparently has been a change in behavior in the tg3 code, but
again it should not matter.  The 3.7 kernel that I tested on behaved the
same as your non-working case but it worked for me.  I guess I can try
to restore the original behavior.  I'll need time to look through all
the changes.




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: tg3 v3.123 in 100Mbps Full-Duplex mode with autoneg off

2013-01-17 Thread Michael Chan
On Thu, 2013-01-17 at 17:59 +0100, Marcin Miotk wrote:
 root@XX ~ # cat miitool_non_working_3.4.11
 Using SIOCGMIIPHY=0x8947
 eth1: 100 Mbit, full duplex, no link
   registers for MII PHY 1:
 2100 7949 0020 6340 0041  0004 2001
        3000
 0001 2000      4000
 7277 1000   2801   
   product info: vendor 00:08:18, model 52 rev 0
   basic mode:   100 Mbit, full duplex
   basic status: no link
   capabilities: 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
   advertising:  10baseT-FD

Autoneg is disabled, so the advertising register that is advertising
only 10Mbps Full Duplex should not matter.  What matters is that
register 0 has set it to 100Mbps Full Duplex.
 
 
 What looks suspicious here is the last line of mii-tool output:
 'advertising: 10baseT-FD', while only 100baseTx-FD is set on a switch.
 'mii-tool -vvv eth1' on working 3.0.9 gives output like below:
 
 root@XXX ~ # cat miitool_working_3.0.9
 Using SIOCGMIIPHY=0x8947
 eth1: 100 Mbit, full duplex, link ok
   registers for MII PHY 1:
 2100 794d 0020 6340 0501  0004 2001
        3000
 0001 0301      4000
 7277 0504   2801   
   product info: vendor 00:08:18, model 52 rev 0
   basic mode:   100 Mbit, full duplex
   basic status: link ok
   capabilities: 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
   advertising:  100baseTx-FD flow-control
 
In the working case, register 0 where it matters is the same (autoneg
disabled and 100Mbps Full Duplex).  Register 4 happens to advertise the
same 100 Mbps Full duplex forced speed, and apparently your link partner
cares about that, which is very strange because autoneg is disabled.

There apparently has been a change in behavior in the tg3 code, but
again it should not matter.  The 3.7 kernel that I tested on behaved the
same as your non-working case but it worked for me.  I guess I can try
to restore the original behavior.  I'll need time to look through all
the changes.




--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] 8250/16?50: Add support for Broadcom TruManage redirected serial port

2013-01-17 Thread Michael Chan
From: Stephen Hurd sh...@broadcom.com

Add support for the UART device present in Broadcom TruManage capable
NetXtreme chips (ie: 5761m 5762, and 5725).

This implementation has a hidden transmit FIFO, so running in single-byte
interrupt mode results in too many interrupts.  The UART_CAP_HFIFO
capability was added to track this.  It continues to reload the THR as long
as the THRE and TSRE bits are set in the LSR up to a specified limit (1024
is used here).

Signed-off-by: Stephen Hurd sh...@broadcom.com
Signed-off-by: Michael Chan mc...@broadcom.com
---
 drivers/tty/serial/8250/8250.c |   11 ++
 drivers/tty/serial/8250/8250.h |1 +
 drivers/tty/serial/8250/8250_pci.c |   38 
 include/uapi/linux/serial_core.h   |3 +-
 4 files changed, 52 insertions(+), 1 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
index d085e3a..f932043 100644
--- a/drivers/tty/serial/8250/8250.c
+++ b/drivers/tty/serial/8250/8250.c
@@ -300,6 +300,12 @@ static const struct serial8250_config uart_config[] = {
  UART_FCR_R_TRIG_00 | UART_FCR_T_TRIG_00,
.flags  = UART_CAP_FIFO,
},
+   [PORT_BRCM_TRUMANAGE] = {
+   .name   = TruManage,
+   .fifo_size  = 1,
+   .tx_loadsz  = 1024,
+   .flags  = UART_CAP_HFIFO,
+   },
[PORT_8250_CIR] = {
.name   = CIR port
}
@@ -1490,6 +1496,11 @@ void serial8250_tx_chars(struct uart_8250_port *up)
port-icount.tx++;
if (uart_circ_empty(xmit))
break;
+   if (up-capabilities  UART_CAP_HFIFO) {
+   if ((serial_port_in(port, UART_LSR)  BOTH_EMPTY) !=
+   BOTH_EMPTY)
+   break;
+   }
} while (--count  0);
 
if (uart_circ_chars_pending(xmit)  WAKEUP_CHARS)
diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index 3b4ea84..12caa12 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -40,6 +40,7 @@ struct serial8250_config {
 #define UART_CAP_AFE   (1  11)   /* MCR-based hw flow control */
 #define UART_CAP_UUE   (1  12)   /* UART needs IER bit 6 set (Xscale) */
 #define UART_CAP_RTOIE (1  13)   /* UART needs IER bit 4 set (Xscale, 
Tegra) */
+#define UART_CAP_HFIFO (1  14)   /* UART has a hidden FIFO */
 
 #define UART_BUG_QUOT  (1  0)/* UART has buggy quot LSB */
 #define UART_BUG_TXEN  (1  1)/* UART has buggy TX IIR status */
diff --git a/drivers/tty/serial/8250/8250_pci.c 
b/drivers/tty/serial/8250/8250_pci.c
index 26b9dc0..c6777b5 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1085,6 +1085,18 @@ pci_omegapci_setup(struct serial_private *priv,
return setup_port(priv, port, 2, idx * 8, 0);
 }
 
+static int
+pci_brcm_trumanage_setup(struct serial_private *priv,
+const struct pciserial_board *board,
+struct uart_8250_port *port, int idx)
+{
+   int ret = pci_default_setup(priv, board, port, idx);
+
+   port-port.type = PORT_BRCM_TRUMANAGE;
+   port-port.flags = (port-port.flags | UPF_FIXED_PORT | UPF_FIXED_TYPE);
+   return ret;
+}
+
 static int skip_tx_en_setup(struct serial_private *priv,
const struct pciserial_board *board,
struct uart_8250_port *port, int idx)
@@ -1304,6 +1316,7 @@ pci_wch_ch353_setup(struct serial_private *priv,
 #define PCI_DEVICE_ID_COMMTECH_4222PCIE 0x0019
 #define PCI_DEVICE_ID_COMMTECH_4224PCIE0x0020
 #define PCI_DEVICE_ID_COMMTECH_4228PCIE0x0021
+#define PCI_DEVICE_ID_BROADCOM_TRUMANAGE 0x160a
 
 
 /* Unknown vendors/cards - this should not be in linux/pci_ids.h */
@@ -1954,6 +1967,17 @@ static struct pci_serial_quirk pci_serial_quirks[] 
__refdata = {
.setup  = pci_xr17v35x_setup,
},
/*
+* Broadcom TruManage (NetXtreme)
+*/
+   {
+   .vendor = PCI_VENDOR_ID_BROADCOM,
+   .device = PCI_DEVICE_ID_BROADCOM_TRUMANAGE,
+   .subvendor  = PCI_ANY_ID,
+   .subdevice  = PCI_ANY_ID,
+   .setup  = pci_brcm_trumanage_setup,
+   },
+
+   /*
 * Default match everything terminator entry
 */
{
@@ -2148,6 +2172,7 @@ enum pci_board_num_t {
pbn_ce4100_1_115200,
pbn_omegapci,
pbn_NETMOS9900_2s_115200,
+   pbn_brcm_trumanage,
 };
 
 /*
@@ -2892,6 +2917,12 @@ static struct pciserial_board pci_boards[] = {
.num_ports  = 2,
.base_baud  = 115200,
},
+   [pbn_brcm_trumanage] = {
+   .flags  = FL_BASE0

Re: tg3 v3.123 in 100Mbps Full-Duplex mode with autoneg off

2013-01-14 Thread Michael Chan
On Wed, 2013-01-09 at 08:03 +0100, Marcin Miotk wrote:
> 03:04.0 Ethernet controller: Broadcom Corporation NetXtreme BCM5715
> Gigabit Ethernet (rev a3)
> 03:04.1 Ethernet controller: Broadcom Corporation NetXtreme BCM5715
> Gigabit Ethernet (rev a3) 

I tested on 3.7 kernel and it seemed to work.  Bear in mind that you'll
lose auto MDI-X when both sides don't autoneg.  Please provide the
following:

1. ethtool eth0 with a working kernel.  I'd like to see the MDI-X
setting when it is working.

2. mii-tool -vvv eth0 with the non-working kernel.

Thanks.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: tg3 v3.123 in 100Mbps Full-Duplex mode with autoneg off

2013-01-14 Thread Michael Chan
On Wed, 2013-01-09 at 08:03 +0100, Marcin Miotk wrote:
 03:04.0 Ethernet controller: Broadcom Corporation NetXtreme BCM5715
 Gigabit Ethernet (rev a3)
 03:04.1 Ethernet controller: Broadcom Corporation NetXtreme BCM5715
 Gigabit Ethernet (rev a3) 

I tested on 3.7 kernel and it seemed to work.  Bear in mind that you'll
lose auto MDI-X when both sides don't autoneg.  Please provide the
following:

1. ethtool eth0 with a working kernel.  I'd like to see the MDI-X
setting when it is working.

2. mii-tool -vvv eth0 with the non-working kernel.

Thanks.


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: tg3 v3.123 in 100Mbps Full-Duplex mode with autoneg off

2013-01-08 Thread Michael Chan
Please tell us what tg3 device you're using.  You can provide lspci
output or tg3 probing dmesg output.  Thanks.

On Wed, 2013-01-09 at 11:20 +0800, 王金浦 wrote: 
> For this kind of driver related question, I suggest you send mail to
> tg3 driver developers, who I already cced.
> 
> 
> I think they should know what's going on, right?
> 
> 
> Jack
> 
> 
> 2013/1/8 Marcin Miotk 
> Hi,
> 
> any conclusions re this? 
> 
> Regards,
> Marcin Miotk
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at
>  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> 
> 



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: tg3 v3.123 in 100Mbps Full-Duplex mode with autoneg off

2013-01-08 Thread Michael Chan
Please tell us what tg3 device you're using.  You can provide lspci
output or tg3 probing dmesg output.  Thanks.

On Wed, 2013-01-09 at 11:20 +0800, 王金浦 wrote: 
 For this kind of driver related question, I suggest you send mail to
 tg3 driver developers, who I already cced.
 
 
 I think they should know what's going on, right?
 
 
 Jack
 
 
 2013/1/8 Marcin Miotk marcinmiot...@gmail.com
 Hi,
 
 any conclusions re this? 
 
 Regards,
 Marcin Miotk
 --
 To unsubscribe from this list: send the line unsubscribe
 linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at
  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/
 
 
 



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: tg3 driver upgrade (Linux 2.6.32 -> 3.2) breaks IBM Bladecenter SoL

2012-10-02 Thread Michael Chan
On Tue, 2012-10-02 at 18:49 +0200, Ferenc Wagner wrote:
> Going into the opposite direction: I found that Linux 3.6 does not
> permanently break the SoL console on upping eth0!  I'll try to find
> the
> commit which (sort of) fixed it.

These are the likely fixes:
> 
commit cf9ecf4b631f649a964fa611f1a5e8874f2a76db 
Author: Matt Carlson 
Date: Mon Nov 28 09:41:03 2011 +

tg3: Fix TSO CAP for 5704 devs w / ASF enabled

commit 7196cd6c3d4863000ef88b09f34d6dd75610ec3e
Author: Matt Carlson 
Date: Thu May 19 16:02:44 2011 +

tg3: Add braces around 5906 workaround.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: tg3 driver upgrade (Linux 2.6.32 -> 3.2) breaks IBM Bladecenter SoL

2012-10-02 Thread Michael Chan
On Tue, 2012-10-02 at 14:07 +0200, Ferenc Wagner wrote:
> I'm done with bisecting it: the first bad commit is:
> 
> commit dabc5c670d3f86d15ee4f42ab38ec5bd2682487d
> Author: Matt Carlson 
> Date:   Thu May 19 12:12:52 2011 +
> 
> tg3: Move TSO_CAPABLE assignment
> 
> This patch moves the code that asserts the TSO_CAPABLE flag closer
> to
> where the TSO capabilities flags are set.  There isn't a good
> enough
> reason for the code to be separated.
> 
> Signed-off-by: Matt Carlson 
> Reviewed-by: Michael Chan 
> Signed-off-by: David S. Miller 

Thanks, I'll look into this.
> 
> On the other hand, losing the SoL console even temporarily during boot
> (as it happens with a minimal kernel before this commit) isn't nice
> either.  I'll try to look after that, too, just mentioning it here... 

This is expected as the driver has to reset the link and you'll lose SoL
for a few seconds until link comes back up.  We can look into an
enhancement to not touch the link if it is already in a good state when
the driver comes up.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: tg3 driver upgrade (Linux 2.6.32 - 3.2) breaks IBM Bladecenter SoL

2012-10-02 Thread Michael Chan
On Tue, 2012-10-02 at 14:07 +0200, Ferenc Wagner wrote:
 I'm done with bisecting it: the first bad commit is:
 
 commit dabc5c670d3f86d15ee4f42ab38ec5bd2682487d
 Author: Matt Carlson mcarl...@broadcom.com
 Date:   Thu May 19 12:12:52 2011 +
 
 tg3: Move TSO_CAPABLE assignment
 
 This patch moves the code that asserts the TSO_CAPABLE flag closer
 to
 where the TSO capabilities flags are set.  There isn't a good
 enough
 reason for the code to be separated.
 
 Signed-off-by: Matt Carlson mcarl...@broadcom.com
 Reviewed-by: Michael Chan mc...@broadcom.com
 Signed-off-by: David S. Miller da...@davemloft.net

Thanks, I'll look into this.
 
 On the other hand, losing the SoL console even temporarily during boot
 (as it happens with a minimal kernel before this commit) isn't nice
 either.  I'll try to look after that, too, just mentioning it here... 

This is expected as the driver has to reset the link and you'll lose SoL
for a few seconds until link comes back up.  We can look into an
enhancement to not touch the link if it is already in a good state when
the driver comes up.


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: tg3 driver upgrade (Linux 2.6.32 - 3.2) breaks IBM Bladecenter SoL

2012-10-02 Thread Michael Chan
On Tue, 2012-10-02 at 18:49 +0200, Ferenc Wagner wrote:
 Going into the opposite direction: I found that Linux 3.6 does not
 permanently break the SoL console on upping eth0!  I'll try to find
 the
 commit which (sort of) fixed it.

These are the likely fixes:
 
commit cf9ecf4b631f649a964fa611f1a5e8874f2a76db 
Author: Matt Carlson mcarl...@broadcom.com
Date: Mon Nov 28 09:41:03 2011 +

tg3: Fix TSO CAP for 5704 devs w / ASF enabled

commit 7196cd6c3d4863000ef88b09f34d6dd75610ec3e
Author: Matt Carlson mcarl...@broadcom.com
Date: Thu May 19 16:02:44 2011 +

tg3: Add braces around 5906 workaround.



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: tg3 driver upgrade (Linux 2.6.32 -> 3.2) breaks IBM Bladecenter SoL

2012-10-01 Thread Michael Chan
On Fri, 2012-09-28 at 22:45 +0200, Ferenc Wagner wrote: 
> Hi,
> 
> Upgrading the kernel on our HS20 blades resulted in their SoL (serial
> over LAN) connection being broken.  The disconnection happens when eth0
> (the interface involved in SoL) is brought up during the boot sequence.
> If I later "ip link set eth0 down", then the connection is restored, but
> "ip link set eth0 up" breaks it again on 3.2.  ethtool -a, -c, -g, -k
> and -u show no difference; ethtool -i on the 2.6.32 kernel reports:
> 
> driver: tg3
> version: 3.116
> firmware-version: 5704s-v3.38, ASFIPMIs v2.47
> bus-info: :05:01.0
> 
> In the 3.2 kernel the driver version is 3.121.

2.6.32 to 3.2 is a big jump.  Can you narrow this down further?  It will
be hard for us to find a HS20 with 5704 to test this.  Thanks.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: tg3 driver upgrade (Linux 2.6.32 - 3.2) breaks IBM Bladecenter SoL

2012-10-01 Thread Michael Chan
On Fri, 2012-09-28 at 22:45 +0200, Ferenc Wagner wrote: 
 Hi,
 
 Upgrading the kernel on our HS20 blades resulted in their SoL (serial
 over LAN) connection being broken.  The disconnection happens when eth0
 (the interface involved in SoL) is brought up during the boot sequence.
 If I later ip link set eth0 down, then the connection is restored, but
 ip link set eth0 up breaks it again on 3.2.  ethtool -a, -c, -g, -k
 and -u show no difference; ethtool -i on the 2.6.32 kernel reports:
 
 driver: tg3
 version: 3.116
 firmware-version: 5704s-v3.38, ASFIPMIs v2.47
 bus-info: :05:01.0
 
 In the 3.2 kernel the driver version is 3.121.

2.6.32 to 3.2 is a big jump.  Can you narrow this down further?  It will
be hard for us to find a HS20 with 5704 to test this.  Thanks.


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] bnx2: turn off the network statck during initialization

2012-08-16 Thread Michael Chan
On Thu, 2012-08-16 at 21:48 +, Jiang Wang wrote:
>  I see. I was confused by the name of bnx2_set_power_state()  and
> I think the PHY is actually powered up by bnx2_reset_nic, right?

bnx2_init_phy() in bnx2_init_nic() will bring up the PHY if it is down.

> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] bnx2: turn off the network statck during initialization

2012-08-16 Thread Michael Chan
On Thu, 2012-08-16 at 20:28 +, Jiang Wang wrote:
> Also, I have another comment related to link state.
> 
> Right now, the bnx2 driver powers up the device in bnx2_init_board(),
> regardless the netif_carrier is on or off.

We actually don't power up the device.  bnx2_init_board() just probes
the device.  If link is already up, it will stay up.  If it is down, it
will stay down.

> This may introduce following inconsistent behaviors:
> 1) suppose the cable is plugged in to the NIC and the other end is
> connected to a switch
> 2) user powers up the box

The link may already be up before or when you power up the box because
of management firmware (iLO, etc) or WoL.

> 3) the Linux does not bring up the interface; i.e, ifconfig ethx shows
> it is down
> 4) ethtool ethx will show no link
> 5) if the user goes to check the light on the physical NIC, he will
> see the green link light is ON. That means the link is up, right? 
> 
> I think it is better to power down the device until bnx2_open is
> called. In this way, ethtool report and the physical link light will
> be consistent. 

We cannot power it down.  If link is up, it is up for a reason (e.g. it
is an iLO port, etc).

Thanks.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] bnx2: turn off the network statck during initialization

2012-08-16 Thread Michael Chan
On Thu, 2012-08-16 at 19:15 +, Jiang Wang wrote: 
> Hi Michael,
> 
> I just checked the code for netif_carrier_off(), and it will return if
> the dev is not initialized (registered) as follows:
> 
> void netif_carrier_off(struct net_device *dev)
> {
> if (!test_and_set_bit(__LINK_STATE_NOCARRIER, >state)) {
> if (dev->reg_state == NETREG_UNINITIALIZED)
> return;
> linkwatch_fire_event(dev);
> }
> }
> 
> So linkwatch_fire_event should not fire in this case, right? Thanks
> for the quick response.
> 
Yes, you're right.  Your patch should not cause any problem then.

But I still don't understand what problem you're trying to solve.  The
link status from ethtool is determined by bnx2_get_link() and it should
always return link down before bnx2_open() is called.  Can you please
elborate?  Thanks.

> Regards,
> 
> Jiang
> 
> -
> Jiang Wang
> Member of Technical Staff
> Riverbed Technology
> Tel: (408) 522-5109
> Email: jiang.w...@riverbed.com
> www.riverbed.com
> 
> 
> -Original Message-
> From: Michael Chan [mailto:mc...@broadcom.com] 
> Sent: Thursday, August 16, 2012 11:57 AM
> To: Jiang Wang
> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; Chaitanya Lala; 
> Francis St. Amant; Jiang Wang
> Subject: Re: [PATCH] bnx2: turn off the network statck during initialization
> 
> On Thu, 2012-08-16 at 11:21 -0700, Jiang Wang wrote: 
> > The initialization state of bnx2 driver is wrong. It does not turn of 
> > the Linux network stack using netif_carrier_off. This may lead to 
> > inconsistent report from ethtool as the link is up but speed is 
> > unknown when the cable is not plugged in.
> > 
> > E.g.
> > Speed: Unknown! (0)<--
> > Duplex: Half   <--
> > MDI: Unknown! (0)
> > Port: Twisted Pair
> > PHYAD: 1
> > Transceiver: internal
> > Auto-negotiation: on
> > Supports Wake-on: g
> > Wake-on: d
> > Link detected: yes <---
> > 
> > This patches fixed the problem by turning off the network stack during 
> > initialization.
> > 
> > Signed-off-by: Jiang Wang 
> > ---
> >  drivers/net/ethernet/broadcom/bnx2.c |4 
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/broadcom/bnx2.c 
> > b/drivers/net/ethernet/broadcom/bnx2.c
> > index ac7b744..ce4548d 100644
> > --- a/drivers/net/ethernet/broadcom/bnx2.c
> > +++ b/drivers/net/ethernet/broadcom/bnx2.c
> > @@ -8463,6 +8463,10 @@ bnx2_init_one(struct pci_dev *pdev, const struct 
> > pci_device_id *ent)
> > dev->features |= dev->hw_features;
> > dev->priv_flags |= IFF_UNICAST_FLT;
> >  
> > +   /* tell the stack to leave us alone until bnx2_open() is called */
> > +   netif_carrier_off(dev);
> 
> We have tried this before and this didn't work.  netif_carrier_off() calls 
> linkwatch_fire_event() to schedule the event.  If
> register_netdev() fails for whatever reason, the netdev will be freed but the 
> link_watch event may still be scheduled.
> 
> Calling netif_carrier_off() after register_netdev() returns successfully also 
> may not work as it will race with bnx2_open() which enables IRQ.
> An linkup IRQ can happen at time and we may end up with the wrong carrier 
> state because of the race condition.
> 
> > +   netif_stop_queue(dev);
> > +
> > if ((rc = register_netdev(dev))) {
> > dev_err(>dev, "Cannot register net device\n");
> > goto error;
> 
> 
> 
> 



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] bnx2: turn off the network statck during initialization

2012-08-16 Thread Michael Chan
On Thu, 2012-08-16 at 11:21 -0700, Jiang Wang wrote: 
> The initialization state of bnx2 driver is wrong. It does not turn
> of the Linux network stack using netif_carrier_off. This may lead to
> inconsistent report from ethtool as the link is up but speed is
> unknown when the cable is not plugged in.
> 
> E.g.
> Speed: Unknown! (0)<--
> Duplex: Half   <--
> MDI: Unknown! (0)
> Port: Twisted Pair
> PHYAD: 1
> Transceiver: internal
> Auto-negotiation: on
> Supports Wake-on: g
> Wake-on: d
> Link detected: yes <---
> 
> This patches fixed the problem by turning off the network stack
> during initialization.
> 
> Signed-off-by: Jiang Wang 
> ---
>  drivers/net/ethernet/broadcom/bnx2.c |4 
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnx2.c 
> b/drivers/net/ethernet/broadcom/bnx2.c
> index ac7b744..ce4548d 100644
> --- a/drivers/net/ethernet/broadcom/bnx2.c
> +++ b/drivers/net/ethernet/broadcom/bnx2.c
> @@ -8463,6 +8463,10 @@ bnx2_init_one(struct pci_dev *pdev, const struct 
> pci_device_id *ent)
>   dev->features |= dev->hw_features;
>   dev->priv_flags |= IFF_UNICAST_FLT;
>  
> + /* tell the stack to leave us alone until bnx2_open() is called */
> + netif_carrier_off(dev);

We have tried this before and this didn't work.  netif_carrier_off()
calls linkwatch_fire_event() to schedule the event.  If
register_netdev() fails for whatever reason, the netdev will be freed
but the link_watch event may still be scheduled.

Calling netif_carrier_off() after register_netdev() returns successfully
also may not work as it will race with bnx2_open() which enables IRQ.
An linkup IRQ can happen at time and we may end up with the wrong
carrier state because of the race condition.

> + netif_stop_queue(dev);
> +
>   if ((rc = register_netdev(dev))) {
>   dev_err(>dev, "Cannot register net device\n");
>   goto error;



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] bnx2: turn off the network statck during initialization

2012-08-16 Thread Michael Chan
On Thu, 2012-08-16 at 11:21 -0700, Jiang Wang wrote: 
 The initialization state of bnx2 driver is wrong. It does not turn
 of the Linux network stack using netif_carrier_off. This may lead to
 inconsistent report from ethtool as the link is up but speed is
 unknown when the cable is not plugged in.
 
 E.g.
 Speed: Unknown! (0)--
 Duplex: Half   --
 MDI: Unknown! (0)
 Port: Twisted Pair
 PHYAD: 1
 Transceiver: internal
 Auto-negotiation: on
 Supports Wake-on: g
 Wake-on: d
 Link detected: yes ---
 
 This patches fixed the problem by turning off the network stack
 during initialization.
 
 Signed-off-by: Jiang Wang jw...@riverbed.com
 ---
  drivers/net/ethernet/broadcom/bnx2.c |4 
  1 files changed, 4 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/net/ethernet/broadcom/bnx2.c 
 b/drivers/net/ethernet/broadcom/bnx2.c
 index ac7b744..ce4548d 100644
 --- a/drivers/net/ethernet/broadcom/bnx2.c
 +++ b/drivers/net/ethernet/broadcom/bnx2.c
 @@ -8463,6 +8463,10 @@ bnx2_init_one(struct pci_dev *pdev, const struct 
 pci_device_id *ent)
   dev-features |= dev-hw_features;
   dev-priv_flags |= IFF_UNICAST_FLT;
  
 + /* tell the stack to leave us alone until bnx2_open() is called */
 + netif_carrier_off(dev);

We have tried this before and this didn't work.  netif_carrier_off()
calls linkwatch_fire_event() to schedule the event.  If
register_netdev() fails for whatever reason, the netdev will be freed
but the link_watch event may still be scheduled.

Calling netif_carrier_off() after register_netdev() returns successfully
also may not work as it will race with bnx2_open() which enables IRQ.
An linkup IRQ can happen at time and we may end up with the wrong
carrier state because of the race condition.

 + netif_stop_queue(dev);
 +
   if ((rc = register_netdev(dev))) {
   dev_err(pdev-dev, Cannot register net device\n);
   goto error;



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] bnx2: turn off the network statck during initialization

2012-08-16 Thread Michael Chan
On Thu, 2012-08-16 at 19:15 +, Jiang Wang wrote: 
 Hi Michael,
 
 I just checked the code for netif_carrier_off(), and it will return if
 the dev is not initialized (registered) as follows:
 
 void netif_carrier_off(struct net_device *dev)
 {
 if (!test_and_set_bit(__LINK_STATE_NOCARRIER, dev-state)) {
 if (dev-reg_state == NETREG_UNINITIALIZED)
 return;
 linkwatch_fire_event(dev);
 }
 }
 
 So linkwatch_fire_event should not fire in this case, right? Thanks
 for the quick response.
 
Yes, you're right.  Your patch should not cause any problem then.

But I still don't understand what problem you're trying to solve.  The
link status from ethtool is determined by bnx2_get_link() and it should
always return link down before bnx2_open() is called.  Can you please
elborate?  Thanks.

 Regards,
 
 Jiang
 
 -
 Jiang Wang
 Member of Technical Staff
 Riverbed Technology
 Tel: (408) 522-5109
 Email: jiang.w...@riverbed.com
 www.riverbed.com
 
 
 -Original Message-
 From: Michael Chan [mailto:mc...@broadcom.com] 
 Sent: Thursday, August 16, 2012 11:57 AM
 To: Jiang Wang
 Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; Chaitanya Lala; 
 Francis St. Amant; Jiang Wang
 Subject: Re: [PATCH] bnx2: turn off the network statck during initialization
 
 On Thu, 2012-08-16 at 11:21 -0700, Jiang Wang wrote: 
  The initialization state of bnx2 driver is wrong. It does not turn of 
  the Linux network stack using netif_carrier_off. This may lead to 
  inconsistent report from ethtool as the link is up but speed is 
  unknown when the cable is not plugged in.
  
  E.g.
  Speed: Unknown! (0)--
  Duplex: Half   --
  MDI: Unknown! (0)
  Port: Twisted Pair
  PHYAD: 1
  Transceiver: internal
  Auto-negotiation: on
  Supports Wake-on: g
  Wake-on: d
  Link detected: yes ---
  
  This patches fixed the problem by turning off the network stack during 
  initialization.
  
  Signed-off-by: Jiang Wang jw...@riverbed.com
  ---
   drivers/net/ethernet/broadcom/bnx2.c |4 
   1 files changed, 4 insertions(+), 0 deletions(-)
  
  diff --git a/drivers/net/ethernet/broadcom/bnx2.c 
  b/drivers/net/ethernet/broadcom/bnx2.c
  index ac7b744..ce4548d 100644
  --- a/drivers/net/ethernet/broadcom/bnx2.c
  +++ b/drivers/net/ethernet/broadcom/bnx2.c
  @@ -8463,6 +8463,10 @@ bnx2_init_one(struct pci_dev *pdev, const struct 
  pci_device_id *ent)
  dev-features |= dev-hw_features;
  dev-priv_flags |= IFF_UNICAST_FLT;
   
  +   /* tell the stack to leave us alone until bnx2_open() is called */
  +   netif_carrier_off(dev);
 
 We have tried this before and this didn't work.  netif_carrier_off() calls 
 linkwatch_fire_event() to schedule the event.  If
 register_netdev() fails for whatever reason, the netdev will be freed but the 
 link_watch event may still be scheduled.
 
 Calling netif_carrier_off() after register_netdev() returns successfully also 
 may not work as it will race with bnx2_open() which enables IRQ.
 An linkup IRQ can happen at time and we may end up with the wrong carrier 
 state because of the race condition.
 
  +   netif_stop_queue(dev);
  +
  if ((rc = register_netdev(dev))) {
  dev_err(pdev-dev, Cannot register net device\n);
  goto error;
 
 
 
 



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] bnx2: turn off the network statck during initialization

2012-08-16 Thread Michael Chan
On Thu, 2012-08-16 at 20:28 +, Jiang Wang wrote:
 Also, I have another comment related to link state.
 
 Right now, the bnx2 driver powers up the device in bnx2_init_board(),
 regardless the netif_carrier is on or off.

We actually don't power up the device.  bnx2_init_board() just probes
the device.  If link is already up, it will stay up.  If it is down, it
will stay down.

 This may introduce following inconsistent behaviors:
 1) suppose the cable is plugged in to the NIC and the other end is
 connected to a switch
 2) user powers up the box

The link may already be up before or when you power up the box because
of management firmware (iLO, etc) or WoL.

 3) the Linux does not bring up the interface; i.e, ifconfig ethx shows
 it is down
 4) ethtool ethx will show no link
 5) if the user goes to check the light on the physical NIC, he will
 see the green link light is ON. That means the link is up, right? 
 
 I think it is better to power down the device until bnx2_open is
 called. In this way, ethtool report and the physical link light will
 be consistent. 

We cannot power it down.  If link is up, it is up for a reason (e.g. it
is an iLO port, etc).

Thanks.


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] bnx2: turn off the network statck during initialization

2012-08-16 Thread Michael Chan
On Thu, 2012-08-16 at 21:48 +, Jiang Wang wrote:
  I see. I was confused by the name of bnx2_set_power_state()  and
 I think the PHY is actually powered up by bnx2_reset_nic, right?

bnx2_init_phy() in bnx2_init_nic() will bring up the PHY if it is down.

 
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] bnx2: update bnx2-mips-09 firmware to bnx2-mips-09-6.2.1b

2012-07-13 Thread Michael Chan
On Fri, 2012-07-13 at 15:09 +0100, Chris Webb wrote: 
> Is there a more automatic method than going through the source for each
> configured driver and setting CONFIG_EXTRA_FIRMWARE manually to list the
> relevant firmwares? Is there any way to give the kernel the location of
> linux-firmware and have it compile in everything needed for the selected
> drivers, as used to happen with the firmware/ subdirectory?
> CONFIG_EXTRA_FIRMWARE_DIR doesn't seem to do anything with
> CONFIG_EXTRA_FIRMWARE empty, so I don't think it does what I'm hoping?
> 

Most users will just download the linux-firmware tree:

//git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git

and copy all the firmware files to /lib/firmware.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] bnx2: update bnx2-mips-09 firmware to bnx2-mips-09-6.2.1b

2012-07-13 Thread Michael Chan
On Fri, 2012-07-13 at 15:09 +0100, Chris Webb wrote: 
 Is there a more automatic method than going through the source for each
 configured driver and setting CONFIG_EXTRA_FIRMWARE manually to list the
 relevant firmwares? Is there any way to give the kernel the location of
 linux-firmware and have it compile in everything needed for the selected
 drivers, as used to happen with the firmware/ subdirectory?
 CONFIG_EXTRA_FIRMWARE_DIR doesn't seem to do anything with
 CONFIG_EXTRA_FIRMWARE empty, so I don't think it does what I'm hoping?
 

Most users will just download the linux-firmware tree:

//git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git

and copy all the firmware files to /lib/firmware.


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: TG3 network data corruption regression 2.6.24/2.6.23.4

2008-02-20 Thread Michael Chan
On Wed, 2008-02-20 at 15:08 -0800, David Miller wrote:
> From: Tony Battersby <[EMAIL PROTECTED]>
> Date: Wed, 20 Feb 2008 18:04:09 -0500
> 
> > The following patch fixes the problem for me.  Do we want to accept this
> > patch and call it a day or continue investigating the source of the problem?
> > 
> > Patch applies to 2.6.24.2, but doesn't apply to 2.6.25-rc.  If everyone
> > agrees that this is the right solution, I will resubmit with a proper
> > subject line and description.
> 
> A chipset bug, if it even exists, should be worked around in the
> driver for that hardware.  We shouldn't make every other piece
> of hardware in the world suffer too.
> 
> 

Yes, we should workaround this in the TG3 driver once we understand what
the problem is and how to workaround it.  We are still looking through
the errata list to sort this out.  It looks like it is the starting DMA
address of the TX buffer that is causing the problem.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: TG3 network data corruption regression 2.6.24/2.6.23.4

2008-02-20 Thread Michael Chan
On Wed, 2008-02-20 at 15:08 -0800, David Miller wrote:
 From: Tony Battersby [EMAIL PROTECTED]
 Date: Wed, 20 Feb 2008 18:04:09 -0500
 
  The following patch fixes the problem for me.  Do we want to accept this
  patch and call it a day or continue investigating the source of the problem?
  
  Patch applies to 2.6.24.2, but doesn't apply to 2.6.25-rc.  If everyone
  agrees that this is the right solution, I will resubmit with a proper
  subject line and description.
 
 A chipset bug, if it even exists, should be worked around in the
 driver for that hardware.  We shouldn't make every other piece
 of hardware in the world suffer too.
 
 

Yes, we should workaround this in the TG3 driver once we understand what
the problem is and how to workaround it.  We are still looking through
the errata list to sort this out.  It looks like it is the starting DMA
address of the TX buffer that is causing the problem.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: TG3 network data corruption regression 2.6.24/2.6.23.4

2008-02-19 Thread Michael Chan
On Tue, 2008-02-19 at 17:14 -0500, Tony Battersby wrote:

> 
> Update: when I revert Herbert's patch in addition to applying your
> patch, the iSCSI performance goes back up to 115 MB/s again in both
> directions.  So it looks like turning off SG for TX didn't itself cause
> the performance drop, but rather that the performance drop is just
> another manifestation of whatever bug is causing the data corruption.
> 
> I do not regularly use wireshark or look at network packet dumps, so I
> am not really sure what to look for.  Given the above information, do
> you still believe that there is value in examining the packet dump?
> 

Can you confirm whether you're getting TCP checksum errors on the other
side that is receiving packets from the 5701?  You can just check
statistics using netstat -s.  I suspect that after we turn off SG,
checksum is no longer offloaded and we are getting lots of TCP checksum
errors instead that are slowing the performance.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: TG3 network data corruption regression 2.6.24/2.6.23.4

2008-02-19 Thread Michael Chan
On Tue, 2008-02-19 at 11:16 -0500, Tony Battersby wrote:
> iSCSI
> performance drops to 6 - 15 MB/s when the 3Com NIC is doing heavy rx
> with light tx,

That's strange.  The patch should only affect TX performance slightly
since we are just turning off SG for TX.  Please take an ethereal trace
to see what's happening and compare with a good trace.

> but remains at a decent 115 MB/s when the 3Com NIC is
> doing heavy tx with light rx.  When I revert Herbert's patch instead
> of
> applying the patch above, I get 115 MB/s in both cases.  (With a stock
> unpatched kernel, the test fails almost immediately because the iSCSI
> control PDUs are corrupted, causing the TCP connection to be dropped.)
> 
> The SysKonnect NIC that does not exhibit this problem has a chip that
> says "BCM5411KQM" "TT0128 P2Q" and "56975E".
> 

I think this is the 5700, but please send me the tg3 output that
identifies the chip and the revision.  Something like this:

eth2: Tigon3 [partno(BCM95705) rev 3003 PHY(5705)] (PCI:66MHz:32-bit) 
10/100/1000Base-T Ethernet 00:10:18:04:57:0d
eth2: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[0] WireSpeed[0] TSOcap[1]





--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: TG3 network data corruption regression 2.6.24/2.6.23.4

2008-02-19 Thread Michael Chan
On Tue, 2008-02-19 at 11:16 -0500, Tony Battersby wrote:
 iSCSI
 performance drops to 6 - 15 MB/s when the 3Com NIC is doing heavy rx
 with light tx,

That's strange.  The patch should only affect TX performance slightly
since we are just turning off SG for TX.  Please take an ethereal trace
to see what's happening and compare with a good trace.

 but remains at a decent 115 MB/s when the 3Com NIC is
 doing heavy tx with light rx.  When I revert Herbert's patch instead
 of
 applying the patch above, I get 115 MB/s in both cases.  (With a stock
 unpatched kernel, the test fails almost immediately because the iSCSI
 control PDUs are corrupted, causing the TCP connection to be dropped.)
 
 The SysKonnect NIC that does not exhibit this problem has a chip that
 says BCM5411KQM TT0128 P2Q and 56975E.
 

I think this is the 5700, but please send me the tg3 output that
identifies the chip and the revision.  Something like this:

eth2: Tigon3 [partno(BCM95705) rev 3003 PHY(5705)] (PCI:66MHz:32-bit) 
10/100/1000Base-T Ethernet 00:10:18:04:57:0d
eth2: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[0] WireSpeed[0] TSOcap[1]





--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: TG3 network data corruption regression 2.6.24/2.6.23.4

2008-02-19 Thread Michael Chan
On Tue, 2008-02-19 at 17:14 -0500, Tony Battersby wrote:

 
 Update: when I revert Herbert's patch in addition to applying your
 patch, the iSCSI performance goes back up to 115 MB/s again in both
 directions.  So it looks like turning off SG for TX didn't itself cause
 the performance drop, but rather that the performance drop is just
 another manifestation of whatever bug is causing the data corruption.
 
 I do not regularly use wireshark or look at network packet dumps, so I
 am not really sure what to look for.  Given the above information, do
 you still believe that there is value in examining the packet dump?
 

Can you confirm whether you're getting TCP checksum errors on the other
side that is receiving packets from the 5701?  You can just check
statistics using netstat -s.  I suspect that after we turn off SG,
checksum is no longer offloaded and we are getting lots of TCP checksum
errors instead that are slowing the performance.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: TG3 network data corruption regression 2.6.24/2.6.23.4

2008-02-18 Thread Michael Chan
On Mon, 2008-02-18 at 16:35 -0800, David Miller wrote:

> One consequence of Herbert's change is that the chip will see a
> different datastream.  The initial skb->data linear area will be
> smaller, and the transition to the fragmented area of pages will be
> quicker.
> 

I see.  Perhaps when we get to the end of the data-stream, there is a
tiny frag that the chip cannot handle.  That's the only thing I can
think of.

Please try this patch to see if the problem goes away.  This will
disable SG on 5701 so we always get linear SKBs.

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index db606b6..bb37e76 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -12717,6 +12717,9 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
} else
tp->tg3_flags &= ~TG3_FLAG_RX_CHECKSUMS;
 
+   if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5701)
+   dev->features &= ~(NETIF_F_IP_CSUM | NETIF_F_SG);
+
/* flow control autonegotiation is default behavior */
tp->tg3_flags |= TG3_FLAG_PAUSE_AUTONEG;
tp->link_config.flowctrl = TG3_FLOW_CTRL_TX | TG3_FLOW_CTRL_RX;


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: TG3 network data corruption regression 2.6.24/2.6.23.4

2008-02-18 Thread Michael Chan
On Mon, 2008-02-18 at 17:41 -0500, Tony Battersby wrote:
> I am experiencing network data corruption with a 3Com 3C996B-T NIC
> (Broadcom NetXtreme BCM5701; driver tg3.ko).  I have identified the
> following patch as the trigger:

Assuming this problem is unique to the 5701, I'm not sure how it is
exposed by Herbert's patch.  One thing unique on the 5701 is that it
double-copies all RX packets so that the data starts at offset 2, but
that's quite unrelated to the patch below.

> 
> commit fb93134dfc2a6e6fbedc7c270a31da03fce88db9
> Author: Herbert Xu <[EMAIL PROTECTED]>
> Date:   Wed Nov 14 15:45:21 2007 -0800
> 
> [TCP]: Fix size calculation in sk_stream_alloc_pskb
>
> 

> I do not get data corruption when substituting a SysKonnect 9D21 NIC
> (which also uses the tg3.ko driver)

What Broadcom chip is on the Syskonnect card?



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: TG3 network data corruption regression 2.6.24/2.6.23.4

2008-02-18 Thread Michael Chan
On Mon, 2008-02-18 at 16:35 -0800, David Miller wrote:

 One consequence of Herbert's change is that the chip will see a
 different datastream.  The initial skb-data linear area will be
 smaller, and the transition to the fragmented area of pages will be
 quicker.
 

I see.  Perhaps when we get to the end of the data-stream, there is a
tiny frag that the chip cannot handle.  That's the only thing I can
think of.

Please try this patch to see if the problem goes away.  This will
disable SG on 5701 so we always get linear SKBs.

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index db606b6..bb37e76 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -12717,6 +12717,9 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
} else
tp-tg3_flags = ~TG3_FLAG_RX_CHECKSUMS;
 
+   if (GET_ASIC_REV(tp-pci_chip_rev_id) == ASIC_REV_5701)
+   dev-features = ~(NETIF_F_IP_CSUM | NETIF_F_SG);
+
/* flow control autonegotiation is default behavior */
tp-tg3_flags |= TG3_FLAG_PAUSE_AUTONEG;
tp-link_config.flowctrl = TG3_FLOW_CTRL_TX | TG3_FLOW_CTRL_RX;


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: TG3 network data corruption regression 2.6.24/2.6.23.4

2008-02-18 Thread Michael Chan
On Mon, 2008-02-18 at 17:41 -0500, Tony Battersby wrote:
 I am experiencing network data corruption with a 3Com 3C996B-T NIC
 (Broadcom NetXtreme BCM5701; driver tg3.ko).  I have identified the
 following patch as the trigger:

Assuming this problem is unique to the 5701, I'm not sure how it is
exposed by Herbert's patch.  One thing unique on the 5701 is that it
double-copies all RX packets so that the data starts at offset 2, but
that's quite unrelated to the patch below.

 
 commit fb93134dfc2a6e6fbedc7c270a31da03fce88db9
 Author: Herbert Xu [EMAIL PROTECTED]
 Date:   Wed Nov 14 15:45:21 2007 -0800
 
 [TCP]: Fix size calculation in sk_stream_alloc_pskb

 

 I do not get data corruption when substituting a SysKonnect 9D21 NIC
 (which also uses the tg3.ko driver)

What Broadcom chip is on the Syskonnect card?



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [drivers/net/bnx2.c] ADVERTISE_1000XPSE_ASYM

2008-01-30 Thread Michael Chan
On Wed, 2008-01-30 at 15:07 +0100, Roel Kluin wrote:
> In drivers/net/bnx2.c:1285: it reads in function bnx2_setup_remote_phy():
> 
> if (pause_adv & (ADVERTISE_1000XPSE_ASYM | ADVERTISE_1000XPSE_ASYM))
> 
> Note that the two are the same and this is therefore equivalent to
> 
> if (pause_adv & ADVERTISE_1000XPSE_ASYM)
> 
> This appears to be incorrect, was maybe '| ADVERTISE_1000XPAUSE' intended?
> 

Thanks for catching this.  The patch below will fix it.

[BNX2]: Fix ASYM PAUSE advertisement for remote PHY.

We were checking for the ASYM_PAUSE bit for 1000Base-X twice instead
checking for both the 1000Base-X bit and the 10/100/1000Base-T bit.
The purpose of the logic is to tell the firmware that ASYM_PAUSE is
set on either the Serdes or Copper interface.

Problem was discovered by Roel Kluin <[EMAIL PROTECTED]>

Signed-off-by: Michael Chan <[EMAIL PROTECTED]>

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index be7e8f8..77400ad 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -1429,7 +1429,7 @@ bnx2_setup_remote_phy(struct bnx2 *bp, u8 port)
 
if (pause_adv & (ADVERTISE_1000XPAUSE | ADVERTISE_PAUSE_CAP))
speed_arg |= BNX2_NETLINK_SET_LINK_FC_SYM_PAUSE;
-   if (pause_adv & (ADVERTISE_1000XPSE_ASYM | ADVERTISE_1000XPSE_ASYM))
+   if (pause_adv & (ADVERTISE_1000XPSE_ASYM | ADVERTISE_PAUSE_ASYM))
speed_arg |= BNX2_NETLINK_SET_LINK_FC_ASYM_PAUSE;
 
if (port == PORT_TP)


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [drivers/net/bnx2.c] ADVERTISE_1000XPSE_ASYM

2008-01-30 Thread Michael Chan
On Wed, 2008-01-30 at 15:07 +0100, Roel Kluin wrote:
 In drivers/net/bnx2.c:1285: it reads in function bnx2_setup_remote_phy():
 
 if (pause_adv  (ADVERTISE_1000XPSE_ASYM | ADVERTISE_1000XPSE_ASYM))
 
 Note that the two are the same and this is therefore equivalent to
 
 if (pause_adv  ADVERTISE_1000XPSE_ASYM)
 
 This appears to be incorrect, was maybe '| ADVERTISE_1000XPAUSE' intended?
 

Thanks for catching this.  The patch below will fix it.

[BNX2]: Fix ASYM PAUSE advertisement for remote PHY.

We were checking for the ASYM_PAUSE bit for 1000Base-X twice instead
checking for both the 1000Base-X bit and the 10/100/1000Base-T bit.
The purpose of the logic is to tell the firmware that ASYM_PAUSE is
set on either the Serdes or Copper interface.

Problem was discovered by Roel Kluin [EMAIL PROTECTED]

Signed-off-by: Michael Chan [EMAIL PROTECTED]

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index be7e8f8..77400ad 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -1429,7 +1429,7 @@ bnx2_setup_remote_phy(struct bnx2 *bp, u8 port)
 
if (pause_adv  (ADVERTISE_1000XPAUSE | ADVERTISE_PAUSE_CAP))
speed_arg |= BNX2_NETLINK_SET_LINK_FC_SYM_PAUSE;
-   if (pause_adv  (ADVERTISE_1000XPSE_ASYM | ADVERTISE_1000XPSE_ASYM))
+   if (pause_adv  (ADVERTISE_1000XPSE_ASYM | ADVERTISE_PAUSE_ASYM))
speed_arg |= BNX2_NETLINK_SET_LINK_FC_ASYM_PAUSE;
 
if (port == PORT_TP)


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: 2.6.23-mm1 tg3 wake-on-lan oddity...

2007-11-27 Thread Michael Chan
On Tue, 2007-11-27 at 01:35 -0800, [EMAIL PROTECTED] wrote:

> 
> Issue:
> 
> I (for unrelated reasons) run powertop, and it suggests I conserve power
> by doing 'ethtool -s eth0 wol d'.  I look at it, and think that it's daft,
> because (a) the Dell factory default is WOL disabled and (b) if it wasn't
> the default, I'd have *set* it to disabled, and (c) I even went back and
> rebooted and checked the BIOS setting - disabled. Nonetheless:
> 
> # ethtool eth0 | grep Wake
> Supports Wake-on: g
> Wake-on: g
> 
> Is this expected behavior?

What's happening is that there are 2 WoL settings: one in the BIOS and
one in the NIC's NVRAM.  For WoL to work, I think both settings have to
be enabled.  Apparently in this case, when you turn off WoL in the BIOS,
the NVRAM's WoL setting is unchanged, and will be seen by tg3 as
enabled.

Ideally, the BIOS should modify the NVRAM's setting when it is changed.
We will talk to Dell to get their opinion on this as this is very
confusing to the user.

Thanks.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: 2.6.23-mm1 tg3 wake-on-lan oddity...

2007-11-27 Thread Michael Chan
[EMAIL PROTECTED] wrote:

> (a) the Dell factory default is WOL disabled and (b) 
> if it wasn't
> the default, I'd have *set* it to disabled, and (c) I even 
> went back and
> rebooted and checked the BIOS setting - disabled. Nonetheless:
> 
> # ethtool eth0 | grep Wake
> Supports Wake-on: g
> Wake-on: g
> 
> Is this expected behavior?
> 

The new tg3 is supposed to follow the WoL setting in the
NVRAM, so this is not expected.  We'll have to look into
this.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: 2.6.23-mm1 tg3 wake-on-lan oddity...

2007-11-27 Thread Michael Chan
[EMAIL PROTECTED] wrote:

 (a) the Dell factory default is WOL disabled and (b) 
 if it wasn't
 the default, I'd have *set* it to disabled, and (c) I even 
 went back and
 rebooted and checked the BIOS setting - disabled. Nonetheless:
 
 # ethtool eth0 | grep Wake
 Supports Wake-on: g
 Wake-on: g
 
 Is this expected behavior?
 

The new tg3 is supposed to follow the WoL setting in the
NVRAM, so this is not expected.  We'll have to look into
this.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: 2.6.23-mm1 tg3 wake-on-lan oddity...

2007-11-27 Thread Michael Chan
On Tue, 2007-11-27 at 01:35 -0800, [EMAIL PROTECTED] wrote:

 
 Issue:
 
 I (for unrelated reasons) run powertop, and it suggests I conserve power
 by doing 'ethtool -s eth0 wol d'.  I look at it, and think that it's daft,
 because (a) the Dell factory default is WOL disabled and (b) if it wasn't
 the default, I'd have *set* it to disabled, and (c) I even went back and
 rebooted and checked the BIOS setting - disabled. Nonetheless:
 
 # ethtool eth0 | grep Wake
 Supports Wake-on: g
 Wake-on: g
 
 Is this expected behavior?

What's happening is that there are 2 WoL settings: one in the BIOS and
one in the NIC's NVRAM.  For WoL to work, I think both settings have to
be enabled.  Apparently in this case, when you turn off WoL in the BIOS,
the NVRAM's WoL setting is unchanged, and will be seen by tg3 as
enabled.

Ideally, the BIOS should modify the NVRAM's setting when it is changed.
We will talk to Dell to get their opinion on this as this is very
confusing to the user.

Thanks.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: tg3: strange errors and non-working-ness

2007-11-15 Thread Michael Chan
On Thu, 2007-11-15 at 13:17 -0600, Jon Nelson wrote:

> Is this what you mean? I pulled this from the quoted text:
> 
> Nov 10 22:45:52 frank kernel: NETDEV WATCHDOG: eth0: transmit timed out
> 

Right.  This explains the reset at 22:45:52, but not the earlier reset
at 22:24:40.  Link never came up after that earlier reset.

Is this a new problem introduced by a new driver?  I notice you are
using tg3 3.65.  Have you used newer versions or older versions?

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: tg3: strange errors and non-working-ness

2007-11-15 Thread Michael Chan
On Thu, 2007-11-15 at 10:47 +0100, Jarek Poplawski wrote:
> On 13-11-2007 19:57, Jon Nelson wrote:
> > The best info I've got is this:

It looks like the card is being reset periodically.  Every time the card
gets reset, you'll see those PM messages in the version of the driver
you're using.  Do you see NETDEV WATCHDOG message as well in the dmesg
log?

> > 
> > Nov 10 22:21:19 frank kernel: tg3.c:v3.65 (August 07, 2006)
> > Nov 10 22:21:19 frank kernel: ACPI: PCI Interrupt :00:0b.0[A] ->
> > Link [LNKB] -> GSI 3 (level, low) -> IRQ 3
> > Nov 10 22:21:19 frank kernel: eth0: Tigon3 [partno(AC91002A1) rev 0105
> > PHY(5701)] (PCI:33MHz:32-bit) 10/100/1000BaseT Ethernet
> > 00:09:5b:09:b1:69
> > Nov 10 22:21:19 frank kernel: eth0: RXcsums[1] LinkChgREG[0] MIirq[0]
> > ASF[0] Split[0] WireSpeed[1] TSOcap[0]
> > Nov 10 22:21:19 frank kernel: eth0: dma_rwctrl[76ff000f] dma_mask[64-bit]
> > Nov 10 22:21:19 frank kernel: PM: Writing back config space on device
> > :00:0b.0 at offset b (was 164514e4, writing 302a1385)
> > Nov 10 22:21:19 frank kernel: PM: Writing back config space on device
> > :00:0b.0 at offset 3 (was 0, writing 4008)
> > Nov 10 22:21:19 frank kernel: PM: Writing back config space on device
> > :00:0b.0 at offset 2 (was 200, writing 215)
> > Nov 10 22:21:19 frank kernel: PM: Writing back config space on device
> > :00:0b.0 at offset 1 (was 2b0, writing 2b00106)
> > Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
> > :00:0b.0 at offset 0 (was 164514e4, writing 3ea173b)
> > Nov 10 22:21:20 frank kernel: tg3: eth0: Link is up at 1000 Mbps, full 
> > duplex.
> > Nov 10 22:21:20 frank kernel: tg3: eth0: Flow control is on for TX and
> > on for RX.
> > Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
> > :00:0b.0 at offset b (was 164514e4, writing 302a1385)
> > Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
> > :00:0b.0 at offset 3 (was 0, writing 4008)
> > Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
> > :00:0b.0 at offset 2 (was 200, writing 215)
> > Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
> > :00:0b.0 at offset 1 (was 2b0, writing 2b00106)
> > Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
> > :00:0b.0 at offset 0 (was 164514e4, writing 3ea173b)
> > Nov 10 22:21:20 frank kernel: ACPI: PCI interrupt for device
> > :00:0b.0 disabled
> > Nov 10 22:21:20 frank kernel: PCI: Enabling device :00:0b.0 (0100 -> 
> > 0102)
> > Nov 10 22:21:20 frank kernel: ACPI: PCI Interrupt :00:0b.0[A] ->
> > Link [LNKB] -> GSI 3 (level, low) -> IRQ 3
> > Nov 10 22:21:20 frank kernel: eth0: Tigon3 [partno(AC91002A1) rev 0105
> > PHY(5701)] (PCI:33MHz:32-bit) 10/100/1000BaseT Ethernet
> > 00:09:5b:09:b1:69
> > Nov 10 22:21:20 frank kernel: eth0: RXcsums[1] LinkChgREG[0] MIirq[0]
> > ASF[0] Split[0] WireSpeed[1] TSOcap[0]
> > Nov 10 22:21:20 frank kernel: eth0: dma_rwctrl[76ff000f] dma_mask[64-bit]
> > Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
> > :00:0b.0 at offset b (was 164514e4, writing 302a1385)
> > Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
> > :00:0b.0 at offset 3 (was 0, writing 4008)
> > Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
> > :00:0b.0 at offset 2 (was 200, writing 215)
> > Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
> > :00:0b.0 at offset 1 (was 2b0, writing 2b00106)
> > Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
> > :00:0b.0 at offset 0 (was 164514e4, writing 3ea173b)
> > Nov 10 22:21:20 frank kernel: tg3: eth0: Link is up at 1000 Mbps, full 
> > duplex.
> > Nov 10 22:21:20 frank kernel: tg3: eth0: Flow control is on for TX and
> > on for RX.
> > Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
> > :00:0b.0 at offset b (was 164514e4, writing 302a1385)
> > Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
> > :00:0b.0 at offset 3 (was 0, writing 4008)
> > Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
> > :00:0b.0 at offset 2 (was 200, writing 215)
> > Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
> > :00:0b.0 at offset 1 (was 2b0, writing 2b00106)
> > Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
> > :00:0b.0 at offset 0 (was 164514e4, writing 3ea173b)
> > Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
> > :00:0b.0 at offset b (was 164514e4, writing 302a1385)
> > Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
> > :00:0b.0 at offset 3 (was 0, writing 4008)
> > Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
> > :00:0b.0 at offset 2 (was 200, writing 215)
> > Nov 10 22:21:20 frank kernel: PM: 

Re: tg3: strange errors and non-working-ness

2007-11-15 Thread Michael Chan
On Thu, 2007-11-15 at 10:47 +0100, Jarek Poplawski wrote:
 On 13-11-2007 19:57, Jon Nelson wrote:
  The best info I've got is this:

It looks like the card is being reset periodically.  Every time the card
gets reset, you'll see those PM messages in the version of the driver
you're using.  Do you see NETDEV WATCHDOG message as well in the dmesg
log?

  
  Nov 10 22:21:19 frank kernel: tg3.c:v3.65 (August 07, 2006)
  Nov 10 22:21:19 frank kernel: ACPI: PCI Interrupt :00:0b.0[A] -
  Link [LNKB] - GSI 3 (level, low) - IRQ 3
  Nov 10 22:21:19 frank kernel: eth0: Tigon3 [partno(AC91002A1) rev 0105
  PHY(5701)] (PCI:33MHz:32-bit) 10/100/1000BaseT Ethernet
  00:09:5b:09:b1:69
  Nov 10 22:21:19 frank kernel: eth0: RXcsums[1] LinkChgREG[0] MIirq[0]
  ASF[0] Split[0] WireSpeed[1] TSOcap[0]
  Nov 10 22:21:19 frank kernel: eth0: dma_rwctrl[76ff000f] dma_mask[64-bit]
  Nov 10 22:21:19 frank kernel: PM: Writing back config space on device
  :00:0b.0 at offset b (was 164514e4, writing 302a1385)
  Nov 10 22:21:19 frank kernel: PM: Writing back config space on device
  :00:0b.0 at offset 3 (was 0, writing 4008)
  Nov 10 22:21:19 frank kernel: PM: Writing back config space on device
  :00:0b.0 at offset 2 (was 200, writing 215)
  Nov 10 22:21:19 frank kernel: PM: Writing back config space on device
  :00:0b.0 at offset 1 (was 2b0, writing 2b00106)
  Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
  :00:0b.0 at offset 0 (was 164514e4, writing 3ea173b)
  Nov 10 22:21:20 frank kernel: tg3: eth0: Link is up at 1000 Mbps, full 
  duplex.
  Nov 10 22:21:20 frank kernel: tg3: eth0: Flow control is on for TX and
  on for RX.
  Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
  :00:0b.0 at offset b (was 164514e4, writing 302a1385)
  Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
  :00:0b.0 at offset 3 (was 0, writing 4008)
  Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
  :00:0b.0 at offset 2 (was 200, writing 215)
  Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
  :00:0b.0 at offset 1 (was 2b0, writing 2b00106)
  Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
  :00:0b.0 at offset 0 (was 164514e4, writing 3ea173b)
  Nov 10 22:21:20 frank kernel: ACPI: PCI interrupt for device
  :00:0b.0 disabled
  Nov 10 22:21:20 frank kernel: PCI: Enabling device :00:0b.0 (0100 - 
  0102)
  Nov 10 22:21:20 frank kernel: ACPI: PCI Interrupt :00:0b.0[A] -
  Link [LNKB] - GSI 3 (level, low) - IRQ 3
  Nov 10 22:21:20 frank kernel: eth0: Tigon3 [partno(AC91002A1) rev 0105
  PHY(5701)] (PCI:33MHz:32-bit) 10/100/1000BaseT Ethernet
  00:09:5b:09:b1:69
  Nov 10 22:21:20 frank kernel: eth0: RXcsums[1] LinkChgREG[0] MIirq[0]
  ASF[0] Split[0] WireSpeed[1] TSOcap[0]
  Nov 10 22:21:20 frank kernel: eth0: dma_rwctrl[76ff000f] dma_mask[64-bit]
  Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
  :00:0b.0 at offset b (was 164514e4, writing 302a1385)
  Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
  :00:0b.0 at offset 3 (was 0, writing 4008)
  Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
  :00:0b.0 at offset 2 (was 200, writing 215)
  Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
  :00:0b.0 at offset 1 (was 2b0, writing 2b00106)
  Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
  :00:0b.0 at offset 0 (was 164514e4, writing 3ea173b)
  Nov 10 22:21:20 frank kernel: tg3: eth0: Link is up at 1000 Mbps, full 
  duplex.
  Nov 10 22:21:20 frank kernel: tg3: eth0: Flow control is on for TX and
  on for RX.
  Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
  :00:0b.0 at offset b (was 164514e4, writing 302a1385)
  Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
  :00:0b.0 at offset 3 (was 0, writing 4008)
  Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
  :00:0b.0 at offset 2 (was 200, writing 215)
  Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
  :00:0b.0 at offset 1 (was 2b0, writing 2b00106)
  Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
  :00:0b.0 at offset 0 (was 164514e4, writing 3ea173b)
  Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
  :00:0b.0 at offset b (was 164514e4, writing 302a1385)
  Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
  :00:0b.0 at offset 3 (was 0, writing 4008)
  Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
  :00:0b.0 at offset 2 (was 200, writing 215)
  Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
  :00:0b.0 at offset 1 (was 2b0, writing 2b00106)
  Nov 10 22:21:20 frank kernel: PM: Writing back config space on device
  

Re: tg3: strange errors and non-working-ness

2007-11-15 Thread Michael Chan
On Thu, 2007-11-15 at 13:17 -0600, Jon Nelson wrote:

 Is this what you mean? I pulled this from the quoted text:
 
 Nov 10 22:45:52 frank kernel: NETDEV WATCHDOG: eth0: transmit timed out
 

Right.  This explains the reset at 22:45:52, but not the earlier reset
at 22:24:40.  Link never came up after that earlier reset.

Is this a new problem introduced by a new driver?  I notice you are
using tg3 3.65.  Have you used newer versions or older versions?

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] - [2/15] - remove defconfig ptr comparisons to 0 - drivers/net

2007-11-13 Thread Michael Chan
On Tue, 2007-11-13 at 18:04 -0800, Joe Perches wrote:
> Remove defconfig ptr comparison to 0
> 
> Remove sparse warning: Using plain integer as NULL pointer
> 
> Signed-off-by: Joe Perches <[EMAIL PROTECTED]>
> 

Acked-by: Michael Chan <[EMAIL PROTECTED]>

Thanks.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] - [2/15] - remove defconfig ptr comparisons to 0 - drivers/net

2007-11-13 Thread Michael Chan
On Tue, 2007-11-13 at 18:04 -0800, Joe Perches wrote:
 Remove defconfig ptr comparison to 0
 
 Remove sparse warning: Using plain integer as NULL pointer
 
 Signed-off-by: Joe Perches [EMAIL PROTECTED]
 

Acked-by: Michael Chan [EMAIL PROTECTED]

Thanks.


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: expected behavior of PF_PACKET on NETIF_F_HW_VLAN_RX device?

2007-11-02 Thread Michael Chan
On Fri, 2007-11-02 at 14:08 -0400, Dave Johnson wrote:

> > 
> > *2:  If chip supports 'ASF', tag is always stripped (see *1 above).
> >  Looks ok if ASF is not supported. 
> 
> Michael,
> 
> These changes seems to cause this issue:
> 
> >  [BNX2]: Fix VLAN on ASF
> >  
> >  Always set up the device to strip incoming VLAN tags when ASF is
> >  enabled. ASF firmware will not parse packets correctly if VLAN tags
> >  are not stripped.
> >  
> >  Signed-off-by: Michael Chan <[EMAIL PROTECTED]>
> >  Signed-off-by: David S. Miller <[EMAIL PROTECTED]>
> >  
> >  GIT: e29054f92d7d575631691865c1b95bee5bc974cc
> 
> and
> 
> > [EMAIL PROTECTED], 2003-12-02 02:34:13-08:00, [EMAIL PROTECTED] +1 -0
> >   [TG3]: Do not set RX_MODE_KEEP_VLAN_TAG when ASF is enabled.
> 
> 
> Could you elaborate if this is really needed, if so is there some
> workaround that could be done instead?

This is needed for management firmware to work properly.  Management
firmware expects any VLAN tags to be stripped.  Unfortunately, VLAN
stripping cannot be done independently between the driver and the
firmware.  The workaround is to disable management firmware.

> 
> Simply removing the check seemed to work for me, but I'm unsure if
> this is actually a valid thing to do with these MACs.

Most of these on-board devices are shipped with management firmware
enabled.  Removing the check will make the firmware not functional.

We realize this VLAN limitation is causing problems to many users and we
are looking for ways to address it.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: expected behavior of PF_PACKET on NETIF_F_HW_VLAN_RX device?

2007-11-02 Thread Michael Chan
On Fri, 2007-11-02 at 14:08 -0400, Dave Johnson wrote:

  
  *2:  If chip supports 'ASF', tag is always stripped (see *1 above).
   Looks ok if ASF is not supported. 
 
 Michael,
 
 These changes seems to cause this issue:
 
   [BNX2]: Fix VLAN on ASF
   
   Always set up the device to strip incoming VLAN tags when ASF is
   enabled. ASF firmware will not parse packets correctly if VLAN tags
   are not stripped.
   
   Signed-off-by: Michael Chan [EMAIL PROTECTED]
   Signed-off-by: David S. Miller [EMAIL PROTECTED]
   
   GIT: e29054f92d7d575631691865c1b95bee5bc974cc
 
 and
 
  [EMAIL PROTECTED], 2003-12-02 02:34:13-08:00, [EMAIL PROTECTED] +1 -0
[TG3]: Do not set RX_MODE_KEEP_VLAN_TAG when ASF is enabled.
 
 
 Could you elaborate if this is really needed, if so is there some
 workaround that could be done instead?

This is needed for management firmware to work properly.  Management
firmware expects any VLAN tags to be stripped.  Unfortunately, VLAN
stripping cannot be done independently between the driver and the
firmware.  The workaround is to disable management firmware.

 
 Simply removing the check seemed to work for me, but I'm unsure if
 this is actually a valid thing to do with these MACs.

Most of these on-board devices are shipped with management firmware
enabled.  Removing the check will make the firmware not functional.

We realize this VLAN limitation is causing problems to many users and we
are looking for ways to address it.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/4]: [PCI]: Add quirk for devices which disable MSI when INTX_DISABLE is set.

2007-10-23 Thread Michael Chan
David Miller wrote:

> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_BROADCOM,
> + PCI_DEVICE_ID_TIGON3_5780,
> + quirk_msi_intx_disable_bug);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_BROADCOM,
> + PCI_DEVICE_ID_TIGON3_5780S,
> + quirk_msi_intx_disable_bug);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_BROADCOM,
> + PCI_DEVICE_ID_TIGON3_5714,
> + quirk_msi_intx_disable_bug);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_BROADCOM,
> + PCI_DEVICE_ID_TIGON3_5714S,
> + quirk_msi_intx_disable_bug);
> +

Please add PCI_DEVICE_ID_TIGON3_5715 and
PCI_DEVICE_ID_TIGON3_5715S as well.

Other than that,
Acked-by: Michael Chan <[EMAIL PROTECTED]>

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/4]: [PCI]: Add quirk for devices which disable MSI when INTX_DISABLE is set.

2007-10-23 Thread Michael Chan
David Miller wrote:

 +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_BROADCOM,
 + PCI_DEVICE_ID_TIGON3_5780,
 + quirk_msi_intx_disable_bug);
 +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_BROADCOM,
 + PCI_DEVICE_ID_TIGON3_5780S,
 + quirk_msi_intx_disable_bug);
 +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_BROADCOM,
 + PCI_DEVICE_ID_TIGON3_5714,
 + quirk_msi_intx_disable_bug);
 +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_BROADCOM,
 + PCI_DEVICE_ID_TIGON3_5714S,
 + quirk_msi_intx_disable_bug);
 +

Please add PCI_DEVICE_ID_TIGON3_5715 and
PCI_DEVICE_ID_TIGON3_5715S as well.

Other than that,
Acked-by: Michael Chan [EMAIL PROTECTED]

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] bnx2: factor out gzip unpacker

2007-09-21 Thread Michael Chan
On Fri, 2007-09-21 at 10:49 -0700, David Miller wrote:
> From: Denys Vlasenko <[EMAIL PROTECTED]>
> Date: Fri, 21 Sep 2007 18:03:55 +0100
> 
> > Do patches look ok to you?
> 
> I'm travelling so I haven't looked closely yet :-)
> 
> Michael can take a look and I'll try to do so as well
> tonight.
> 

I've already reviewed the earlier versions of the patch and have made
some suggestions.  This latest one looks ok to me and tested ok.

I'll follow up later with another patch to remove all the zeros in other
firmware sections, and to remove the gzip headers completely.

Acked-by: Michael Chan <[EMAIL PROTECTED]>

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] bnx2: factor out gzip unpacker

2007-09-21 Thread Michael Chan
On Fri, 2007-09-21 at 10:49 -0700, David Miller wrote:
 From: Denys Vlasenko [EMAIL PROTECTED]
 Date: Fri, 21 Sep 2007 18:03:55 +0100
 
  Do patches look ok to you?
 
 I'm travelling so I haven't looked closely yet :-)
 
 Michael can take a look and I'll try to do so as well
 tonight.
 

I've already reviewed the earlier versions of the patch and have made
some suggestions.  This latest one looks ok to me and tested ok.

I'll follow up later with another patch to remove all the zeros in other
firmware sections, and to remove the gzip headers completely.

Acked-by: Michael Chan [EMAIL PROTECTED]

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: bnx2 dirver's firmware images

2007-09-20 Thread Michael Chan
On Thu, 2007-09-20 at 15:49 +0100, Denys Vlasenko wrote:
> 
> 
> Please test these two patches.
> I updated them according to your comments.
> 
> 

I've only tested patch #1.  It worked after some minor modifications
below.

> 
> 
> 
> 
> 
> plain text
> document
> attachment
> (linux-2.6.23-
> rc6.bnx2-1.patch)
> 
> 
> @@ -2767,89 +2769,44 @@ bnx2_set_rx_mode(struct net_device *dev)
> spin_unlock_bh(>phy_lock);
>  }
>  
> -#define FW_BUF_SIZE0x8000
> -
> +/* To be moved to generic lib/ */
>  static int
> -bnx2_gunzip_init(struct bnx2 *bp)
> +bnx2_gunzip(void *gunzip_buf, unsigned sz, u8 *zbuf, int len, void **outbuf)

outbuf is no longer needed.

> +   rc = zlib_inflateInit2(strm, -MAX_WBITS);
> +   if (rc == Z_OK) {
> +   rc = zlib_inflate(strm, Z_FINISH);
> +   if (rc == Z_OK)

rc will always be Z_STREAM_END in this case since we provide a big
enough gunzip_buf for the whole blob.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: bnx2 dirver's firmware images

2007-09-20 Thread Michael Chan
On Thu, 2007-09-20 at 15:49 +0100, Denys Vlasenko wrote:
 
 
 Please test these two patches.
 I updated them according to your comments.
 
 

I've only tested patch #1.  It worked after some minor modifications
below.

 
 
 
 
 
 plain text
 document
 attachment
 (linux-2.6.23-
 rc6.bnx2-1.patch)
 
 
 @@ -2767,89 +2769,44 @@ bnx2_set_rx_mode(struct net_device *dev)
 spin_unlock_bh(bp-phy_lock);
  }
  
 -#define FW_BUF_SIZE0x8000
 -
 +/* To be moved to generic lib/ */
  static int
 -bnx2_gunzip_init(struct bnx2 *bp)
 +bnx2_gunzip(void *gunzip_buf, unsigned sz, u8 *zbuf, int len, void **outbuf)

outbuf is no longer needed.

 +   rc = zlib_inflateInit2(strm, -MAX_WBITS);
 +   if (rc == Z_OK) {
 +   rc = zlib_inflate(strm, Z_FINISH);
 +   if (rc == Z_OK)

rc will always be Z_STREAM_END in this case since we provide a big
enough gunzip_buf for the whole blob.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: bnx2 dirver's firmware images

2007-09-19 Thread Michael Chan
On Wed, 2007-09-19 at 21:29 +0100, Denys Vlasenko wrote:

> Are you saying that you successfully run-tested it?

I've only reviewed the code.  Let's resolve these issues first before
testing the code.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: bnx2 dirver's firmware images

2007-09-19 Thread Michael Chan
On Wed, 2007-09-19 at 09:30 +0100, Denys Vlasenko wrote:
+   /* gzip header (1f,8b,08... 10 bytes total + possible asciz filename)
+* is stripped, 32-bit unpacked size (LE) is prepended instead */
+   sz = *zbuf++;
+   sz = (sz << 8) + *zbuf++;
+   sz = (sz << 8) + *zbuf++;
+   sz = (sz << 8) + *zbuf++;

I don't have a problem with removing the gzip header.  It doesn't
contain very useful information other than a valid header for sanity
check.  But I don't think we need to arbitrarily add the unpacked size
in front of the gzipped data.  The driver knows the size (e.g. the size
of RAM on the chip) and should pass it to the call.  The driver should
also allocate the memory for the unpacked data instead of allocating the
memory inside the call and freeing it by the caller.  For example, the
driver may need to use pci_alloc_consistent() if the firmware is to be
DMA'ed to the chip.

Other than that, everything else looks fine.  Thanks.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: bnx2 dirver's firmware images

2007-09-19 Thread Michael Chan
On Wed, 2007-09-19 at 09:30 +0100, Denys Vlasenko wrote:
+   /* gzip header (1f,8b,08... 10 bytes total + possible asciz filename)
+* is stripped, 32-bit unpacked size (LE) is prepended instead */
+   sz = *zbuf++;
+   sz = (sz  8) + *zbuf++;
+   sz = (sz  8) + *zbuf++;
+   sz = (sz  8) + *zbuf++;

I don't have a problem with removing the gzip header.  It doesn't
contain very useful information other than a valid header for sanity
check.  But I don't think we need to arbitrarily add the unpacked size
in front of the gzipped data.  The driver knows the size (e.g. the size
of RAM on the chip) and should pass it to the call.  The driver should
also allocate the memory for the unpacked data instead of allocating the
memory inside the call and freeing it by the caller.  For example, the
driver may need to use pci_alloc_consistent() if the firmware is to be
DMA'ed to the chip.

Other than that, everything else looks fine.  Thanks.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: bnx2 dirver's firmware images

2007-09-19 Thread Michael Chan
On Wed, 2007-09-19 at 21:29 +0100, Denys Vlasenko wrote:

 Are you saying that you successfully run-tested it?

I've only reviewed the code.  Let's resolve these issues first before
testing the code.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: bnx2 dirver's firmware images

2007-09-18 Thread Michael Chan
On Tue, 2007-09-18 at 23:37 +0200, Willy Tarreau wrote:

> > > Michael, doesn't a functional-yet-suboptimal firmware exist ? I mean,
> > > just the same principle as we all have kernels, boot CDs, statically
> > > built tools, etc... which run everywhere. If you have such a beast,
> > > maybe it would be a good start to have it in the kernel, and provide
> > > the users with the ability to upgrade the firmware once the system
> > > is able to do more complex things.
> > > 
> > > Just a thought...
> > 
> > So let's save 60K instead of 80K.
> 
> That's not for this reason I said this. Michael said the firmware needs
> to be updated somewhat often. What I was wondering was if it was not
> possible to stick to a stable one (and hopefully small) so that the
> driver could require less frequent updates. Sorry if it's not the main
> point of the discussion, but I grepped on this :-)
> 

The bnx2 chip requires a lot of firmware to begin with, so it won't save
much space no matter what version is used in the kernel.  We update the
firmware to fix bugs and sometimes to add new features.  New features
often require matching changes in the driver.  For example, we're
planning to add S/G support for jumbo rx frames and this requires
changes in both driver and firmware.

It's possible to make the driver work with multiple firmware versions,
but that adds complexity to enable/disable certain features.  Testing
also becomes more difficult as it has to cover different combinations.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: bnx2 dirver's firmware images

2007-09-18 Thread Michael Chan
On Tue, 2007-09-18 at 11:23 -0700, David Miller wrote:

> I don't like it because it means people have to setup full initrd's
> in order to do network booting with such network cards.
> 
> But the days of my opinion mattering on that issue are long gone,
> the momentum is just too greatly behind using request_firmware()
> across the board, so there is no reason for bnx2 to be any different.
> 

The bnx2 firmware changes quite frequently.  A new driver quite often
requires new firmware to work correctly.  Splitting them up makes things
difficult for the user.

The firmware in tg3 is a lot more mature and I don't expect it to
change.  I think tg3 is better suited for using request_firmware().

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: bnx2 dirver's firmware images

2007-09-18 Thread Michael Chan
On Tue, 2007-09-18 at 18:55 +0100, Denys Vlasenko wrote:
> On Tuesday 18 September 2007 19:45, Michael Chan wrote:
> > We can compress all the different sections of the firmware.  Currently,
> > we only compress the biggest chunks and the rest are uncompressed.
> 
> > These zeros should compress to almost nothing.  But I agree that the
> > firmware is still big.
> 
> You don't need to store and fetch zeros at all. You *know* that
> they are zeros, right? So do this:
> 
> - REG_WR_IND(bp, offset, fw->bss[j]);
> + REG_WR_IND(bp, offset, 0);
> 
Good point.  We can do that.  Looking at the file, there are other non-
zero data that can be compressed to save some more room.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: bnx2 dirver's firmware images

2007-09-18 Thread Michael Chan
On Tue, 2007-09-18 at 18:23 +0100, Denys Vlasenko wrote:
> Hi Michael,
> 
> In bnx2_fw.h I see the following:
> 
> static u32 bnx2_RXP_b06FwBss[(0x13dc/4) + 1] = { 0x0 };
> 
> static struct fw_info bnx2_rxp_fw_06 = {
> ...
> .bss= bnx2_RXP_b06FwBss,
> ...
> };
> 
> I grepped for the usage of .bss member (grepped for '[.>]bss[^_]')
> and it is used only here:
> 
> if (fw->bss) {
> int j;
> 
> for (j = 0; j < (fw->bss_len/4); j++, offset += 4) {
> REG_WR_IND(bp, offset, fw->bss[j]);
> }
> }
> 
> If I understand it correctly, you read zero words one by one from
> bnx2_RXP_b06FwBss and writing them into the card. This is very
> suboptimal usage of nearly 5k of kernel unswappable memory.
> 
> Do you plan to fix it?

We can compress all the different sections of the firmware.  Currently,
we only compress the biggest chunks and the rest are uncompressed.
These zeros should compress to almost nothing.  But I agree that the
firmware is still big.

> 
> Do you have any plans to switch to request_firmware() interface,
> which will allow you to avoid keeping firmware in unswappable kernel
> memory and thus free ~80k?

Matching the correct version of the firmware with the driver is the main
issue.

David, what's your opinion on this?

> 
> $ size bnx2.o
>textdata bss dec hex filename
>   52255   815516360  140166   22386 bnx2.o
> --
> vda
> 

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: bnx2 dirver's firmware images

2007-09-18 Thread Michael Chan
On Tue, 2007-09-18 at 18:23 +0100, Denys Vlasenko wrote:
 Hi Michael,
 
 In bnx2_fw.h I see the following:
 
 static u32 bnx2_RXP_b06FwBss[(0x13dc/4) + 1] = { 0x0 };
 
 static struct fw_info bnx2_rxp_fw_06 = {
 ...
 .bss= bnx2_RXP_b06FwBss,
 ...
 };
 
 I grepped for the usage of .bss member (grepped for '[.]bss[^_]')
 and it is used only here:
 
 if (fw-bss) {
 int j;
 
 for (j = 0; j  (fw-bss_len/4); j++, offset += 4) {
 REG_WR_IND(bp, offset, fw-bss[j]);
 }
 }
 
 If I understand it correctly, you read zero words one by one from
 bnx2_RXP_b06FwBss and writing them into the card. This is very
 suboptimal usage of nearly 5k of kernel unswappable memory.
 
 Do you plan to fix it?

We can compress all the different sections of the firmware.  Currently,
we only compress the biggest chunks and the rest are uncompressed.
These zeros should compress to almost nothing.  But I agree that the
firmware is still big.

 
 Do you have any plans to switch to request_firmware() interface,
 which will allow you to avoid keeping firmware in unswappable kernel
 memory and thus free ~80k?

Matching the correct version of the firmware with the driver is the main
issue.

David, what's your opinion on this?

 
 $ size bnx2.o
textdata bss dec hex filename
   52255   815516360  140166   22386 bnx2.o
 --
 vda
 

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: bnx2 dirver's firmware images

2007-09-18 Thread Michael Chan
On Tue, 2007-09-18 at 18:55 +0100, Denys Vlasenko wrote:
 On Tuesday 18 September 2007 19:45, Michael Chan wrote:
  We can compress all the different sections of the firmware.  Currently,
  we only compress the biggest chunks and the rest are uncompressed.
 
  These zeros should compress to almost nothing.  But I agree that the
  firmware is still big.
 
 You don't need to store and fetch zeros at all. You *know* that
 they are zeros, right? So do this:
 
 - REG_WR_IND(bp, offset, fw-bss[j]);
 + REG_WR_IND(bp, offset, 0);
 
Good point.  We can do that.  Looking at the file, there are other non-
zero data that can be compressed to save some more room.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: bnx2 dirver's firmware images

2007-09-18 Thread Michael Chan
On Tue, 2007-09-18 at 11:23 -0700, David Miller wrote:

 I don't like it because it means people have to setup full initrd's
 in order to do network booting with such network cards.
 
 But the days of my opinion mattering on that issue are long gone,
 the momentum is just too greatly behind using request_firmware()
 across the board, so there is no reason for bnx2 to be any different.
 

The bnx2 firmware changes quite frequently.  A new driver quite often
requires new firmware to work correctly.  Splitting them up makes things
difficult for the user.

The firmware in tg3 is a lot more mature and I don't expect it to
change.  I think tg3 is better suited for using request_firmware().

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: bnx2 dirver's firmware images

2007-09-18 Thread Michael Chan
On Tue, 2007-09-18 at 23:37 +0200, Willy Tarreau wrote:

   Michael, doesn't a functional-yet-suboptimal firmware exist ? I mean,
   just the same principle as we all have kernels, boot CDs, statically
   built tools, etc... which run everywhere. If you have such a beast,
   maybe it would be a good start to have it in the kernel, and provide
   the users with the ability to upgrade the firmware once the system
   is able to do more complex things.
   
   Just a thought...
  
  So let's save 60K instead of 80K.
 
 That's not for this reason I said this. Michael said the firmware needs
 to be updated somewhat often. What I was wondering was if it was not
 possible to stick to a stable one (and hopefully small) so that the
 driver could require less frequent updates. Sorry if it's not the main
 point of the discussion, but I grepped on this :-)
 

The bnx2 chip requires a lot of firmware to begin with, so it won't save
much space no matter what version is used in the kernel.  We update the
firmware to fix bugs and sometimes to add new features.  New features
often require matching changes in the driver.  For example, we're
planning to add S/G support for jumbo rx frames and this requires
changes in both driver and firmware.

It's possible to make the driver work with multiple firmware versions,
but that adds complexity to enable/disable certain features.  Testing
also becomes more difficult as it has to cover different combinations.


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [ofa-general] InfiniBand/RDMA merge plans for 2.6.24

2007-09-14 Thread Michael Chan
On Fri, 2007-09-14 at 09:18 -0700, Roland Dreier wrote:

> However, do you have any plans to support iSCSI offload for targets?
> Also, looking at the first CNIC patch, I can't help but notice that
> you seem to have at least some support for iWARP there.  How does the
> CNIC look?  Does it share the same interface/addresses as the
> non-offload NIC, or does it create a completely separate netdevice?

We will support iWARP in the future and it should be similar to the way
we do iSCSI - using the same interface/addresses as the bnx2 NIC.

> 
> I want to make sure that whatever solution we come up with for cxgb3
> doesn't cause problems for you.  And of course if you have a better
> idea than what Steve has come up with, that would be great :)
> 

We are looking at these discussions with great interest.  If we have any
new ideas, we'll definitely let everyone know.  Thanks.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [ofa-general] InfiniBand/RDMA merge plans for 2.6.24

2007-09-14 Thread Michael Chan
On Fri, 2007-09-14 at 09:18 -0700, Roland Dreier wrote:

 However, do you have any plans to support iSCSI offload for targets?
 Also, looking at the first CNIC patch, I can't help but notice that
 you seem to have at least some support for iWARP there.  How does the
 CNIC look?  Does it share the same interface/addresses as the
 non-offload NIC, or does it create a completely separate netdevice?

We will support iWARP in the future and it should be similar to the way
we do iSCSI - using the same interface/addresses as the bnx2 NIC.

 
 I want to make sure that whatever solution we come up with for cxgb3
 doesn't cause problems for you.  And of course if you have a better
 idea than what Steve has come up with, that would be great :)
 

We are looking at these discussions with great interest.  If we have any
new ideas, we'll definitely let everyone know.  Thanks.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


<    1   2   3   >