b43 wireless driver includes an internal implementation of
cordic algorithm, although there's a common cordic library
which was split out from brcmsmac driver. Use that and drop
internal implementation.
During the process, cordic-algorithm related macros in
brcmfmac driver were also removed and use general macros.
Please note that this series is only compile-tested, as I
do not have access to the hardware.
--
Changes since v3:
- Added ACKed-By bits for patches 1, 4
- Added new patch for stable by Larry as patch 3
- Fixed argument scaling for b43 driver in patch 4
Changes since v2:
- Improvements to commit messages. No functional changes.
- Collect reviewed-by bits.
Changes since v1:
- Merged brcmsmac driver patches into single patch
- Merged b43 driver patches into single patch
Larry Finger (1):
b43: Fix error in cordic routine
Priit Laes (3):
lib: cordic: Move cordic macros and defines to header file
brcmsmac: Use cordic-related macros from common cordic library
b43: Use cordic algorithm from kernel library
drivers/net/wireless/broadcom/b43/Kconfig | 1 +-
drivers/net/wireless/broadcom/b43/phy_common.c | 47 +-------
drivers/net/wireless/broadcom/b43/phy_common.h | 9 +-
drivers/net/wireless/broadcom/b43/phy_lp.c | 13 +-
drivers/net/wireless/broadcom/b43/phy_n.c | 13 +-
drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_int.h | 7 +-
drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c | 4 +-
drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c | 4 +-
include/linux/cordic.h | 9 +-
lib/cordic.c | 23 +---
10 files changed, 35 insertions(+), 95 deletions(-)
base-commit: 9ff01193a20d391e8dbce4403dd5ef87c7eaaca6
--
git-series 0.9.1
Now that these macros are in header file, we can eventually
clean up the duplicate macros present in the drivers that
utilize the same cordic algorithm implementation.
Also add CORDIC_ prefix to nonprefixed macros.
Reviewed-by: Arend van Spriel <[email protected]>
Signed-off-by: Priit Laes <[email protected]>
Acked-by: Larry Finger <[email protected]>
---
include/linux/cordic.h | 9 +++++++++
lib/cordic.c | 23 +++++++----------------
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/include/linux/cordic.h b/include/linux/cordic.h
index cf68ca4..3d656f5 100644
--- a/include/linux/cordic.h
+++ b/include/linux/cordic.h
@@ -18,6 +18,15 @@
#include <linux/types.h>
+#define CORDIC_ANGLE_GEN 39797
+#define CORDIC_PRECISION_SHIFT 16
+#define CORDIC_NUM_ITER (CORDIC_PRECISION_SHIFT + 2)
+
+#define CORDIC_FIXED(X) ((s32)((X) << CORDIC_PRECISION_SHIFT))
+#define CORDIC_FLOAT(X) (((X) >= 0) \
+ ? ((((X) >> (CORDIC_PRECISION_SHIFT - 1)) + 1) >> 1) \
+ : -((((-(X)) >> (CORDIC_PRECISION_SHIFT - 1)) + 1) >> 1))
+
/**
* struct cordic_iq - i/q coordinate.
*
diff --git a/lib/cordic.c b/lib/cordic.c
index 6cf4778..8ef27c1 100644
--- a/lib/cordic.c
+++ b/lib/cordic.c
@@ -16,15 +16,6 @@
#include <linux/module.h>
#include <linux/cordic.h>
-#define CORDIC_ANGLE_GEN 39797
-#define CORDIC_PRECISION_SHIFT 16
-#define CORDIC_NUM_ITER (CORDIC_PRECISION_SHIFT + 2)
-
-#define FIXED(X) ((s32)((X) << CORDIC_PRECISION_SHIFT))
-#define FLOAT(X) (((X) >= 0) \
- ? ((((X) >> (CORDIC_PRECISION_SHIFT - 1)) + 1) >> 1) \
- : -((((-(X)) >> (CORDIC_PRECISION_SHIFT - 1)) + 1) >> 1))
-
static const s32 arctan_table[] = {
2949120,
1740967,
@@ -64,16 +55,16 @@ struct cordic_iq cordic_calc_iq(s32 theta)
coord.q = 0;
angle = 0;
- theta = FIXED(theta);
+ theta = CORDIC_FIXED(theta);
signtheta = (theta < 0) ? -1 : 1;
- theta = ((theta + FIXED(180) * signtheta) % FIXED(360)) -
- FIXED(180) * signtheta;
+ theta = ((theta + CORDIC_FIXED(180) * signtheta) % CORDIC_FIXED(360)) -
+ CORDIC_FIXED(180) * signtheta;
- if (FLOAT(theta) > 90) {
- theta -= FIXED(180);
+ if (CORDIC_FLOAT(theta) > 90) {
+ theta -= CORDIC_FIXED(180);
signx = -1;
- } else if (FLOAT(theta) < -90) {
- theta += FIXED(180);
+ } else if (CORDIC_FLOAT(theta) < -90) {
+ theta += CORDIC_FIXED(180);
signx = -1;
}
--
git-series 0.9.1
From: Larry Finger <[email protected]>
The cordic routine for calculating sines and cosines that was added in
commit 6f98e62a9f1b ("b43: update cordic code to match current specs")
contains an error whereby a quantity declared u32 can in fact go negative.
This problem was detected by Priit Laes who is switching b43 to use the
routine in the library functions of the kernel.
Fixes: 986504540306 ("b43: make cordic common (LP-PHY and N-PHY need it)")
Reported-by: Priit Laes <[email protected]>
Cc: Rafał Miłecki <[email protected]>
Cc: Stable <[email protected]> # 2.6.34
Signed-off-by: Larry Finger <[email protected]>
Signed-off-by: Priit Laes <[email protected]>
---
drivers/net/wireless/broadcom/b43/phy_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/b43/phy_common.c b/drivers/net/wireless/broadcom/b43/phy_common.c
index 85f2ca9..ef3ffa5 100644
--- a/drivers/net/wireless/broadcom/b43/phy_common.c
+++ b/drivers/net/wireless/broadcom/b43/phy_common.c
@@ -616,7 +616,7 @@ struct b43_c32 b43_cordic(int theta)
u8 i;
s32 tmp;
s8 signx = 1;
- u32 angle = 0;
+ s32 angle = 0;
struct b43_c32 ret = { .i = 39797, .q = 0, };
while (theta > (180 << 16))
--
git-series 0.9.1
Kernel library has a common cordic algorithm which is identical
to internally implemented one, so use it and drop the duplicate
implementation.
Acked-by: Larry Finger <[email protected]>
Signed-off-by: Priit Laes <[email protected]>
---
drivers/net/wireless/broadcom/b43/Kconfig | 1 +-
drivers/net/wireless/broadcom/b43/phy_common.c | 47 +-------------------
drivers/net/wireless/broadcom/b43/phy_common.h | 9 +----
drivers/net/wireless/broadcom/b43/phy_lp.c | 13 ++---
drivers/net/wireless/broadcom/b43/phy_n.c | 13 ++---
5 files changed, 15 insertions(+), 68 deletions(-)
diff --git a/drivers/net/wireless/broadcom/b43/Kconfig b/drivers/net/wireless/broadcom/b43/Kconfig
index fba8560..3e41457 100644
--- a/drivers/net/wireless/broadcom/b43/Kconfig
+++ b/drivers/net/wireless/broadcom/b43/Kconfig
@@ -4,6 +4,7 @@ config B43
select BCMA if B43_BCMA
select SSB if B43_SSB
select FW_LOADER
+ select CORDIC
---help---
b43 is a driver for the Broadcom 43xx series wireless devices.
diff --git a/drivers/net/wireless/broadcom/b43/phy_common.c b/drivers/net/wireless/broadcom/b43/phy_common.c
index ef3ffa5..98c4fa5 100644
--- a/drivers/net/wireless/broadcom/b43/phy_common.c
+++ b/drivers/net/wireless/broadcom/b43/phy_common.c
@@ -604,50 +604,3 @@ void b43_phy_force_clock(struct b43_wldev *dev, bool force)
#endif
}
}
-
-/* http://bcm-v4.sipsolutions.net/802.11/PHY/Cordic */
-struct b43_c32 b43_cordic(int theta)
-{
- static const u32 arctg[] = {
- 2949120, 1740967, 919879, 466945, 234379, 117304,
- 58666, 29335, 14668, 7334, 3667, 1833,
- 917, 458, 229, 115, 57, 29,
- };
- u8 i;
- s32 tmp;
- s8 signx = 1;
- s32 angle = 0;
- struct b43_c32 ret = { .i = 39797, .q = 0, };
-
- while (theta > (180 << 16))
- theta -= (360 << 16);
- while (theta < -(180 << 16))
- theta += (360 << 16);
-
- if (theta > (90 << 16)) {
- theta -= (180 << 16);
- signx = -1;
- } else if (theta < -(90 << 16)) {
- theta += (180 << 16);
- signx = -1;
- }
-
- for (i = 0; i <= 17; i++) {
- if (theta > angle) {
- tmp = ret.i - (ret.q >> i);
- ret.q += ret.i >> i;
- ret.i = tmp;
- angle += arctg[i];
- } else {
- tmp = ret.i + (ret.q >> i);
- ret.q -= ret.i >> i;
- ret.i = tmp;
- angle -= arctg[i];
- }
- }
-
- ret.i *= signx;
- ret.q *= signx;
-
- return ret;
-}
diff --git a/drivers/net/wireless/broadcom/b43/phy_common.h b/drivers/net/wireless/broadcom/b43/phy_common.h
index 57a1ad8..4213cac 100644
--- a/drivers/net/wireless/broadcom/b43/phy_common.h
+++ b/drivers/net/wireless/broadcom/b43/phy_common.h
@@ -7,13 +7,6 @@
struct b43_wldev;
-/* Complex number using 2 32-bit signed integers */
-struct b43_c32 { s32 i, q; };
-
-#define CORDIC_CONVERT(value) (((value) >= 0) ? \
- ((((value) >> 15) + 1) >> 1) : \
- -((((-(value)) >> 15) + 1) >> 1))
-
/* PHY register routing bits */
#define B43_PHYROUTE 0x0C00 /* PHY register routing bits mask */
#define B43_PHYROUTE_BASE 0x0000 /* Base registers */
@@ -450,6 +443,4 @@ bool b43_is_40mhz(struct b43_wldev *dev);
void b43_phy_force_clock(struct b43_wldev *dev, bool force);
-struct b43_c32 b43_cordic(int theta);
-
#endif /* LINUX_B43_PHY_COMMON_H_ */
diff --git a/drivers/net/wireless/broadcom/b43/phy_lp.c b/drivers/net/wireless/broadcom/b43/phy_lp.c
index 6922cbb..46408a5 100644
--- a/drivers/net/wireless/broadcom/b43/phy_lp.c
+++ b/drivers/net/wireless/broadcom/b43/phy_lp.c
@@ -23,6 +23,7 @@
*/
+#include <linux/cordic.h>
#include <linux/slab.h>
#include "b43.h"
@@ -1780,9 +1781,9 @@ static void lpphy_start_tx_tone(struct b43_wldev *dev, s32 freq, u16 max)
{
struct b43_phy_lp *lpphy = dev->phy.lp;
u16 buf[64];
- int i, samples = 0, angle = 0;
+ int i, samples = 0, theta = 0;
int rotation = (((36 * freq) / 20) << 16) / 100;
- struct b43_c32 sample;
+ struct cordic_iq sample;
lpphy->tx_tone_freq = freq;
@@ -1798,10 +1799,10 @@ static void lpphy_start_tx_tone(struct b43_wldev *dev, s32 freq, u16 max)
}
for (i = 0; i < samples; i++) {
- sample = b43_cordic(angle);
- angle += rotation;
- buf[i] = CORDIC_CONVERT((sample.i * max) & 0xFF) << 8;
- buf[i] |= CORDIC_CONVERT((sample.q * max) & 0xFF);
+ sample = cordic_calc_iq(CORDIC_FIXED(theta));
+ theta += rotation;
+ buf[i] = CORDIC_FLOAT((sample.i * max) & 0xFF) << 8;
+ buf[i] |= CORDIC_FLOAT((sample.q * max) & 0xFF);
}
b43_lptab_write_bulk(dev, B43_LPTAB16(5, 0), samples, buf);
diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c b/drivers/net/wireless/broadcom/b43/phy_n.c
index 44ab080..7d30036 100644
--- a/drivers/net/wireless/broadcom/b43/phy_n.c
+++ b/drivers/net/wireless/broadcom/b43/phy_n.c
@@ -23,6 +23,7 @@
*/
+#include <linux/cordic.h>
#include <linux/delay.h>
#include <linux/slab.h>
#include <linux/types.h>
@@ -1513,7 +1514,7 @@ static void b43_radio_init2055(struct b43_wldev *dev)
/* http://bcm-v4.sipsolutions.net/802.11/PHY/N/LoadSampleTable */
static int b43_nphy_load_samples(struct b43_wldev *dev,
- struct b43_c32 *samples, u16 len) {
+ struct cordic_iq *samples, u16 len) {
struct b43_phy_n *nphy = dev->phy.n;
u16 i;
u32 *data;
@@ -1544,7 +1545,7 @@ static u16 b43_nphy_gen_load_samples(struct b43_wldev *dev, u32 freq, u16 max,
{
int i;
u16 bw, len, rot, angle;
- struct b43_c32 *samples;
+ struct cordic_iq *samples;
bw = b43_is_40mhz(dev) ? 40 : 20;
len = bw << 3;
@@ -1561,7 +1562,7 @@ static u16 b43_nphy_gen_load_samples(struct b43_wldev *dev, u32 freq, u16 max,
len = bw << 1;
}
- samples = kcalloc(len, sizeof(struct b43_c32), GFP_KERNEL);
+ samples = kcalloc(len, sizeof(struct cordic_iq), GFP_KERNEL);
if (!samples) {
b43err(dev->wl, "allocation for samples generation failed\n");
return 0;
@@ -1570,10 +1571,10 @@ static u16 b43_nphy_gen_load_samples(struct b43_wldev *dev, u32 freq, u16 max,
angle = 0;
for (i = 0; i < len; i++) {
- samples[i] = b43_cordic(angle);
+ samples[i] = cordic_calc_iq(CORDIC_FIXED(angle));
angle += rot;
- samples[i].q = CORDIC_CONVERT(samples[i].q * max);
- samples[i].i = CORDIC_CONVERT(samples[i].i * max);
+ samples[i].q = CORDIC_FLOAT(samples[i].q * max);
+ samples[i].i = CORDIC_FLOAT(samples[i].i * max);
}
i = b43_nphy_load_samples(dev, samples, len);
--
git-series 0.9.1
Current driver includes macro that is available from general cordic
library. Use that and drop unused duplicate and unneeded internal
definitions.
Signed-off-by: Priit Laes <[email protected]>
---
drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_int.h | 7 +-------
drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c | 4 ++--
drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c | 4 ++--
3 files changed, 4 insertions(+), 11 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_int.h b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_int.h
index 4960f7d..e9e8337 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_int.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_int.h
@@ -220,13 +220,6 @@ enum phy_cal_mode {
#define BB_MULT_MASK 0x0000ffff
#define BB_MULT_VALID_MASK 0x80000000
-#define CORDIC_AG 39797
-#define CORDIC_NI 18
-#define FIXED(X) ((s32)((X) << 16))
-
-#define FLOAT(X) \
- (((X) >= 0) ? ((((X) >> 15) + 1) >> 1) : -((((-(X)) >> 15) + 1) >> 1))
-
#define PHY_CHAIN_TX_DISABLE_TEMP 115
#define PHY_HYSTERESIS_DELTATEMP 5
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c
index 9fb0d9f..e78a93a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c
@@ -3447,8 +3447,8 @@ wlc_lcnphy_start_tx_tone(struct brcms_phy *pi, s32 f_kHz, u16 max_val,
theta += rot;
- i_samp = (u16) (FLOAT(tone_samp.i * max_val) & 0x3ff);
- q_samp = (u16) (FLOAT(tone_samp.q * max_val) & 0x3ff);
+ i_samp = (u16)(CORDIC_FLOAT(tone_samp.i * max_val) & 0x3ff);
+ q_samp = (u16)(CORDIC_FLOAT(tone_samp.q * max_val) & 0x3ff);
data_buf[t] = (i_samp << 10) | q_samp;
}
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c
index a57f271..f4f5e90 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c
@@ -23089,8 +23089,8 @@ wlc_phy_gen_load_samples_nphy(struct brcms_phy *pi, u32 f_kHz, u16 max_val,
theta += rot;
- tone_buf[t].q = (s32) FLOAT(tone_buf[t].q * max_val);
- tone_buf[t].i = (s32) FLOAT(tone_buf[t].i * max_val);
+ tone_buf[t].q = (s32)CORDIC_FLOAT(tone_buf[t].q * max_val);
+ tone_buf[t].i = (s32)CORDIC_FLOAT(tone_buf[t].i * max_val);
}
wlc_phy_loadsampletable_nphy(pi, tone_buf, num_samps);
--
git-series 0.9.1
On Mon, Nov 19, 2018 at 08:01:21PM +0200, Priit Laes wrote:
> b43 wireless driver includes an internal implementation of
> cordic algorithm, although there's a common cordic library
> which was split out from brcmsmac driver. Use that and drop
> internal implementation.
>
> During the process, cordic-algorithm related macros in
> brcmfmac driver were also removed and use general macros.
>
> Please note that this series is only compile-tested, as I
> do not have access to the hardware.
Gah.. I missed the series version in subject. Shall I resend?
>
> --
>
> Changes since v3:
> - Added ACKed-By bits for patches 1, 4
> - Added new patch for stable by Larry as patch 3
> - Fixed argument scaling for b43 driver in patch 4
>
> Changes since v2:
> - Improvements to commit messages. No functional changes.
> - Collect reviewed-by bits.
>
> Changes since v1:
> - Merged brcmsmac driver patches into single patch
> - Merged b43 driver patches into single patch
>
> Larry Finger (1):
> b43: Fix error in cordic routine
>
> Priit Laes (3):
> lib: cordic: Move cordic macros and defines to header file
> brcmsmac: Use cordic-related macros from common cordic library
> b43: Use cordic algorithm from kernel library
>
> drivers/net/wireless/broadcom/b43/Kconfig | 1 +-
> drivers/net/wireless/broadcom/b43/phy_common.c | 47 +-------
> drivers/net/wireless/broadcom/b43/phy_common.h | 9 +-
> drivers/net/wireless/broadcom/b43/phy_lp.c | 13 +-
> drivers/net/wireless/broadcom/b43/phy_n.c | 13 +-
> drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_int.h | 7 +-
> drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c | 4 +-
> drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c | 4 +-
> include/linux/cordic.h | 9 +-
> lib/cordic.c | 23 +---
> 10 files changed, 35 insertions(+), 95 deletions(-)
>
> base-commit: 9ff01193a20d391e8dbce4403dd5ef87c7eaaca6
> --
> git-series 0.9.1
Priit Laes <[email protected]> writes:
> On Mon, Nov 19, 2018 at 08:01:21PM +0200, Priit Laes wrote:
>> b43 wireless driver includes an internal implementation of
>> cordic algorithm, although there's a common cordic library
>> which was split out from brcmsmac driver. Use that and drop
>> internal implementation.
>>
>> During the process, cordic-algorithm related macros in
>> brcmfmac driver were also removed and use general macros.
>>
>> Please note that this series is only compile-tested, as I
>> do not have access to the hardware.
>
>
> Gah.. I missed the series version in subject. Shall I resend?
No need, I'll remember now that this is v4.
--
Kalle Valo
Priit Laes <[email protected]> wrote:
> Now that these macros are in header file, we can eventually
> clean up the duplicate macros present in the drivers that
> utilize the same cordic algorithm implementation.
>
> Also add CORDIC_ prefix to nonprefixed macros.
>
> Reviewed-by: Arend van Spriel <[email protected]>
> Signed-off-by: Priit Laes <[email protected]>
> Acked-by: Larry Finger <[email protected]>
4 patches applied to wireless-drivers-next.git, thanks.
58d81d64e06f lib: cordic: Move cordic macros and defines to header file
ea3edda9ddba brcmsmac: Use cordic-related macros from common cordic library
8ea3819c0bbe b43: Fix error in cordic routine
d5a433556d09 b43: Use cordic algorithm from kernel library
--
https://patchwork.kernel.org/patch/10689235/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches