[PATCH 4/5] b43: Use common cordic algorithm from kernel lib

Discussion:

Priit Laes

2018-11-03 09:59:43 UTC

Signed-off-by: Priit Laes <***@plaes.org>
---
drivers/net/wireless/broadcom/b43/Kconfig | 1 +
drivers/net/wireless/broadcom/b43/phy_lp.c | 13 +++++++------
drivers/net/wireless/broadcom/b43/phy_n.c | 13 +++++++------
3 files changed, 15 insertions(+), 12 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_lp.c b/drivers/net/wireless/broadcom/b43/phy_lp.c
index 6922cbb..1718e3b 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(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..1f9378a 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(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

Priit Laes

2018-11-03 09:59:44 UTC

Permalink

Signed-off-by: Priit Laes <***@plaes.org>
---
drivers/net/wireless/broadcom/b43/phy_common.c | 47 +-------------------
drivers/net/wireless/broadcom/b43/phy_common.h | 9 +----
2 files changed, 56 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43/phy_common.c b/drivers/net/wireless/broadcom/b43/phy_common.c
index 85f2ca9..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;
- u32 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_ */

--
git-series 0.9.1

Kalle Valo

2018-11-05 09:09:59 UTC

Permalink

No empty commit logs, please.

And IMHO you could fold patch 5 into patch 4.

--
Kalle Valo

Arend van Spriel

2018-11-05 09:11:41 UTC

Permalink

Post by Kalle Valo
No empty commit logs, please.
And IMHO you could fold patch 5 into patch 4.

Similarly 2 and 3.

Regards,
Arend

Larry Finger

2018-11-03 17:37:20 UTC

Permalink

Where is the commit message? The stuff in the cover letter (Patch 0/N) never
makes it to the git repository. You must have a message in each of the
individual patches.

NACK.

Larry

Post by Priit Laes
---
drivers/net/wireless/broadcom/b43/Kconfig | 1 +
drivers/net/wireless/broadcom/b43/phy_lp.c | 13 +++++++------
drivers/net/wireless/broadcom/b43/phy_n.c | 13 +++++++------
3 files changed, 15 insertions(+), 12 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_lp.c b/drivers/net/wireless/broadcom/b43/phy_lp.c
index 6922cbb..1718e3b 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(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..1f9378a 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(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);

Kalle Valo

2018-11-05 08:02:17 UTC

Permalink

b43 wireless driver included internal implementation of cordic
algorithm which has now been removed in favor of library
implementation.
During the process, brcmfmac was driver was also cleaned.
Please note that this series is only compile-tested, as I
do not have access to the hardware.
lib: cordic: Move cordic macros and defines to header file
brcmfmac: Use common CORDIC_FLOAT macro from lib
brcmfmac: Drop unused cordic defines and macros
b43: Use common cordic algorithm from kernel lib
b43: Drop internal cordic algorithm implementation
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(-)

I don't see patch 1 in linux-wireless patchwork:

https://patchwork.kernel.org/project/linux-wireless/list/?series=38033&state=*

Via which tree are you planning to push these? These could potentially
go via my wireless-drivers-next tree (if review goes ok) but I need to
have all five patches in patchwork.

Also I don't see MAINTAINERS entry for cordic.[c|h], that would be good
to have as well.

--
Kalle Valo

Kalle Valo

2018-11-05 08:07:07 UTC

Permalink

Post by Kalle Valo

https://patchwork.kernel.org/project/linux-wireless/list/?series=38033&state=*
Via which tree are you planning to push these? These could potentially
go via my wireless-drivers-next tree (if review goes ok) but I need to
have all five patches in patchwork.

Oh, forgot to mention that please resubmit all five patches, not just
patch 1, because then it's easier for me to apply them.

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#resubmit_the_whole_patchset

--
Kalle Valo

Arend van Spriel

2018-11-05 08:17:40 UTC

Permalink

Post by Kalle Valo
Also I don't see MAINTAINERS entry for cordic.[c|h], that would be good
to have as well.

We added the cordic library functions during brcm80211 staging cleanup.
We can add it to MAINTAINERS file.

Regards,
Arend

Kalle Valo

2018-11-05 09:02:29 UTC

Permalink

Post by Arend van Spriel

Post by Kalle Valo
Also I don't see MAINTAINERS entry for cordic.[c|h], that would be good
to have as well.

We added the cordic library functions during brcm80211 staging
cleanup. We can add it to MAINTAINERS file.

Great, thanks.

--
Kalle Valo

Arend van Spriel

2018-11-05 08:24:16 UTC

Permalink

b43 wireless driver included internal implementation of cordic
algorithm which has now been removed in favor of library
implementation.
During the process, brcmfmac was driver was also cleaned.

You actually touched the *brcmsmac* driver, not brcmfmac. Please fix the
driver prefix where appropriate in this series, ie. patches 2 and 3.

Please note that this series is only compile-tested, as I
do not have access to the hardware.

I can/will verify brcmsmac. As Kalle mentioned it makes more sense to
push the 'lib: cordic:' patch through the wireless tree as well as it
only is used by wireless drivers right now.

Regards,
Arend