[PATCH 0/3] ath9k patches

[PATCH 0/3] ath9k patches

[Sujith Manoharan]

From: Sujith Manoharan <c_manoha@qca.qualcomm.com>

Hi,

Pending patches for ath9k, rebased over 3.14.0-rc1.

Sujith

Sujith Manoharan (3):
  ath9k: Fix build error on ARM
  ath9k: Do not support PowerSave by default
  ath9k: Fix TX power calculation

 drivers/net/wireless/ath/ath9k/ar9003_eeprom.c | 4 ++++
 drivers/net/wireless/ath/ath9k/hw.c            | 5 ++---
 drivers/net/wireless/ath/ath9k/init.c          | 8 +++++++-
 3 files changed, 13 insertions(+), 4 deletions(-)

-- 
1.8.5.3

[ath9k-devel] [PATCH 0/3] ath9k patches

[Sujith Manoharan]

From: Sujith Manoharan <c_manoha@qca.qualcomm.com>

Hi,

Pending patches for ath9k, rebased over 3.14.0-rc1.

Sujith

Sujith Manoharan (3):
  ath9k: Fix build error on ARM
  ath9k: Do not support PowerSave by default
  ath9k: Fix TX power calculation

 drivers/net/wireless/ath/ath9k/ar9003_eeprom.c | 4 ++++
 drivers/net/wireless/ath/ath9k/hw.c            | 5 ++---
 drivers/net/wireless/ath/ath9k/init.c          | 8 +++++++-
 3 files changed, 13 insertions(+), 4 deletions(-)

-- 
1.8.5.3

[PATCH 1/3] ath9k: Fix build error on ARM

[Sujith Manoharan]

From: Sujith Manoharan <c_manoha@qca.qualcomm.com>

Use mdelay instead of udelay to fix this error:

ERROR: "__bad_udelay" [drivers/net/wireless/ath/ath9k/ath9k_hw.ko] undefined!
make[1]: *** [__modpost] Error 1
make: *** [modules] Error 2

Reported-by: Josh Boyer <jwboyer@fedoraproject.org>
Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath9k/hw.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index fbf43c0..11eab9f 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -1316,7 +1316,7 @@ static bool ath9k_hw_set_reset(struct ath_hw *ah, int type)
 	if (AR_SREV_9300_20_OR_LATER(ah))
 		udelay(50);
 	else if (AR_SREV_9100(ah))
-		udelay(10000);
+		mdelay(10);
 	else
 		udelay(100);
 
@@ -2051,9 +2051,8 @@ static bool ath9k_hw_set_power_awake(struct ath_hw *ah)
 
 	REG_SET_BIT(ah, AR_RTC_FORCE_WAKE,
 		    AR_RTC_FORCE_WAKE_EN);
-
 	if (AR_SREV_9100(ah))
-		udelay(10000);
+		mdelay(10);
 	else
 		udelay(50);
 
-- 
1.8.5.3

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Sujith Manoharan]

From: Sujith Manoharan <c_manoha@qca.qualcomm.com>

Use mdelay instead of udelay to fix this error:

ERROR: "__bad_udelay" [drivers/net/wireless/ath/ath9k/ath9k_hw.ko] undefined!
make[1]: *** [__modpost] Error 1
make: *** [modules] Error 2

Reported-by: Josh Boyer <jwboyer@fedoraproject.org>
Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath9k/hw.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index fbf43c0..11eab9f 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -1316,7 +1316,7 @@ static bool ath9k_hw_set_reset(struct ath_hw *ah, int type)
 	if (AR_SREV_9300_20_OR_LATER(ah))
 		udelay(50);
 	else if (AR_SREV_9100(ah))
-		udelay(10000);
+		mdelay(10);
 	else
 		udelay(100);
 
@@ -2051,9 +2051,8 @@ static bool ath9k_hw_set_power_awake(struct ath_hw *ah)
 
 	REG_SET_BIT(ah, AR_RTC_FORCE_WAKE,
 		    AR_RTC_FORCE_WAKE_EN);
-
 	if (AR_SREV_9100(ah))
-		udelay(10000);
+		mdelay(10);
 	else
 		udelay(50);
 
-- 
1.8.5.3

[PATCH 2/3] ath9k: Do not support PowerSave by default

[Sujith Manoharan]

From: Sujith Manoharan <c_manoha@qca.qualcomm.com>

Even though we make sure PowerSave is not enabled by default
by disabling the flag, WIPHY_FLAG_PS_ON_BY_DEFAULT on init,
PS could be enabled by userspace based on various factors
like battery usage etc. Since PS in ath9k is just broken
and has been untested for years, remove support for it, but
allow a user to explicitly enable it using a module parameter.

Cc: stable@vger.kernel.org
Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath9k/init.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index c36de30..1fc2e5a 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -57,6 +57,10 @@ static int ath9k_bt_ant_diversity;
 module_param_named(bt_ant_diversity, ath9k_bt_ant_diversity, int, 0444);
 MODULE_PARM_DESC(bt_ant_diversity, "Enable WLAN/BT RX antenna diversity");
 
+static int ath9k_ps_enable;
+module_param_named(ps_enable, ath9k_ps_enable, int, 0444);
+MODULE_PARM_DESC(ps_enable, "Enable WLAN PowerSave");
+
 bool is_ath9k_unloaded;
 /* We use the hw_value as an index into our private channel structure */
 
@@ -903,13 +907,15 @@ static void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw)
 	hw->flags = IEEE80211_HW_RX_INCLUDES_FCS |
 		IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING |
 		IEEE80211_HW_SIGNAL_DBM |
-		IEEE80211_HW_SUPPORTS_PS |
 		IEEE80211_HW_PS_NULLFUNC_STACK |
 		IEEE80211_HW_SPECTRUM_MGMT |
 		IEEE80211_HW_REPORTS_TX_ACK_STATUS |
 		IEEE80211_HW_SUPPORTS_RC_TABLE |
 		IEEE80211_HW_SUPPORTS_HT_CCK_RATES;
 
+	if (ath9k_ps_enable)
+		hw->flags |= IEEE80211_HW_SUPPORTS_PS;
+
 	if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_HT) {
 		hw->flags |= IEEE80211_HW_AMPDU_AGGREGATION;
 
-- 
1.8.5.3

[ath9k-devel] [PATCH 2/3] ath9k: Do not support PowerSave by default

[Sujith Manoharan]

From: Sujith Manoharan <c_manoha@qca.qualcomm.com>

Even though we make sure PowerSave is not enabled by default
by disabling the flag, WIPHY_FLAG_PS_ON_BY_DEFAULT on init,
PS could be enabled by userspace based on various factors
like battery usage etc. Since PS in ath9k is just broken
and has been untested for years, remove support for it, but
allow a user to explicitly enable it using a module parameter.

Cc: stable at vger.kernel.org
Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath9k/init.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index c36de30..1fc2e5a 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -57,6 +57,10 @@ static int ath9k_bt_ant_diversity;
 module_param_named(bt_ant_diversity, ath9k_bt_ant_diversity, int, 0444);
 MODULE_PARM_DESC(bt_ant_diversity, "Enable WLAN/BT RX antenna diversity");
 
+static int ath9k_ps_enable;
+module_param_named(ps_enable, ath9k_ps_enable, int, 0444);
+MODULE_PARM_DESC(ps_enable, "Enable WLAN PowerSave");
+
 bool is_ath9k_unloaded;
 /* We use the hw_value as an index into our private channel structure */
 
@@ -903,13 +907,15 @@ static void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw)
 	hw->flags = IEEE80211_HW_RX_INCLUDES_FCS |
 		IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING |
 		IEEE80211_HW_SIGNAL_DBM |
-		IEEE80211_HW_SUPPORTS_PS |
 		IEEE80211_HW_PS_NULLFUNC_STACK |
 		IEEE80211_HW_SPECTRUM_MGMT |
 		IEEE80211_HW_REPORTS_TX_ACK_STATUS |
 		IEEE80211_HW_SUPPORTS_RC_TABLE |
 		IEEE80211_HW_SUPPORTS_HT_CCK_RATES;
 
+	if (ath9k_ps_enable)
+		hw->flags |= IEEE80211_HW_SUPPORTS_PS;
+
 	if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_HT) {
 		hw->flags |= IEEE80211_HW_AMPDU_AGGREGATION;
 
-- 
1.8.5.3

[PATCH 3/3] ath9k: Fix TX power calculation

[Sujith Manoharan]

From: Sujith Manoharan <c_manoha@qca.qualcomm.com>

The commit, "ath9k_hw: Fix incorrect Tx control power in AR9003 template"
fixed the incorrect values in the eeprom templates, but if
boards have already been calibrated with incorrect values,
they would still be using the wrong TX power. Fix this by assigning
a default value in such cases.

Cc: Rajkumar Manoharan <rmanohar@qti.qualcomm.com>
Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath9k/ar9003_eeprom.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
index 25243cb..b8daff7 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
@@ -5065,6 +5065,10 @@ static u16 ar9003_hw_get_max_edge_power(struct ar9300_eeprom *eep,
 			break;
 		}
 	}
+
+	if (is2GHz && !twiceMaxEdgePower)
+		twiceMaxEdgePower = 60;
+
 	return twiceMaxEdgePower;
 }
 
-- 
1.8.5.3

[ath9k-devel] [PATCH 3/3] ath9k: Fix TX power calculation

[Sujith Manoharan]

From: Sujith Manoharan <c_manoha@qca.qualcomm.com>

The commit, "ath9k_hw: Fix incorrect Tx control power in AR9003 template"
fixed the incorrect values in the eeprom templates, but if
boards have already been calibrated with incorrect values,
they would still be using the wrong TX power. Fix this by assigning
a default value in such cases.

Cc: Rajkumar Manoharan <rmanohar@qti.qualcomm.com>
Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath9k/ar9003_eeprom.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
index 25243cb..b8daff7 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
@@ -5065,6 +5065,10 @@ static u16 ar9003_hw_get_max_edge_power(struct ar9300_eeprom *eep,
 			break;
 		}
 	}
+
+	if (is2GHz && !twiceMaxEdgePower)
+		twiceMaxEdgePower = 60;
+
 	return twiceMaxEdgePower;
 }
 
-- 
1.8.5.3

Re: [PATCH 1/3] ath9k: Fix build error on ARM

[Joe Perches]

On Tue, 2014-02-04 at 08:37 +0530, Sujith Manoharan wrote:
> From: Sujith Manoharan <c_manoha@qca.qualcomm.com>
> 
> Use mdelay instead of udelay to fix this error:
> 
> ERROR: "__bad_udelay" [drivers/net/wireless/ath/ath9k/ath9k_hw.ko] undefined!
> make[1]: *** [__modpost] Error 1
> make: *** [modules] Error 2
> 
> Reported-by: Josh Boyer <jwboyer@fedoraproject.org>
> Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
> ---
>  drivers/net/wireless/ath/ath9k/hw.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
> index fbf43c0..11eab9f 100644
> --- a/drivers/net/wireless/ath/ath9k/hw.c
> +++ b/drivers/net/wireless/ath/ath9k/hw.c
> @@ -1316,7 +1316,7 @@ static bool ath9k_hw_set_reset(struct ath_hw *ah, int type)
>  	if (AR_SREV_9300_20_OR_LATER(ah))
>  		udelay(50);
>  	else if (AR_SREV_9100(ah))
> -		udelay(10000);
> +		mdelay(10);
>  	else
>  		udelay(100);

How can this help?
There are udelays above and below this line.

>  
> @@ -2051,9 +2051,8 @@ static bool ath9k_hw_set_power_awake(struct ath_hw *ah)
>  
>  	REG_SET_BIT(ah, AR_RTC_FORCE_WAKE,
>  		    AR_RTC_FORCE_WAKE_EN);
> -
>  	if (AR_SREV_9100(ah))
> -		udelay(10000);
> +		mdelay(10);
>  	else
>  		udelay(50);
>  

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Joe Perches]

On Tue, 2014-02-04 at 08:37 +0530, Sujith Manoharan wrote:
> From: Sujith Manoharan <c_manoha@qca.qualcomm.com>
> 
> Use mdelay instead of udelay to fix this error:
> 
> ERROR: "__bad_udelay" [drivers/net/wireless/ath/ath9k/ath9k_hw.ko] undefined!
> make[1]: *** [__modpost] Error 1
> make: *** [modules] Error 2
> 
> Reported-by: Josh Boyer <jwboyer@fedoraproject.org>
> Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
> ---
>  drivers/net/wireless/ath/ath9k/hw.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
> index fbf43c0..11eab9f 100644
> --- a/drivers/net/wireless/ath/ath9k/hw.c
> +++ b/drivers/net/wireless/ath/ath9k/hw.c
> @@ -1316,7 +1316,7 @@ static bool ath9k_hw_set_reset(struct ath_hw *ah, int type)
>  	if (AR_SREV_9300_20_OR_LATER(ah))
>  		udelay(50);
>  	else if (AR_SREV_9100(ah))
> -		udelay(10000);
> +		mdelay(10);
>  	else
>  		udelay(100);

How can this help?
There are udelays above and below this line.

>  
> @@ -2051,9 +2051,8 @@ static bool ath9k_hw_set_power_awake(struct ath_hw *ah)
>  
>  	REG_SET_BIT(ah, AR_RTC_FORCE_WAKE,
>  		    AR_RTC_FORCE_WAKE_EN);
> -
>  	if (AR_SREV_9100(ah))
> -		udelay(10000);
> +		mdelay(10);
>  	else
>  		udelay(50);
>  

Re: [PATCH 1/3] ath9k: Fix build error on ARM

[Sujith Manoharan]

> >  	if (AR_SREV_9300_20_OR_LATER(ah))
> >  		udelay(50);
> >  	else if (AR_SREV_9100(ah))
> > -		udelay(10000);
> > +		mdelay(10);
> >  	else
> >  		udelay(100);
> 
> How can this help?
> There are udelays above and below this line.

A large value causes a build error on ARM:
https://lkml.org/lkml/2014/1/26/92

Sujith

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Sujith Manoharan]

> >  	if (AR_SREV_9300_20_OR_LATER(ah))
> >  		udelay(50);
> >  	else if (AR_SREV_9100(ah))
> > -		udelay(10000);
> > +		mdelay(10);
> >  	else
> >  		udelay(100);
> 
> How can this help?
> There are udelays above and below this line.

A large value causes a build error on ARM:
https://lkml.org/lkml/2014/1/26/92

Sujith

Re: [ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Holger Schurig]

Joe, look in linux/arch/arm/include/asm/delay.h. The macro udelay
cannot handle large values because of lost-of-precision.

IMHO udelay on ARM is broken, because it also cannot work with fast
ARM processors (where bogomips >= 3355, which is in sight now). It's
just not broken enought that someone did something against it ...   so
the current kludge is good enought.

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Holger Schurig]

Joe, look in linux/arch/arm/include/asm/delay.h. The macro udelay
cannot handle large values because of lost-of-precision.

IMHO udelay on ARM is broken, because it also cannot work with fast
ARM processors (where bogomips >= 3355, which is in sight now). It's
just not broken enought that someone did something against it ...   so
the current kludge is good enought.

[PATCH] checkpatch: Add test for long udelay

[Joe Perches]

On Tue, 2014-02-04 at 08:03 +0100, Holger Schurig wrote:
> The macro udelay
> cannot handle large values because of lost-of-precision.
> 
> IMHO udelay on ARM is broken, because it also cannot work with fast
> ARM processors (where bogomips >= 3355, which is in sight now). It's
> just not broken enought that someone did something against it ...   so
> the current kludge is good enought.

Until then, warn on long udelay uses.

Also fix uses of $line that should have been $herecurr.

Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 0ea2a1e..36275c5 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3912,10 +3912,15 @@ sub process {
 
 # prefer usleep_range over udelay
 		if ($line =~ /\budelay\s*\(\s*(\d+)\s*\)/) {
+			my $delay = $1;
 			# ignore udelay's < 10, however
-			if (! ($1 < 10) ) {
+			if (! ($delay < 10) ) {
 				CHK("USLEEP_RANGE",
-				    "usleep_range is preferred over udelay; see Documentation/timers/timers-howto.txt\n" . $line);
+				    "usleep_range is preferred over udelay; see Documentation/timers/timers-howto.txt\n" . $herecurr);
+			}
+			if ($delay > 2000) {
+				WARN("LONG_UDELAY",
+				     "long udelay - prefer mdelay; see arch/arm/include/asm/delay.h\n" . $herecurr);
 			}
 		}
 
@@ -3923,7 +3928,7 @@ sub process {
 		if ($line =~ /\bmsleep\s*\((\d+)\);/) {
 			if ($1 < 20) {
 				WARN("MSLEEP",
-				     "msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.txt\n" . $line);
+				     "msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.txt\n" . $herecurr);
 			}
 		}
 

[PATCH] checkpatch: Add test for long udelay

[Joe Perches]

On Tue, 2014-02-04 at 08:03 +0100, Holger Schurig wrote:
> The macro udelay
> cannot handle large values because of lost-of-precision.
> 
> IMHO udelay on ARM is broken, because it also cannot work with fast
> ARM processors (where bogomips >= 3355, which is in sight now). It's
> just not broken enought that someone did something against it ...   so
> the current kludge is good enought.

Until then, warn on long udelay uses.

Also fix uses of $line that should have been $herecurr.

Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 0ea2a1e..36275c5 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3912,10 +3912,15 @@ sub process {
 
 # prefer usleep_range over udelay
 		if ($line =~ /\budelay\s*\(\s*(\d+)\s*\)/) {
+			my $delay = $1;
 			# ignore udelay's < 10, however
-			if (! ($1 < 10) ) {
+			if (! ($delay < 10) ) {
 				CHK("USLEEP_RANGE",
-				    "usleep_range is preferred over udelay; see Documentation/timers/timers-howto.txt\n" . $line);
+				    "usleep_range is preferred over udelay; see Documentation/timers/timers-howto.txt\n" . $herecurr);
+			}
+			if ($delay > 2000) {
+				WARN("LONG_UDELAY",
+				     "long udelay - prefer mdelay; see arch/arm/include/asm/delay.h\n" . $herecurr);
 			}
 		}
 
@@ -3923,7 +3928,7 @@ sub process {
 		if ($line =~ /\bmsleep\s*\((\d+)\);/) {
 			if ($1 < 20) {
 				WARN("MSLEEP",
-				     "msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.txt\n" . $line);
+				     "msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.txt\n" . $herecurr);
 			}
 		}
 

[ath9k-devel] [PATCH] checkpatch: Add test for long udelay

[Joe Perches]

On Tue, 2014-02-04 at 08:03 +0100, Holger Schurig wrote:
> The macro udelay
> cannot handle large values because of lost-of-precision.
> 
> IMHO udelay on ARM is broken, because it also cannot work with fast
> ARM processors (where bogomips >= 3355, which is in sight now). It's
> just not broken enought that someone did something against it ...   so
> the current kludge is good enought.

Until then, warn on long udelay uses.

Also fix uses of $line that should have been $herecurr.

Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 0ea2a1e..36275c5 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3912,10 +3912,15 @@ sub process {
 
 # prefer usleep_range over udelay
 		if ($line =~ /\budelay\s*\(\s*(\d+)\s*\)/) {
+			my $delay = $1;
 			# ignore udelay's < 10, however
-			if (! ($1 < 10) ) {
+			if (! ($delay < 10) ) {
 				CHK("USLEEP_RANGE",
-				    "usleep_range is preferred over udelay; see Documentation/timers/timers-howto.txt\n" . $line);
+				    "usleep_range is preferred over udelay; see Documentation/timers/timers-howto.txt\n" . $herecurr);
+			}
+			if ($delay > 2000) {
+				WARN("LONG_UDELAY",
+				     "long udelay - prefer mdelay; see arch/arm/include/asm/delay.h\n" . $herecurr);
 			}
 		}
 
@@ -3923,7 +3928,7 @@ sub process {
 		if ($line =~ /\bmsleep\s*\((\d+)\);/) {
 			if ($1 < 20) {
 				WARN("MSLEEP",
-				     "msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.txt\n" . $line);
+				     "msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.txt\n" . $herecurr);
 			}
 		}
 

Re: [ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Joe Perches]

On Tue, 2014-02-04 at 08:03 +0100, Holger Schurig wrote:
> Joe, look in linux/arch/arm/include/asm/delay.h. The macro udelay
> cannot handle large values because of lost-of-precision.
> 
> IMHO udelay on ARM is broken, because it also cannot work with fast
> ARM processors (where bogomips >= 3355, which is in sight now). It's
> just not broken enought that someone did something against it ...   so
> the current kludge is good enought.

Maybe something like this would be better?

---
 arch/arm/include/asm/delay.h | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/arch/arm/include/asm/delay.h b/arch/arm/include/asm/delay.h
index dff714d..ac33c56 100644
--- a/arch/arm/include/asm/delay.h
+++ b/arch/arm/include/asm/delay.h
@@ -15,6 +15,8 @@
 
 #ifndef __ASSEMBLY__
 
+#include <linux/printk.h>
+
 struct delay_timer {
 	unsigned long (*read_current_timer)(void);
 	unsigned long freq;
@@ -51,11 +53,34 @@ extern void __bad_udelay(void);
 #define __udelay(n)		arm_delay_ops.udelay(n)
 #define __const_udelay(n)	arm_delay_ops.const_udelay(n)
 
+#ifdef DEBUG
+#define __udelay_debug_max_delay(n)					\
+do {									\
+	if (n > MAX_UDELAY_MS * 1000) {					\
+		pr_debug("udelay(%d) too large - Convert to mdelay\n", n); \
+		dump_stack();						\
+	}								\
+} while (0)
+#else
+#define __udelay_debug_max_delay(n)					\
+	do {} while (0)
+#endif
+
 #define udelay(n)							\
-	(__builtin_constant_p(n) ?					\
-	  ((n) > (MAX_UDELAY_MS * 1000) ? __bad_udelay() :		\
-			__const_udelay((n) * UDELAY_MULT)) :		\
-	  __udelay(n))
+({									\
+	if (__builtin_constant_p(n)) {					\
+		typeof n _n = n;					\
+		while (_n > MAX_UDELAY_MS * 1000) {			\
+			__const_udelay(MAX_UDELAY_MS * 1000 * UDELAY_MULT); \
+			_n -= MAX_UDELAY_MS * 1000;			\
+		}							\
+		if (_n)							\
+			__const_udelay(_n * 1000 * UDELAY_MULT);	\
+	} else {							\
+		__udelay_debug_max_delay(n);				\
+		__udelay(n);						\
+	}								\
+})
 
 /* Loop-based definitions for assembly code. */
 extern void __loop_delay(unsigned long loops);

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Joe Perches]

On Tue, 2014-02-04 at 08:03 +0100, Holger Schurig wrote:
> Joe, look in linux/arch/arm/include/asm/delay.h. The macro udelay
> cannot handle large values because of lost-of-precision.
> 
> IMHO udelay on ARM is broken, because it also cannot work with fast
> ARM processors (where bogomips >= 3355, which is in sight now). It's
> just not broken enought that someone did something against it ...   so
> the current kludge is good enought.

Maybe something like this would be better?

---
 arch/arm/include/asm/delay.h | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/arch/arm/include/asm/delay.h b/arch/arm/include/asm/delay.h
index dff714d..ac33c56 100644
--- a/arch/arm/include/asm/delay.h
+++ b/arch/arm/include/asm/delay.h
@@ -15,6 +15,8 @@
 
 #ifndef __ASSEMBLY__
 
+#include <linux/printk.h>
+
 struct delay_timer {
 	unsigned long (*read_current_timer)(void);
 	unsigned long freq;
@@ -51,11 +53,34 @@ extern void __bad_udelay(void);
 #define __udelay(n)		arm_delay_ops.udelay(n)
 #define __const_udelay(n)	arm_delay_ops.const_udelay(n)
 
+#ifdef DEBUG
+#define __udelay_debug_max_delay(n)					\
+do {									\
+	if (n > MAX_UDELAY_MS * 1000) {					\
+		pr_debug("udelay(%d) too large - Convert to mdelay\n", n); \
+		dump_stack();						\
+	}								\
+} while (0)
+#else
+#define __udelay_debug_max_delay(n)					\
+	do {} while (0)
+#endif
+
 #define udelay(n)							\
-	(__builtin_constant_p(n) ?					\
-	  ((n) > (MAX_UDELAY_MS * 1000) ? __bad_udelay() :		\
-			__const_udelay((n) * UDELAY_MULT)) :		\
-	  __udelay(n))
+({									\
+	if (__builtin_constant_p(n)) {					\
+		typeof n _n = n;					\
+		while (_n > MAX_UDELAY_MS * 1000) {			\
+			__const_udelay(MAX_UDELAY_MS * 1000 * UDELAY_MULT); \
+			_n -= MAX_UDELAY_MS * 1000;			\
+		}							\
+		if (_n)							\
+			__const_udelay(_n * 1000 * UDELAY_MULT);	\
+	} else {							\
+		__udelay_debug_max_delay(n);				\
+		__udelay(n);						\
+	}								\
+})
 
 /* Loop-based definitions for assembly code. */
 extern void __loop_delay(unsigned long loops);

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Joe Perches]

On Tue, 2014-02-04 at 08:03 +0100, Holger Schurig wrote:
> Joe, look in linux/arch/arm/include/asm/delay.h. The macro udelay
> cannot handle large values because of lost-of-precision.
> 
> IMHO udelay on ARM is broken, because it also cannot work with fast
> ARM processors (where bogomips >= 3355, which is in sight now). It's
> just not broken enought that someone did something against it ...   so
> the current kludge is good enought.

Maybe something like this would be better?

---
 arch/arm/include/asm/delay.h | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/arch/arm/include/asm/delay.h b/arch/arm/include/asm/delay.h
index dff714d..ac33c56 100644
--- a/arch/arm/include/asm/delay.h
+++ b/arch/arm/include/asm/delay.h
@@ -15,6 +15,8 @@
 
 #ifndef __ASSEMBLY__
 
+#include <linux/printk.h>
+
 struct delay_timer {
 	unsigned long (*read_current_timer)(void);
 	unsigned long freq;
@@ -51,11 +53,34 @@ extern void __bad_udelay(void);
 #define __udelay(n)		arm_delay_ops.udelay(n)
 #define __const_udelay(n)	arm_delay_ops.const_udelay(n)
 
+#ifdef DEBUG
+#define __udelay_debug_max_delay(n)					\
+do {									\
+	if (n > MAX_UDELAY_MS * 1000) {					\
+		pr_debug("udelay(%d) too large - Convert to mdelay\n", n); \
+		dump_stack();						\
+	}								\
+} while (0)
+#else
+#define __udelay_debug_max_delay(n)					\
+	do {} while (0)
+#endif
+
 #define udelay(n)							\
-	(__builtin_constant_p(n) ?					\
-	  ((n) > (MAX_UDELAY_MS * 1000) ? __bad_udelay() :		\
-			__const_udelay((n) * UDELAY_MULT)) :		\
-	  __udelay(n))
+({									\
+	if (__builtin_constant_p(n)) {					\
+		typeof n _n = n;					\
+		while (_n > MAX_UDELAY_MS * 1000) {			\
+			__const_udelay(MAX_UDELAY_MS * 1000 * UDELAY_MULT); \
+			_n -= MAX_UDELAY_MS * 1000;			\
+		}							\
+		if (_n)							\
+			__const_udelay(_n * 1000 * UDELAY_MULT);	\
+	} else {							\
+		__udelay_debug_max_delay(n);				\
+		__udelay(n);						\
+	}								\
+})
 
 /* Loop-based definitions for assembly code. */
 extern void __loop_delay(unsigned long loops);

Re: [ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Arnd Bergmann]

On Tuesday 04 February 2014 08:36:36 Joe Perches wrote:
> On Tue, 2014-02-04 at 08:03 +0100, Holger Schurig wrote:
> > Joe, look in linux/arch/arm/include/asm/delay.h. The macro udelay
> > cannot handle large values because of lost-of-precision.
> > 
> > IMHO udelay on ARM is broken, because it also cannot work with fast
> > ARM processors (where bogomips >= 3355, which is in sight now). It's
> > just not broken enought that someone did something against it ...   so
> > the current kludge is good enought.
> 
> Maybe something like this would be better?
> 

I actually like the fact that we get link errors for insane 'udelay'
times. In most cases it's a driver bug because we shouldn't keep
the CPU busy for an eternity in the kernel (and call msleep() instead).
For the rare cases where mdelay makes sense, we also want to add
a comment to the code explaining why msleep cannot be used.

	Arnd

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Arnd Bergmann]

On Tuesday 04 February 2014 08:36:36 Joe Perches wrote:
> On Tue, 2014-02-04 at 08:03 +0100, Holger Schurig wrote:
> > Joe, look in linux/arch/arm/include/asm/delay.h. The macro udelay
> > cannot handle large values because of lost-of-precision.
> > 
> > IMHO udelay on ARM is broken, because it also cannot work with fast
> > ARM processors (where bogomips >= 3355, which is in sight now). It's
> > just not broken enought that someone did something against it ...   so
> > the current kludge is good enought.
> 
> Maybe something like this would be better?
> 

I actually like the fact that we get link errors for insane 'udelay'
times. In most cases it's a driver bug because we shouldn't keep
the CPU busy for an eternity in the kernel (and call msleep() instead).
For the rare cases where mdelay makes sense, we also want to add
a comment to the code explaining why msleep cannot be used.

	Arnd

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Arnd Bergmann]

On Tuesday 04 February 2014 08:36:36 Joe Perches wrote:
> On Tue, 2014-02-04 at 08:03 +0100, Holger Schurig wrote:
> > Joe, look in linux/arch/arm/include/asm/delay.h. The macro udelay
> > cannot handle large values because of lost-of-precision.
> > 
> > IMHO udelay on ARM is broken, because it also cannot work with fast
> > ARM processors (where bogomips >= 3355, which is in sight now). It's
> > just not broken enought that someone did something against it ...   so
> > the current kludge is good enought.
> 
> Maybe something like this would be better?
> 

I actually like the fact that we get link errors for insane 'udelay'
times. In most cases it's a driver bug because we shouldn't keep
the CPU busy for an eternity in the kernel (and call msleep() instead).
For the rare cases where mdelay makes sense, we also want to add
a comment to the code explaining why msleep cannot be used.

	Arnd

Re: [ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Joe Perches]

On Tue, 2014-02-04 at 19:40 +0100, Arnd Bergmann wrote:
> On Tuesday 04 February 2014 08:36:36 Joe Perches wrote:
> > On Tue, 2014-02-04 at 08:03 +0100, Holger Schurig wrote:
> > > Joe, look in linux/arch/arm/include/asm/delay.h. The macro udelay
> > > cannot handle large values because of lost-of-precision.
> > > 
> > > IMHO udelay on ARM is broken, because it also cannot work with fast
> > > ARM processors (where bogomips >= 3355, which is in sight now). It's
> > > just not broken enought that someone did something against it ...   so
> > > the current kludge is good enought.
> > 
> > Maybe something like this would be better?
> > 
> 
> I actually like the fact that we get link errors for insane 'udelay'
> times.

For static values yes, for computed values no.

This emits a warning/dump_stack on the non-static
values.  It could also emit some similar #warning
on static values > insane too.

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Joe Perches]

On Tue, 2014-02-04 at 19:40 +0100, Arnd Bergmann wrote:
> On Tuesday 04 February 2014 08:36:36 Joe Perches wrote:
> > On Tue, 2014-02-04 at 08:03 +0100, Holger Schurig wrote:
> > > Joe, look in linux/arch/arm/include/asm/delay.h. The macro udelay
> > > cannot handle large values because of lost-of-precision.
> > > 
> > > IMHO udelay on ARM is broken, because it also cannot work with fast
> > > ARM processors (where bogomips >= 3355, which is in sight now). It's
> > > just not broken enought that someone did something against it ...   so
> > > the current kludge is good enought.
> > 
> > Maybe something like this would be better?
> > 
> 
> I actually like the fact that we get link errors for insane 'udelay'
> times.

For static values yes, for computed values no.

This emits a warning/dump_stack on the non-static
values.  It could also emit some similar #warning
on static values > insane too.

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Joe Perches]

On Tue, 2014-02-04 at 19:40 +0100, Arnd Bergmann wrote:
> On Tuesday 04 February 2014 08:36:36 Joe Perches wrote:
> > On Tue, 2014-02-04 at 08:03 +0100, Holger Schurig wrote:
> > > Joe, look in linux/arch/arm/include/asm/delay.h. The macro udelay
> > > cannot handle large values because of lost-of-precision.
> > > 
> > > IMHO udelay on ARM is broken, because it also cannot work with fast
> > > ARM processors (where bogomips >= 3355, which is in sight now). It's
> > > just not broken enought that someone did something against it ...   so
> > > the current kludge is good enought.
> > 
> > Maybe something like this would be better?
> > 
> 
> I actually like the fact that we get link errors for insane 'udelay'
> times.

For static values yes, for computed values no.

This emits a warning/dump_stack on the non-static
values.  It could also emit some similar #warning
on static values > insane too.

Re: [ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Russell King - ARM Linux]

On Tue, Feb 04, 2014 at 08:36:36AM -0800, Joe Perches wrote:
> On Tue, 2014-02-04 at 08:03 +0100, Holger Schurig wrote:
> > Joe, look in linux/arch/arm/include/asm/delay.h. The macro udelay
> > cannot handle large values because of lost-of-precision.
> > 
> > IMHO udelay on ARM is broken, because it also cannot work with fast
> > ARM processors (where bogomips >= 3355, which is in sight now). It's
> > just not broken enought that someone did something against it ...   so
> > the current kludge is good enought.
> 
> Maybe something like this would be better?

No, the point of __bad_udelay() is that people doing stupidly large
udelay()s result in build errors, rather than having to run the kernel
and trip over a non-existent debugging message beacuse they haven't
built the kernel with DEBUG defined.

NAK.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Russell King - ARM Linux]

On Tue, Feb 04, 2014 at 08:36:36AM -0800, Joe Perches wrote:
> On Tue, 2014-02-04 at 08:03 +0100, Holger Schurig wrote:
> > Joe, look in linux/arch/arm/include/asm/delay.h. The macro udelay
> > cannot handle large values because of lost-of-precision.
> > 
> > IMHO udelay on ARM is broken, because it also cannot work with fast
> > ARM processors (where bogomips >= 3355, which is in sight now). It's
> > just not broken enought that someone did something against it ...   so
> > the current kludge is good enought.
> 
> Maybe something like this would be better?

No, the point of __bad_udelay() is that people doing stupidly large
udelay()s result in build errors, rather than having to run the kernel
and trip over a non-existent debugging message beacuse they haven't
built the kernel with DEBUG defined.

NAK.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Russell King - ARM Linux]

On Tue, Feb 04, 2014 at 08:36:36AM -0800, Joe Perches wrote:
> On Tue, 2014-02-04 at 08:03 +0100, Holger Schurig wrote:
> > Joe, look in linux/arch/arm/include/asm/delay.h. The macro udelay
> > cannot handle large values because of lost-of-precision.
> > 
> > IMHO udelay on ARM is broken, because it also cannot work with fast
> > ARM processors (where bogomips >= 3355, which is in sight now). It's
> > just not broken enought that someone did something against it ...   so
> > the current kludge is good enought.
> 
> Maybe something like this would be better?

No, the point of __bad_udelay() is that people doing stupidly large
udelay()s result in build errors, rather than having to run the kernel
and trip over a non-existent debugging message beacuse they haven't
built the kernel with DEBUG defined.

NAK.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

Re: [ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Joe Perches]

On Wed, 2014-02-05 at 11:50 +0000, Russell King - ARM Linux wrote:
> On Tue, Feb 04, 2014 at 08:36:36AM -0800, Joe Perches wrote:
> > On Tue, 2014-02-04 at 08:03 +0100, Holger Schurig wrote:
> > > Joe, look in linux/arch/arm/include/asm/delay.h. The macro udelay
> > > cannot handle large values because of lost-of-precision.
> > > 
> > > IMHO udelay on ARM is broken, because it also cannot work with fast
> > > ARM processors (where bogomips >= 3355, which is in sight now). It's
> > > just not broken enought that someone did something against it ...   so
> > > the current kludge is good enought.
> > 
> > Maybe something like this would be better?
> 
> No, the point of __bad_udelay() is that people doing stupidly large
> udelay()s result in build errors,

Apparently, people just convert stupidly large udelay()s
to mdelay and not be bothered.

>  rather than having to run the kernel
> and trip over a non-existent debugging message beacuse they haven't
> built the kernel with DEBUG defined.
> 
> NAK.

<shrug>

Perhaps there should be some runtime udelay > maximum
supported check.

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Joe Perches]

On Wed, 2014-02-05 at 11:50 +0000, Russell King - ARM Linux wrote:
> On Tue, Feb 04, 2014 at 08:36:36AM -0800, Joe Perches wrote:
> > On Tue, 2014-02-04 at 08:03 +0100, Holger Schurig wrote:
> > > Joe, look in linux/arch/arm/include/asm/delay.h. The macro udelay
> > > cannot handle large values because of lost-of-precision.
> > > 
> > > IMHO udelay on ARM is broken, because it also cannot work with fast
> > > ARM processors (where bogomips >= 3355, which is in sight now). It's
> > > just not broken enought that someone did something against it ...   so
> > > the current kludge is good enought.
> > 
> > Maybe something like this would be better?
> 
> No, the point of __bad_udelay() is that people doing stupidly large
> udelay()s result in build errors,

Apparently, people just convert stupidly large udelay()s
to mdelay and not be bothered.

>  rather than having to run the kernel
> and trip over a non-existent debugging message beacuse they haven't
> built the kernel with DEBUG defined.
> 
> NAK.

<shrug>

Perhaps there should be some runtime udelay > maximum
supported check.

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Joe Perches]

On Wed, 2014-02-05 at 11:50 +0000, Russell King - ARM Linux wrote:
> On Tue, Feb 04, 2014 at 08:36:36AM -0800, Joe Perches wrote:
> > On Tue, 2014-02-04 at 08:03 +0100, Holger Schurig wrote:
> > > Joe, look in linux/arch/arm/include/asm/delay.h. The macro udelay
> > > cannot handle large values because of lost-of-precision.
> > > 
> > > IMHO udelay on ARM is broken, because it also cannot work with fast
> > > ARM processors (where bogomips >= 3355, which is in sight now). It's
> > > just not broken enought that someone did something against it ...   so
> > > the current kludge is good enought.
> > 
> > Maybe something like this would be better?
> 
> No, the point of __bad_udelay() is that people doing stupidly large
> udelay()s result in build errors,

Apparently, people just convert stupidly large udelay()s
to mdelay and not be bothered.

>  rather than having to run the kernel
> and trip over a non-existent debugging message beacuse they haven't
> built the kernel with DEBUG defined.
> 
> NAK.

<shrug>

Perhaps there should be some runtime udelay > maximum
supported check.

Re: [ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Russell King - ARM Linux]

On Wed, Feb 05, 2014 at 04:32:46AM -0800, Joe Perches wrote:
> On Wed, 2014-02-05 at 11:50 +0000, Russell King - ARM Linux wrote:
> > On Tue, Feb 04, 2014 at 08:36:36AM -0800, Joe Perches wrote:
> > > On Tue, 2014-02-04 at 08:03 +0100, Holger Schurig wrote:
> > > > Joe, look in linux/arch/arm/include/asm/delay.h. The macro udelay
> > > > cannot handle large values because of lost-of-precision.
> > > > 
> > > > IMHO udelay on ARM is broken, because it also cannot work with fast
> > > > ARM processors (where bogomips >= 3355, which is in sight now). It's
> > > > just not broken enought that someone did something against it ...   so
> > > > the current kludge is good enought.
> > > 
> > > Maybe something like this would be better?
> > 
> > No, the point of __bad_udelay() is that people doing stupidly large
> > udelay()s result in build errors,
> 
> Apparently, people just convert stupidly large udelay()s
> to mdelay and not be bothered.

And that's the correct answer.  Having udelay(10000) rather than mdelay(10)
is a sign that they weren't paying that much attention when writing the
code.

> Perhaps there should be some runtime udelay > maximum supported check.

Having both a runtime check _and_ a compile time check would actually
be a good thing, but any runtime check needs to be suitably rate-
limited.

The compile time check is very important because it catches a lot of
cases which wouldn't otherwise be found (eg, in drivers which hardly
anyone uses on ARM.)

Maybe the compile time check should be something which is implemented
in a cross-architecture way in linux/delay.h with the maximum set to
the lowest that any architecture can do?

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Russell King - ARM Linux]

On Wed, Feb 05, 2014 at 04:32:46AM -0800, Joe Perches wrote:
> On Wed, 2014-02-05 at 11:50 +0000, Russell King - ARM Linux wrote:
> > On Tue, Feb 04, 2014 at 08:36:36AM -0800, Joe Perches wrote:
> > > On Tue, 2014-02-04 at 08:03 +0100, Holger Schurig wrote:
> > > > Joe, look in linux/arch/arm/include/asm/delay.h. The macro udelay
> > > > cannot handle large values because of lost-of-precision.
> > > > 
> > > > IMHO udelay on ARM is broken, because it also cannot work with fast
> > > > ARM processors (where bogomips >= 3355, which is in sight now). It's
> > > > just not broken enought that someone did something against it ...   so
> > > > the current kludge is good enought.
> > > 
> > > Maybe something like this would be better?
> > 
> > No, the point of __bad_udelay() is that people doing stupidly large
> > udelay()s result in build errors,
> 
> Apparently, people just convert stupidly large udelay()s
> to mdelay and not be bothered.

And that's the correct answer.  Having udelay(10000) rather than mdelay(10)
is a sign that they weren't paying that much attention when writing the
code.

> Perhaps there should be some runtime udelay > maximum supported check.

Having both a runtime check _and_ a compile time check would actually
be a good thing, but any runtime check needs to be suitably rate-
limited.

The compile time check is very important because it catches a lot of
cases which wouldn't otherwise be found (eg, in drivers which hardly
anyone uses on ARM.)

Maybe the compile time check should be something which is implemented
in a cross-architecture way in linux/delay.h with the maximum set to
the lowest that any architecture can do?

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Russell King - ARM Linux]

On Wed, Feb 05, 2014 at 04:32:46AM -0800, Joe Perches wrote:
> On Wed, 2014-02-05 at 11:50 +0000, Russell King - ARM Linux wrote:
> > On Tue, Feb 04, 2014 at 08:36:36AM -0800, Joe Perches wrote:
> > > On Tue, 2014-02-04 at 08:03 +0100, Holger Schurig wrote:
> > > > Joe, look in linux/arch/arm/include/asm/delay.h. The macro udelay
> > > > cannot handle large values because of lost-of-precision.
> > > > 
> > > > IMHO udelay on ARM is broken, because it also cannot work with fast
> > > > ARM processors (where bogomips >= 3355, which is in sight now). It's
> > > > just not broken enought that someone did something against it ...   so
> > > > the current kludge is good enought.
> > > 
> > > Maybe something like this would be better?
> > 
> > No, the point of __bad_udelay() is that people doing stupidly large
> > udelay()s result in build errors,
> 
> Apparently, people just convert stupidly large udelay()s
> to mdelay and not be bothered.

And that's the correct answer.  Having udelay(10000) rather than mdelay(10)
is a sign that they weren't paying that much attention when writing the
code.

> Perhaps there should be some runtime udelay > maximum supported check.

Having both a runtime check _and_ a compile time check would actually
be a good thing, but any runtime check needs to be suitably rate-
limited.

The compile time check is very important because it catches a lot of
cases which wouldn't otherwise be found (eg, in drivers which hardly
anyone uses on ARM.)

Maybe the compile time check should be something which is implemented
in a cross-architecture way in linux/delay.h with the maximum set to
the lowest that any architecture can do?

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

Re: [ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Joe Perches]

On Wed, 2014-02-05 at 12:41 +0000, Russell King - ARM Linux wrote:
> On Wed, Feb 05, 2014 at 04:32:46AM -0800, Joe Perches wrote:
> > Apparently, people just convert stupidly large udelay()s
> > to mdelay and not be bothered.
> 
> And that's the correct answer.  Having udelay(10000) rather than mdelay(10)
> is a sign that they weren't paying that much attention when writing the
> code.

Not really.

Look at the code that brought this up in the first place.

On Tue, 2014-02-04 at 08:37 +0530, Sujith Manoharan wrote:
> From: Sujith Manoharan <c_manoha@qca.qualcomm.com>
> 
> Use mdelay instead of udelay to fix this error:
> 
> ERROR: "__bad_udelay" [drivers/net/wireless/ath/ath9k/ath9k_hw.ko] undefined!
[]
diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
[]
> @@ -1316,7 +1316,7 @@ static bool ath9k_hw_set_reset(struct ath_hw *ah, int type)
>  	if (AR_SREV_9300_20_OR_LATER(ah))
>  		udelay(50);
>  	else if (AR_SREV_9100(ah))
> -		udelay(10000);
> +		mdelay(10);
>  	else
>  		udelay(100);
>  
> 
> >     if (AR_SREV_9300_20_OR_LATER(ah))
> >             udelay(50);
> >     else if (AR_SREV_9100(ah))
> > -           udelay(10000);
> > +           mdelay(10);
> >     else
> >             udelay(100);

One chip needs a larger delay than the others.

It's not so much not paying attention as not
knowing ARM is broken for large udelay().

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Joe Perches]

On Wed, 2014-02-05 at 12:41 +0000, Russell King - ARM Linux wrote:
> On Wed, Feb 05, 2014 at 04:32:46AM -0800, Joe Perches wrote:
> > Apparently, people just convert stupidly large udelay()s
> > to mdelay and not be bothered.
> 
> And that's the correct answer.  Having udelay(10000) rather than mdelay(10)
> is a sign that they weren't paying that much attention when writing the
> code.

Not really.

Look at the code that brought this up in the first place.

On Tue, 2014-02-04 at 08:37 +0530, Sujith Manoharan wrote:
> From: Sujith Manoharan <c_manoha@qca.qualcomm.com>
> 
> Use mdelay instead of udelay to fix this error:
> 
> ERROR: "__bad_udelay" [drivers/net/wireless/ath/ath9k/ath9k_hw.ko] undefined!
[]
diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
[]
> @@ -1316,7 +1316,7 @@ static bool ath9k_hw_set_reset(struct ath_hw *ah, int type)
>  	if (AR_SREV_9300_20_OR_LATER(ah))
>  		udelay(50);
>  	else if (AR_SREV_9100(ah))
> -		udelay(10000);
> +		mdelay(10);
>  	else
>  		udelay(100);
>  
> 
> >     if (AR_SREV_9300_20_OR_LATER(ah))
> >             udelay(50);
> >     else if (AR_SREV_9100(ah))
> > -           udelay(10000);
> > +           mdelay(10);
> >     else
> >             udelay(100);

One chip needs a larger delay than the others.

It's not so much not paying attention as not
knowing ARM is broken for large udelay().

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Joe Perches]

On Wed, 2014-02-05 at 12:41 +0000, Russell King - ARM Linux wrote:
> On Wed, Feb 05, 2014 at 04:32:46AM -0800, Joe Perches wrote:
> > Apparently, people just convert stupidly large udelay()s
> > to mdelay and not be bothered.
> 
> And that's the correct answer.  Having udelay(10000) rather than mdelay(10)
> is a sign that they weren't paying that much attention when writing the
> code.

Not really.

Look at the code that brought this up in the first place.

On Tue, 2014-02-04 at 08:37 +0530, Sujith Manoharan wrote:
> From: Sujith Manoharan <c_manoha@qca.qualcomm.com>
> 
> Use mdelay instead of udelay to fix this error:
> 
> ERROR: "__bad_udelay" [drivers/net/wireless/ath/ath9k/ath9k_hw.ko] undefined!
[]
diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
[]
> @@ -1316,7 +1316,7 @@ static bool ath9k_hw_set_reset(struct ath_hw *ah, int type)
>  	if (AR_SREV_9300_20_OR_LATER(ah))
>  		udelay(50);
>  	else if (AR_SREV_9100(ah))
> -		udelay(10000);
> +		mdelay(10);
>  	else
>  		udelay(100);
>  
> 
> >     if (AR_SREV_9300_20_OR_LATER(ah))
> >             udelay(50);
> >     else if (AR_SREV_9100(ah))
> > -           udelay(10000);
> > +           mdelay(10);
> >     else
> >             udelay(100);

One chip needs a larger delay than the others.

It's not so much not paying attention as not
knowing ARM is broken for large udelay().

Re: [ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Russell King - ARM Linux]

On Wed, Feb 05, 2014 at 05:04:54AM -0800, Joe Perches wrote:
> On Wed, 2014-02-05 at 12:41 +0000, Russell King - ARM Linux wrote:
> > On Wed, Feb 05, 2014 at 04:32:46AM -0800, Joe Perches wrote:
> > > Apparently, people just convert stupidly large udelay()s
> > > to mdelay and not be bothered.
> > 
> > And that's the correct answer.  Having udelay(10000) rather than mdelay(10)
> > is a sign that they weren't paying that much attention when writing the
> > code.
> 
> Not really.
> 
> Look at the code that brought this up in the first place.
> 
> On Tue, 2014-02-04 at 08:37 +0530, Sujith Manoharan wrote:
> > From: Sujith Manoharan <c_manoha@qca.qualcomm.com>
> > 
> > Use mdelay instead of udelay to fix this error:
> > 
> > ERROR: "__bad_udelay" [drivers/net/wireless/ath/ath9k/ath9k_hw.ko] undefined!
> []
> diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
> []
> > @@ -1316,7 +1316,7 @@ static bool ath9k_hw_set_reset(struct ath_hw *ah, int type)
> >  	if (AR_SREV_9300_20_OR_LATER(ah))
> >  		udelay(50);
> >  	else if (AR_SREV_9100(ah))
> > -		udelay(10000);
> > +		mdelay(10);
> >  	else
> >  		udelay(100);
> >  
> > 
> > >     if (AR_SREV_9300_20_OR_LATER(ah))
> > >             udelay(50);
> > >     else if (AR_SREV_9100(ah))
> > > -           udelay(10000);
> > > +           mdelay(10);
> > >     else
> > >             udelay(100);
> 
> One chip needs a larger delay than the others.
> 
> It's not so much not paying attention as not
> knowing ARM is broken for large udelay().

And now read my suggestion about how to avoid the "not knowing" problem. :)

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Russell King - ARM Linux]

On Wed, Feb 05, 2014 at 05:04:54AM -0800, Joe Perches wrote:
> On Wed, 2014-02-05 at 12:41 +0000, Russell King - ARM Linux wrote:
> > On Wed, Feb 05, 2014 at 04:32:46AM -0800, Joe Perches wrote:
> > > Apparently, people just convert stupidly large udelay()s
> > > to mdelay and not be bothered.
> > 
> > And that's the correct answer.  Having udelay(10000) rather than mdelay(10)
> > is a sign that they weren't paying that much attention when writing the
> > code.
> 
> Not really.
> 
> Look at the code that brought this up in the first place.
> 
> On Tue, 2014-02-04 at 08:37 +0530, Sujith Manoharan wrote:
> > From: Sujith Manoharan <c_manoha@qca.qualcomm.com>
> > 
> > Use mdelay instead of udelay to fix this error:
> > 
> > ERROR: "__bad_udelay" [drivers/net/wireless/ath/ath9k/ath9k_hw.ko] undefined!
> []
> diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
> []
> > @@ -1316,7 +1316,7 @@ static bool ath9k_hw_set_reset(struct ath_hw *ah, int type)
> >  	if (AR_SREV_9300_20_OR_LATER(ah))
> >  		udelay(50);
> >  	else if (AR_SREV_9100(ah))
> > -		udelay(10000);
> > +		mdelay(10);
> >  	else
> >  		udelay(100);
> >  
> > 
> > >     if (AR_SREV_9300_20_OR_LATER(ah))
> > >             udelay(50);
> > >     else if (AR_SREV_9100(ah))
> > > -           udelay(10000);
> > > +           mdelay(10);
> > >     else
> > >             udelay(100);
> 
> One chip needs a larger delay than the others.
> 
> It's not so much not paying attention as not
> knowing ARM is broken for large udelay().

And now read my suggestion about how to avoid the "not knowing" problem. :)

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Russell King - ARM Linux]

On Wed, Feb 05, 2014 at 05:04:54AM -0800, Joe Perches wrote:
> On Wed, 2014-02-05 at 12:41 +0000, Russell King - ARM Linux wrote:
> > On Wed, Feb 05, 2014 at 04:32:46AM -0800, Joe Perches wrote:
> > > Apparently, people just convert stupidly large udelay()s
> > > to mdelay and not be bothered.
> > 
> > And that's the correct answer.  Having udelay(10000) rather than mdelay(10)
> > is a sign that they weren't paying that much attention when writing the
> > code.
> 
> Not really.
> 
> Look at the code that brought this up in the first place.
> 
> On Tue, 2014-02-04 at 08:37 +0530, Sujith Manoharan wrote:
> > From: Sujith Manoharan <c_manoha@qca.qualcomm.com>
> > 
> > Use mdelay instead of udelay to fix this error:
> > 
> > ERROR: "__bad_udelay" [drivers/net/wireless/ath/ath9k/ath9k_hw.ko] undefined!
> []
> diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
> []
> > @@ -1316,7 +1316,7 @@ static bool ath9k_hw_set_reset(struct ath_hw *ah, int type)
> >  	if (AR_SREV_9300_20_OR_LATER(ah))
> >  		udelay(50);
> >  	else if (AR_SREV_9100(ah))
> > -		udelay(10000);
> > +		mdelay(10);
> >  	else
> >  		udelay(100);
> >  
> > 
> > >     if (AR_SREV_9300_20_OR_LATER(ah))
> > >             udelay(50);
> > >     else if (AR_SREV_9100(ah))
> > > -           udelay(10000);
> > > +           mdelay(10);
> > >     else
> > >             udelay(100);
> 
> One chip needs a larger delay than the others.
> 
> It's not so much not paying attention as not
> knowing ARM is broken for large udelay().

And now read my suggestion about how to avoid the "not knowing" problem. :)

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

Re: [ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Joe Perches]

On Wed, 2014-02-05 at 13:39 +0000, Russell King - ARM Linux wrote:
> On Wed, Feb 05, 2014 at 05:04:54AM -0800, Joe Perches wrote:
> > On Wed, 2014-02-05 at 12:41 +0000, Russell King - ARM Linux wrote:
> > > On Wed, Feb 05, 2014 at 04:32:46AM -0800, Joe Perches wrote:
> > > > Apparently, people just convert stupidly large udelay()s
> > > > to mdelay and not be bothered.
> > > 
> > > And that's the correct answer.  Having udelay(10000) rather than mdelay(10)
> > > is a sign that they weren't paying that much attention when writing the
> > > code.
> > 
> > Not really.
[]
> > It's not so much not paying attention as not
> > knowing ARM is broken for large udelay().
> 
> And now read my suggestion about how to avoid the "not knowing" problem. :)

I'd read it already.  I didn't and don't disagree.

I still think adding a #warning on large static udelay()s
would be sensible.  Maybe adding another option like
#define UDELAY_TOO_BIG_I_KNOW_ALREADY_DONT_BOTHER_ME
guard to avoid seeing the #warning when there's no other
option.

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Joe Perches]

On Wed, 2014-02-05 at 13:39 +0000, Russell King - ARM Linux wrote:
> On Wed, Feb 05, 2014 at 05:04:54AM -0800, Joe Perches wrote:
> > On Wed, 2014-02-05 at 12:41 +0000, Russell King - ARM Linux wrote:
> > > On Wed, Feb 05, 2014 at 04:32:46AM -0800, Joe Perches wrote:
> > > > Apparently, people just convert stupidly large udelay()s
> > > > to mdelay and not be bothered.
> > > 
> > > And that's the correct answer.  Having udelay(10000) rather than mdelay(10)
> > > is a sign that they weren't paying that much attention when writing the
> > > code.
> > 
> > Not really.
[]
> > It's not so much not paying attention as not
> > knowing ARM is broken for large udelay().
> 
> And now read my suggestion about how to avoid the "not knowing" problem. :)

I'd read it already.  I didn't and don't disagree.

I still think adding a #warning on large static udelay()s
would be sensible.  Maybe adding another option like
#define UDELAY_TOO_BIG_I_KNOW_ALREADY_DONT_BOTHER_ME
guard to avoid seeing the #warning when there's no other
option.

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Joe Perches]

On Wed, 2014-02-05 at 13:39 +0000, Russell King - ARM Linux wrote:
> On Wed, Feb 05, 2014 at 05:04:54AM -0800, Joe Perches wrote:
> > On Wed, 2014-02-05 at 12:41 +0000, Russell King - ARM Linux wrote:
> > > On Wed, Feb 05, 2014 at 04:32:46AM -0800, Joe Perches wrote:
> > > > Apparently, people just convert stupidly large udelay()s
> > > > to mdelay and not be bothered.
> > > 
> > > And that's the correct answer.  Having udelay(10000) rather than mdelay(10)
> > > is a sign that they weren't paying that much attention when writing the
> > > code.
> > 
> > Not really.
[]
> > It's not so much not paying attention as not
> > knowing ARM is broken for large udelay().
> 
> And now read my suggestion about how to avoid the "not knowing" problem. :)

I'd read it already.  I didn't and don't disagree.

I still think adding a #warning on large static udelay()s
would be sensible.  Maybe adding another option like
#define UDELAY_TOO_BIG_I_KNOW_ALREADY_DONT_BOTHER_ME
guard to avoid seeing the #warning when there's no other
option.

Re: [ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Russell King - ARM Linux]

On Wed, Feb 05, 2014 at 06:03:13AM -0800, Joe Perches wrote:
> On Wed, 2014-02-05 at 13:39 +0000, Russell King - ARM Linux wrote:
> > On Wed, Feb 05, 2014 at 05:04:54AM -0800, Joe Perches wrote:
> > > On Wed, 2014-02-05 at 12:41 +0000, Russell King - ARM Linux wrote:
> > > > On Wed, Feb 05, 2014 at 04:32:46AM -0800, Joe Perches wrote:
> > > > > Apparently, people just convert stupidly large udelay()s
> > > > > to mdelay and not be bothered.
> > > > 
> > > > And that's the correct answer.  Having udelay(10000) rather than mdelay(10)
> > > > is a sign that they weren't paying that much attention when writing the
> > > > code.
> > > 
> > > Not really.
> []
> > > It's not so much not paying attention as not
> > > knowing ARM is broken for large udelay().
> > 
> > And now read my suggestion about how to avoid the "not knowing" problem. :)
> 
> I'd read it already.  I didn't and don't disagree.

Please explain /why/ you don't agree.

> I still think adding a #warning on large static udelay()s
> would be sensible.  Maybe adding another option like
> #define UDELAY_TOO_BIG_I_KNOW_ALREADY_DONT_BOTHER_ME
> guard to avoid seeing the #warning when there's no other
> option.

How does that help?  It's /not/ a warning situation - if you ask for
udelay(10000) on ARM, you will /not/ get a 10ms delay.  You'll instead
get something much shorter because the arithmetic will overflow.

Now, you could argue that maybe ARM's udelay() implementation should
know about this and implement a loop around the underlying udelay()
implementation to achieve it, and I might agree with you - but I
don't for one very simple reason: we /already/ have such an
implementation.  It's called mdelay():

#ifndef mdelay
#define mdelay(n) (\
        (__builtin_constant_p(n) && (n)<=MAX_UDELAY_MS) ? udelay((n)*1000) : \
        ({unsigned long __ms=(n); while (__ms--) udelay(1000);}))
#endif

So, the right answer is /not/ do duplicate this, but to /use/ the
appropriate facilities provided by the kernel.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Russell King - ARM Linux]

On Wed, Feb 05, 2014 at 06:03:13AM -0800, Joe Perches wrote:
> On Wed, 2014-02-05 at 13:39 +0000, Russell King - ARM Linux wrote:
> > On Wed, Feb 05, 2014 at 05:04:54AM -0800, Joe Perches wrote:
> > > On Wed, 2014-02-05 at 12:41 +0000, Russell King - ARM Linux wrote:
> > > > On Wed, Feb 05, 2014 at 04:32:46AM -0800, Joe Perches wrote:
> > > > > Apparently, people just convert stupidly large udelay()s
> > > > > to mdelay and not be bothered.
> > > > 
> > > > And that's the correct answer.  Having udelay(10000) rather than mdelay(10)
> > > > is a sign that they weren't paying that much attention when writing the
> > > > code.
> > > 
> > > Not really.
> []
> > > It's not so much not paying attention as not
> > > knowing ARM is broken for large udelay().
> > 
> > And now read my suggestion about how to avoid the "not knowing" problem. :)
> 
> I'd read it already.  I didn't and don't disagree.

Please explain /why/ you don't agree.

> I still think adding a #warning on large static udelay()s
> would be sensible.  Maybe adding another option like
> #define UDELAY_TOO_BIG_I_KNOW_ALREADY_DONT_BOTHER_ME
> guard to avoid seeing the #warning when there's no other
> option.

How does that help?  It's /not/ a warning situation - if you ask for
udelay(10000) on ARM, you will /not/ get a 10ms delay.  You'll instead
get something much shorter because the arithmetic will overflow.

Now, you could argue that maybe ARM's udelay() implementation should
know about this and implement a loop around the underlying udelay()
implementation to achieve it, and I might agree with you - but I
don't for one very simple reason: we /already/ have such an
implementation.  It's called mdelay():

#ifndef mdelay
#define mdelay(n) (\
        (__builtin_constant_p(n) && (n)<=MAX_UDELAY_MS) ? udelay((n)*1000) : \
        ({unsigned long __ms=(n); while (__ms--) udelay(1000);}))
#endif

So, the right answer is /not/ do duplicate this, but to /use/ the
appropriate facilities provided by the kernel.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Russell King - ARM Linux]

On Wed, Feb 05, 2014 at 06:03:13AM -0800, Joe Perches wrote:
> On Wed, 2014-02-05 at 13:39 +0000, Russell King - ARM Linux wrote:
> > On Wed, Feb 05, 2014 at 05:04:54AM -0800, Joe Perches wrote:
> > > On Wed, 2014-02-05 at 12:41 +0000, Russell King - ARM Linux wrote:
> > > > On Wed, Feb 05, 2014 at 04:32:46AM -0800, Joe Perches wrote:
> > > > > Apparently, people just convert stupidly large udelay()s
> > > > > to mdelay and not be bothered.
> > > > 
> > > > And that's the correct answer.  Having udelay(10000) rather than mdelay(10)
> > > > is a sign that they weren't paying that much attention when writing the
> > > > code.
> > > 
> > > Not really.
> []
> > > It's not so much not paying attention as not
> > > knowing ARM is broken for large udelay().
> > 
> > And now read my suggestion about how to avoid the "not knowing" problem. :)
> 
> I'd read it already.  I didn't and don't disagree.

Please explain /why/ you don't agree.

> I still think adding a #warning on large static udelay()s
> would be sensible.  Maybe adding another option like
> #define UDELAY_TOO_BIG_I_KNOW_ALREADY_DONT_BOTHER_ME
> guard to avoid seeing the #warning when there's no other
> option.

How does that help?  It's /not/ a warning situation - if you ask for
udelay(10000) on ARM, you will /not/ get a 10ms delay.  You'll instead
get something much shorter because the arithmetic will overflow.

Now, you could argue that maybe ARM's udelay() implementation should
know about this and implement a loop around the underlying udelay()
implementation to achieve it, and I might agree with you - but I
don't for one very simple reason: we /already/ have such an
implementation.  It's called mdelay():

#ifndef mdelay
#define mdelay(n) (\
        (__builtin_constant_p(n) && (n)<=MAX_UDELAY_MS) ? udelay((n)*1000) : \
        ({unsigned long __ms=(n); while (__ms--) udelay(1000);}))
#endif

So, the right answer is /not/ do duplicate this, but to /use/ the
appropriate facilities provided by the kernel.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

Re: [ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Joe Perches]

On Wed, 2014-02-05 at 14:21 +0000, Russell King - ARM Linux wrote:
> On Wed, Feb 05, 2014 at 06:03:13AM -0800, Joe Perches wrote:
> > On Wed, 2014-02-05 at 13:39 +0000, Russell King - ARM Linux wrote:
> > > On Wed, Feb 05, 2014 at 05:04:54AM -0800, Joe Perches wrote:
> > > > On Wed, 2014-02-05 at 12:41 +0000, Russell King - ARM Linux wrote:
> > > > > On Wed, Feb 05, 2014 at 04:32:46AM -0800, Joe Perches wrote:
> > > > > > Apparently, people just convert stupidly large udelay()s
> > > > > > to mdelay and not be bothered.
> > > > > 
> > > > > And that's the correct answer.  Having udelay(10000) rather than mdelay(10)
> > > > > is a sign that they weren't paying that much attention when writing the
> > > > > code.
> > > > 
> > > > Not really.
> > []
> > > > It's not so much not paying attention as not
> > > > knowing ARM is broken for large udelay().
> > > 
> > > And now read my suggestion about how to avoid the "not knowing" problem. :)
> > 
> > I'd read it already.  I didn't and don't disagree.
> 
> Please explain /why/ you don't agree.

Please reread what I wrote.

We agree a lot more than you seem to think.

> > I still think adding a #warning on large static udelay()s
> > would be sensible.  Maybe adding another option like
> > #define UDELAY_TOO_BIG_I_KNOW_ALREADY_DONT_BOTHER_ME
> > guard to avoid seeing the #warning when there's no other
> > option.
> 
> How does that help?  It's /not/ a warning situation - if you ask for
> udelay(10000) on ARM, you will /not/ get a 10ms delay.  You'll instead
> get something much shorter because the arithmetic will overflow.

> Now, you could argue that maybe ARM's udelay() implementation should
> know about this and implement a loop around the underlying udelay()
> implementation to achieve it,

I suggested something like that was possible.

> and I might agree with you - but I
> don't for one very simple reason: we /already/ have such an
> implementation.  It's called mdelay():

I think mdelay should be for the case where the range
is too big for a udelay.  I think arm's 11 bit range
is unfortunately small.

I think we agree be nice to get a generic compiler
#warning when a __builtin_constant_p value is too large
and a ratelimited dmesg/warning for the runtime case.

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Joe Perches]

On Wed, 2014-02-05 at 14:21 +0000, Russell King - ARM Linux wrote:
> On Wed, Feb 05, 2014 at 06:03:13AM -0800, Joe Perches wrote:
> > On Wed, 2014-02-05 at 13:39 +0000, Russell King - ARM Linux wrote:
> > > On Wed, Feb 05, 2014 at 05:04:54AM -0800, Joe Perches wrote:
> > > > On Wed, 2014-02-05 at 12:41 +0000, Russell King - ARM Linux wrote:
> > > > > On Wed, Feb 05, 2014 at 04:32:46AM -0800, Joe Perches wrote:
> > > > > > Apparently, people just convert stupidly large udelay()s
> > > > > > to mdelay and not be bothered.
> > > > > 
> > > > > And that's the correct answer.  Having udelay(10000) rather than mdelay(10)
> > > > > is a sign that they weren't paying that much attention when writing the
> > > > > code.
> > > > 
> > > > Not really.
> > []
> > > > It's not so much not paying attention as not
> > > > knowing ARM is broken for large udelay().
> > > 
> > > And now read my suggestion about how to avoid the "not knowing" problem. :)
> > 
> > I'd read it already.  I didn't and don't disagree.
> 
> Please explain /why/ you don't agree.

Please reread what I wrote.

We agree a lot more than you seem to think.

> > I still think adding a #warning on large static udelay()s
> > would be sensible.  Maybe adding another option like
> > #define UDELAY_TOO_BIG_I_KNOW_ALREADY_DONT_BOTHER_ME
> > guard to avoid seeing the #warning when there's no other
> > option.
> 
> How does that help?  It's /not/ a warning situation - if you ask for
> udelay(10000) on ARM, you will /not/ get a 10ms delay.  You'll instead
> get something much shorter because the arithmetic will overflow.

> Now, you could argue that maybe ARM's udelay() implementation should
> know about this and implement a loop around the underlying udelay()
> implementation to achieve it,

I suggested something like that was possible.

> and I might agree with you - but I
> don't for one very simple reason: we /already/ have such an
> implementation.  It's called mdelay():

I think mdelay should be for the case where the range
is too big for a udelay.  I think arm's 11 bit range
is unfortunately small.

I think we agree be nice to get a generic compiler
#warning when a __builtin_constant_p value is too large
and a ratelimited dmesg/warning for the runtime case.

[ath9k-devel] [PATCH 1/3] ath9k: Fix build error on ARM

[Joe Perches]

On Wed, 2014-02-05 at 14:21 +0000, Russell King - ARM Linux wrote:
> On Wed, Feb 05, 2014 at 06:03:13AM -0800, Joe Perches wrote:
> > On Wed, 2014-02-05 at 13:39 +0000, Russell King - ARM Linux wrote:
> > > On Wed, Feb 05, 2014 at 05:04:54AM -0800, Joe Perches wrote:
> > > > On Wed, 2014-02-05 at 12:41 +0000, Russell King - ARM Linux wrote:
> > > > > On Wed, Feb 05, 2014 at 04:32:46AM -0800, Joe Perches wrote:
> > > > > > Apparently, people just convert stupidly large udelay()s
> > > > > > to mdelay and not be bothered.
> > > > > 
> > > > > And that's the correct answer.  Having udelay(10000) rather than mdelay(10)
> > > > > is a sign that they weren't paying that much attention when writing the
> > > > > code.
> > > > 
> > > > Not really.
> > []
> > > > It's not so much not paying attention as not
> > > > knowing ARM is broken for large udelay().
> > > 
> > > And now read my suggestion about how to avoid the "not knowing" problem. :)
> > 
> > I'd read it already.  I didn't and don't disagree.
> 
> Please explain /why/ you don't agree.

Please reread what I wrote.

We agree a lot more than you seem to think.

> > I still think adding a #warning on large static udelay()s
> > would be sensible.  Maybe adding another option like
> > #define UDELAY_TOO_BIG_I_KNOW_ALREADY_DONT_BOTHER_ME
> > guard to avoid seeing the #warning when there's no other
> > option.
> 
> How does that help?  It's /not/ a warning situation - if you ask for
> udelay(10000) on ARM, you will /not/ get a 10ms delay.  You'll instead
> get something much shorter because the arithmetic will overflow.

> Now, you could argue that maybe ARM's udelay() implementation should
> know about this and implement a loop around the underlying udelay()
> implementation to achieve it,

I suggested something like that was possible.

> and I might agree with you - but I
> don't for one very simple reason: we /already/ have such an
> implementation.  It's called mdelay():

I think mdelay should be for the case where the range
is too big for a udelay.  I think arm's 11 bit range
is unfortunately small.

I think we agree be nice to get a generic compiler
#warning when a __builtin_constant_p value is too large
and a ratelimited dmesg/warning for the runtime case.

[PATCH 0/3] ath9k patches

[Sujith Manoharan]

From: Sujith Manoharan <c_manoha@qca.qualcomm.com>

Fixes related to reset.

Sujith Manoharan (3):
  ath9k: Unify reset API
  ath9k: Set ATH_OP_HW_RESET before HW reset
  ath9k: Disable beacon tasklet during reset

 drivers/net/wireless/ath/ath9k/ath9k.h   |  3 +--
 drivers/net/wireless/ath/ath9k/channel.c |  2 +-
 drivers/net/wireless/ath/ath9k/main.c    | 26 +++++++++++++++++++++-----
 drivers/net/wireless/ath/ath9k/tx99.c    |  4 ++--
 4 files changed, 25 insertions(+), 10 deletions(-)

-- 
2.1.2

[PATCH 0/3] ath9k patches

[Sujith Manoharan]

From: Sujith Manoharan <c_manoha@qca.qualcomm.com>

A few more MCC fixes for 3.18.

Sujith Manoharan (3):
  ath9k: Use a subroutine to assign HW queues
  ath9k: Fix offchannel operation
  ath9k: Fix MCC scanning

 drivers/net/wireless/ath/ath9k/ath9k.h   |  1 +
 drivers/net/wireless/ath/ath9k/channel.c |  8 +++++++-
 drivers/net/wireless/ath/ath9k/init.c    |  2 +-
 drivers/net/wireless/ath/ath9k/main.c    | 33 ++++++++++++++++----------------
 4 files changed, 26 insertions(+), 18 deletions(-)

-- 
2.1.0

[PATCH 0/3] ath9k patches

[Sujith Manoharan]

From: Sujith Manoharan <c_manoha@qca.qualcomm.com>

This series adds initial support for AR9462 2.1

Sujith Manoharan (3):
  ath9k: Add version macros for AR9462 2.1
  ath9k: Add initvals for AR9462 2.1
  ath9k: Program initvals for AR9462 2.1

 drivers/net/wireless/ath/ath9k/ar9003_hw.c         |   59 +-
 .../net/wireless/ath/ath9k/ar9462_2p1_initvals.h   | 1774 ++++++++++++++++++++
 drivers/net/wireless/ath/ath9k/reg.h               |   13 +-
 3 files changed, 1842 insertions(+), 4 deletions(-)
 create mode 100644 drivers/net/wireless/ath/ath9k/ar9462_2p1_initvals.h

-- 
1.8.3.1