Re: [PATCH 1/3] tg3: Limit minimum tx queue wakeup threshold
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
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()
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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...
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...
[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...
[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...
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
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
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
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
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
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
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?
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?
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.
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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/