linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] Use common cordic algorithm for b43
@ 2018-11-03  9:59 Priit Laes
  2018-11-03  9:59 ` [PATCH 1/5] lib: cordic: Move cordic macros and defines to header file Priit Laes
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: Priit Laes @ 2018-11-03  9:59 UTC (permalink / raw)
  To: linux-wireless, b43-dev
  Cc: netdev, linux-kernel, brcm80211-dev-list.pdl, brcm80211-dev-list

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.

Priit Laes (5):
  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(-)

base-commit: 5f21585384a4a69b8bfdd2cae7e3648ae805f57d
-- 
git-series 0.9.1

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/5] lib: cordic: Move cordic macros and defines to header file
  2018-11-03  9:59 [PATCH 0/5] Use common cordic algorithm for b43 Priit Laes
@ 2018-11-03  9:59 ` Priit Laes
  2018-11-03  9:59 ` [PATCH 2/5] brcmfmac: Use common CORDIC_FLOAT macro from lib Priit Laes
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Priit Laes @ 2018-11-03  9:59 UTC (permalink / raw)
  To: linux-kernel

Also append CORDIC_ prefix to nonprefixed macros.

Signed-off-by: Priit Laes <plaes@plaes.org>
---
 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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/5] brcmfmac: Use common CORDIC_FLOAT macro from lib
  2018-11-03  9:59 [PATCH 0/5] Use common cordic algorithm for b43 Priit Laes
  2018-11-03  9:59 ` [PATCH 1/5] lib: cordic: Move cordic macros and defines to header file Priit Laes
@ 2018-11-03  9:59 ` Priit Laes
  2018-11-05  9:05   ` Kalle Valo
  2018-11-03  9:59 ` [PATCH 3/5] brcmfmac: Drop unused cordic defines and macros Priit Laes
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Priit Laes @ 2018-11-03  9:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
	Wright Feng, Kalle Valo, David S. Miller, linux-wireless,
	brcm80211-dev-list.pdl, brcm80211-dev-list, netdev

Now that cordic library has the CORDIC_FLOAT macro, use that

Signed-off-by: Priit Laes <plaes@plaes.org>
---
 drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c | 4 ++--
 drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c   | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/5] brcmfmac: Drop unused cordic defines and macros
  2018-11-03  9:59 [PATCH 0/5] Use common cordic algorithm for b43 Priit Laes
  2018-11-03  9:59 ` [PATCH 1/5] lib: cordic: Move cordic macros and defines to header file Priit Laes
  2018-11-03  9:59 ` [PATCH 2/5] brcmfmac: Use common CORDIC_FLOAT macro from lib Priit Laes
@ 2018-11-03  9:59 ` Priit Laes
  2018-11-05  9:07   ` Kalle Valo
  2018-11-03  9:59 ` [PATCH 4/5] b43: Use common cordic algorithm from kernel lib Priit Laes
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Priit Laes @ 2018-11-03  9:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
	Wright Feng, Kalle Valo, David S. Miller, linux-wireless,
	brcm80211-dev-list.pdl, brcm80211-dev-list, netdev

Now that we use library macros, we can drop internal copies

Signed-off-by: Priit Laes <plaes@plaes.org>
---
 drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_int.h | 7 +-------
 1 file changed, 7 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
 
-- 
git-series 0.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/5] b43: Use common cordic algorithm from kernel lib
  2018-11-03  9:59 [PATCH 0/5] Use common cordic algorithm for b43 Priit Laes
                   ` (2 preceding siblings ...)
  2018-11-03  9:59 ` [PATCH 3/5] brcmfmac: Drop unused cordic defines and macros Priit Laes
@ 2018-11-03  9:59 ` Priit Laes
  2018-11-03 17:37   ` Larry Finger
  2018-11-03  9:59 ` [PATCH 5/5] b43: Drop internal cordic algorithm implementation Priit Laes
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Priit Laes @ 2018-11-03  9:59 UTC (permalink / raw)
  To: linux-kernel; +Cc: Kalle Valo, David S. Miller, linux-wireless, b43-dev, netdev

Signed-off-by: Priit Laes <plaes@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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 5/5] b43: Drop internal cordic algorithm implementation
  2018-11-03  9:59 [PATCH 0/5] Use common cordic algorithm for b43 Priit Laes
                   ` (3 preceding siblings ...)
  2018-11-03  9:59 ` [PATCH 4/5] b43: Use common cordic algorithm from kernel lib Priit Laes
@ 2018-11-03  9:59 ` Priit Laes
  2018-11-05  9:09   ` Kalle Valo
  2018-11-05  8:02 ` [PATCH 0/5] Use common cordic algorithm for b43 Kalle Valo
  2018-11-05  8:24 ` Arend van Spriel
  6 siblings, 1 reply; 17+ messages in thread
From: Priit Laes @ 2018-11-03  9:59 UTC (permalink / raw)
  To: linux-kernel; +Cc: Kalle Valo, David S. Miller, linux-wireless, b43-dev, netdev

Signed-off-by: Priit Laes <plaes@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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/5] b43: Use common cordic algorithm from kernel lib
  2018-11-03  9:59 ` [PATCH 4/5] b43: Use common cordic algorithm from kernel lib Priit Laes
@ 2018-11-03 17:37   ` Larry Finger
  0 siblings, 0 replies; 17+ messages in thread
From: Larry Finger @ 2018-11-03 17:37 UTC (permalink / raw)
  To: Priit Laes, linux-kernel
  Cc: netdev, linux-wireless, David S. Miller, Kalle Valo, b43-dev

On 11/3/18 4:59 AM, Priit Laes wrote:
> Signed-off-by: Priit Laes <plaes@plaes.org>

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

> ---
>   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);
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/5] Use common cordic algorithm for b43
  2018-11-03  9:59 [PATCH 0/5] Use common cordic algorithm for b43 Priit Laes
                   ` (4 preceding siblings ...)
  2018-11-03  9:59 ` [PATCH 5/5] b43: Drop internal cordic algorithm implementation Priit Laes
@ 2018-11-05  8:02 ` Kalle Valo
  2018-11-05  8:07   ` Kalle Valo
  2018-11-05  8:17   ` Arend van Spriel
  2018-11-05  8:24 ` Arend van Spriel
  6 siblings, 2 replies; 17+ messages in thread
From: Kalle Valo @ 2018-11-05  8:02 UTC (permalink / raw)
  To: Priit Laes
  Cc: linux-wireless, b43-dev, netdev, linux-kernel,
	brcm80211-dev-list.pdl, brcm80211-dev-list

Priit Laes <plaes@plaes.org> writes:

> 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.
>
> Priit Laes (5):
>   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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/5] Use common cordic algorithm for b43
  2018-11-05  8:02 ` [PATCH 0/5] Use common cordic algorithm for b43 Kalle Valo
@ 2018-11-05  8:07   ` Kalle Valo
  2018-11-05  8:17   ` Arend van Spriel
  1 sibling, 0 replies; 17+ messages in thread
From: Kalle Valo @ 2018-11-05  8:07 UTC (permalink / raw)
  To: Priit Laes
  Cc: linux-wireless, b43-dev, netdev, linux-kernel,
	brcm80211-dev-list.pdl, brcm80211-dev-list

Kalle Valo <kvalo@codeaurora.org> writes:

> Priit Laes <plaes@plaes.org> writes:
>
>> 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.
>>
>> Priit Laes (5):
>>   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.

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/5] Use common cordic algorithm for b43
  2018-11-05  8:02 ` [PATCH 0/5] Use common cordic algorithm for b43 Kalle Valo
  2018-11-05  8:07   ` Kalle Valo
@ 2018-11-05  8:17   ` Arend van Spriel
  2018-11-05  9:02     ` Kalle Valo
  1 sibling, 1 reply; 17+ messages in thread
From: Arend van Spriel @ 2018-11-05  8:17 UTC (permalink / raw)
  To: Kalle Valo, Priit Laes
  Cc: linux-wireless, b43-dev, netdev, linux-kernel,
	brcm80211-dev-list.pdl, brcm80211-dev-list

On 11/5/2018 9:02 AM, Kalle Valo wrote:
> 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/5] Use common cordic algorithm for b43
  2018-11-03  9:59 [PATCH 0/5] Use common cordic algorithm for b43 Priit Laes
                   ` (5 preceding siblings ...)
  2018-11-05  8:02 ` [PATCH 0/5] Use common cordic algorithm for b43 Kalle Valo
@ 2018-11-05  8:24 ` Arend van Spriel
  6 siblings, 0 replies; 17+ messages in thread
From: Arend van Spriel @ 2018-11-05  8:24 UTC (permalink / raw)
  To: Priit Laes, linux-wireless, b43-dev
  Cc: netdev, linux-kernel, brcm80211-dev-list.pdl, brcm80211-dev-list

On 11/3/2018 10:59 AM, Priit Laes wrote:
> 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/5] Use common cordic algorithm for b43
  2018-11-05  8:17   ` Arend van Spriel
@ 2018-11-05  9:02     ` Kalle Valo
  0 siblings, 0 replies; 17+ messages in thread
From: Kalle Valo @ 2018-11-05  9:02 UTC (permalink / raw)
  To: Arend van Spriel
  Cc: Priit Laes, linux-wireless, b43-dev, netdev, linux-kernel,
	brcm80211-dev-list.pdl, brcm80211-dev-list

Arend van Spriel <arend.vanspriel@broadcom.com> writes:

> On 11/5/2018 9:02 AM, Kalle Valo wrote:
>> 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/5] brcmfmac: Use common CORDIC_FLOAT macro from lib
  2018-11-03  9:59 ` [PATCH 2/5] brcmfmac: Use common CORDIC_FLOAT macro from lib Priit Laes
@ 2018-11-05  9:05   ` Kalle Valo
  2018-11-05  9:13     ` Arend van Spriel
  0 siblings, 1 reply; 17+ messages in thread
From: Kalle Valo @ 2018-11-05  9:05 UTC (permalink / raw)
  To: Priit Laes
  Cc: linux-kernel, Arend van Spriel, Franky Lin, Hante Meuleman,
	Chi-Hsien Lin, Wright Feng, David S. Miller, linux-wireless,
	brcm80211-dev-list.pdl, brcm80211-dev-list, netdev

Priit Laes <plaes@plaes.org> writes:

> Now that cordic library has the CORDIC_FLOAT macro, use that
>
> Signed-off-by: Priit Laes <plaes@plaes.org>
> ---
>  drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c | 4 ++--
>  drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c   | 4 ++--

The driver is "brcmsmac" (note the 's', not 'f'), you should fix the
title accordingly.

> --- 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);

I haven't seen the patch 1 yet, but just from seeing this patch I don't
get what's the benefit.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/5] brcmfmac: Drop unused cordic defines and macros
  2018-11-03  9:59 ` [PATCH 3/5] brcmfmac: Drop unused cordic defines and macros Priit Laes
@ 2018-11-05  9:07   ` Kalle Valo
  0 siblings, 0 replies; 17+ messages in thread
From: Kalle Valo @ 2018-11-05  9:07 UTC (permalink / raw)
  To: Priit Laes
  Cc: linux-kernel, Arend van Spriel, Franky Lin, Hante Meuleman,
	Chi-Hsien Lin, Wright Feng, David S. Miller, linux-wireless,
	brcm80211-dev-list.pdl, brcm80211-dev-list, netdev

Priit Laes <plaes@plaes.org> writes:

> Now that we use library macros, we can drop internal copies
>
> Signed-off-by: Priit Laes <plaes@plaes.org>
> ---
>  drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_int.h | 7 +-------

Also here this is about brcmsmac.

> --- 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))
> -

Ah, now I see the benefit from patch 2. IMHO you could just fold patch 3
into patch 2, no need to split them.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/5] b43: Drop internal cordic algorithm implementation
  2018-11-03  9:59 ` [PATCH 5/5] b43: Drop internal cordic algorithm implementation Priit Laes
@ 2018-11-05  9:09   ` Kalle Valo
  2018-11-05  9:11     ` Arend van Spriel
  0 siblings, 1 reply; 17+ messages in thread
From: Kalle Valo @ 2018-11-05  9:09 UTC (permalink / raw)
  To: Priit Laes; +Cc: linux-kernel, David S. Miller, linux-wireless, b43-dev, netdev

Priit Laes <plaes@plaes.org> writes:

> Signed-off-by: Priit Laes <plaes@plaes.org>

No empty commit logs, please.

And IMHO you could fold patch 5 into patch 4.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/5] b43: Drop internal cordic algorithm implementation
  2018-11-05  9:09   ` Kalle Valo
@ 2018-11-05  9:11     ` Arend van Spriel
  0 siblings, 0 replies; 17+ messages in thread
From: Arend van Spriel @ 2018-11-05  9:11 UTC (permalink / raw)
  To: Kalle Valo, Priit Laes
  Cc: linux-kernel, David S. Miller, linux-wireless, b43-dev, netdev

On 11/5/2018 10:09 AM, Kalle Valo wrote:
> Priit Laes <plaes@plaes.org> writes:
>
>> Signed-off-by: Priit Laes <plaes@plaes.org>
>
> No empty commit logs, please.
>
> And IMHO you could fold patch 5 into patch 4.

Similarly 2 and 3.

Regards,
Arend


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/5] brcmfmac: Use common CORDIC_FLOAT macro from lib
  2018-11-05  9:05   ` Kalle Valo
@ 2018-11-05  9:13     ` Arend van Spriel
  0 siblings, 0 replies; 17+ messages in thread
From: Arend van Spriel @ 2018-11-05  9:13 UTC (permalink / raw)
  To: Kalle Valo, Priit Laes
  Cc: linux-kernel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
	Wright Feng, David S. Miller, linux-wireless,
	brcm80211-dev-list.pdl, brcm80211-dev-list, netdev

On 11/5/2018 10:05 AM, Kalle Valo wrote:
> Priit Laes <plaes@plaes.org> writes:
>
>> Now that cordic library has the CORDIC_FLOAT macro, use that
>>
>> Signed-off-by: Priit Laes <plaes@plaes.org>
>> ---
>>  drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c | 4 ++--
>>  drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c   | 4 ++--
>
> The driver is "brcmsmac" (note the 's', not 'f'), you should fix the
> title accordingly.
>
>> --- 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);
>
> I haven't seen the patch 1 yet, but just from seeing this patch I don't
> get what's the benefit.

The FLOAT macro was defined in brcmsmac (see patch 3). It is now moved 
to the cordic library simply because it is more closely related to that.

Regards,
Arend


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2018-11-05  9:13 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-03  9:59 [PATCH 0/5] Use common cordic algorithm for b43 Priit Laes
2018-11-03  9:59 ` [PATCH 1/5] lib: cordic: Move cordic macros and defines to header file Priit Laes
2018-11-03  9:59 ` [PATCH 2/5] brcmfmac: Use common CORDIC_FLOAT macro from lib Priit Laes
2018-11-05  9:05   ` Kalle Valo
2018-11-05  9:13     ` Arend van Spriel
2018-11-03  9:59 ` [PATCH 3/5] brcmfmac: Drop unused cordic defines and macros Priit Laes
2018-11-05  9:07   ` Kalle Valo
2018-11-03  9:59 ` [PATCH 4/5] b43: Use common cordic algorithm from kernel lib Priit Laes
2018-11-03 17:37   ` Larry Finger
2018-11-03  9:59 ` [PATCH 5/5] b43: Drop internal cordic algorithm implementation Priit Laes
2018-11-05  9:09   ` Kalle Valo
2018-11-05  9:11     ` Arend van Spriel
2018-11-05  8:02 ` [PATCH 0/5] Use common cordic algorithm for b43 Kalle Valo
2018-11-05  8:07   ` Kalle Valo
2018-11-05  8:17   ` Arend van Spriel
2018-11-05  9:02     ` Kalle Valo
2018-11-05  8:24 ` Arend van Spriel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).