* Re: [RFC 1/4] iwlwifi: dump stack when fail to gain access to the device
2012-02-06 16:09 [RFC 1/4] iwlwifi: dump stack when fail to gain access to the device Stanislaw Gruszka
@ 2012-02-06 16:08 ` wwguy
2012-02-07 8:01 ` Stanislaw Gruszka
2012-02-06 16:09 ` [RFC 2/4] iwlwifi: always check if got h/w access before write Stanislaw Gruszka
` (3 subsequent siblings)
4 siblings, 1 reply; 19+ messages in thread
From: wwguy @ 2012-02-06 16:08 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Intel Linux Wireless, linux-wireless
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
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC 1/4] iwlwifi: dump stack when fail to gain access to the device
2012-02-06 16:08 ` wwguy
@ 2012-02-07 8:01 ` Stanislaw Gruszka
2012-02-07 14:21 ` Guy, Wey-Yi
0 siblings, 1 reply; 19+ messages in thread
From: Stanislaw Gruszka @ 2012-02-07 8:01 UTC (permalink / raw)
To: wwguy; +Cc: Intel Linux Wireless, linux-wireless
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
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC 1/4] iwlwifi: dump stack when fail to gain access to the device
2012-02-07 8:01 ` Stanislaw Gruszka
@ 2012-02-07 14:21 ` Guy, Wey-Yi
2012-02-08 10:52 ` Stanislaw Gruszka
0 siblings, 1 reply; 19+ messages in thread
From: Guy, Wey-Yi @ 2012-02-07 14:21 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Intel Linux Wireless, linux-wireless
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
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC 1/4] iwlwifi: dump stack when fail to gain access to the device
2012-02-07 14:21 ` Guy, Wey-Yi
@ 2012-02-08 10:52 ` Stanislaw Gruszka
0 siblings, 0 replies; 19+ messages in thread
From: Stanislaw Gruszka @ 2012-02-08 10:52 UTC (permalink / raw)
To: Guy, Wey-Yi; +Cc: Intel Linux Wireless, linux-wireless
> > > 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
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC 2/4] iwlwifi: always check if got h/w access before write
2012-02-06 16:09 [RFC 1/4] iwlwifi: dump stack when fail to gain access to the device Stanislaw Gruszka
2012-02-06 16:08 ` wwguy
@ 2012-02-06 16:09 ` Stanislaw Gruszka
2012-02-06 16:13 ` wwguy
2012-02-06 16:09 ` [RFC 3/4] iwlwifi: cleanup/fix memory barriers Stanislaw Gruszka
` (2 subsequent siblings)
4 siblings, 1 reply; 19+ messages in thread
From: Stanislaw Gruszka @ 2012-02-06 16:09 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 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
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC 2/4] iwlwifi: always check if got h/w access before write
2012-02-06 16:09 ` [RFC 2/4] iwlwifi: always check if got h/w access before write Stanislaw Gruszka
@ 2012-02-06 16:13 ` wwguy
2012-02-07 8:05 ` Stanislaw Gruszka
0 siblings, 1 reply; 19+ messages in thread
From: wwguy @ 2012-02-06 16:13 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Intel Linux Wireless, linux-wireless
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
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC 2/4] iwlwifi: always check if got h/w access before write
2012-02-06 16:13 ` wwguy
@ 2012-02-07 8:05 ` Stanislaw Gruszka
2012-02-07 14:20 ` Guy, Wey-Yi
0 siblings, 1 reply; 19+ messages in thread
From: Stanislaw Gruszka @ 2012-02-07 8:05 UTC (permalink / raw)
To: wwguy; +Cc: Intel Linux Wireless, linux-wireless
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
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC 2/4] iwlwifi: always check if got h/w access before write
2012-02-07 8:05 ` Stanislaw Gruszka
@ 2012-02-07 14:20 ` Guy, Wey-Yi
0 siblings, 0 replies; 19+ messages in thread
From: Guy, Wey-Yi @ 2012-02-07 14:20 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Intel Linux Wireless, linux-wireless
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC 3/4] iwlwifi: cleanup/fix memory barriers
2012-02-06 16:09 [RFC 1/4] iwlwifi: dump stack when fail to gain access to the device Stanislaw Gruszka
2012-02-06 16:08 ` wwguy
2012-02-06 16:09 ` [RFC 2/4] iwlwifi: always check if got h/w access before write Stanislaw Gruszka
@ 2012-02-06 16:09 ` Stanislaw Gruszka
2012-02-06 16:13 ` Johannes Berg
2012-02-06 16:09 ` [RFC 4/4] iwlwifi: use writeb,writel,readl directly Stanislaw Gruszka
2012-02-10 12:01 ` [RFC 1/4] iwlwifi: dump stack when fail to gain access to the device Stanislaw Gruszka
4 siblings, 1 reply; 19+ messages in thread
From: Stanislaw Gruszka @ 2012-02-06 16:09 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 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
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC 3/4] iwlwifi: cleanup/fix memory barriers
2012-02-06 16:09 ` [RFC 3/4] iwlwifi: cleanup/fix memory barriers Stanislaw Gruszka
@ 2012-02-06 16:13 ` Johannes Berg
2012-02-07 8:11 ` Stanislaw Gruszka
0 siblings, 1 reply; 19+ messages in thread
From: Johannes Berg @ 2012-02-06 16:13 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Wey-Yi Guy, Intel Linux Wireless, linux-wireless
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC 3/4] iwlwifi: cleanup/fix memory barriers
2012-02-06 16:13 ` Johannes Berg
@ 2012-02-07 8:11 ` Stanislaw Gruszka
2012-02-07 8:25 ` Johannes Berg
0 siblings, 1 reply; 19+ messages in thread
From: Stanislaw Gruszka @ 2012-02-07 8:11 UTC (permalink / raw)
To: Johannes Berg; +Cc: Wey-Yi Guy, Intel Linux Wireless, linux-wireless
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC 3/4] iwlwifi: cleanup/fix memory barriers
2012-02-07 8:11 ` Stanislaw Gruszka
@ 2012-02-07 8:25 ` Johannes Berg
0 siblings, 0 replies; 19+ messages in thread
From: Johannes Berg @ 2012-02-07 8:25 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Wey-Yi Guy, Intel Linux Wireless, linux-wireless
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC 4/4] iwlwifi: use writeb,writel,readl directly
2012-02-06 16:09 [RFC 1/4] iwlwifi: dump stack when fail to gain access to the device Stanislaw Gruszka
` (2 preceding siblings ...)
2012-02-06 16:09 ` [RFC 3/4] iwlwifi: cleanup/fix memory barriers Stanislaw Gruszka
@ 2012-02-06 16:09 ` Stanislaw Gruszka
2012-02-06 16:32 ` wwguy
2012-02-10 12:01 ` [RFC 1/4] iwlwifi: dump stack when fail to gain access to the device Stanislaw Gruszka
4 siblings, 1 reply; 19+ messages in thread
From: Stanislaw Gruszka @ 2012-02-06 16:09 UTC (permalink / raw)
To: Wey-Yi Guy; +Cc: Intel Linux Wireless, linux-wireless, 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
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC 4/4] iwlwifi: use writeb,writel,readl directly
2012-02-06 16:09 ` [RFC 4/4] iwlwifi: use writeb,writel,readl directly Stanislaw Gruszka
@ 2012-02-06 16:32 ` wwguy
2012-02-07 8:22 ` Stanislaw Gruszka
0 siblings, 1 reply; 19+ messages in thread
From: wwguy @ 2012-02-06 16:32 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Intel Linux Wireless, linux-wireless
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
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC 4/4] iwlwifi: use writeb,writel,readl directly
2012-02-06 16:32 ` wwguy
@ 2012-02-07 8:22 ` Stanislaw Gruszka
2012-02-07 14:22 ` Guy, Wey-Yi
0 siblings, 1 reply; 19+ messages in thread
From: Stanislaw Gruszka @ 2012-02-07 8:22 UTC (permalink / raw)
To: wwguy; +Cc: Intel Linux Wireless, linux-wireless
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC 4/4] iwlwifi: use writeb,writel,readl directly
2012-02-07 8:22 ` Stanislaw Gruszka
@ 2012-02-07 14:22 ` Guy, Wey-Yi
0 siblings, 0 replies; 19+ messages in thread
From: Guy, Wey-Yi @ 2012-02-07 14:22 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Intel Linux Wireless, linux-wireless
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC 1/4] iwlwifi: dump stack when fail to gain access to the device
2012-02-06 16:09 [RFC 1/4] iwlwifi: dump stack when fail to gain access to the device Stanislaw Gruszka
` (3 preceding siblings ...)
2012-02-06 16:09 ` [RFC 4/4] iwlwifi: use writeb,writel,readl directly Stanislaw Gruszka
@ 2012-02-10 12:01 ` Stanislaw Gruszka
2012-02-23 10:14 ` Stanislaw Gruszka
4 siblings, 1 reply; 19+ messages in thread
From: Stanislaw Gruszka @ 2012-02-10 12:01 UTC (permalink / raw)
To: Wey-Yi Guy; +Cc: Intel Linux Wireless, linux-wireless
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
^ permalink raw reply [flat|nested] 19+ messages in thread