linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/6] iwlwifi: dump stack when fail to gain access to the device
@ 2012-02-23 10:16 Stanislaw Gruszka
  2012-02-23 10:16 ` [PATCH 2/6] iwlwifi: always check if got h/w access before write Stanislaw Gruszka
                   ` (6 more replies)
  0 siblings, 7 replies; 13+ messages in thread
From: Stanislaw Gruszka @ 2012-02-23 10:16 UTC (permalink / raw)
  To: Wey-Yi Guy; +Cc: Intel Linux Wireless, linux-wireless, Stanislaw Gruszka

Print dump stack when the device is not responding. This should give
some more clue about the reason of failure. Also change the message we
print, since "MAC in deep sleep" is kinda confusing.

On the way add unlikely(), as fail to gain NIC access is hmm ...
unlikely.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-io.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-io.c b/drivers/net/wireless/iwlwifi/iwl-io.c
index e2e3b5c..71015a3 100644
--- a/drivers/net/wireless/iwlwifi/iwl-io.c
+++ b/drivers/net/wireless/iwlwifi/iwl-io.c
@@ -121,10 +121,10 @@ int iwl_grab_nic_access_silent(struct iwl_trans *trans)
 int iwl_grab_nic_access(struct iwl_trans *trans)
 {
 	int ret = iwl_grab_nic_access_silent(trans);
-	if (ret) {
+	if (unlikely(ret)) {
 		u32 val = iwl_read32(trans, CSR_GP_CNTRL);
-		IWL_ERR(trans,
-			"MAC is in deep sleep!. CSR_GP_CNTRL = 0x%08X\n", val);
+		WARN_ONCE(1, "Timeout waiting for ucode processor access "
+			     "(CSR_GP_CNTRL 0x%08x)\n", val);
 	}
 
 	return ret;
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/6] iwlwifi: always check if got h/w access before write
  2012-02-23 10:16 [PATCH 1/6] iwlwifi: dump stack when fail to gain access to the device Stanislaw Gruszka
@ 2012-02-23 10:16 ` Stanislaw Gruszka
  2012-02-23 10:16 ` [PATCH 3/6] iwlwifi: cleanup/fix memory barriers Stanislaw Gruszka
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Stanislaw Gruszka @ 2012-02-23 10:16 UTC (permalink / raw)
  To: Wey-Yi Guy; +Cc: Intel Linux Wireless, linux-wireless, Stanislaw Gruszka

Before we write to the device registers always check if
iwl_grap_nic_access() was successful.

On the way change return type of grab_nic_access() to bool, and add
likely()/unlikely() statement.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-agn-tt.c        |    2 +-
 drivers/net/wireless/iwlwifi/iwl-agn.c           |    2 +-
 drivers/net/wireless/iwlwifi/iwl-io.c            |   52 +++++++++++----------
 drivers/net/wireless/iwlwifi/iwl-io.h            |    2 +-
 drivers/net/wireless/iwlwifi/iwl-mac80211.c      |    2 +-
 drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c |    4 +-
 6 files changed, 34 insertions(+), 30 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-tt.c b/drivers/net/wireless/iwlwifi/iwl-agn-tt.c
index c728ed7..65d1454 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-tt.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-tt.c
@@ -188,7 +188,7 @@ static void iwl_tt_check_exit_ct_kill(unsigned long data)
 		}
 		iwl_read32(trans(priv), CSR_UCODE_DRV_GP1);
 		spin_lock_irqsave(&trans(priv)->reg_lock, flags);
-		if (!iwl_grab_nic_access(trans(priv)))
+		if (likely(iwl_grab_nic_access(trans(priv))))
 			iwl_release_nic_access(trans(priv));
 		spin_unlock_irqrestore(&trans(priv)->reg_lock, flags);
 
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
index 8837171..925fb71 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
@@ -329,7 +329,7 @@ static void iwl_print_cont_event_trace(struct iwl_priv *priv, u32 base,
 
 	/* Make sure device is powered up for SRAM reads */
 	spin_lock_irqsave(&trans(priv)->reg_lock, reg_flags);
-	if (iwl_grab_nic_access(trans(priv))) {
+	if (unlikely(!iwl_grab_nic_access(trans(priv)))) {
 		spin_unlock_irqrestore(&trans(priv)->reg_lock, reg_flags);
 		return;
 	}
diff --git a/drivers/net/wireless/iwlwifi/iwl-io.c b/drivers/net/wireless/iwlwifi/iwl-io.c
index 71015a3..829531a 100644
--- a/drivers/net/wireless/iwlwifi/iwl-io.c
+++ b/drivers/net/wireless/iwlwifi/iwl-io.c
@@ -118,16 +118,17 @@ int iwl_grab_nic_access_silent(struct iwl_trans *trans)
 	return 0;
 }
 
-int iwl_grab_nic_access(struct iwl_trans *trans)
+bool iwl_grab_nic_access(struct iwl_trans *trans)
 {
 	int ret = iwl_grab_nic_access_silent(trans);
 	if (unlikely(ret)) {
 		u32 val = iwl_read32(trans, CSR_GP_CNTRL);
 		WARN_ONCE(1, "Timeout waiting for ucode processor access "
 			     "(CSR_GP_CNTRL 0x%08x)\n", val);
+		return false;
 	}
 
-	return ret;
+	return true;
 }
 
 void iwl_release_nic_access(struct iwl_trans *trans)
@@ -156,7 +157,7 @@ void iwl_write_direct32(struct iwl_trans *trans, u32 reg, u32 value)
 	unsigned long flags;
 
 	spin_lock_irqsave(&trans->reg_lock, flags);
-	if (!iwl_grab_nic_access(trans)) {
+	if (likely(iwl_grab_nic_access(trans))) {
 		iwl_write32(trans, reg, value);
 		iwl_release_nic_access(trans);
 	}
@@ -211,7 +212,7 @@ void iwl_write_prph(struct iwl_trans *trans, u32 addr, u32 val)
 	unsigned long flags;
 
 	spin_lock_irqsave(&trans->reg_lock, flags);
-	if (!iwl_grab_nic_access(trans)) {
+	if (likely(iwl_grab_nic_access(trans))) {
 		__iwl_write_prph(trans, addr, val);
 		iwl_release_nic_access(trans);
 	}
@@ -223,9 +224,10 @@ void iwl_set_bits_prph(struct iwl_trans *trans, u32 reg, u32 mask)
 	unsigned long flags;
 
 	spin_lock_irqsave(&trans->reg_lock, flags);
-	iwl_grab_nic_access(trans);
-	__iwl_write_prph(trans, reg, __iwl_read_prph(trans, reg) | mask);
-	iwl_release_nic_access(trans);
+	if (likely(iwl_grab_nic_access(trans))) {
+		__iwl_write_prph(trans, reg, __iwl_read_prph(trans, reg) | mask);
+		iwl_release_nic_access(trans);
+	}
 	spin_unlock_irqrestore(&trans->reg_lock, flags);
 }
 
@@ -235,10 +237,11 @@ void iwl_set_bits_mask_prph(struct iwl_trans *trans, u32 reg,
 	unsigned long flags;
 
 	spin_lock_irqsave(&trans->reg_lock, flags);
-	iwl_grab_nic_access(trans);
-	__iwl_write_prph(trans, reg,
-			 (__iwl_read_prph(trans, reg) & mask) | bits);
-	iwl_release_nic_access(trans);
+	if (likely(iwl_grab_nic_access(trans))) {
+		__iwl_write_prph(trans, reg,
+				 (__iwl_read_prph(trans, reg) & mask) | bits);
+		iwl_release_nic_access(trans);
+	}
 	spin_unlock_irqrestore(&trans->reg_lock, flags);
 }
 
@@ -248,10 +251,11 @@ void iwl_clear_bits_prph(struct iwl_trans *trans, u32 reg, u32 mask)
 	u32 val;
 
 	spin_lock_irqsave(&trans->reg_lock, flags);
-	iwl_grab_nic_access(trans);
-	val = __iwl_read_prph(trans, reg);
-	__iwl_write_prph(trans, reg, (val & ~mask));
-	iwl_release_nic_access(trans);
+	if (likely(iwl_grab_nic_access(trans))) {
+		val = __iwl_read_prph(trans, reg);
+		__iwl_write_prph(trans, reg, (val & ~mask));
+		iwl_release_nic_access(trans);
+	}
 	spin_unlock_irqrestore(&trans->reg_lock, flags);
 }
 
@@ -263,15 +267,13 @@ void _iwl_read_targ_mem_words(struct iwl_trans *trans, u32 addr,
 	u32 *vals = buf;
 
 	spin_lock_irqsave(&trans->reg_lock, flags);
-	iwl_grab_nic_access(trans);
-
-	iwl_write32(trans, HBUS_TARG_MEM_RADDR, addr);
-	rmb();
-
-	for (offs = 0; offs < words; offs++)
-		vals[offs] = iwl_read32(trans, HBUS_TARG_MEM_RDAT);
-
-	iwl_release_nic_access(trans);
+	if (likely(iwl_grab_nic_access(trans))) {
+		iwl_write32(trans, HBUS_TARG_MEM_RADDR, addr);
+		rmb();
+		for (offs = 0; offs < words; offs++)
+			vals[offs] = iwl_read32(trans, HBUS_TARG_MEM_RDAT);
+		iwl_release_nic_access(trans);
+	}
 	spin_unlock_irqrestore(&trans->reg_lock, flags);
 }
 
@@ -292,7 +294,7 @@ int _iwl_write_targ_mem_words(struct iwl_trans *trans, u32 addr,
 	u32 *vals = buf;
 
 	spin_lock_irqsave(&trans->reg_lock, flags);
-	if (!iwl_grab_nic_access(trans)) {
+	if (likely(iwl_grab_nic_access(trans))) {
 		iwl_write32(trans, HBUS_TARG_MEM_WADDR, addr);
 		wmb();
 
diff --git a/drivers/net/wireless/iwlwifi/iwl-io.h b/drivers/net/wireless/iwlwifi/iwl-io.h
index 782486f..74f0976 100644
--- a/drivers/net/wireless/iwlwifi/iwl-io.h
+++ b/drivers/net/wireless/iwlwifi/iwl-io.h
@@ -61,7 +61,7 @@ int iwl_poll_direct_bit(struct iwl_trans *trans, u32 addr, u32 mask,
 			int timeout);
 
 int iwl_grab_nic_access_silent(struct iwl_trans *trans);
-int iwl_grab_nic_access(struct iwl_trans *trans);
+bool iwl_grab_nic_access(struct iwl_trans *trans);
 void iwl_release_nic_access(struct iwl_trans *trans);
 
 u32 iwl_read_direct32(struct iwl_trans *trans, u32 reg);
diff --git a/drivers/net/wireless/iwlwifi/iwl-mac80211.c b/drivers/net/wireless/iwlwifi/iwl-mac80211.c
index d8025fe..b6e61d2 100644
--- a/drivers/net/wireless/iwlwifi/iwl-mac80211.c
+++ b/drivers/net/wireless/iwlwifi/iwl-mac80211.c
@@ -444,7 +444,7 @@ static int iwlagn_mac_resume(struct ieee80211_hw *hw)
 	if (iwlagn_hw_valid_rtc_data_addr(base)) {
 		spin_lock_irqsave(&trans(priv)->reg_lock, flags);
 		ret = iwl_grab_nic_access_silent(trans(priv));
-		if (ret == 0) {
+		if (likely(ret == 0)) {
 			iwl_write32(trans(priv), HBUS_TARG_MEM_RADDR, base);
 			status = iwl_read32(trans(priv), HBUS_TARG_MEM_RDAT);
 			iwl_release_nic_access(trans(priv));
diff --git a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c
index 3826852..ea60584 100644
--- a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c
@@ -746,7 +746,8 @@ static int iwl_print_event_log(struct iwl_trans *trans, u32 start_idx,
 
 	/* Make sure device is powered up for SRAM reads */
 	spin_lock_irqsave(&trans->reg_lock, reg_flags);
-	iwl_grab_nic_access(trans);
+	if (unlikely(!iwl_grab_nic_access(trans)))
+		goto out_unlock;
 
 	/* Set starting address; reads will auto-increment */
 	iwl_write32(trans, HBUS_TARG_MEM_RADDR, ptr);
@@ -786,6 +787,7 @@ static int iwl_print_event_log(struct iwl_trans *trans, u32 start_idx,
 
 	/* Allow device to power down */
 	iwl_release_nic_access(trans);
+out_unlock:
 	spin_unlock_irqrestore(&trans->reg_lock, reg_flags);
 	return pos;
 }
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/6] iwlwifi: cleanup/fix memory barriers
  2012-02-23 10:16 [PATCH 1/6] iwlwifi: dump stack when fail to gain access to the device Stanislaw Gruszka
  2012-02-23 10:16 ` [PATCH 2/6] iwlwifi: always check if got h/w access before write Stanislaw Gruszka
@ 2012-02-23 10:16 ` Stanislaw Gruszka
  2012-02-23 12:22   ` Johannes Berg
  2012-02-23 10:16 ` [PATCH 4/6] iwlwifi: use writeb,writel,readl directly Stanislaw Gruszka
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Stanislaw Gruszka @ 2012-02-23 10:16 UTC (permalink / raw)
  To: Wey-Yi Guy; +Cc: Intel Linux Wireless, linux-wireless, Stanislaw Gruszka

wmb(), rmb() are not needed when writel(), readl() are used as
accessors for MMIO. We use them indirectly via iowrite32(),
ioread32().

What is needed mmiowb(), for synchronizing writes coming from
different CPUs on PCIe bridge (see in patch comments). This
fortunately is not needed on x86, where mmiowb() is just
defined as compiler barrier. As iwlwifi devices are most likely
not used on anything other than x86, this is not so important
fix.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-agn.c           |    1 -
 drivers/net/wireless/iwlwifi/iwl-io.c            |   12 +++++++-----
 drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c |    1 -
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
index 925fb71..ad8dfb9 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
@@ -336,7 +336,6 @@ static void iwl_print_cont_event_trace(struct iwl_priv *priv, u32 base,
 
 	/* Set starting address; reads will auto-increment */
 	iwl_write32(trans(priv), HBUS_TARG_MEM_RADDR, ptr);
-	rmb();
 
 	/*
 	 * Refuse to read more than would have fit into the log from
diff --git a/drivers/net/wireless/iwlwifi/iwl-io.c b/drivers/net/wireless/iwlwifi/iwl-io.c
index 829531a..493c434 100644
--- a/drivers/net/wireless/iwlwifi/iwl-io.c
+++ b/drivers/net/wireless/iwlwifi/iwl-io.c
@@ -136,6 +136,13 @@ void iwl_release_nic_access(struct iwl_trans *trans)
 	lockdep_assert_held(&trans->reg_lock);
 	__iwl_clear_bit(trans, CSR_GP_CNTRL,
 			CSR_GP_CNTRL_REG_FLAG_MAC_ACCESS_REQ);
+	/*
+	 * In above we are reading CSR_GP_CNTRL register, what will flush any
+	 * previous writes, but still want write, which clear MAC_ACCESS_REQ
+	 * bit, be performed on PCI bus before any other writes scheduled on
+	 * different CPUs (after we drop reg_lock).
+	 */
+	mmiowb();
 }
 
 u32 iwl_read_direct32(struct iwl_trans *trans, u32 reg)
@@ -182,7 +189,6 @@ int iwl_poll_direct_bit(struct iwl_trans *trans, u32 addr, u32 mask,
 static inline u32 __iwl_read_prph(struct iwl_trans *trans, u32 reg)
 {
 	iwl_write32(trans, HBUS_TARG_PRPH_RADDR, reg | (3 << 24));
-	rmb();
 	return iwl_read32(trans, HBUS_TARG_PRPH_RDAT);
 }
 
@@ -190,7 +196,6 @@ static inline void __iwl_write_prph(struct iwl_trans *trans, u32 addr, u32 val)
 {
 	iwl_write32(trans, HBUS_TARG_PRPH_WADDR,
 		    ((addr & 0x0000FFFF) | (3 << 24)));
-	wmb();
 	iwl_write32(trans, HBUS_TARG_PRPH_WDAT, val);
 }
 
@@ -269,7 +274,6 @@ void _iwl_read_targ_mem_words(struct iwl_trans *trans, u32 addr,
 	spin_lock_irqsave(&trans->reg_lock, flags);
 	if (likely(iwl_grab_nic_access(trans))) {
 		iwl_write32(trans, HBUS_TARG_MEM_RADDR, addr);
-		rmb();
 		for (offs = 0; offs < words; offs++)
 			vals[offs] = iwl_read32(trans, HBUS_TARG_MEM_RDAT);
 		iwl_release_nic_access(trans);
@@ -296,8 +300,6 @@ int _iwl_write_targ_mem_words(struct iwl_trans *trans, u32 addr,
 	spin_lock_irqsave(&trans->reg_lock, flags);
 	if (likely(iwl_grab_nic_access(trans))) {
 		iwl_write32(trans, HBUS_TARG_MEM_WADDR, addr);
-		wmb();
-
 		for (offs = 0; offs < words; offs++)
 			iwl_write32(trans, HBUS_TARG_MEM_WDAT, vals[offs]);
 		iwl_release_nic_access(trans);
diff --git a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c
index ea60584..a38a5b3 100644
--- a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c
@@ -751,7 +751,6 @@ static int iwl_print_event_log(struct iwl_trans *trans, u32 start_idx,
 
 	/* Set starting address; reads will auto-increment */
 	iwl_write32(trans, HBUS_TARG_MEM_RADDR, ptr);
-	rmb();
 
 	/* "time" is actually "data" for mode 0 (no timestamp).
 	* place event id # at far right for easier visual parsing. */
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/6] iwlwifi: use writeb,writel,readl directly
  2012-02-23 10:16 [PATCH 1/6] iwlwifi: dump stack when fail to gain access to the device Stanislaw Gruszka
  2012-02-23 10:16 ` [PATCH 2/6] iwlwifi: always check if got h/w access before write Stanislaw Gruszka
  2012-02-23 10:16 ` [PATCH 3/6] iwlwifi: cleanup/fix memory barriers Stanislaw Gruszka
@ 2012-02-23 10:16 ` Stanislaw Gruszka
  2012-02-23 12:23   ` Johannes Berg
  2012-02-23 10:16 ` [PATCH 5/6] iwlwifi: print DMA stop timeout message only if that error happened Stanislaw Gruszka
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Stanislaw Gruszka @ 2012-02-23 10:16 UTC (permalink / raw)
  To: Wey-Yi Guy; +Cc: Intel Linux Wireless, linux-wireless, Stanislaw Gruszka

That change will save us some CPU cycles at run time. Having port-based
I/O seems to be not possible for PCIe devices.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/Kconfig          |    1 +
 drivers/net/wireless/iwlwifi/iwl-trans-pcie.c |   13 ++++++-------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/Kconfig b/drivers/net/wireless/iwlwifi/Kconfig
index ae08498..404ac69 100644
--- a/drivers/net/wireless/iwlwifi/Kconfig
+++ b/drivers/net/wireless/iwlwifi/Kconfig
@@ -6,6 +6,7 @@ config IWLWIFI
 	select LEDS_CLASS
 	select LEDS_TRIGGERS
 	select MAC80211_LEDS
+	select HAS_IOMEM
 	---help---
 	  Select to build the driver supporting the:
 
diff --git a/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c b/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
index f5cb5d3..b6909b4 100644
--- a/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
+++ b/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
@@ -1583,18 +1583,17 @@ static int iwl_trans_pcie_reclaim(struct iwl_trans *trans, int sta_id, int tid,
 
 static void iwl_trans_pcie_write8(struct iwl_trans *trans, u32 ofs, u8 val)
 {
-	iowrite8(val, IWL_TRANS_GET_PCIE_TRANS(trans)->hw_base + ofs);
+	writeb(val, IWL_TRANS_GET_PCIE_TRANS(trans)->hw_base + ofs);
 }
 
 static void iwl_trans_pcie_write32(struct iwl_trans *trans, u32 ofs, u32 val)
 {
-	iowrite32(val, IWL_TRANS_GET_PCIE_TRANS(trans)->hw_base + ofs);
+	writel(val, IWL_TRANS_GET_PCIE_TRANS(trans)->hw_base + ofs);
 }
 
 static u32 iwl_trans_pcie_read32(struct iwl_trans *trans, u32 ofs)
 {
-	u32 val = ioread32(IWL_TRANS_GET_PCIE_TRANS(trans)->hw_base + ofs);
-	return val;
+	return readl(IWL_TRANS_GET_PCIE_TRANS(trans)->hw_base + ofs);
 }
 
 static void iwl_trans_pcie_free(struct iwl_trans *trans)
@@ -1613,7 +1612,7 @@ static void iwl_trans_pcie_free(struct iwl_trans *trans)
 	}
 
 	pci_disable_msi(trans_pcie->pci_dev);
-	pci_iounmap(trans_pcie->pci_dev, trans_pcie->hw_base);
+	iounmap(trans_pcie->hw_base);
 	pci_release_regions(trans_pcie->pci_dev);
 	pci_disable_device(trans_pcie->pci_dev);
 
@@ -2294,9 +2293,9 @@ struct iwl_trans *iwl_trans_pcie_alloc(struct iwl_shared *shrd,
 		goto out_pci_disable_device;
 	}
 
-	trans_pcie->hw_base = pci_iomap(pdev, 0, 0);
+	trans_pcie->hw_base = pci_ioremap_bar(pdev, 0);
 	if (!trans_pcie->hw_base) {
-		dev_printk(KERN_ERR, &pdev->dev, "pci_iomap failed");
+		dev_printk(KERN_ERR, &pdev->dev, "pci_ioremap_bar failed");
 		err = -ENODEV;
 		goto out_pci_release_regions;
 	}
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 5/6] iwlwifi: print DMA stop timeout message only if that error happened
  2012-02-23 10:16 [PATCH 1/6] iwlwifi: dump stack when fail to gain access to the device Stanislaw Gruszka
                   ` (2 preceding siblings ...)
  2012-02-23 10:16 ` [PATCH 4/6] iwlwifi: use writeb,writel,readl directly Stanislaw Gruszka
@ 2012-02-23 10:16 ` Stanislaw Gruszka
  2012-02-23 10:16 ` [PATCH 6/6] iwlwifi: reintroduce iwl_enable_rfkill_int Stanislaw Gruszka
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Stanislaw Gruszka @ 2012-02-23 10:16 UTC (permalink / raw)
  To: Wey-Yi Guy; +Cc: Intel Linux Wireless, linux-wireless, Stanislaw Gruszka

iwl_poll_direct_bit() return negative error value on timeout, positive
values does not indicate an error.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-trans-pcie.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c b/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
index b6909b4..a0e716f 100644
--- a/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
+++ b/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
@@ -1209,7 +1209,7 @@ static void iwl_trans_pcie_fw_alive(struct iwl_trans *trans)
  */
 static int iwl_trans_tx_stop(struct iwl_trans *trans)
 {
-	int ch, txq_id;
+	int ch, txq_id, ret;
 	unsigned long flags;
 	struct iwl_trans_pcie *trans_pcie = IWL_TRANS_GET_PCIE_TRANS(trans);
 
@@ -1222,9 +1222,10 @@ static int iwl_trans_tx_stop(struct iwl_trans *trans)
 	for (ch = 0; ch < FH_TCSR_CHNL_NUM; ch++) {
 		iwl_write_direct32(trans,
 				   FH_TCSR_CHNL_TX_CONFIG_REG(ch), 0x0);
-		if (iwl_poll_direct_bit(trans, FH_TSSR_TX_STATUS_REG,
+		ret = iwl_poll_direct_bit(trans, FH_TSSR_TX_STATUS_REG,
 				    FH_TSSR_TX_STATUS_REG_MSK_CHNL_IDLE(ch),
-				    1000))
+				    1000);
+		if (ret < 0)
 			IWL_ERR(trans, "Failing on timeout while stopping"
 			    " DMA channel %d [0x%08x]", ch,
 			    iwl_read_direct32(trans,
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 6/6] iwlwifi: reintroduce iwl_enable_rfkill_int
  2012-02-23 10:16 [PATCH 1/6] iwlwifi: dump stack when fail to gain access to the device Stanislaw Gruszka
                   ` (3 preceding siblings ...)
  2012-02-23 10:16 ` [PATCH 5/6] iwlwifi: print DMA stop timeout message only if that error happened Stanislaw Gruszka
@ 2012-02-23 10:16 ` Stanislaw Gruszka
  2012-02-23 12:22 ` [PATCH 1/6] iwlwifi: dump stack when fail to gain access to the device Johannes Berg
  2012-02-23 14:55 ` Guy, Wey-Yi
  6 siblings, 0 replies; 13+ messages in thread
From: Stanislaw Gruszka @ 2012-02-23 10:16 UTC (permalink / raw)
  To: Wey-Yi Guy; +Cc: Intel Linux Wireless, linux-wireless, Stanislaw Gruszka

If device is disabled by rfkill switch, do not enable all interrupts,
but only CSR_INT_BIT_RF_KILL to receive rfkill state change. Unblocking
other interrupts might cause problems, since driver can not be prepared
for receive them.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-trans-pcie-int.h |    6 ++++++
 drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c  |    6 ++----
 drivers/net/wireless/iwlwifi/iwl-trans-pcie.c     |   14 +++++++-------
 3 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-int.h b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-int.h
index 561865f..d5f3d2f 100644
--- a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-int.h
+++ b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-int.h
@@ -338,6 +338,12 @@ static inline void iwl_enable_interrupts(struct iwl_trans *trans)
 	iwl_write32(trans, CSR_INT_MASK, trans_pcie->inta_mask);
 }
 
+static inline void iwl_enable_rfkill_int(struct iwl_trans *trans)
+{
+	IWL_DEBUG_ISR(trans, "Enabling rfkill interrupt\n");
+	iwl_write32(trans, CSR_INT_MASK, CSR_INT_BIT_RF_KILL);
+}
+
 /*
  * we have 8 bits used like this:
  *
diff --git a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c
index a38a5b3..4175cb8 100644
--- a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c
@@ -1150,10 +1150,8 @@ void iwl_irq_tasklet(struct iwl_trans *trans)
 	if (test_bit(STATUS_INT_ENABLED, &trans->shrd->status))
 		iwl_enable_interrupts(trans);
 	/* Re-enable RF_KILL if it occurred */
-	else if (handled & CSR_INT_BIT_RF_KILL) {
-		IWL_DEBUG_ISR(trans, "Enabling rfkill interrupt\n");
-		iwl_write32(trans, CSR_INT_MASK, CSR_INT_BIT_RF_KILL);
-	}
+	else if (handled & CSR_INT_BIT_RF_KILL)
+		iwl_enable_rfkill_int(trans);
 }
 
 /******************************************************************************
diff --git a/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c b/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
index a0e716f..33f119a 100644
--- a/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
+++ b/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
@@ -1045,7 +1045,7 @@ static int iwl_trans_pcie_start_fw(struct iwl_trans *trans, struct fw_img *fw)
 
 	if (iwl_is_rfkill(trans->shrd)) {
 		iwl_set_hw_rfkill_state(priv(trans), true);
-		iwl_enable_interrupts(trans);
+		iwl_enable_rfkill_int(trans);
 		return -ERFKILL;
 	}
 
@@ -1538,8 +1538,7 @@ static void iwl_trans_pcie_stop_hw(struct iwl_trans *trans)
 	iwl_write32(trans, CSR_INT, 0xFFFFFFFF);
 
 	/* Even if we stop the HW, we still want the RF kill interrupt */
-	IWL_DEBUG_ISR(trans, "Enabling rfkill interrupt\n");
-	iwl_write32(trans, CSR_INT_MASK, CSR_INT_BIT_RF_KILL);
+	iwl_enable_rfkill_int(trans);
 }
 
 static int iwl_trans_pcie_reclaim(struct iwl_trans *trans, int sta_id, int tid,
@@ -1650,16 +1649,17 @@ static int iwl_trans_pcie_resume(struct iwl_trans *trans)
 {
 	bool hw_rfkill = false;
 
-	iwl_enable_interrupts(trans);
-
 	if (!(iwl_read32(trans, CSR_GP_CNTRL) &
 				CSR_GP_CNTRL_REG_FLAG_HW_RF_KILL_SW))
 		hw_rfkill = true;
 
-	if (hw_rfkill)
+	if (hw_rfkill) {
 		set_bit(STATUS_RF_KILL_HW, &trans->shrd->status);
-	else
+		iwl_enable_rfkill_int(trans);
+	} else {
 		clear_bit(STATUS_RF_KILL_HW, &trans->shrd->status);
+		iwl_enable_interrupts(trans);
+	}
 
 	iwl_set_hw_rfkill_state(priv(trans), hw_rfkill);
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/6] iwlwifi: dump stack when fail to gain access to the device
  2012-02-23 10:16 [PATCH 1/6] iwlwifi: dump stack when fail to gain access to the device Stanislaw Gruszka
                   ` (4 preceding siblings ...)
  2012-02-23 10:16 ` [PATCH 6/6] iwlwifi: reintroduce iwl_enable_rfkill_int Stanislaw Gruszka
@ 2012-02-23 12:22 ` Johannes Berg
  2012-02-23 14:55 ` Guy, Wey-Yi
  6 siblings, 0 replies; 13+ messages in thread
From: Johannes Berg @ 2012-02-23 12:22 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Wey-Yi Guy, Intel Linux Wireless, linux-wireless

Thanks Stanislaw.

As these patches conflict a bit with some internal work we have pending,
I've picked them up. We'll post them back as soon as possible.

johannes


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/6] iwlwifi: cleanup/fix memory barriers
  2012-02-23 10:16 ` [PATCH 3/6] iwlwifi: cleanup/fix memory barriers Stanislaw Gruszka
@ 2012-02-23 12:22   ` Johannes Berg
  0 siblings, 0 replies; 13+ messages in thread
From: Johannes Berg @ 2012-02-23 12:22 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Wey-Yi Guy, Intel Linux Wireless, linux-wireless

On Thu, 2012-02-23 at 11:16 +0100, Stanislaw Gruszka wrote:

>  	__iwl_clear_bit(trans, CSR_GP_CNTRL,
>  			CSR_GP_CNTRL_REG_FLAG_MAC_ACCESS_REQ);
> +	/*
> +	 * In above we are reading CSR_GP_CNTRL register, what will flush any
> +	 * previous writes, but still want write, which clear MAC_ACCESS_REQ
> +	 * bit, be performed on PCI bus before any other writes scheduled on
> +	 * different CPUs (after we drop reg_lock).
> +	 */
> +	mmiowb();
>  }

I changed this comment to the, imho, more readable

        /*
         * Above we read the CSR_GP_CNTRL register, which will flush
         * any previous writes, but we need the write that clears the
         * MAC_ACCESS_REQ bit to be performed before any other writes
         * scheduled on different CPUs (after we drop reg_lock).
         */

johannes


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/6] iwlwifi: use writeb,writel,readl directly
  2012-02-23 10:16 ` [PATCH 4/6] iwlwifi: use writeb,writel,readl directly Stanislaw Gruszka
@ 2012-02-23 12:23   ` Johannes Berg
  0 siblings, 0 replies; 13+ messages in thread
From: Johannes Berg @ 2012-02-23 12:23 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Wey-Yi Guy, Intel Linux Wireless, linux-wireless

On Thu, 2012-02-23 at 11:16 +0100, Stanislaw Gruszka wrote:
> That change will save us some CPU cycles at run time. Having port-based
> I/O seems to be not possible for PCIe devices.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/iwlwifi/Kconfig          |    1 +
>  drivers/net/wireless/iwlwifi/iwl-trans-pcie.c |   13 ++++++-------
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/wireless/iwlwifi/Kconfig b/drivers/net/wireless/iwlwifi/Kconfig
> index ae08498..404ac69 100644
> --- a/drivers/net/wireless/iwlwifi/Kconfig
> +++ b/drivers/net/wireless/iwlwifi/Kconfig
> @@ -6,6 +6,7 @@ config IWLWIFI
>  	select LEDS_CLASS
>  	select LEDS_TRIGGERS
>  	select MAC80211_LEDS
> +	select HAS_IOMEM

According to every other user of HAS_IOMEM, this should be
	depends on HAS_IOMEM

I've made the change.

johannes


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/6] iwlwifi: dump stack when fail to gain access to the device
  2012-02-23 10:16 [PATCH 1/6] iwlwifi: dump stack when fail to gain access to the device Stanislaw Gruszka
                   ` (5 preceding siblings ...)
  2012-02-23 12:22 ` [PATCH 1/6] iwlwifi: dump stack when fail to gain access to the device Johannes Berg
@ 2012-02-23 14:55 ` Guy, Wey-Yi
  2012-02-24  9:32   ` Stanislaw Gruszka
  6 siblings, 1 reply; 13+ messages in thread
From: Guy, Wey-Yi @ 2012-02-23 14:55 UTC (permalink / raw)
  To: Stanislaw Gruszka, linville; +Cc: Intel Linux Wireless, linux-wireless

Hi Stanislaw,

We are doing major driver re-factor works internally, I understand your
patches are important and these patches improve the quality of the
driver. I just wonder if it is ok for you to allow us to merge your
patches into our internal development tree first, then I will push these
patches upstream for you. By doing so, we can make sure your patches can
fully integrated with our on-going driver re-factor work and not cause
any conflict.

Linville, would it be ok for you?
 
Thanks in advance
Wey


On Thu, 2012-02-23 at 11:16 +0100, Stanislaw Gruszka wrote:
> Print dump stack when the device is not responding. This should give
> some more clue about the reason of failure. Also change the message we
> print, since "MAC in deep sleep" is kinda confusing.
> 
> On the way add unlikely(), as fail to gain NIC access is hmm ...
> unlikely.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl-io.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/iwlwifi/iwl-io.c b/drivers/net/wireless/iwlwifi/iwl-io.c
> index e2e3b5c..71015a3 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-io.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-io.c
> @@ -121,10 +121,10 @@ int iwl_grab_nic_access_silent(struct iwl_trans *trans)
>  int iwl_grab_nic_access(struct iwl_trans *trans)
>  {
>  	int ret = iwl_grab_nic_access_silent(trans);
> -	if (ret) {
> +	if (unlikely(ret)) {
>  		u32 val = iwl_read32(trans, CSR_GP_CNTRL);
> -		IWL_ERR(trans,
> -			"MAC is in deep sleep!. CSR_GP_CNTRL = 0x%08X\n", val);
> +		WARN_ONCE(1, "Timeout waiting for ucode processor access "
> +			     "(CSR_GP_CNTRL 0x%08x)\n", val);
>  	}
>  
>  	return ret;



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/6] iwlwifi: dump stack when fail to gain access to the device
  2012-02-23 14:55 ` Guy, Wey-Yi
@ 2012-02-24  9:32   ` Stanislaw Gruszka
  2012-02-24 14:45     ` Guy, Wey-Yi
  0 siblings, 1 reply; 13+ messages in thread
From: Stanislaw Gruszka @ 2012-02-24  9:32 UTC (permalink / raw)
  To: Guy, Wey-Yi; +Cc: linville, Intel Linux Wireless, linux-wireless

Hi

On Thu, Feb 23, 2012 at 06:55:17AM -0800, Guy, Wey-Yi wrote:
> We are doing major driver re-factor works internally, I understand your
> patches are important and these patches improve the quality of the
> driver. I just wonder if it is ok for you to allow us to merge your
> patches into our internal development tree first, then I will push these
> patches upstream for you. By doing so, we can make sure your patches can
> fully integrated with our on-going driver re-factor work and not cause
> any conflict.
No problem with that.

Thanks
Stanislaw

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/6] iwlwifi: dump stack when fail to gain access to the device
  2012-02-24  9:32   ` Stanislaw Gruszka
@ 2012-02-24 14:45     ` Guy, Wey-Yi
  2012-02-24 16:01       ` Johannes Berg
  0 siblings, 1 reply; 13+ messages in thread
From: Guy, Wey-Yi @ 2012-02-24 14:45 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: linville, Intel Linux Wireless, linux-wireless

Hi Stanislaw,

On Fri, 2012-02-24 at 10:32 +0100, Stanislaw Gruszka wrote:
> Hi
> 
> On Thu, Feb 23, 2012 at 06:55:17AM -0800, Guy, Wey-Yi wrote:
> > We are doing major driver re-factor works internally, I understand your
> > patches are important and these patches improve the quality of the
> > driver. I just wonder if it is ok for you to allow us to merge your
> > patches into our internal development tree first, then I will push these
> > patches upstream for you. By doing so, we can make sure your patches can
> > fully integrated with our on-going driver re-factor work and not cause
> > any conflict.
> No problem with that.
> 
Thank you so much for the understanding, now is critical time for us, we
are doing huge amount of re-factor works and I will like to make it as
smooth and as quick as possible.

btw, are you going to send another updated version of patches set to
address Johannes's comments?

Best Regards
Wey



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/6] iwlwifi: dump stack when fail to gain access to the device
  2012-02-24 14:45     ` Guy, Wey-Yi
@ 2012-02-24 16:01       ` Johannes Berg
  0 siblings, 0 replies; 13+ messages in thread
From: Johannes Berg @ 2012-02-24 16:01 UTC (permalink / raw)
  To: Guy, Wey-Yi
  Cc: Stanislaw Gruszka, linville, Intel Linux Wireless, linux-wireless

On Fri, 2012-02-24 at 06:45 -0800, Guy, Wey-Yi wrote:
> Hi Stanislaw,
> 
> On Fri, 2012-02-24 at 10:32 +0100, Stanislaw Gruszka wrote:
> > Hi
> > 
> > On Thu, Feb 23, 2012 at 06:55:17AM -0800, Guy, Wey-Yi wrote:
> > > We are doing major driver re-factor works internally, I understand your
> > > patches are important and these patches improve the quality of the
> > > driver. I just wonder if it is ok for you to allow us to merge your
> > > patches into our internal development tree first, then I will push these
> > > patches upstream for you. By doing so, we can make sure your patches can
> > > fully integrated with our on-going driver re-factor work and not cause
> > > any conflict.
> > No problem with that.
> > 
> Thank you so much for the understanding, now is critical time for us, we
> are doing huge amount of re-factor works and I will like to make it as
> smooth and as quick as possible.
> 
> btw, are you going to send another updated version of patches set to
> address Johannes's comments?

No need, I fixed these issues already.

johannes


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2012-02-24 16:01 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-23 10:16 [PATCH 1/6] iwlwifi: dump stack when fail to gain access to the device Stanislaw Gruszka
2012-02-23 10:16 ` [PATCH 2/6] iwlwifi: always check if got h/w access before write Stanislaw Gruszka
2012-02-23 10:16 ` [PATCH 3/6] iwlwifi: cleanup/fix memory barriers Stanislaw Gruszka
2012-02-23 12:22   ` Johannes Berg
2012-02-23 10:16 ` [PATCH 4/6] iwlwifi: use writeb,writel,readl directly Stanislaw Gruszka
2012-02-23 12:23   ` Johannes Berg
2012-02-23 10:16 ` [PATCH 5/6] iwlwifi: print DMA stop timeout message only if that error happened Stanislaw Gruszka
2012-02-23 10:16 ` [PATCH 6/6] iwlwifi: reintroduce iwl_enable_rfkill_int Stanislaw Gruszka
2012-02-23 12:22 ` [PATCH 1/6] iwlwifi: dump stack when fail to gain access to the device Johannes Berg
2012-02-23 14:55 ` Guy, Wey-Yi
2012-02-24  9:32   ` Stanislaw Gruszka
2012-02-24 14:45     ` Guy, Wey-Yi
2012-02-24 16:01       ` Johannes Berg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).