Re: [RFC 1/4] iwlwifi: dump stack when fail to gain access to the device

Re: [RFC 1/4] iwlwifi: dump stack when fail to gain access to the device

[wwguy]

On Mon, 2012-02-06 at 17:09 +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 83fdff3..ce6d9c1 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-io.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-io.c
> @@ -120,10 +120,10 @@ int iwl_grab_nic_access_silent(struct iwl_bus *bus)
>  int iwl_grab_nic_access(struct iwl_bus *bus)
>  {
>  	int ret = iwl_grab_nic_access_silent(bus);
> -	if (ret) {
> +	if (unlikely(ret)) {
>  		u32 val = iwl_read32(bus, CSR_GP_CNTRL);
> -		IWL_ERR(bus,
> -			"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);
>  	}
>  
I agree the message and check make sense, but not sure what new clue we
got by doing this?

Wey

[RFC 1/4] iwlwifi: dump stack when fail to gain access to the device

[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 83fdff3..ce6d9c1 100644
--- a/drivers/net/wireless/iwlwifi/iwl-io.c
+++ b/drivers/net/wireless/iwlwifi/iwl-io.c
@@ -120,10 +120,10 @@ int iwl_grab_nic_access_silent(struct iwl_bus *bus)
 int iwl_grab_nic_access(struct iwl_bus *bus)
 {
 	int ret = iwl_grab_nic_access_silent(bus);
-	if (ret) {
+	if (unlikely(ret)) {
 		u32 val = iwl_read32(bus, CSR_GP_CNTRL);
-		IWL_ERR(bus,
-			"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

[RFC 2/4] iwlwifi: always check if got h/w access before write

[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 56c6def..e9a0e93 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(bus(priv), CSR_UCODE_DRV_GP1);
 		spin_lock_irqsave(&bus(priv)->reg_lock, flags);
-		if (!iwl_grab_nic_access(bus(priv)))
+		if (likely(iwl_grab_nic_access(bus(priv))))
 			iwl_release_nic_access(bus(priv));
 		spin_unlock_irqrestore(&bus(priv)->reg_lock, flags);
 
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
index 90315c6..2642bf9 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(&bus(priv)->reg_lock, reg_flags);
-	if (iwl_grab_nic_access(bus(priv))) {
+	if (unlikely(!iwl_grab_nic_access(bus(priv)))) {
 		spin_unlock_irqrestore(&bus(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 ce6d9c1..b5d0bcb 100644
--- a/drivers/net/wireless/iwlwifi/iwl-io.c
+++ b/drivers/net/wireless/iwlwifi/iwl-io.c
@@ -117,16 +117,17 @@ int iwl_grab_nic_access_silent(struct iwl_bus *bus)
 	return 0;
 }
 
-int iwl_grab_nic_access(struct iwl_bus *bus)
+bool iwl_grab_nic_access(struct iwl_bus *bus)
 {
 	int ret = iwl_grab_nic_access_silent(bus);
 	if (unlikely(ret)) {
 		u32 val = iwl_read32(bus, 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_bus *bus)
@@ -155,7 +156,7 @@ void iwl_write_direct32(struct iwl_bus *bus, u32 reg, u32 value)
 	unsigned long flags;
 
 	spin_lock_irqsave(&bus->reg_lock, flags);
-	if (!iwl_grab_nic_access(bus)) {
+	if (likely(iwl_grab_nic_access(bus))) {
 		iwl_write32(bus, reg, value);
 		iwl_release_nic_access(bus);
 	}
@@ -210,7 +211,7 @@ void iwl_write_prph(struct iwl_bus *bus, u32 addr, u32 val)
 	unsigned long flags;
 
 	spin_lock_irqsave(&bus->reg_lock, flags);
-	if (!iwl_grab_nic_access(bus)) {
+	if (likely(iwl_grab_nic_access(bus))) {
 		__iwl_write_prph(bus, addr, val);
 		iwl_release_nic_access(bus);
 	}
@@ -222,9 +223,10 @@ void iwl_set_bits_prph(struct iwl_bus *bus, u32 reg, u32 mask)
 	unsigned long flags;
 
 	spin_lock_irqsave(&bus->reg_lock, flags);
-	iwl_grab_nic_access(bus);
-	__iwl_write_prph(bus, reg, __iwl_read_prph(bus, reg) | mask);
-	iwl_release_nic_access(bus);
+	if (likely(iwl_grab_nic_access(bus))) {
+		__iwl_write_prph(bus, reg, __iwl_read_prph(bus, reg) | mask);
+		iwl_release_nic_access(bus);
+	}
 	spin_unlock_irqrestore(&bus->reg_lock, flags);
 }
 
@@ -234,10 +236,11 @@ void iwl_set_bits_mask_prph(struct iwl_bus *bus, u32 reg,
 	unsigned long flags;
 
 	spin_lock_irqsave(&bus->reg_lock, flags);
-	iwl_grab_nic_access(bus);
-	__iwl_write_prph(bus, reg,
-			 (__iwl_read_prph(bus, reg) & mask) | bits);
-	iwl_release_nic_access(bus);
+	if (likely(iwl_grab_nic_access(bus))) {
+		__iwl_write_prph(bus, reg,
+				 (__iwl_read_prph(bus, reg) & mask) | bits);
+		iwl_release_nic_access(bus);
+	}
 	spin_unlock_irqrestore(&bus->reg_lock, flags);
 }
 
@@ -247,10 +250,11 @@ void iwl_clear_bits_prph(struct iwl_bus *bus, u32 reg, u32 mask)
 	u32 val;
 
 	spin_lock_irqsave(&bus->reg_lock, flags);
-	iwl_grab_nic_access(bus);
-	val = __iwl_read_prph(bus, reg);
-	__iwl_write_prph(bus, reg, (val & ~mask));
-	iwl_release_nic_access(bus);
+	if (likely(iwl_grab_nic_access(bus))) {
+		val = __iwl_read_prph(bus, reg);
+		__iwl_write_prph(bus, reg, (val & ~mask));
+		iwl_release_nic_access(bus);
+	}
 	spin_unlock_irqrestore(&bus->reg_lock, flags);
 }
 
@@ -262,15 +266,13 @@ void _iwl_read_targ_mem_words(struct iwl_bus *bus, u32 addr,
 	u32 *vals = buf;
 
 	spin_lock_irqsave(&bus->reg_lock, flags);
-	iwl_grab_nic_access(bus);
-
-	iwl_write32(bus, HBUS_TARG_MEM_RADDR, addr);
-	rmb();
-
-	for (offs = 0; offs < words; offs++)
-		vals[offs] = iwl_read32(bus, HBUS_TARG_MEM_RDAT);
-
-	iwl_release_nic_access(bus);
+	if (likely(iwl_grab_nic_access(bus))) {
+		iwl_write32(bus, HBUS_TARG_MEM_RADDR, addr);
+		rmb();
+		for (offs = 0; offs < words; offs++)
+			vals[offs] = iwl_read32(bus, HBUS_TARG_MEM_RDAT);
+		iwl_release_nic_access(bus);
+	}
 	spin_unlock_irqrestore(&bus->reg_lock, flags);
 }
 
@@ -291,7 +293,7 @@ int _iwl_write_targ_mem_words(struct iwl_bus *bus, u32 addr,
 	u32 *vals = buf;
 
 	spin_lock_irqsave(&bus->reg_lock, flags);
-	if (!iwl_grab_nic_access(bus)) {
+	if (likely(iwl_grab_nic_access(bus))) {
 		iwl_write32(bus, 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 427d065..f4763d8 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_bus *bus, u32 addr, u32 mask,
 			int timeout);
 
 int iwl_grab_nic_access_silent(struct iwl_bus *bus);
-int iwl_grab_nic_access(struct iwl_bus *bus);
+bool iwl_grab_nic_access(struct iwl_bus *bus);
 void iwl_release_nic_access(struct iwl_bus *bus);
 
 u32 iwl_read_direct32(struct iwl_bus *bus, u32 reg);
diff --git a/drivers/net/wireless/iwlwifi/iwl-mac80211.c b/drivers/net/wireless/iwlwifi/iwl-mac80211.c
index 965d047..218071a 100644
--- a/drivers/net/wireless/iwlwifi/iwl-mac80211.c
+++ b/drivers/net/wireless/iwlwifi/iwl-mac80211.c
@@ -443,7 +443,7 @@ static int iwlagn_mac_resume(struct ieee80211_hw *hw)
 	if (iwlagn_hw_valid_rtc_data_addr(base)) {
 		spin_lock_irqsave(&bus(priv)->reg_lock, flags);
 		ret = iwl_grab_nic_access_silent(bus(priv));
-		if (ret == 0) {
+		if (likely(ret == 0)) {
 			iwl_write32(bus(priv), HBUS_TARG_MEM_RADDR, base);
 			status = iwl_read32(bus(priv), HBUS_TARG_MEM_RDAT);
 			iwl_release_nic_access(bus(priv));
diff --git a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c
index 2900db9..7b7899d 100644
--- a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c
@@ -742,7 +742,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(&bus(trans)->reg_lock, reg_flags);
-	iwl_grab_nic_access(bus(trans));
+	if (unlikely(!iwl_grab_nic_access(bus(trans))))
+		goto out_unlock;
 
 	/* Set starting address; reads will auto-increment */
 	iwl_write32(bus(trans), HBUS_TARG_MEM_RADDR, ptr);
@@ -782,6 +783,7 @@ static int iwl_print_event_log(struct iwl_trans *trans, u32 start_idx,
 
 	/* Allow device to power down */
 	iwl_release_nic_access(bus(trans));
+out_unlock:
 	spin_unlock_irqrestore(&bus(trans)->reg_lock, reg_flags);
 	return pos;
 }
-- 
1.7.1

[RFC 3/4] iwlwifi: cleanup/fix memory barriers

[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 2642bf9..6636149 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(bus(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 b5d0bcb..111182b 100644
--- a/drivers/net/wireless/iwlwifi/iwl-io.c
+++ b/drivers/net/wireless/iwlwifi/iwl-io.c
@@ -135,6 +135,13 @@ void iwl_release_nic_access(struct iwl_bus *bus)
 	lockdep_assert_held(&bus->reg_lock);
 	__iwl_clear_bit(bus, 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 to 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_bus *bus, u32 reg)
@@ -181,7 +188,6 @@ int iwl_poll_direct_bit(struct iwl_bus *bus, u32 addr, u32 mask,
 static inline u32 __iwl_read_prph(struct iwl_bus *bus, u32 reg)
 {
 	iwl_write32(bus, HBUS_TARG_PRPH_RADDR, reg | (3 << 24));
-	rmb();
 	return iwl_read32(bus, HBUS_TARG_PRPH_RDAT);
 }
 
@@ -189,7 +195,6 @@ static inline void __iwl_write_prph(struct iwl_bus *bus, u32 addr, u32 val)
 {
 	iwl_write32(bus, HBUS_TARG_PRPH_WADDR,
 		    ((addr & 0x0000FFFF) | (3 << 24)));
-	wmb();
 	iwl_write32(bus, HBUS_TARG_PRPH_WDAT, val);
 }
 
@@ -268,7 +273,6 @@ void _iwl_read_targ_mem_words(struct iwl_bus *bus, u32 addr,
 	spin_lock_irqsave(&bus->reg_lock, flags);
 	if (likely(iwl_grab_nic_access(bus))) {
 		iwl_write32(bus, HBUS_TARG_MEM_RADDR, addr);
-		rmb();
 		for (offs = 0; offs < words; offs++)
 			vals[offs] = iwl_read32(bus, HBUS_TARG_MEM_RDAT);
 		iwl_release_nic_access(bus);
@@ -295,8 +299,6 @@ int _iwl_write_targ_mem_words(struct iwl_bus *bus, u32 addr,
 	spin_lock_irqsave(&bus->reg_lock, flags);
 	if (likely(iwl_grab_nic_access(bus))) {
 		iwl_write32(bus, HBUS_TARG_MEM_WADDR, addr);
-		wmb();
-
 		for (offs = 0; offs < words; offs++)
 			iwl_write32(bus, HBUS_TARG_MEM_WDAT, vals[offs]);
 		iwl_release_nic_access(bus);
diff --git a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c
index 7b7899d..3564b86 100644
--- a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c
@@ -747,7 +747,6 @@ static int iwl_print_event_log(struct iwl_trans *trans, u32 start_idx,
 
 	/* Set starting address; reads will auto-increment */
 	iwl_write32(bus(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

[RFC 4/4] iwlwifi: use writeb,writel,readl directly

[Stanislaw Gruszka]

That will save us some CPU cycles at run time.

Obviously I do not think port-based IO is possible, but I added
check for that during probe just in case.

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

diff --git a/drivers/net/wireless/iwlwifi/iwl-pci.c b/drivers/net/wireless/iwlwifi/iwl-pci.c
index 03702a2..32b326d 100644
--- a/drivers/net/wireless/iwlwifi/iwl-pci.c
+++ b/drivers/net/wireless/iwlwifi/iwl-pci.c
@@ -153,18 +153,17 @@ static u32 iwl_pci_get_hw_id(struct iwl_bus *bus)
 
 static void iwl_pci_write8(struct iwl_bus *bus, u32 ofs, u8 val)
 {
-	iowrite8(val, IWL_BUS_GET_PCI_BUS(bus)->hw_base + ofs);
+	writeb(val, IWL_BUS_GET_PCI_BUS(bus)->hw_base + ofs);
 }
 
 static void iwl_pci_write32(struct iwl_bus *bus, u32 ofs, u32 val)
 {
-	iowrite32(val, IWL_BUS_GET_PCI_BUS(bus)->hw_base + ofs);
+	writel(val, IWL_BUS_GET_PCI_BUS(bus)->hw_base + ofs);
 }
 
 static u32 iwl_pci_read32(struct iwl_bus *bus, u32 ofs)
 {
-	u32 val = ioread32(IWL_BUS_GET_PCI_BUS(bus)->hw_base + ofs);
-	return val;
+	return readl(IWL_BUS_GET_PCI_BUS(bus)->hw_base + ofs);
 }
 
 static const struct iwl_bus_ops bus_ops_pci = {
@@ -417,6 +416,11 @@ static int iwl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto out_pci_disable_device;
 	}
 
+	if (pci_resource_flags(pdev, 0) & IORESOURCE_IO) {
+		dev_printk(KERN_ERR, bus->dev, "not supported I/O method");
+		goto out_pci_disable_device;
+	}
+
 	pci_bus->hw_base = pci_iomap(pdev, 0, 0);
 	if (!pci_bus->hw_base) {
 		dev_printk(KERN_ERR, bus->dev, "pci_iomap failed");
-- 
1.7.1

Re: [RFC 2/4] iwlwifi: always check if got h/w access before write

[wwguy]

On Mon, 2012-02-06 at 17:09 +0100, Stanislaw Gruszka wrote:
> 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 56c6def..e9a0e93 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(bus(priv), CSR_UCODE_DRV_GP1);
>  		spin_lock_irqsave(&bus(priv)->reg_lock, flags);
> -		if (!iwl_grab_nic_access(bus(priv)))
> +		if (likely(iwl_grab_nic_access(bus(priv))))
>  			iwl_release_nic_access(bus(priv));
>  		spin_unlock_irqrestore(&bus(priv)->reg_lock, flags);
>  
> diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
> index 90315c6..2642bf9 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(&bus(priv)->reg_lock, reg_flags);
> -	if (iwl_grab_nic_access(bus(priv))) {
> +	if (unlikely(!iwl_grab_nic_access(bus(priv)))) {
>  		spin_unlock_irqrestore(&bus(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 ce6d9c1..b5d0bcb 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-io.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-io.c
> @@ -117,16 +117,17 @@ int iwl_grab_nic_access_silent(struct iwl_bus *bus)
>  	return 0;
>  }
>  
> -int iwl_grab_nic_access(struct iwl_bus *bus)
> +bool iwl_grab_nic_access(struct iwl_bus *bus)
>  {
>  	int ret = iwl_grab_nic_access_silent(bus);
>  	if (unlikely(ret)) {
>  		u32 val = iwl_read32(bus, 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_bus *bus)
> @@ -155,7 +156,7 @@ void iwl_write_direct32(struct iwl_bus *bus, u32 reg, u32 value)
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&bus->reg_lock, flags);
> -	if (!iwl_grab_nic_access(bus)) {
> +	if (likely(iwl_grab_nic_access(bus))) {
>  		iwl_write32(bus, reg, value);
>  		iwl_release_nic_access(bus);
>  	}
> @@ -210,7 +211,7 @@ void iwl_write_prph(struct iwl_bus *bus, u32 addr, u32 val)
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&bus->reg_lock, flags);
> -	if (!iwl_grab_nic_access(bus)) {
> +	if (likely(iwl_grab_nic_access(bus))) {
>  		__iwl_write_prph(bus, addr, val);
>  		iwl_release_nic_access(bus);
>  	}
> @@ -222,9 +223,10 @@ void iwl_set_bits_prph(struct iwl_bus *bus, u32 reg, u32 mask)
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&bus->reg_lock, flags);
> -	iwl_grab_nic_access(bus);
> -	__iwl_write_prph(bus, reg, __iwl_read_prph(bus, reg) | mask);
> -	iwl_release_nic_access(bus);
> +	if (likely(iwl_grab_nic_access(bus))) {
> +		__iwl_write_prph(bus, reg, __iwl_read_prph(bus, reg) | mask);
> +		iwl_release_nic_access(bus);
> +	}
>  	spin_unlock_irqrestore(&bus->reg_lock, flags);
>  }
>  
> @@ -234,10 +236,11 @@ void iwl_set_bits_mask_prph(struct iwl_bus *bus, u32 reg,
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&bus->reg_lock, flags);
> -	iwl_grab_nic_access(bus);
> -	__iwl_write_prph(bus, reg,
> -			 (__iwl_read_prph(bus, reg) & mask) | bits);
> -	iwl_release_nic_access(bus);
> +	if (likely(iwl_grab_nic_access(bus))) {
> +		__iwl_write_prph(bus, reg,
> +				 (__iwl_read_prph(bus, reg) & mask) | bits);
> +		iwl_release_nic_access(bus);
> +	}
>  	spin_unlock_irqrestore(&bus->reg_lock, flags);
>  }
>  
> @@ -247,10 +250,11 @@ void iwl_clear_bits_prph(struct iwl_bus *bus, u32 reg, u32 mask)
>  	u32 val;
>  
>  	spin_lock_irqsave(&bus->reg_lock, flags);
> -	iwl_grab_nic_access(bus);
> -	val = __iwl_read_prph(bus, reg);
> -	__iwl_write_prph(bus, reg, (val & ~mask));
> -	iwl_release_nic_access(bus);
> +	if (likely(iwl_grab_nic_access(bus))) {
> +		val = __iwl_read_prph(bus, reg);
> +		__iwl_write_prph(bus, reg, (val & ~mask));
> +		iwl_release_nic_access(bus);
> +	}
>  	spin_unlock_irqrestore(&bus->reg_lock, flags);
>  }
>  
> @@ -262,15 +266,13 @@ void _iwl_read_targ_mem_words(struct iwl_bus *bus, u32 addr,
>  	u32 *vals = buf;
>  
>  	spin_lock_irqsave(&bus->reg_lock, flags);
> -	iwl_grab_nic_access(bus);
> -
> -	iwl_write32(bus, HBUS_TARG_MEM_RADDR, addr);
> -	rmb();
> -
> -	for (offs = 0; offs < words; offs++)
> -		vals[offs] = iwl_read32(bus, HBUS_TARG_MEM_RDAT);
> -
> -	iwl_release_nic_access(bus);
> +	if (likely(iwl_grab_nic_access(bus))) {
> +		iwl_write32(bus, HBUS_TARG_MEM_RADDR, addr);
> +		rmb();
> +		for (offs = 0; offs < words; offs++)
> +			vals[offs] = iwl_read32(bus, HBUS_TARG_MEM_RDAT);
> +		iwl_release_nic_access(bus);
> +	}
>  	spin_unlock_irqrestore(&bus->reg_lock, flags);
>  }
>  
> @@ -291,7 +293,7 @@ int _iwl_write_targ_mem_words(struct iwl_bus *bus, u32 addr,
>  	u32 *vals = buf;
>  
>  	spin_lock_irqsave(&bus->reg_lock, flags);
> -	if (!iwl_grab_nic_access(bus)) {
> +	if (likely(iwl_grab_nic_access(bus))) {
>  		iwl_write32(bus, 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 427d065..f4763d8 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_bus *bus, u32 addr, u32 mask,
>  			int timeout);
>  
>  int iwl_grab_nic_access_silent(struct iwl_bus *bus);
> -int iwl_grab_nic_access(struct iwl_bus *bus);
> +bool iwl_grab_nic_access(struct iwl_bus *bus);
>  void iwl_release_nic_access(struct iwl_bus *bus);
>  
>  u32 iwl_read_direct32(struct iwl_bus *bus, u32 reg);
> diff --git a/drivers/net/wireless/iwlwifi/iwl-mac80211.c b/drivers/net/wireless/iwlwifi/iwl-mac80211.c
> index 965d047..218071a 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-mac80211.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-mac80211.c
> @@ -443,7 +443,7 @@ static int iwlagn_mac_resume(struct ieee80211_hw *hw)
>  	if (iwlagn_hw_valid_rtc_data_addr(base)) {
>  		spin_lock_irqsave(&bus(priv)->reg_lock, flags);
>  		ret = iwl_grab_nic_access_silent(bus(priv));
> -		if (ret == 0) {
> +		if (likely(ret == 0)) {

you change the return to bool, right, but "ret" is "int"

Wey

Re: [RFC 3/4] iwlwifi: cleanup/fix memory barriers

[Johannes Berg]

On 2/6/2012 5:09 PM, Stanislaw Gruszka wrote:
> 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.

Technically, PCIe bridges can reorder writes as well, so we really need 
to do a dummy read somewhere -- we were discussing this internally 
recently as well.

johannes

Re: [RFC 4/4] iwlwifi: use writeb,writel,readl directly

[wwguy]

On Mon, 2012-02-06 at 17:09 +0100, Stanislaw Gruszka wrote:
> That will save us some CPU cycles at run time.
> 
> Obviously I do not think port-based IO is possible, but I added
> check for that during probe just in case.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl-pci.c |   12 ++++++++----
>  1 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/wireless/iwlwifi/iwl-pci.c b/drivers/net/wireless/iwlwifi/iwl-pci.c
> index 03702a2..32b326d 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-pci.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-pci.c
> @@ -153,18 +153,17 @@ static u32 iwl_pci_get_hw_id(struct iwl_bus *bus)
>  
>  static void iwl_pci_write8(struct iwl_bus *bus, u32 ofs, u8 val)
>  {
> -	iowrite8(val, IWL_BUS_GET_PCI_BUS(bus)->hw_base + ofs);
> +	writeb(val, IWL_BUS_GET_PCI_BUS(bus)->hw_base + ofs);
>  }
>  
>  static void iwl_pci_write32(struct iwl_bus *bus, u32 ofs, u32 val)
>  {
> -	iowrite32(val, IWL_BUS_GET_PCI_BUS(bus)->hw_base + ofs);
> +	writel(val, IWL_BUS_GET_PCI_BUS(bus)->hw_base + ofs);
>  }
>  http://www.gossamer-threads.com/lists/linux/kernel/960145
>  static u32 iwl_pci_read32(struct iwl_bus *bus, u32 ofs)
>  {
> -	u32 val = ioread32(IWL_BUS_GET_PCI_BUS(bus)->hw_base + ofs);
> -	return val;
> +	return readl(IWL_BUS_GET_PCI_BUS(bus)->hw_base + ofs);
>  }
>  
>  static const struct iwl_bus_ops bus_ops_pci = {
> @@ -417,6 +416,11 @@ static int iwl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  		goto out_pci_disable_device;
>  	}
>  
> +	if (pci_resource_flags(pdev, 0) & IORESOURCE_IO) {
> +		dev_printk(KERN_ERR, bus->dev, "not supported I/O method");
> +		goto out_pci_disable_device;
> +	}
> +
>  	pci_bus->hw_base = pci_iomap(pdev, 0, 0);
>  	if (!pci_bus->hw_base) {
>  		dev_printk(KERN_ERR, bus->dev, "pci_iomap failed");

I believe iwl driver were use readl/writel before switch to
ioread32/iowrite32 (long time ago)

http://www.gossamer-threads.com/lists/linux/kernel/960145

also, AFAIK, the readl/writel IO functions still works but their use in
new code is discouraged.

Wey
 

Re: [RFC 1/4] iwlwifi: dump stack when fail to gain access to the device

[Stanislaw Gruszka]

On Mon, Feb 06, 2012 at 08:08:16AM -0800, wwguy wrote:
> On Mon, 2012-02-06 at 17:09 +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 83fdff3..ce6d9c1 100644
> > --- a/drivers/net/wireless/iwlwifi/iwl-io.c
> > +++ b/drivers/net/wireless/iwlwifi/iwl-io.c
> > @@ -120,10 +120,10 @@ int iwl_grab_nic_access_silent(struct iwl_bus *bus)
> >  int iwl_grab_nic_access(struct iwl_bus *bus)
> >  {
> >  	int ret = iwl_grab_nic_access_silent(bus);
> > -	if (ret) {
> > +	if (unlikely(ret)) {
> >  		u32 val = iwl_read32(bus, CSR_GP_CNTRL);
> > -		IWL_ERR(bus,
> > -			"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);
> >  	}
> >  
> I agree the message and check make sense, but not sure what new clue we
> got by doing this?

Calltrace with very first grab nic access failure.

Stanislaw

Re: [RFC 2/4] iwlwifi: always check if got h/w access before write

[Stanislaw Gruszka]

On Mon, Feb 06, 2012 at 08:13:11AM -0800, wwguy wrote:
> On Mon, 2012-02-06 at 17:09 +0100, Stanislaw Gruszka wrote:
> > @@ -117,16 +117,17 @@ int iwl_grab_nic_access_silent(struct iwl_bus *bus)
> >  	return 0;
> >  }
> >  
> > -int iwl_grab_nic_access(struct iwl_bus *bus)
> > +bool iwl_grab_nic_access(struct iwl_bus *bus)
> >  {
> >  	int ret = iwl_grab_nic_access_silent(bus);
> >  	if (unlikely(ret)) {
> >  		u32 val = iwl_read32(bus, 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;
> >  }
> >  u32 iwl_read_direct32(struct iwl_bus *bus, u32 reg);
> > diff --git a/drivers/net/wireless/iwlwifi/iwl-mac80211.c b/drivers/net/wireless/iwlwifi/iwl-mac80211.c
> > index 965d047..218071a 100644
> > --- a/drivers/net/wireless/iwlwifi/iwl-mac80211.c
> > +++ b/drivers/net/wireless/iwlwifi/iwl-mac80211.c
> > @@ -443,7 +443,7 @@ static int iwlagn_mac_resume(struct ieee80211_hw *hw)
> >  	if (iwlagn_hw_valid_rtc_data_addr(base)) {
> >  		spin_lock_irqsave(&bus(priv)->reg_lock, flags);
> >  		ret = iwl_grab_nic_access_silent(bus(priv));
> > -		if (ret == 0) {
> > +		if (likely(ret == 0)) {
> 
> you change the return to bool, right, but "ret" is "int"

I only changed iwl_grab_nic_access(),
iwl_grab_nic_access_silent() still returns int.

Stanislaw 

Re: [RFC 3/4] iwlwifi: cleanup/fix memory barriers

[Stanislaw Gruszka]

On Mon, Feb 06, 2012 at 05:13:25PM +0100, Johannes Berg wrote:
> On 2/6/2012 5:09 PM, Stanislaw Gruszka wrote:
> >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.
> 
> Technically, PCIe bridges can reorder writes as well, so we really
> need to do a dummy read somewhere -- we were discussing this
> internally recently as well.

PCIe can reorder writes only if they come from different CPUs,
good examples are in "LOCKS VS I/O ACCESSES" in
Documentation/memory-barriers.txt

Stanislaw

Re: [RFC 4/4] iwlwifi: use writeb,writel,readl directly

[Stanislaw Gruszka]

On Mon, Feb 06, 2012 at 08:32:52AM -0800, wwguy wrote:
> I believe iwl driver were use readl/writel before switch to
> ioread32/iowrite32 (long time ago)
> 
> http://www.gossamer-threads.com/lists/linux/kernel/960145
>
> also, AFAIK, the readl/writel IO functions still works but their use in
> new code is discouraged.
I do not think so, we have tons of drivers that use readl/writel.
PCI express devices can not use port-based IO, they will be
incredible slow then.

I think confusion come from a fact that iwlwifi use pci_ioremp() for
mapping registers, which can also maps port-based IO areas. Simply
changing that into pci_ioremap_bar(), and make driver depend on
CONFIG_HAS_IOMEM will remove that inconsistency.

Stanislaw 

Re: [RFC 3/4] iwlwifi: cleanup/fix memory barriers

[Johannes Berg]

On 2/7/2012 9:11 AM, Stanislaw Gruszka wrote:
>> Technically, PCIe bridges can reorder writes as well, so we really
>> need to do a dummy read somewhere -- we were discussing this
>> internally recently as well.
>
> PCIe can reorder writes only if they come from different CPUs,
> good examples are in "LOCKS VS I/O ACCESSES" in
> Documentation/memory-barriers.txt

Good point, I was confusing this and write posting. Don't think we have 
an issue with write posting here so we should be OK.

johannes

Re: [RFC 2/4] iwlwifi: always check if got h/w access before write

[Guy, Wey-Yi]

On Tue, 2012-02-07 at 09:05 +0100, Stanislaw Gruszka wrote:
> On Mon, Feb 06, 2012 at 08:13:11AM -0800, wwguy wrote:
> > On Mon, 2012-02-06 at 17:09 +0100, Stanislaw Gruszka wrote:
> > > @@ -117,16 +117,17 @@ int iwl_grab_nic_access_silent(struct iwl_bus *bus)
> > >  	return 0;
> > >  }
> > >  
> > > -int iwl_grab_nic_access(struct iwl_bus *bus)
> > > +bool iwl_grab_nic_access(struct iwl_bus *bus)
> > >  {
> > >  	int ret = iwl_grab_nic_access_silent(bus);
> > >  	if (unlikely(ret)) {
> > >  		u32 val = iwl_read32(bus, 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;
> > >  }
> > >  u32 iwl_read_direct32(struct iwl_bus *bus, u32 reg);
> > > diff --git a/drivers/net/wireless/iwlwifi/iwl-mac80211.c b/drivers/net/wireless/iwlwifi/iwl-mac80211.c
> > > index 965d047..218071a 100644
> > > --- a/drivers/net/wireless/iwlwifi/iwl-mac80211.c
> > > +++ b/drivers/net/wireless/iwlwifi/iwl-mac80211.c
> > > @@ -443,7 +443,7 @@ static int iwlagn_mac_resume(struct ieee80211_hw *hw)
> > >  	if (iwlagn_hw_valid_rtc_data_addr(base)) {
> > >  		spin_lock_irqsave(&bus(priv)->reg_lock, flags);
> > >  		ret = iwl_grab_nic_access_silent(bus(priv));
> > > -		if (ret == 0) {
> > > +		if (likely(ret == 0)) {
> > 
> > you change the return to bool, right, but "ret" is "int"
> 
> I only changed iwl_grab_nic_access(),
> iwl_grab_nic_access_silent() still returns int.
> 
you are correct, I miss that

Wey

Re: [RFC 1/4] iwlwifi: dump stack when fail to gain access to the device

[Guy, Wey-Yi]

On Tue, 2012-02-07 at 09:01 +0100, Stanislaw Gruszka wrote:
> On Mon, Feb 06, 2012 at 08:08:16AM -0800, wwguy wrote:
> > On Mon, 2012-02-06 at 17:09 +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 83fdff3..ce6d9c1 100644
> > > --- a/drivers/net/wireless/iwlwifi/iwl-io.c
> > > +++ b/drivers/net/wireless/iwlwifi/iwl-io.c
> > > @@ -120,10 +120,10 @@ int iwl_grab_nic_access_silent(struct iwl_bus *bus)
> > >  int iwl_grab_nic_access(struct iwl_bus *bus)
> > >  {
> > >  	int ret = iwl_grab_nic_access_silent(bus);
> > > -	if (ret) {
> > > +	if (unlikely(ret)) {
> > >  		u32 val = iwl_read32(bus, CSR_GP_CNTRL);
> > > -		IWL_ERR(bus,
> > > -			"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);
> > >  	}
> > >  
> > I agree the message and check make sense, but not sure what new clue we
> > got by doing this?
> 
> Calltrace with very first grab nic access failure.
> 
Good point, we have this "PCI bus disappear" problem on all of our
devices and very difficult to debug.

btw, do you have any system can reproduce this problem? we have nothing.

Thanks
Wey

Re: [RFC 4/4] iwlwifi: use writeb,writel,readl directly

[Guy, Wey-Yi]

On Tue, 2012-02-07 at 09:22 +0100, Stanislaw Gruszka wrote:
> On Mon, Feb 06, 2012 at 08:32:52AM -0800, wwguy wrote:
> > I believe iwl driver were use readl/writel before switch to
> > ioread32/iowrite32 (long time ago)
> > 
> > http://www.gossamer-threads.com/lists/linux/kernel/960145
> >
> > also, AFAIK, the readl/writel IO functions still works but their use in
> > new code is discouraged.
> I do not think so, we have tons of drivers that use readl/writel.
> PCI express devices can not use port-based IO, they will be
> incredible slow then.

Agree

> I think confusion come from a fact that iwlwifi use pci_ioremp() for
> mapping registers, which can also maps port-based IO areas. Simply
> changing that into pci_ioremap_bar(), and make driver depend on
> CONFIG_HAS_IOMEM will remove that inconsistency.
> 
> Stanislaw 

Re: [RFC 1/4] iwlwifi: dump stack when fail to gain access to the device

[Stanislaw Gruszka]

> > > I agree the message and check make sense, but not sure what new clue we
> > > got by doing this?
> > 
> > Calltrace with very first grab nic access failure.
> > 
> Good point, we have this "PCI bus disappear" problem on all of our
> devices and very difficult to debug.
> 
> btw, do you have any system can reproduce this problem? we have nothing.

No, those problems are not reproducible here as well.

Stanislaw

Re: [RFC 1/4] iwlwifi: dump stack when fail to gain access to the device

[Stanislaw Gruszka]

On Mon, Feb 06, 2012 at 05:09:21PM +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 83fdff3..ce6d9c1 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-io.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-io.c
> @@ -120,10 +120,10 @@ int iwl_grab_nic_access_silent(struct iwl_bus *bus)
>  int iwl_grab_nic_access(struct iwl_bus *bus)
>  {
>  	int ret = iwl_grab_nic_access_silent(bus);
> -	if (ret) {
> +	if (unlikely(ret)) {
>  		u32 val = iwl_read32(bus, CSR_GP_CNTRL);
> -		IWL_ERR(bus,
> -			"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);
>  	}
I need to do a bit more testing before posting this. On iwlegacy the waring is
triggered on rfkill, and currently I have no access to iwlwifi with rfkill
switch, but seems there will be the same problem with iwlwifi: iwl_grab_nic_access()
fail when rfkill is on.

Stanislaw

Re: [RFC 1/4] iwlwifi: dump stack when fail to gain access to the device

[Stanislaw Gruszka]

On Fri, Feb 10, 2012 at 01:01:01PM +0100, Stanislaw Gruszka wrote:
> I need to do a bit more testing before posting this. On iwlegacy the waring is
> triggered on rfkill, and currently I have no access to iwlwifi with rfkill
> switch, but seems there will be the same problem with iwlwifi: iwl_grab_nic_access()
> fail when rfkill is on.

Done, no problem notice with those patches and rfkill.

Stanislaw